From d1cdf8ce2075ce88846fd636c028b4334a40e2d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= Date: Sun, 7 Dec 2025 13:49:50 +0100 Subject: [PATCH] Extend `redundant_self_in_closure` to find all redundant `self`s (#6346) --- .swiftlint.yml | 1 + CHANGELOG.md | 9 +- .../Models/BuiltInRules.swift | 2 +- .../RedundantSelfConfiguration.swift | 9 + .../Style/RedundantSelfInClosureRule.swift | 161 ------------------ .../Rules/Style/RedundantSelfRule.swift | 153 +++++++++++++++++ ....swift => RedundantSelfRuleExamples.swift} | 49 +++++- Source/SwiftLintCore/Helpers/Stack.swift | 18 +- .../DeclaredIdentifiersTrackingVisitor.swift | 7 + Tests/GeneratedTests/GeneratedTests_08.swift | 4 +- .../default_rule_configurations.yml | 3 +- 11 files changed, 244 insertions(+), 172 deletions(-) create mode 100644 Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/RedundantSelfConfiguration.swift delete mode 100644 Source/SwiftLintBuiltInRules/Rules/Style/RedundantSelfInClosureRule.swift create mode 100644 Source/SwiftLintBuiltInRules/Rules/Style/RedundantSelfRule.swift rename Source/SwiftLintBuiltInRules/Rules/Style/{RedundantSelfInClosureRuleExamples.swift => RedundantSelfRuleExamples.swift} (84%) diff --git a/.swiftlint.yml b/.swiftlint.yml index 0c068841e..f9eea368c 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -41,6 +41,7 @@ disabled_rules: - one_declaration_per_file - prefer_nimble - prefixed_toplevel_constant + - redundant_self_in_closure - required_deinit - sorted_enum_cases - strict_fileprivate diff --git a/CHANGELOG.md b/CHANGELOG.md index e972d6551..1970d5315 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,9 @@ ### Breaking -* None. +* The `redundant_self_in_closure` rule has been renamed to `redundant_self` (with + `redundant_self_in_closure` as a deprecated alias) to reflect its now broader scope. + [SimplyDanny](https://github.com/SimplyDanny) ### Experimental @@ -12,6 +14,11 @@ ### Enhancements +* Extend `redundant_self_in_closure` rule to detect all redundant uses of `self`, + not just in closures. Initializers (which commonly prefer an explicit `self` prefix) + can be ignored by setting `keep_in_initializers` to `true`. + [SimplyDanny](https://github.com/SimplyDanny) + * Add a `separation` configuration option to the `vertical_whitespace_between_cases` rule to allow customizing blank line separation between switch cases. The default value is `always` (require at least one blank line). Setting it to `never` enforces no blank diff --git a/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift b/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift index 38104f3ec..d3d2b8221 100644 --- a/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift +++ b/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift @@ -178,7 +178,7 @@ public let builtInRules: [any Rule.Type] = [ RedundantDiscardableLetRule.self, RedundantNilCoalescingRule.self, RedundantObjcAttributeRule.self, - RedundantSelfInClosureRule.self, + RedundantSelfRule.self, RedundantSendableRule.self, RedundantSetAccessControlRule.self, RedundantStringEnumValueRule.self, diff --git a/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/RedundantSelfConfiguration.swift b/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/RedundantSelfConfiguration.swift new file mode 100644 index 000000000..ae7e3fdbb --- /dev/null +++ b/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/RedundantSelfConfiguration.swift @@ -0,0 +1,9 @@ +import SwiftLintCore + +@AutoConfigParser +struct RedundantSelfConfiguration: SeverityBasedRuleConfiguration { + @ConfigurationElement(key: "severity") + private(set) var severityConfiguration = SeverityConfiguration(.warning) + @ConfigurationElement(key: "keep_in_initializers") + private(set) var keepInInitializers = false +} diff --git a/Source/SwiftLintBuiltInRules/Rules/Style/RedundantSelfInClosureRule.swift b/Source/SwiftLintBuiltInRules/Rules/Style/RedundantSelfInClosureRule.swift deleted file mode 100644 index 49b1f5510..000000000 --- a/Source/SwiftLintBuiltInRules/Rules/Style/RedundantSelfInClosureRule.swift +++ /dev/null @@ -1,161 +0,0 @@ -import SwiftSyntax - -@SwiftSyntaxRule(correctable: true, optIn: true) -struct RedundantSelfInClosureRule: Rule { - var configuration = SeverityConfiguration(.warning) - - static let description = RuleDescription( - identifier: "redundant_self_in_closure", - name: "Redundant Self in Closure", - description: "Explicit use of 'self' is not required", - kind: .style, - nonTriggeringExamples: RedundantSelfInClosureRuleExamples.nonTriggeringExamples, - triggeringExamples: RedundantSelfInClosureRuleExamples.triggeringExamples, - corrections: RedundantSelfInClosureRuleExamples.corrections - ) -} - -private enum TypeDeclarationKind { - case likeStruct - case likeClass -} - -private enum FunctionCallType { - case anonymousClosure - case function -} - -private enum SelfCaptureKind { - case strong - case weak - case uncaptured -} - -private extension RedundantSelfInClosureRule { - final class Visitor: DeclaredIdentifiersTrackingVisitor { - private var typeDeclarations = Stack() - private var functionCalls = Stack() - private var selfCaptures = Stack() - - override var skippableDeclarations: [any DeclSyntaxProtocol.Type] { [ProtocolDeclSyntax.self] } - - override func visit(_: ActorDeclSyntax) -> SyntaxVisitorContinueKind { - typeDeclarations.push(.likeClass) - return .visitChildren - } - - override func visitPost(_: ActorDeclSyntax) { - typeDeclarations.pop() - } - - override func visit(_: ClassDeclSyntax) -> SyntaxVisitorContinueKind { - typeDeclarations.push(.likeClass) - return .visitChildren - } - - override func visitPost(_: ClassDeclSyntax) { - typeDeclarations.pop() - } - - override func visit(_ node: ClosureExprSyntax) -> SyntaxVisitorContinueKind { - if let selfItem = node.signature?.capture?.items.first(where: \.capturesSelf) { - selfCaptures.push(selfItem.capturesWeakly ? .weak : .strong) - } else { - selfCaptures.push(.uncaptured) - } - return .visitChildren - } - - override func visitPost(_ node: ClosureExprSyntax) { - guard let activeTypeDeclarationKind = typeDeclarations.peek(), - let activeFunctionCallType = functionCalls.peek(), - let activeSelfCaptureKind = selfCaptures.peek() else { - return - } - let localViolationCorrections = ExplicitSelfVisitor( - configuration: configuration, - file: file, - typeDeclarationKind: activeTypeDeclarationKind, - functionCallType: activeFunctionCallType, - selfCaptureKind: activeSelfCaptureKind, - scope: scope - ).walk(tree: node.statements, handler: \.violations) - violations.append(contentsOf: localViolationCorrections) - selfCaptures.pop() - } - - override func visit(_: EnumDeclSyntax) -> SyntaxVisitorContinueKind { - typeDeclarations.push(.likeStruct) - return .visitChildren - } - - override func visitPost(_: EnumDeclSyntax) { - typeDeclarations.pop() - } - - override func visit(_ node: FunctionCallExprSyntax) -> SyntaxVisitorContinueKind { - if node.calledExpression.is(ClosureExprSyntax.self) { - functionCalls.push(.anonymousClosure) - } else { - functionCalls.push(.function) - } - return .visitChildren - } - - override func visitPost(_: FunctionCallExprSyntax) { - functionCalls.pop() - } - - override func visit(_: StructDeclSyntax) -> SyntaxVisitorContinueKind { - typeDeclarations.push(.likeStruct) - return .visitChildren - } - - override func visitPost(_: StructDeclSyntax) { - typeDeclarations.pop() - } - } -} - -private class ExplicitSelfVisitor: DeclaredIdentifiersTrackingVisitor { - private let typeDeclKind: TypeDeclarationKind - private let functionCallType: FunctionCallType - private let selfCaptureKind: SelfCaptureKind - - init(configuration: Configuration, - file: SwiftLintFile, - typeDeclarationKind: TypeDeclarationKind, - functionCallType: FunctionCallType, - selfCaptureKind: SelfCaptureKind, - scope: Scope) { - self.typeDeclKind = typeDeclarationKind - self.functionCallType = functionCallType - self.selfCaptureKind = selfCaptureKind - super.init(configuration: configuration, file: file, scope: scope) - } - - override func visitPost(_ node: MemberAccessExprSyntax) { - if !hasSeenDeclaration(for: node.declName.baseName.text), node.isBaseSelf, isSelfRedundant { - violations.append( - at: node.positionAfterSkippingLeadingTrivia, - correction: .init( - start: node.positionAfterSkippingLeadingTrivia, - end: node.period.endPositionBeforeTrailingTrivia, - replacement: "" - ) - ) - } - } - - override func visit(_: ClosureExprSyntax) -> SyntaxVisitorContinueKind { - // Will be handled separately by the parent visitor. - .skipChildren - } - - var isSelfRedundant: Bool { - typeDeclKind == .likeStruct - || functionCallType == .anonymousClosure - || selfCaptureKind == .strong && SwiftVersion.current >= .fiveDotThree - || selfCaptureKind == .weak && SwiftVersion.current >= .fiveDotEight - } -} diff --git a/Source/SwiftLintBuiltInRules/Rules/Style/RedundantSelfRule.swift b/Source/SwiftLintBuiltInRules/Rules/Style/RedundantSelfRule.swift new file mode 100644 index 000000000..75a5c3307 --- /dev/null +++ b/Source/SwiftLintBuiltInRules/Rules/Style/RedundantSelfRule.swift @@ -0,0 +1,153 @@ +@_spi(Diagnostics) +import SwiftParser +@_spi(RawSyntax) +import SwiftSyntax + +@SwiftSyntaxRule(correctable: true, optIn: true) +struct RedundantSelfRule: Rule { + var configuration = RedundantSelfConfiguration() + + static let description = RuleDescription( + identifier: "redundant_self", + name: "Redundant Self", + description: "Explicit use of 'self' is not required", + kind: .style, + nonTriggeringExamples: RedundantSelfRuleExamples.nonTriggeringExamples, + triggeringExamples: RedundantSelfRuleExamples.triggeringExamples, + corrections: RedundantSelfRuleExamples.corrections, + deprecatedAliases: ["redundant_self_in_closure"] + ) +} + +private enum TypeDeclarationKind { + case likeStruct, likeClass +} + +private enum ClosureExprType { + case anonymousCall, functionArgument +} + +private enum SelfCaptureKind { + case strong, weak, uncaptured +} + +private extension RedundantSelfRule { + final class Visitor: DeclaredIdentifiersTrackingVisitor { + private var typeDeclarations = Stack() + private var closureExprScopes = Stack<(ClosureExprType, SelfCaptureKind)>() + private var initializerScopes = Stack() + + override var skippableDeclarations: [any DeclSyntaxProtocol.Type] { [ProtocolDeclSyntax.self] } + + override func visit(_: ActorDeclSyntax) -> SyntaxVisitorContinueKind { + typeDeclarations.push(.likeClass) + return .visitChildren + } + + override func visitPost(_: ActorDeclSyntax) { + typeDeclarations.pop() + } + + override func visit(_: ClassDeclSyntax) -> SyntaxVisitorContinueKind { + typeDeclarations.push(.likeClass) + return .visitChildren + } + + override func visitPost(_: ClassDeclSyntax) { + typeDeclarations.pop() + } + + override func visit(_ node: ClosureExprSyntax) -> SyntaxVisitorContinueKind { + let captureType: SelfCaptureKind = + if let selfItem = node.signature?.capture?.items.first(where: \.capturesSelf) { + selfItem.capturesWeakly ? .weak : .strong + } else { + .uncaptured + } + let exprType: ClosureExprType = + if node.keyPathInParent == \FunctionCallExprSyntax.calledExpression { + .anonymousCall + } else { + .functionArgument + } + closureExprScopes.push((exprType, captureType)) + return .visitChildren + } + + override func visitPost(_: ClosureExprSyntax) { + closureExprScopes.pop() + } + + override func visit(_: EnumDeclSyntax) -> SyntaxVisitorContinueKind { + typeDeclarations.push(.likeStruct) + return .visitChildren + } + + override func visitPost(_: EnumDeclSyntax) { + typeDeclarations.pop() + } + + override func visit(_: InitializerDeclSyntax) -> SyntaxVisitorContinueKind { + initializerScopes.push(true) + return .visitChildren + } + + override func visitPost(_: InitializerDeclSyntax) { + initializerScopes.pop() + } + + override func visitPost(_ node: MemberAccessExprSyntax) { + if configuration.keepInInitializers, initializerScopes.peek() == true { + return + } + if closureExprScopes.isNotEmpty, !isSelfRedundant { + return + } + let declName = node.declName.baseName.text + if !hasSeenDeclaration(for: declName), node.isBaseSelf, declName != "init" { + violations.append( + at: node.positionAfterSkippingLeadingTrivia, + correction: .init( + start: node.positionAfterSkippingLeadingTrivia, + end: node.endPositionBeforeTrailingTrivia, + replacement: node.declName.baseName.needsEscaping + ? "`\(declName)`" + : declName + ) + ) + } + } + + override func visit(_: StructDeclSyntax) -> SyntaxVisitorContinueKind { + typeDeclarations.push(.likeStruct) + return .visitChildren + } + + override func visitPost(_: StructDeclSyntax) { + typeDeclarations.pop() + } + + private var isSelfRedundant: Bool { + if typeDeclarations.peek() == .likeStruct { + return true + } + guard let (closureType, selfCapture) = closureExprScopes.peek() else { + return false + } + return closureType == .anonymousCall + || selfCapture == .strong && SwiftVersion.current >= .fiveDotThree + || selfCapture == .weak && SwiftVersion.current >= .fiveDotEight + } + } +} + +private extension TokenSyntax { + var needsEscaping: Bool { + [UInt8](text.utf8).withUnsafeBufferPointer { + if let keyword = Keyword(SyntaxText(baseAddress: $0.baseAddress, count: text.count)) { + return TokenKind.keyword(keyword).isLexerClassifiedKeyword + } + return false + } + } +} diff --git a/Source/SwiftLintBuiltInRules/Rules/Style/RedundantSelfInClosureRuleExamples.swift b/Source/SwiftLintBuiltInRules/Rules/Style/RedundantSelfRuleExamples.swift similarity index 84% rename from Source/SwiftLintBuiltInRules/Rules/Style/RedundantSelfInClosureRuleExamples.swift rename to Source/SwiftLintBuiltInRules/Rules/Style/RedundantSelfRuleExamples.swift index 9923ab527..003c0310b 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Style/RedundantSelfInClosureRuleExamples.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Style/RedundantSelfRuleExamples.swift @@ -1,4 +1,4 @@ -struct RedundantSelfInClosureRuleExamples { +struct RedundantSelfRuleExamples { static let nonTriggeringExamples = [ Example(""" struct S { @@ -93,6 +93,15 @@ struct RedundantSelfInClosureRuleExamples { func f(_: () -> Void) {} } """, excludeFromDocumentation: true), + Example(""" + class C { + var x = 0, y = 0 + init(x: Int) { + self.x = x + self.y = x + 1 + } + } + """, configuration: ["keep_in_initializers": true]), ] static let triggeringExamples = [ @@ -194,11 +203,11 @@ struct RedundantSelfInClosureRuleExamples { var x = 0 func f(_ work: @escaping () -> Void) { work() } func g() { - f { [weak self] in + f({ [weak self] in self?.x = 1 guard let self else { return } ↓self.x = 1 - } + }) f { [weak self] in self?.x = 1 if let self = self { ↓self.x = 1 } @@ -212,6 +221,23 @@ struct RedundantSelfInClosureRuleExamples { } } """), + Example(""" + class C { + var x = 0 + private lazy var c1: Int = { + ↓self.x = 1 + let f = { self.x = 2 } + let g = { [self] in ↓self.x = 3 } + return 2 + }() + private lazy var c2: Int = { [weak self] in + guard let self else { return 0 } + ↓self.x = 1 + let f = { self.x = 2 } + return 2 + }() + } + """, excludeFromDocumentation: true), ] static let corrections = [ @@ -238,5 +264,22 @@ struct RedundantSelfInClosureRuleExamples { } } """), + Example(""" + struct S { + var x = 0, y = 0 + init(x: Int) { + self.x = x + ↓self.y = 1 + } + } + """): Example(""" + struct S { + var x = 0, y = 0 + init(x: Int) { + self.x = x + y = 1 + } + } + """), ] } diff --git a/Source/SwiftLintCore/Helpers/Stack.swift b/Source/SwiftLintCore/Helpers/Stack.swift index 794919bf2..74a0e9725 100644 --- a/Source/SwiftLintCore/Helpers/Stack.swift +++ b/Source/SwiftLintCore/Helpers/Stack.swift @@ -49,9 +49,21 @@ public struct Stack { } } -extension Stack: Sequence { - public func makeIterator() -> [Element].Iterator { - elements.makeIterator() +extension Stack: Collection { + public var startIndex: Int { + elements.startIndex + } + + public var endIndex: Int { + elements.endIndex + } + + public subscript(position: Int) -> Element { + elements[position] + } + + public func index(after index: Int) -> Int { + elements.index(after: index) } } diff --git a/Source/SwiftLintCore/Visitors/DeclaredIdentifiersTrackingVisitor.swift b/Source/SwiftLintCore/Visitors/DeclaredIdentifiersTrackingVisitor.swift index 798bb2d50..0ba2d0ce5 100644 --- a/Source/SwiftLintCore/Visitors/DeclaredIdentifiersTrackingVisitor.swift +++ b/Source/SwiftLintCore/Visitors/DeclaredIdentifiersTrackingVisitor.swift @@ -132,6 +132,13 @@ open class DeclaredIdentifiersTrackingVisitor: } } + override open func visit(_ node: FunctionDeclSyntax) -> SyntaxVisitorContinueKind { + if node.parent?.is(MemberBlockItemSyntax.self) != true { + scope.addToCurrentScope(.localVariable(name: node.name)) + } + return .visitChildren + } + // MARK: Private methods private func collectIdentifiers(from parameters: FunctionParameterListSyntax) { diff --git a/Tests/GeneratedTests/GeneratedTests_08.swift b/Tests/GeneratedTests/GeneratedTests_08.swift index 134d69cf3..bb3481ea8 100644 --- a/Tests/GeneratedTests/GeneratedTests_08.swift +++ b/Tests/GeneratedTests/GeneratedTests_08.swift @@ -13,9 +13,9 @@ final class RedundantObjcAttributeRuleGeneratedTests: SwiftLintTestCase { } } -final class RedundantSelfInClosureRuleGeneratedTests: SwiftLintTestCase { +final class RedundantSelfRuleGeneratedTests: SwiftLintTestCase { func testWithDefaultConfiguration() { - verifyRule(RedundantSelfInClosureRule.description) + verifyRule(RedundantSelfRule.description) } } diff --git a/Tests/IntegrationTests/default_rule_configurations.yml b/Tests/IntegrationTests/default_rule_configurations.yml index bbd525020..ac97ab336 100644 --- a/Tests/IntegrationTests/default_rule_configurations.yml +++ b/Tests/IntegrationTests/default_rule_configurations.yml @@ -1014,8 +1014,9 @@ redundant_objc_attribute: meta: opt-in: false correctable: true -redundant_self_in_closure: +redundant_self: severity: warning + keep_in_initializers: false meta: opt-in: true correctable: true