Extend redundant_self_in_closure to find all redundant selfs

Author: SimplyDanny

.swiftlint.yml

@@ -40,6 +40,7 @@ disabled_rules:
   - one_declaration_per_file
   - prefer_nimble
   - prefixed_toplevel_constant
+  - redundant_self_in_closure
   - required_deinit
   - sorted_enum_cases
   - strict_fileprivate

Source/SwiftLintBuiltInRules/Rules/Style/RedundantSelfInClosureRule.swift

@@ -1,3 +1,6 @@
+@_spi(Diagnostics)
+import SwiftParser
+@_spi(RawSyntax)
 import SwiftSyntax
 
 @SwiftSyntaxRule(correctable: true, optIn: true)
@@ -16,19 +19,15 @@ struct RedundantSelfInClosureRule: Rule {
 }
 
 private enum TypeDeclarationKind {
-    case likeStruct
-    case likeClass
+    case likeStruct, likeClass
 }
 
 private enum FunctionCallType {
-    case anonymousClosure
-    case function
+    case anonymousClosure, function
 }
 
 private enum SelfCaptureKind {
-    case strong
-    case weak
-    case uncaptured
+    case strong, weak, uncaptured
 }
 
 private extension RedundantSelfInClosureRule {
@@ -63,25 +62,17 @@ private extension RedundantSelfInClosureRule {
             } else {
                 selfCaptures.push(.uncaptured)
             }
+            if node.keyPathInParent == \FunctionCallExprSyntax.calledExpression {
+                functionCalls.push(.anonymousClosure)
+            } else {
+                functionCalls.push(.function)
+            }
             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)
+        override func visitPost(_: ClosureExprSyntax) {
             selfCaptures.pop()
+            functionCalls.pop()
         }
 
         override func visit(_: EnumDeclSyntax) -> SyntaxVisitorContinueKind {
@@ -93,17 +84,26 @@ private extension RedundantSelfInClosureRule {
             typeDeclarations.pop()
         }
 
-        override func visit(_ node: FunctionCallExprSyntax) -> SyntaxVisitorContinueKind {
-            if node.calledExpression.is(ClosureExprSyntax.self) {
-                functionCalls.push(.anonymousClosure)
-            } else {
-                functionCalls.push(.function)
+        override func visitPost(_ node: MemberAccessExprSyntax) {
+            if selfCaptures.isNotEmpty {
+                // In closure ...
+                guard typeDeclarations.isNotEmpty, functionCalls.isNotEmpty, isSelfRedundant else {
+                    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
+                    )
+                )
             }
-            return .visitChildren
-        }
-
-        override func visitPost(_: FunctionCallExprSyntax) {
-            functionCalls.pop()
         }
 
         override func visit(_: StructDeclSyntax) -> SyntaxVisitorContinueKind {
@@ -114,48 +114,23 @@ private extension RedundantSelfInClosureRule {
         override func visitPost(_: StructDeclSyntax) {
             typeDeclarations.pop()
         }
-    }
-}
-
-private class ExplicitSelfVisitor<Configuration: RuleConfiguration>: DeclaredIdentifiersTrackingVisitor<Configuration> {
-    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: ""
-                )
-            )
+        private var isSelfRedundant: Bool {
+               typeDeclarations.peek() == .likeStruct
+            || functionCalls.peek() == .anonymousClosure
+            || selfCaptures.peek() == .strong && SwiftVersion.current >= .fiveDotThree
+            || selfCaptures.peek() == .weak && SwiftVersion.current >= .fiveDotEight
         }
     }
+}
 
-    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
+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
+        }
     }
 }

Source/SwiftLintBuiltInRules/Rules/Style/RedundantSelfInClosureRuleExamples.swift

@@ -194,11 +194,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 +212,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 = [

Source/SwiftLintCore/Helpers/Stack.swift

@@ -49,9 +49,21 @@ public struct Stack<Element> {
     }
 }
 
-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)
     }
 }
 

Source/SwiftLintCore/Visitors/DeclaredIdentifiersTrackingVisitor.swift

@@ -132,6 +132,13 @@ open class DeclaredIdentifiersTrackingVisitor<Configuration: RuleConfiguration>:
         }
     }
 
+    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) {