diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c8062c8f6..cf6cc96e4d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -72,6 +72,12 @@ [Muhammad Zeeshan](https://github.com/mzeeshanid) [#2802](https://github.com/realm/SwiftLint/issues/2802) +* Add new `ignore_attributes` option to `redundant_type_annotation` rule + that allows disabling the rule for properties that are marked with at least + one of the configured attributes. + [tonell-m](https://github.com/tonell-m) + [#5366](https://github.com/realm/SwiftLint/issues/5366) + * Rewrite the following rules with SwiftSyntax: * `explicit_acl` * `extension_access_modifier` @@ -83,6 +89,7 @@ * `nimble_operator` * `opening_brace` * `orphaned_doc_comment` + * `redundant_type_annotation` * `trailing_closure` * `void_return` @@ -91,6 +98,7 @@ [Marcelo Fabri](https://github.com/marcelofabri) [swiftty](https://github.com/swiftty) [KS1019](https://github.com/KS1019) + [tonell-m](https://github.com/tonell-m) * Print invalid keys when configuration parsing fails. [SimplyDanny](https://github.com/SimplyDanny) diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/RedundantTypeAnnotationRule.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/RedundantTypeAnnotationRule.swift index 130066ede8..c797fa5aab 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/RedundantTypeAnnotationRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/RedundantTypeAnnotationRule.swift @@ -1,8 +1,9 @@ -import Foundation -import SourceKittenFramework +import SwiftLintCore +import SwiftSyntax -struct RedundantTypeAnnotationRule: OptInRule, SubstitutionCorrectableRule { - var configuration = SeverityConfiguration(.warning) +@SwiftSyntaxRule +struct RedundantTypeAnnotationRule: OptInRule, SwiftSyntaxCorrectableRule { + var configuration = RedundantTypeAnnotationConfiguration() static let description = RuleDescription( identifier: "redundant_type_annotation", @@ -12,7 +13,23 @@ struct RedundantTypeAnnotationRule: OptInRule, SubstitutionCorrectableRule { nonTriggeringExamples: [ Example("var url = URL()"), Example("var url: CustomStringConvertible = URL()"), - Example("@IBInspectable var color: UIColor = UIColor.white"), + Example("var one: Int = 1, two: Int = 2, three: Int"), + Example("guard let url = URL() else { return }"), + Example("if let url = URL() { return }"), + Example("let alphanumerics = CharacterSet.alphanumerics"), + Example("var set: Set = Set([])"), + Example("var set: Set = Set.init([])"), + Example("var set = Set([])"), + Example("var set = Set.init([])"), + Example("guard var set: Set = Set([]) else { return }"), + Example("if var set: Set = Set.init([]) { return }"), + Example("guard var set = Set([]) else { return }"), + Example("if var set = Set.init([]) { return }"), + Example("var one: A = B()"), + Example("var one: A = B()"), + Example("var one: A = B()"), + Example("let a = A.b.c.d"), + Example("let a: B = A.b.c.d"), Example(""" enum Direction { case up @@ -28,7 +45,14 @@ struct RedundantTypeAnnotationRule: OptInRule, SubstitutionCorrectableRule { } var direction = Direction.up - """) + """), + Example("@IgnoreMe var a: Int = Int(5)", configuration: ["ignore_attributes": ["IgnoreMe"]]), + Example(""" + var a: Int { + @IgnoreMe let i: Int = Int(1) + return i + } + """, configuration: ["ignore_attributes": ["IgnoreMe"]]) ], triggeringExamples: [ Example("var url↓:URL=URL()"), @@ -36,7 +60,23 @@ struct RedundantTypeAnnotationRule: OptInRule, SubstitutionCorrectableRule { Example("var url↓: URL = URL()"), Example("let url↓: URL = URL()"), Example("lazy var url↓: URL = URL()"), + Example("let url↓: URL = URL()!"), + Example("var one: Int = 1, two↓: Int = Int(5), three: Int"), + Example("guard let url↓: URL = URL() else { return }"), + Example("if let url↓: URL = URL() { return }"), Example("let alphanumerics↓: CharacterSet = CharacterSet.alphanumerics"), + Example("var set↓: Set = Set([])"), + Example("var set↓: Set = Set.init([])"), + Example("var set↓: Set = Set([])"), + Example("var set↓: Set = Set.init([])"), + Example("guard var set↓: Set = Set([]) else { return }"), + Example("if var set↓: Set = Set.init([]) { return }"), + Example("guard var set↓: Set = Set([]) else { return }"), + Example("if var set↓: Set = Set.init([]) { return }"), + Example("var set↓: Set = Set([]), otherSet: Set"), + Example("var num↓: Int = Int.random(0..<10)"), + Example("let a↓: A = A.b.c.d"), + Example("let a↓: A = A.f().b"), Example(""" class ViewController: UIViewController { func someMethod() { @@ -52,13 +92,43 @@ struct RedundantTypeAnnotationRule: OptInRule, SubstitutionCorrectableRule { } var direction↓: Direction = Direction.up - """) + """), + Example("@DontIgnoreMe var a↓: Int = Int(5)", configuration: ["ignore_attributes": ["IgnoreMe"]]), + Example(""" + @IgnoreMe + var a: Int { + let i↓: Int = Int(1) + return i + } + """, configuration: ["ignore_attributes": ["IgnoreMe"]]) ], corrections: [ Example("var url↓: URL = URL()"): Example("var url = URL()"), Example("let url↓: URL = URL()"): Example("let url = URL()"), + Example("var one: Int = 1, two↓: Int = Int(5), three: Int"): + Example("var one: Int = 1, two = Int(5), three: Int"), + Example("guard let url↓: URL = URL() else { return }"): + Example("guard let url = URL() else { return }"), + Example("if let url↓: URL = URL() { return }"): + Example("if let url = URL() { return }"), Example("let alphanumerics↓: CharacterSet = CharacterSet.alphanumerics"): Example("let alphanumerics = CharacterSet.alphanumerics"), + Example("var set↓: Set = Set([])"): + Example("var set = Set([])"), + Example("var set↓: Set = Set.init([])"): + Example("var set = Set.init([])"), + Example("var set↓: Set = Set([])"): + Example("var set = Set([])"), + Example("var set↓: Set = Set.init([])"): + Example("var set = Set.init([])"), + Example("guard var set↓: Set = Set([]) else { return }"): + Example("guard var set = Set([]) else { return }"), + Example("if var set↓: Set = Set.init([]) { return }"): + Example("if var set = Set.init([]) { return }"), + Example("var set↓: Set = Set([]), otherSet: Set"): + Example("var set = Set([]), otherSet: Set"), + Example("let a↓: A = A.b.c.d"): + Example("let a = A.b.c.d"), Example(""" class ViewController: UIViewController { func someMethod() { @@ -72,97 +142,137 @@ struct RedundantTypeAnnotationRule: OptInRule, SubstitutionCorrectableRule { let myVar = Int(5) } } + """), + Example("var num: Int = Int.random(0..<10)"): Example("var num = Int.random(0..<10)"), + Example(""" + @IgnoreMe + var a: Int { + let i↓: Int = Int(1) + return i + } + """, configuration: ["ignore_attributes": ["IgnoreMe"]]): + Example(""" + @IgnoreMe + var a: Int { + let i = Int(1) + return i + } """) ] ) +} - func validate(file: SwiftLintFile) -> [StyleViolation] { - return violationRanges(in: file).map { range in - StyleViolation( - ruleDescription: Self.description, - severity: configuration.severity, - location: Location(file: file, characterOffset: range.location) - ) +private extension RedundantTypeAnnotationRule { + final class Visitor: ViolationsSyntaxVisitor { + override func visitPost(_ node: PatternBindingSyntax) { + if node.parentDoesNotContainIgnoredAttributes(for: configuration), + let typeAnnotation = node.typeAnnotation, + let initializer = node.initializer?.value, + typeAnnotation.isRedundant(with: initializer) { + violations.append(typeAnnotation.positionAfterSkippingLeadingTrivia) + violationCorrections.append(ViolationCorrection( + start: typeAnnotation.position, + end: typeAnnotation.endPositionBeforeTrailingTrivia, + replacement: "" + )) + } } - } - func substitution(for violationRange: NSRange, in file: SwiftLintFile) -> (NSRange, String)? { - return (violationRange, "") + override func visitPost(_ node: OptionalBindingConditionSyntax) { + if let typeAnnotation = node.typeAnnotation, + let initializer = node.initializer?.value, + typeAnnotation.isRedundant(with: initializer) { + violations.append(typeAnnotation.positionAfterSkippingLeadingTrivia) + violationCorrections.append(ViolationCorrection( + start: typeAnnotation.position, + end: typeAnnotation.endPositionBeforeTrailingTrivia, + replacement: "" + )) + } + } } +} - private let typeAnnotationPattern: String - private let expressionPattern: String - - init() { - typeAnnotationPattern = - ":\\s*" + // semicolon and any number of whitespaces - "\\w+" // type name - - expressionPattern = - "(var|let)" + // var or let - "\\s+" + // at least single whitespace - "\\w+" + // variable name - "\\s*" + // possible whitespaces - typeAnnotationPattern + - "\\s*=\\s*" + // assignment operator with possible surrounding whitespaces - "\\w+" + // assignee name (type or keyword) - "[\\(\\.]?" // possible opening parenthesis or dot - } +private extension TypeAnnotationSyntax { + func isRedundant(with initializerExpr: ExprSyntax) -> Bool { + // Extract type and type name from type annotation + guard let type = type.as(IdentifierTypeSyntax.self) else { + return false + } + let typeName = type.trimmedDescription - func violationRanges(in file: SwiftLintFile) -> [NSRange] { - return file - .match(pattern: expressionPattern) - .filter { - $0.1 == [.keyword, .identifier, .typeidentifier, .identifier] || - $0.1 == [.keyword, .identifier, .typeidentifier, .keyword] - } - .filter { !isFalsePositive(file: file, range: $0.0) } - .filter { !isIBInspectable(file: file, range: $0.0) } - .compactMap { - file.match(pattern: typeAnnotationPattern, - excludingSyntaxKinds: SyntaxKind.commentAndStringKinds, range: $0.0).first - } - } + var initializer = initializerExpr + if let forceUnwrap = initializer.as(ForceUnwrapExprSyntax.self) { + initializer = forceUnwrap.expression + } - private func isFalsePositive(file: SwiftLintFile, range: NSRange) -> Bool { - guard let typeNames = getPartsOfExpression(in: file, range: range) else { return false } + // If the initializer is a boolean expression, we consider using the `Bool` type + // annotation as redundant. + if initializer.is(BooleanLiteralExprSyntax.self) { + return typeName == "Bool" + } - let lhs = typeNames.variableTypeName - let rhs = typeNames.assigneeName + // If the initializer is a function call (generally a constructor or static builder), + // check if the base type is the same as the one from the type annotation. + if let functionCall = initializer.as(FunctionCallExprSyntax.self) { + if let calledExpression = functionCall.calledExpression.as(DeclReferenceExprSyntax.self) { + return calledExpression.baseName.text == typeName + } + // Parse generic arguments in the intializer if there are any (e.g. var s = Set(...)) + if let genericSpecialization = functionCall.calledExpression.as(GenericSpecializationExprSyntax.self) { + // In this case it should be considered redundant if the type name is the same in the type annotation + // E.g. var s: Set = Set() should trigger a violation + return genericSpecialization.expression.trimmedDescription == type.typeName + } - if lhs == rhs || (lhs == "Bool" && (rhs == "true" || rhs == "false")) { - return false + // If the function call is a member access expression, check if it is a violation + return isMemberAccessViolation(node: functionCall.calledExpression, type: type) } - return true - } - private func getPartsOfExpression( - in file: SwiftLintFile, range: NSRange - ) -> (variableTypeName: String, assigneeName: String)? { - let substring = file.stringView.substring(with: range) - let components = substring.components(separatedBy: "=") + // If the initializer is a member access, check if the base type name is the same as + // the type annotation + return isMemberAccessViolation(node: initializer, type: type) + } - guard - components.count == 2, - let variableTypeName = components[0].components(separatedBy: ":").last?.trimmingCharacters(in: .whitespaces) + /// Checks if the given node is a member access (i.e. an enum case or a static property or function) + /// and if so checks if the base type is the same as the given type name. + private func isMemberAccessViolation(node: ExprSyntax, type: IdentifierTypeSyntax) -> Bool { + guard let memberAccess = node.as(MemberAccessExprSyntax.self), + let base = memberAccess.base else { - return nil + // If the type is implicit, `base` will be nil, meaning there is no redundancy. + return false } - let charactersToTrimFromRhs = CharacterSet(charactersIn: ".(").union(.whitespaces) - let assigneeName = components[1].trimmingCharacters(in: charactersToTrimFromRhs) + // Parse generic arguments in the intializer if there are any (e.g. var s = Set(...)) + if let genericSpecialization = base.as(GenericSpecializationExprSyntax.self) { + // In this case it should be considered redundant if the type name is the same in the type annotation + // E.g. var s: Set = Set() should trigger a violation + return genericSpecialization.expression.trimmedDescription == type.typeName + } - return (variableTypeName, assigneeName) + // In the case of chained MemberAccessExprSyntax (e.g. let a: A = A.b.c), call this function recursively + // with the base sequence as root node (in this case A.b). + if base.is(MemberAccessExprSyntax.self) { + return isMemberAccessViolation(node: base, type: type) + } + // Same for FunctionCallExprSyntax ... + if let call = base.as(FunctionCallExprSyntax.self) { + return isMemberAccessViolation(node: call.calledExpression, type: type) + } + return base.trimmedDescription == type.trimmedDescription } +} - private func isIBInspectable(file: SwiftLintFile, range: NSRange) -> Bool { - guard - let byteRange = file.stringView.NSRangeToByteRange(start: range.location, length: range.length), - let dict = file.structureDictionary.structures(forByteOffset: byteRange.location).last, - let kind = dict.declarationKind, - SwiftDeclarationKind.variableKinds.contains(kind) - else { return false } - - return dict.enclosedSwiftAttributes.contains(.ibinspectable) +private extension PatternBindingSyntax { + /// Checks if none of the attributes flagged as ignored in the configuration + /// are set for this node's parent's parent, if it's a variable declaration + func parentDoesNotContainIgnoredAttributes(for configuration: RedundantTypeAnnotationConfiguration) -> Bool { + guard let variableDecl = parent?.parent?.as(VariableDeclSyntax.self) else { + return true + } + return configuration.ignoreAttributes.allSatisfy { + !variableDecl.attributes.contains(attributeNamed: $0) + } } } diff --git a/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/RedundantTypeAnnotationConfiguration.swift b/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/RedundantTypeAnnotationConfiguration.swift new file mode 100644 index 0000000000..01c22d6005 --- /dev/null +++ b/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/RedundantTypeAnnotationConfiguration.swift @@ -0,0 +1,11 @@ +import SwiftLintCore + +@AutoApply +struct RedundantTypeAnnotationConfiguration: SeverityBasedRuleConfiguration { + typealias Parent = RedundantTypeAnnotationRule + + @ConfigurationElement(key: "severity") + var severityConfiguration = SeverityConfiguration(.warning) + @ConfigurationElement(key: "ignore_attributes") + var ignoreAttributes = Set(["IBInspectable"]) +} diff --git a/Tests/IntegrationTests/default_rule_configurations.yml b/Tests/IntegrationTests/default_rule_configurations.yml index 7607bb6ab0..a8796526ee 100644 --- a/Tests/IntegrationTests/default_rule_configurations.yml +++ b/Tests/IntegrationTests/default_rule_configurations.yml @@ -456,6 +456,7 @@ redundant_string_enum_value: severity: warning redundant_type_annotation: severity: warning + ignore_attributes: ["IBInspectable"] redundant_void_return: severity: warning include_closures: true