Add new `redundant_self_in_closure` rule (#4911)

.swiftlint.yml

@@ -40,6 +40,7 @@ disabled_rules:
   - prefer_nimble
   - prefer_self_in_static_references
   - prefixed_toplevel_constant
+  - redundant_self_in_closure
   - required_deinit
   - self_binding
   - sorted_enum_cases

CHANGELOG.md

@@ -18,6 +18,17 @@
 * Add `sorted_enum_cases` rule which warns when enum cases are not sorted.  
   [kimdv](https://github.com/kimdv)
 
+* Add new `redundant_self_in_closure` rule that triggers in closures on
+  explicitly used `self` when it's actually not needed due to:
+  * Strongly captured `self` (`{ [self] in ... }`)
+  * Closure used in a struct declaration (`self` can always be omitted)
+  * Anonymous closures that are directly called (`{ ... }()`) as they are
+    definitly not escaping
+  * Weakly captured `self` with explicit unwrapping
+
+  [SimplyDanny](https://github.com/SimplyDanny)
+  [#59](https://github.com/realm/SwiftLint/issues/59)
+
 * Extend `xct_specific_matcher` rule to check for boolean asserts on (un)equal
   comparisons. The rule can be configured with the matchers that should trigger
   rule violations. By default, all matchers trigger, but that can be limited to

Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift

@@ -165,6 +165,7 @@ public let builtInRules: [Rule.Type] = [
     RedundantNilCoalescingRule.self,
     RedundantObjcAttributeRule.self,
     RedundantOptionalInitializationRule.self,
+    RedundantSelfInClosureRule.self,
     RedundantSetAccessControlRule.self,
     RedundantStringEnumValueRule.self,
     RedundantTypeAnnotationRule.self,

Source/SwiftLintBuiltInRules/Rules/Style/RedundantSelfInClosureRule.swift

@@ -0,0 +1,315 @@
+import SwiftSyntax
+
+struct RedundantSelfInClosureRule: SwiftSyntaxRule, CorrectableRule, ConfigurationProviderRule, OptInRule {
+    var configuration = SeverityConfiguration(.warning)
+
+    static var description = RuleDescription(
+        identifier: "redundant_self_in_closure",
+        name: "Redundant Self in Closure",
+        description: "Explicit use of 'self' is not required",
+        kind: .style,
+        nonTriggeringExamples: [
+            Example("""
+                struct S {
+                    var x = 0
+                    func f(_ work: @escaping () -> Void) { work() }
+                    func g() {
+                        f {
+                            x = 1
+                            f { x = 1 }
+                            g()
+                        }
+                    }
+                }
+            """),
+            Example("""
+                class C {
+                    var x = 0
+                    func f(_ work: @escaping () -> Void) { work() }
+                    func g() {
+                        f { [weak self] in
+                            self?.x = 1
+                            self?.g()
+                            guard let self = self ?? C() else { return }
+                            self?.x = 1
+                        }
+                        C().f { self.x = 1 }
+                        f { [weak self] in if let self { x = 1 } }
+                    }
+                }
+            """)
+        ],
+        triggeringExamples: [
+            Example("""
+                struct S {
+                    var x = 0
+                    func f(_ work: @escaping () -> Void) { work() }
+                    func g() {
+                        f {
+                            ↓self.x = 1
+                            if ↓self.x == 1 { ↓self.g() }
+                        }
+                    }
+                }
+            """),
+            Example("""
+                class C {
+                    var x = 0
+                    func g() {
+                        {
+                            ↓self.x = 1
+                            ↓self.g()
+                        }()
+                    }
+                }
+            """),
+            Example("""
+                class C {
+                    var x = 0
+                    func f(_ work: @escaping () -> Void) { work() }
+                    func g() {
+                        f { [self] in
+                            ↓self.x = 1
+                            ↓self.g()
+                            f { self.x = 1 }
+                        }
+                    }
+                }
+            """),
+            Example("""
+                class C {
+                    var x = 0
+                    func f(_ work: @escaping () -> Void) { work() }
+                    func g() {
+                        f { [unowned self] in ↓self.x = 1 }
+                        f { [self = self] in ↓self.x = 1 }
+                        f { [s = self] in s.x = 1 }
+                    }
+                }
+            """)
+        ] + triggeringCompilerSpecificExamples,
+        corrections: [
+            Example("""
+                struct S {
+                    var x = 0
+                    func f(_ work: @escaping () -> Void) { work() }
+                    func g() {
+                        f {
+                            ↓self.x = 1
+                            if ↓self.x == 1 { ↓self.g() }
+                        }
+                    }
+                }
+            """): Example("""
+                struct S {
+                    var x = 0
+                    func f(_ work: @escaping () -> Void) { work() }
+                    func g() {
+                        f {
+                            x = 1
+                            if x == 1 { g() }
+                        }
+                    }
+                }
+            """)
+        ]
+    )
+
+#if compiler(>=5.8)
+    private static let triggeringCompilerSpecificExamples = [
+        Example("""
+            class C {
+                var x = 0
+                func f(_ work: @escaping () -> Void) { work() }
+                func g() {
+                    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 else { ↓self.x = 1 }
+                        self?.x = 1
+                    }
+                    f { [weak self] in
+                        self?.x = 1
+                        while let self else { ↓self.x = 1 }
+                        self?.x = 1
+                    }
+                }
+            }
+        """)
+    ]
+#else
+    private static let triggeringCompilerSpecificExamples = [Example]()
+#endif
+
+    func makeVisitor(file: SwiftLintFile) -> ViolationsSyntaxVisitor {
+        ScopeVisitor(viewMode: .sourceAccurate)
+    }
+
+    func correct(file: SwiftLintFile) -> [Correction] {
+        let ranges = ScopeVisitor(viewMode: .sourceAccurate)
+            .walk(file: file, handler: \.corrections)
+            .compactMap { file.stringView.NSRange(start: $0.start, end: $0.end) }
+            .filter { file.ruleEnabled(violatingRange: $0, for: self) != nil }
+            .reversed()
+
+        var corrections = [Correction]()
+        var contents = file.contents
+        for range in ranges {
+            let contentsNSString = contents.bridge()
+            contents = contentsNSString.replacingCharacters(in: range, with: "")
+            let location = Location(file: file, characterOffset: range.location)
+            corrections.append(Correction(ruleDescription: Self.description, location: location))
+        }
+
+        file.write(contents)
+
+        return 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 class ScopeVisitor: ViolationsSyntaxVisitor {
+    private var typeDeclarations = Stack<TypeDeclarationKind>()
+    private var functionCalls = Stack<FunctionCallType>()
+    private var selfCaptures = Stack<SelfCaptureKind>()
+
+    private(set) var corrections = [(start: AbsolutePosition, end: AbsolutePosition)]()
+
+    override var skippableDeclarations: [DeclSyntaxProtocol.Type] { .extensionsAndProtocols }
+
+    override func visit(_ node: ActorDeclSyntax) -> SyntaxVisitorContinueKind {
+        typeDeclarations.push(.likeClass)
+        return .visitChildren
+    }
+
+    override func visitPost(_ node: ActorDeclSyntax) {
+        typeDeclarations.pop()
+    }
+
+    override func visit(_ node: ClassDeclSyntax) -> SyntaxVisitorContinueKind {
+        typeDeclarations.push(.likeClass)
+        return .visitChildren
+    }
+
+    override func visitPost(_ node: 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 localCorrections = ExplicitSelfVisitor(
+            typeDeclarationKind: activeTypeDeclarationKind,
+            functionCallType: activeFunctionCallType,
+            selfCaptureKind: activeSelfCaptureKind
+        ).walk(tree: node.statements, handler: \.corrections)
+        violations.append(contentsOf: localCorrections.map(\.start))
+        corrections.append(contentsOf: localCorrections)
+        selfCaptures.pop()
+    }
+
+    override func visit(_ node: EnumDeclSyntax) -> SyntaxVisitorContinueKind {
+        typeDeclarations.push(.likeStruct)
+        return .visitChildren
+    }
+
+    override func visitPost(_ node: 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(_ node: FunctionCallExprSyntax) {
+        functionCalls.pop()
+    }
+
+    override func visit(_ node: StructDeclSyntax) -> SyntaxVisitorContinueKind {
+        typeDeclarations.push(.likeStruct)
+        return .visitChildren
+    }
+
+    override func visitPost(_ node: StructDeclSyntax) {
+        typeDeclarations.pop()
+    }
+}
+
+private class ExplicitSelfVisitor: ViolationsSyntaxVisitor {
+    private let typeDeclKind: TypeDeclarationKind
+    private let functionCallType: FunctionCallType
+    private let selfCaptureKind: SelfCaptureKind
+
+    private(set) var corrections = [(start: AbsolutePosition, end: AbsolutePosition)]()
+
+    init(typeDeclarationKind: TypeDeclarationKind,
+         functionCallType: FunctionCallType,
+         selfCaptureKind: SelfCaptureKind) {
+        self.typeDeclKind = typeDeclarationKind
+        self.functionCallType = functionCallType
+        self.selfCaptureKind = selfCaptureKind
+        super.init(viewMode: .sourceAccurate)
+    }
+
+    override func visitPost(_ node: MemberAccessExprSyntax) {
+        if node.base?.as(IdentifierExprSyntax.self)?.isSelf == true, isSelfRedundant {
+            corrections.append(
+                (start: node.positionAfterSkippingLeadingTrivia, end: node.dot.endPositionBeforeTrailingTrivia)
+            )
+        }
+    }
+
+    override func visit(_ node: ClosureExprSyntax) -> SyntaxVisitorContinueKind {
+        // Will be handled separately by the parent visitor.
+        .skipChildren
+    }
+
+    var isSelfRedundant: Bool {
+        if typeDeclKind == .likeStruct || functionCallType == .anonymousClosure {
+            return true
+        }
+        if selfCaptureKind == .strong && SwiftVersion.current >= .fiveDotThree {
+            return true
+        }
+        if selfCaptureKind == .weak && SwiftVersion.current >= .fiveDotEight {
+            return true
+        }
+        return false
+    }
+}

Source/SwiftLintCore/Extensions/SwiftSyntax+SwiftLint.swift

@@ -328,7 +328,6 @@ public extension IntegerLiteralExprSyntax {
         guard case let .integerLiteral(number) = digits.tokenKind else {
             return false
         }
-
         return number.isZero
     }
 }
@@ -338,11 +337,32 @@ public extension FloatLiteralExprSyntax {
         guard case let .floatingLiteral(number) = floatingDigits.tokenKind else {
             return false
         }
-
         return number.isZero
     }
 }
 
+public extension IdentifierExprSyntax {
+    var isSelf: Bool {
+        identifier.text == "self"
+    }
+}
+
+public extension MemberAccessExprSyntax {
+    var isSelfAccess: Bool {
+        base?.as(IdentifierExprSyntax.self)?.isSelf == true
+    }
+}
+
+public extension ClosureCaptureItemSyntax {
+    var capturesSelf: Bool {
+        expression.as(IdentifierExprSyntax.self)?.isSelf == true
+    }
+
+    var capturesWeakly: Bool {
+        specifier?.specifier.text == "weak"
+    }
+}
+
 private extension String {
     var isZero: Bool {
         if self == "0" { // fast path

Source/SwiftLintCore/Models/SwiftVersion.swift

@@ -33,6 +33,8 @@ public extension SwiftVersion {
     static let fiveDotSix = SwiftVersion(rawValue: "5.6.0")
     /// Swift 5.7.x - https://swift.org/download/#swift-57
     static let fiveDotSeven = SwiftVersion(rawValue: "5.7.0")
+    /// Swift 5.8.x - https://swift.org/download/#swift-58
+    static let fiveDotEight = SwiftVersion(rawValue: "5.8.0")
 
     /// The current detected Swift compiler version, based on the currently accessible SourceKit version.
     ///

Tests/GeneratedTests/GeneratedTests.swift

@@ -980,6 +980,12 @@ class RedundantOptionalInitializationRuleGeneratedTests: SwiftLintTestCase {
     }
 }
 
+class RedundantSelfInClosureRuleGeneratedTests: SwiftLintTestCase {
+    func testWithDefaultConfiguration() {
+        verifyRule(RedundantSelfInClosureRule.description)
+    }
+}
+
 class RedundantSetAccessControlRuleGeneratedTests: SwiftLintTestCase {
     func testWithDefaultConfiguration() {
         verifyRule(RedundantSetAccessControlRule.description)