Rewrite `explicit_type_interface` rule with SwiftSyntax fixing a false-positive (#4638)

CHANGELOG.md

@@ -39,6 +39,11 @@
   [Martin Redington](https://github.com/mildm8nnered)
   [#3712](https://github.com/realm/SwiftLint/issues/3712)
 
+* Rewrite `explicit_type_interface` rule with SwiftSyntax fixing a
+  false-positive in if-case-let statements.  
+  [SimplyDanny](https://github.com/SimplyDanny)
+  [#4548](https://github.com/realm/SwiftLint/issues/4548)
+
 ## 0.50.3: Bundle of Towels
 
 #### Breaking

Source/SwiftLintFramework/Rules/Idiomatic/ExplicitTypeInterfaceRule.swift

@@ -1,7 +1,6 @@
-import Foundation
+import SwiftSyntax
-import SourceKittenFramework
 
-struct ExplicitTypeInterfaceRule: OptInRule, ConfigurationProviderRule {
+struct ExplicitTypeInterfaceRule: OptInRule, ConfigurationProviderRule, SwiftSyntaxRule {
     var configuration = ExplicitTypeInterfaceConfiguration()
 
     init() {}
@@ -19,7 +18,7 @@ struct ExplicitTypeInterfaceRule: OptInRule, ConfigurationProviderRule {
             """),
             Example("""
             class Foo {
-              let myVar: Int? = 0
+              let myVar: Int? = 0, s: String = ""
             }
             """),
             Example("""
@@ -31,204 +30,115 @@ struct ExplicitTypeInterfaceRule: OptInRule, ConfigurationProviderRule {
             class Foo {
               class var myVar: Int? = 0
             }
-            """)
+            """),
+            Example("""
+            func f() {
+                if case .failure(let error) = errorCompletion {}
+            }
+            """, excludeFromDocumentation: true)
         ],
         triggeringExamples: [
             Example("""
             class Foo {
-              ↓var myVar = 0
+              var ↓myVar = 0
             }
             """),
             Example("""
             class Foo {
-              ↓let mylet = 0
+              let ↓mylet = 0
             }
             """),
             Example("""
             class Foo {
-              ↓static var myStaticVar = 0
+              static var ↓myStaticVar = 0
             }
             """),
             Example("""
             class Foo {
-              ↓class var myClassVar = 0
+              class var ↓myClassVar = 0
             }
             """),
             Example("""
             class Foo {
-              ↓let myVar = Int(0)
+              let ↓myVar = Int(0), ↓s = ""
             }
             """),
             Example("""
             class Foo {
-              ↓let myVar = Set<Int>(0)
+              let ↓myVar = Set<Int>(0)
             }
             """)
         ]
     )
 
-    func validate(file: SwiftLintFile) -> [StyleViolation] {
+    func makeVisitor(file: SwiftLintFile) -> ViolationsSyntaxVisitor {
-        let captureGroupRanges = Lazy(self.captureGroupRanges(in: file))
+        Visitor(configuration: configuration)
-        return file.structureDictionary.traverseWithParentsDepthFirst { parents, subDict in
+    }
-            guard let kind = subDict.declarationKind,
+}
-                  let parent = parents.lastIgnoringCallAndArgument() else {
+
-                return nil
+private class Visitor: ViolationsSyntaxVisitor {
+    let configuration: ExplicitTypeInterfaceConfiguration
+
+    override var skippableDeclarations: [DeclSyntaxProtocol.Type] { [ProtocolDeclSyntax.self] }
+
+    init(configuration: ExplicitTypeInterfaceConfiguration) {
+        self.configuration = configuration
+        super.init(viewMode: .sourceAccurate)
+    }
+
+    override func visitPost(_ node: VariableDeclSyntax) {
+        if node.modifiers.isClass {
+            if configuration.allowedKinds.contains(.class) {
+                checkViolation(node)
+            }
+        } else if node.modifiers.isStatic {
+            if configuration.allowedKinds.contains(.static) {
+                checkViolation(node)
+            }
+        } else if node.parent?.is(MemberDeclListItemSyntax.self) == true {
+            if configuration.allowedKinds.contains(.instance) {
+                checkViolation(node)
+            }
+        } else if node.parent?.is(CodeBlockItemSyntax.self) == true {
+            if configuration.allowedKinds.contains(.local) {
+                checkViolation(node)
             }
-            return validate(file: file, kind: kind, dictionary: subDict, parentStructure: parent,
-                            captureGroupRanges: captureGroupRanges.value)
         }
     }
 
-    private func validate(file: SwiftLintFile,
+    private func checkViolation(_ node: VariableDeclSyntax) {
-                          kind: SwiftDeclarationKind,
+        for binding in node.bindings {
-                          dictionary: SourceKittenDictionary,
+            if configuration.allowRedundancy, let initializer = binding.initializer,
-                          parentStructure: SourceKittenDictionary,
+               initializer.isTypeConstructor || initializer.isTypeReference {
-                          captureGroupRanges: [ByteRange]) -> [StyleViolation] {
+                continue
-        guard configuration.allowedKinds.contains(kind),
+            }
-            let offset = dictionary.offset,
+            if binding.typeAnnotation == nil {
-            !dictionary.containsType,
+                violations.append(binding.positionAfterSkippingLeadingTrivia)
-            (!configuration.allowRedundancy ||
-                (!dictionary.isInitCall(file: file) && !dictionary.isTypeReferenceAssignment(file: file))
-            ),
-            !parentStructure.contains([.forEach, .guard]),
-            !parentStructure.caseStatementPatternRanges.contains(offset),
-            !parentStructure.caseExpressionRanges.contains(offset),
-            !captureGroupRanges.contains(offset) else {
-                return []
-        }
-
-        return [
-            StyleViolation(ruleDescription: Self.description,
-                           severity: configuration.severityConfiguration.severity,
-                           location: Location(file: file, byteOffset: offset))
-        ]
-    }
-
-    private func captureGroupRanges(in file: SwiftLintFile) -> [ByteRange] {
-        return file.match(pattern: "\\{\\s*\\[(\\s*\\w+\\s+\\w+,*)+\\]", excludingSyntaxKinds: SyntaxKind.commentKinds)
-            .compactMap { file.stringView.NSRangeToByteRange(start: $0.location, length: $0.length) }
-    }
-}
-
-private extension SourceKittenDictionary {
-    var containsType: Bool {
-        return typeName != nil
-    }
-
-    func isInitCall(file: SwiftLintFile) -> Bool {
-        guard
-            let nameOffset = nameOffset,
-            let nameLength = nameLength,
-            case let afterNameByteRange = ByteRange(location: nameOffset + nameLength, length: 0),
-            let afterNameRange = file.stringView.byteRangeToNSRange(afterNameByteRange)
-        else {
-            return false
-        }
-
-        let contents = file.stringView
-        let contentAfterName = contents.nsString.substring(from: afterNameRange.location)
-        let initCallRegex =
-            regex("^\\s*=\\s*(?:try[!?]?\\s+)?\\[?\\p{Lu}[^\\(\\s<]*(?:<[^\\>]*>)?(?::\\s*[^\\(\\n]+)?\\]?\\(")
-
-        return initCallRegex.firstMatch(in: contentAfterName, options: [], range: contentAfterName.fullNSRange) != nil
-    }
-
-    func isTypeReferenceAssignment(file: SwiftLintFile) -> Bool {
-        guard
-            let nameOffset = nameOffset,
-            let nameLength = nameLength,
-            case let afterNameByteRange = ByteRange(location: nameOffset + nameLength, length: 0),
-            let afterNameRange = file.stringView.byteRangeToNSRange(afterNameByteRange)
-        else {
-            return false
-        }
-
-        let contents = file.stringView
-        let contentAfterName = contents.nsString.substring(from: afterNameRange.location)
-        let typeAssignment = regex("^\\s*=\\s*(?:\\p{Lu}[^\\(\\s<]*(?:<[^\\>]*>)?\\.)*self")
-
-        return typeAssignment.firstMatch(in: contentAfterName, options: [], range: contentAfterName.fullNSRange) != nil
-    }
-
-    var caseStatementPatternRanges: [ByteRange] {
-        return ranges(with: StatementKind.case.rawValue, for: "source.lang.swift.structure.elem.pattern")
-    }
-
-    var caseExpressionRanges: [ByteRange] {
-        return ranges(with: SwiftExpressionKind.tuple.rawValue, for: "source.lang.swift.structure.elem.expr")
-    }
-
-    func contains(_ statements: Set<StatementKind>) -> Bool {
-        guard let statement = statementKind else {
-            return false
-        }
-        return statements.contains(statement)
-    }
-
-    func ranges(with parentKind: String, for elementKind: String) -> [ByteRange] {
-        guard parentKind == kind else {
-            return []
-        }
-
-        return elements
-            .filter { elementKind == $0.kind }
-            .compactMap { $0.byteRange }
-    }
-}
-
-private extension Collection where Element == ByteRange {
-    func contains(_ index: ByteCount) -> Bool {
-        return contains { $0.contains(index) }
-    }
-}
-
-private extension SourceKittenDictionary {
-    func traverseWithParentsDepthFirst<T>(traverseBlock: ([SourceKittenDictionary], SourceKittenDictionary) -> [T]?)
-        -> [T] {
-        var result: [T] = []
-        traverseWithParentDepthFirst(collectingValuesInto: &result,
-                                     parents: [],
-                                     traverseBlock: traverseBlock)
-        return result
-    }
-
-    private func traverseWithParentDepthFirst<T>(
-        collectingValuesInto array: inout [T],
-        parents: [SourceKittenDictionary],
-        traverseBlock: ([SourceKittenDictionary], SourceKittenDictionary) -> [T]?) {
-        var updatedParents = parents
-        updatedParents.append(self)
-
-        substructure.forEach { subDict in
-            subDict.traverseWithParentDepthFirst(collectingValuesInto: &array,
-                                                 parents: updatedParents,
-                                                 traverseBlock: traverseBlock)
-
-            if let collectedValues = traverseBlock(updatedParents, subDict) {
-                array += collectedValues
             }
         }
     }
 }
 
-private extension Array where Element == SourceKittenDictionary {
+private extension InitializerClauseSyntax {
-    func lastIgnoringCallAndArgument() -> Element? {
+    var isTypeConstructor: Bool {
-        guard SwiftVersion.current >= .fiveDotFour else {
+        if value.as(FunctionCallExprSyntax.self)?.callsPotentialType == true {
-            return last
+            return true
         }
+        if let tryExpr = value.as(TryExprSyntax.self),
+           tryExpr.expression.as(FunctionCallExprSyntax.self)?.callsPotentialType == true {
+            return true
+        }
+        return false
+    }
 
-        return last { element in
+    var isTypeReference: Bool {
-            element.expressionKind != .call && element.expressionKind != .argument
+        value.as(MemberAccessExprSyntax.self)?.name.tokenKind == .selfKeyword
-        }
     }
 }
 
-// extracted from https://forums.swift.org/t/pitch-declaring-local-variables-as-lazy/9287/3
+private extension FunctionCallExprSyntax {
-private class Lazy<Result> {
+    var callsPotentialType: Bool {
-    private var computation: () -> Result
+        let name = calledExpression.debugDescription
-    fileprivate private(set) lazy var value: Result = computation()
+        return name.first?.isUppercase == true || (name.first == "[" && name.last == "]")
-
-    init(_ computation: @escaping @autoclosure () -> Result) {
-        self.computation = computation
     }
 }

Source/SwiftLintFramework/Rules/RuleConfigurations/ExplicitTypeInterfaceConfiguration.swift

@@ -1,59 +1,23 @@
-import SourceKittenFramework
+struct ExplicitTypeInterfaceConfiguration: SeverityBasedRuleConfiguration, Equatable {
+    enum VariableKind: String, CaseIterable {
+        case instance
+        case local
+        case `static`
+        case `class`
 
-private enum VariableKind: String {
+        static let all = Set(allCases)
-    case instance
-    case local
-    case `static`
-    case `class`
-}
-
-private extension SwiftDeclarationKind {
-    init(variableKind: VariableKind) {
-        switch variableKind {
-        case .instance:
-            self = .varInstance
-        case .local:
-            self = .varLocal
-        case .static:
-            self = .varStatic
-        case .class:
-            self = .varClass
-        }
     }
 
-    var variableKind: VariableKind? {
-        switch self {
-        case .varInstance:
-            return .instance
-        case .varLocal:
-            return .local
-        case .varStatic:
-            return .static
-        case .varClass:
-            return .class
-        default:
-            return nil
-        }
-    }
-}
-
-struct ExplicitTypeInterfaceConfiguration: RuleConfiguration, Equatable {
-    private static let variableKinds: Set<SwiftDeclarationKind> = [.varInstance,
-                                                                   .varLocal,
-                                                                   .varStatic,
-                                                                   .varClass]
-
     private(set) var severityConfiguration = SeverityConfiguration(.warning)
 
-    private(set) var allowedKinds = Self.variableKinds
+    private(set) var allowedKinds = VariableKind.all
 
     private(set) var allowRedundancy = false
 
     var consoleDescription: String {
-        let excludedKinds = Self.variableKinds.subtracting(allowedKinds)
+        let excludedKinds = VariableKind.all.subtracting(allowedKinds).map(\.rawValue).sorted()
-        let simplifiedExcludedKinds = excludedKinds.compactMap { $0.variableKind?.rawValue }.sorted()
         return severityConfiguration.consoleDescription +
-            ", excluded: \(simplifiedExcludedKinds)" +
+            ", excluded: \(excludedKinds)" +
             ", allow_redundancy: \(allowRedundancy)"
     }
 
@@ -68,8 +32,7 @@ struct ExplicitTypeInterfaceConfiguration: RuleConfiguration, Equatable {
             case ("severity", let severityString as String):
                 try severityConfiguration.apply(configuration: severityString)
             case ("excluded", let excludedStrings as [String]):
-                let excludedKinds = excludedStrings.compactMap(VariableKind.init(rawValue:))
+                allowedKinds.subtract(excludedStrings.compactMap(VariableKind.init(rawValue:)))
-                allowedKinds.subtract(excludedKinds.map(SwiftDeclarationKind.init(variableKind:)))
             case ("allow_redundancy", let allowRedundancy as Bool):
                 self.allowRedundancy = allowRedundancy
             default:

Tests/SwiftLintFrameworkTests/ExplicitTypeInterfaceConfigurationTests.swift

@@ -5,7 +5,7 @@ class ExplicitTypeInterfaceConfigurationTests: XCTestCase {
     func testDefaultConfiguration() {
         let config = ExplicitTypeInterfaceConfiguration()
         XCTAssertEqual(config.severityConfiguration.severity, .warning)
-        XCTAssertEqual(config.allowedKinds, Set([.varInstance, .varClass, .varStatic, .varLocal]))
+        XCTAssertEqual(config.allowedKinds, Set([.instance, .class, .static, .local]))
     }
 
     func testApplyingCustomConfiguration() throws {
@@ -14,7 +14,7 @@ class ExplicitTypeInterfaceConfigurationTests: XCTestCase {
                                          "excluded": ["local"],
                                          "allow_redundancy": true])
         XCTAssertEqual(config.severityConfiguration.severity, .error)
-        XCTAssertEqual(config.allowedKinds, Set([.varInstance, .varClass, .varStatic]))
+        XCTAssertEqual(config.allowedKinds, Set([.instance, .class, .static]))
         XCTAssertTrue(config.allowRedundancy)
     }
 

Tests/SwiftLintFrameworkTests/ExplicitTypeInterfaceRuleTests.swift

@@ -14,11 +14,11 @@ class ExplicitTypeInterfaceRuleTests: XCTestCase {
             """)
         ]
         let triggeringExamples = [
-            Example("func foo() {\n↓let intVal = 1\n}"),
+            Example("func foo() {\nlet ↓intVal = 1\n}"),
             Example("""
             func foo() {
                 bar {
-                    ↓let x = 1
+                    let ↓x = 1
                 }
             }
             """)
@@ -48,9 +48,9 @@ class ExplicitTypeInterfaceRuleTests: XCTestCase {
             Example("class Foo {\n  static let myStaticLet = 0\n}\n")
         ]
         let triggeringExamples: [Example] = [
-            Example("class Foo {\n  ↓var myVar = 0\n\n}\n"),
+            Example("class Foo {\n  var ↓myVar = 0\n\n}\n"),
-            Example("class Foo {\n  ↓let mylet = 0\n\n}\n"),
+            Example("class Foo {\n  let ↓mylet = 0\n\n}\n"),
-            Example("class Foo {\n  ↓class var myClassVar = 0\n}\n")
+            Example("class Foo {\n  class var ↓myClassVar = 0\n}\n")
         ]
         let description = ExplicitTypeInterfaceRule.description
             .with(triggeringExamples: triggeringExamples)
@@ -76,12 +76,12 @@ class ExplicitTypeInterfaceRuleTests: XCTestCase {
             Example("class Foo {\n  let l10n = L10n.Communication.self\n}\n")
         ]
         let triggeringExamples: [Example] = [
-            Example("class Foo {\n  ↓var myVar = 0\n\n}\n"),
+            Example("class Foo {\n  var ↓myVar = 0\n\n}\n"),
-            Example("class Foo {\n  ↓let mylet = 0\n\n}\n"),
+            Example("class Foo {\n  let ↓mylet = 0\n\n}\n"),
-            Example("class Foo {\n  ↓static var myStaticVar = 0\n}\n"),
+            Example("class Foo {\n  static var ↓myStaticVar = 0\n}\n"),
-            Example("class Foo {\n  ↓class var myClassVar = 0\n}\n"),
+            Example("class Foo {\n  class var ↓myClassVar = 0\n}\n"),
-            Example("class Foo {\n  ↓let array = [\"foo\", \"bar\"]\n}\n"),
+            Example("class Foo {\n  let ↓array = [\"foo\", \"bar\"]\n}\n"),
-            Example("class Foo {\n  ↓let dict = [\"foo\": \"bar\"]\n}\n")
+            Example("class Foo {\n  let ↓dict = [\"foo\": \"bar\"]\n}\n")
         ]
         let description = ExplicitTypeInterfaceRule.description
             .with(triggeringExamples: triggeringExamples)
@@ -211,8 +211,8 @@ class ExplicitTypeInterfaceRuleTests: XCTestCase {
             func bar() {
                 let foo: Foo = .success(1)
                 switch foo {
-                case .failure(let error): ↓let fooBar = 1
+                case .failure(let error): let ↓fooBar = 1
-                case .success(let result): ↓let fooBar = 1
+                case .success(let result): let ↓fooBar = 1
                 }
             }
             """),
@@ -223,8 +223,8 @@ class ExplicitTypeInterfaceRuleTests: XCTestCase {
             func foo() {
                 let foo: Foo = .failure(1, 1)
                 switch foo {
-                case var .failure(x, y): ↓let fooBar = 1
+                case var .failure(x, y): let ↓fooBar = 1
-                default: ↓let fooBar = 1
+                default: let ↓fooBar = 1
                 }
             }
             """)