diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cd9126d7..9c3f63d7e 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/Source/SwiftLintFramework/Rules/Idiomatic/ExplicitTypeInterfaceRule.swift b/Source/SwiftLintFramework/Rules/Idiomatic/ExplicitTypeInterfaceRule.swift index 2ba7c60d1..7fbe39c49 100644 --- a/Source/SwiftLintFramework/Rules/Idiomatic/ExplicitTypeInterfaceRule.swift +++ b/Source/SwiftLintFramework/Rules/Idiomatic/ExplicitTypeInterfaceRule.swift @@ -1,7 +1,6 @@ -import Foundation -import SourceKittenFramework +import SwiftSyntax -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(0) + let ↓myVar = Set(0) } """) ] ) - func validate(file: SwiftLintFile) -> [StyleViolation] { - let captureGroupRanges = Lazy(self.captureGroupRanges(in: file)) - return file.structureDictionary.traverseWithParentsDepthFirst { parents, subDict in - guard let kind = subDict.declarationKind, - let parent = parents.lastIgnoringCallAndArgument() else { - return nil + func makeVisitor(file: SwiftLintFile) -> ViolationsSyntaxVisitor { + Visitor(configuration: configuration) + } +} + +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, - kind: SwiftDeclarationKind, - dictionary: SourceKittenDictionary, - parentStructure: SourceKittenDictionary, - captureGroupRanges: [ByteRange]) -> [StyleViolation] { - guard configuration.allowedKinds.contains(kind), - let offset = dictionary.offset, - !dictionary.containsType, - (!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) -> 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(traverseBlock: ([SourceKittenDictionary], SourceKittenDictionary) -> [T]?) - -> [T] { - var result: [T] = [] - traverseWithParentDepthFirst(collectingValuesInto: &result, - parents: [], - traverseBlock: traverseBlock) - return result - } - - private func traverseWithParentDepthFirst( - 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 func checkViolation(_ node: VariableDeclSyntax) { + for binding in node.bindings { + if configuration.allowRedundancy, let initializer = binding.initializer, + initializer.isTypeConstructor || initializer.isTypeReference { + continue + } + if binding.typeAnnotation == nil { + violations.append(binding.positionAfterSkippingLeadingTrivia) } } } } -private extension Array where Element == SourceKittenDictionary { - func lastIgnoringCallAndArgument() -> Element? { - guard SwiftVersion.current >= .fiveDotFour else { - return last +private extension InitializerClauseSyntax { + var isTypeConstructor: Bool { + if value.as(FunctionCallExprSyntax.self)?.callsPotentialType == true { + return true } + if let tryExpr = value.as(TryExprSyntax.self), + tryExpr.expression.as(FunctionCallExprSyntax.self)?.callsPotentialType == true { + return true + } + return false + } - return last { element in - element.expressionKind != .call && element.expressionKind != .argument - } + var isTypeReference: Bool { + value.as(MemberAccessExprSyntax.self)?.name.tokenKind == .selfKeyword } } -// extracted from https://forums.swift.org/t/pitch-declaring-local-variables-as-lazy/9287/3 -private class Lazy { - private var computation: () -> Result - fileprivate private(set) lazy var value: Result = computation() - - init(_ computation: @escaping @autoclosure () -> Result) { - self.computation = computation +private extension FunctionCallExprSyntax { + var callsPotentialType: Bool { + let name = calledExpression.debugDescription + return name.first?.isUppercase == true || (name.first == "[" && name.last == "]") } } diff --git a/Source/SwiftLintFramework/Rules/RuleConfigurations/ExplicitTypeInterfaceConfiguration.swift b/Source/SwiftLintFramework/Rules/RuleConfigurations/ExplicitTypeInterfaceConfiguration.swift index 9d9587cf5..9f87033cc 100644 --- a/Source/SwiftLintFramework/Rules/RuleConfigurations/ExplicitTypeInterfaceConfiguration.swift +++ b/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 { - 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 - } + static let all = Set(allCases) } - 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 = [.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 simplifiedExcludedKinds = excludedKinds.compactMap { $0.variableKind?.rawValue }.sorted() + let excludedKinds = VariableKind.all.subtracting(allowedKinds).map(\.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(excludedKinds.map(SwiftDeclarationKind.init(variableKind:))) + allowedKinds.subtract(excludedStrings.compactMap(VariableKind.init(rawValue:))) case ("allow_redundancy", let allowRedundancy as Bool): self.allowRedundancy = allowRedundancy default: diff --git a/Tests/SwiftLintFrameworkTests/ExplicitTypeInterfaceConfigurationTests.swift b/Tests/SwiftLintFrameworkTests/ExplicitTypeInterfaceConfigurationTests.swift index cf3ba03d4..568fcfad4 100644 --- a/Tests/SwiftLintFrameworkTests/ExplicitTypeInterfaceConfigurationTests.swift +++ b/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) } diff --git a/Tests/SwiftLintFrameworkTests/ExplicitTypeInterfaceRuleTests.swift b/Tests/SwiftLintFrameworkTests/ExplicitTypeInterfaceRuleTests.swift index c23f1a212..0ba583f2f 100644 --- a/Tests/SwiftLintFrameworkTests/ExplicitTypeInterfaceRuleTests.swift +++ b/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 ↓let mylet = 0\n\n}\n"), - Example("class Foo {\n ↓class var myClassVar = 0\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 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 ↓let mylet = 0\n\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 ↓let array = [\"foo\", \"bar\"]\n}\n"), - Example("class Foo {\n ↓let dict = [\"foo\": \"bar\"]\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 static var ↓myStaticVar = 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 ↓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 .success(let result): ↓let fooBar = 1 + case .failure(let error): 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 - default: ↓let fooBar = 1 + case var .failure(x, y): let ↓fooBar = 1 + default: let ↓fooBar = 1 } } """)