diff --git a/.swiftlint.yml b/.swiftlint.yml index 4007569390..143ad4445f 100644 --- a/.swiftlint.yml +++ b/.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 diff --git a/Source/SwiftLintBuiltInRules/Rules/Style/RedundantSelfInClosureRule.swift b/Source/SwiftLintBuiltInRules/Rules/Style/RedundantSelfInClosureRule.swift index 49b1f55106..5014b1e081 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Style/RedundantSelfInClosureRule.swift +++ b/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: 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: "" - ) - ) + 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 + } } } diff --git a/Source/SwiftLintBuiltInRules/Rules/Style/RedundantSelfInClosureRuleExamples.swift b/Source/SwiftLintBuiltInRules/Rules/Style/RedundantSelfInClosureRuleExamples.swift index 9923ab5271..0f3582bd76 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Style/RedundantSelfInClosureRuleExamples.swift +++ b/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 = [ diff --git a/Source/SwiftLintCore/Helpers/Stack.swift b/Source/SwiftLintCore/Helpers/Stack.swift index 794919bf26..74a0e9725c 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 798bb2d502..0ba2d0ce5b 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) {