diff --git a/CHANGELOG.md b/CHANGELOG.md index ba3ca094f..fea7d6fdc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -71,6 +71,13 @@ [Martin Redington](https://github.com/mildm8nnered) [#4876](https://github.com/realm/SwiftLint/issues/4876) +* The `no_magic_numbers` rule will not trigger for violations in an + extension, if the extended class inherits from one of the specified + `test_parent_classes`, as long as the class declaration and the + extension are in the same source file. + [Martin Redington](https://github.com/mildm8nnered) + [#5137](https://github.com/realm/SwiftLint/issues/5137) + ## 0.52.4: Lid Switch #### Breaking diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/NoMagicNumbersRule.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/NoMagicNumbersRule.swift index 97f2742b5..1e381d652 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/NoMagicNumbersRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/NoMagicNumbersRule.swift @@ -56,6 +56,18 @@ struct NoMagicNumbersRule: SwiftSyntaxRule, OptInRule, ConfigurationProviderRule let bar = array[42] } } + """), + Example(""" + class MyTest: XCTestCase {} + extension MyTest { + let a = Int(3) + } + """), + Example(""" + extension MyTest { + let a = Int(3) + } + class MyTest: XCTestCase {} """) ], triggeringExamples: [ @@ -64,7 +76,13 @@ struct NoMagicNumbersRule: SwiftSyntaxRule, OptInRule, ConfigurationProviderRule Example("array[↓42]"), Example("let box = array[↓12 + ↓14]"), Example("let a = b + ↓2.0"), - Example("Color.primary.opacity(isAnimate ? ↓0.1 : ↓1.5)") + Example("Color.primary.opacity(isAnimate ? ↓0.1 : ↓1.5)"), + Example(""" + class MyTest: XCTestCase {} + extension NSObject { + let a = Int(↓3) + } + """) ] ) @@ -76,22 +94,63 @@ struct NoMagicNumbersRule: SwiftSyntaxRule, OptInRule, ConfigurationProviderRule private extension NoMagicNumbersRule { final class Visitor: ViolationsSyntaxVisitor { private let testParentClasses: Set + private var testClasses: Set = [] + private var nonTestClasses: Set = [] + private var possibleViolations: [String: Set] = [:] init(viewMode: SyntaxTreeViewMode, testParentClasses: Set) { self.testParentClasses = testParentClasses super.init(viewMode: viewMode) } - override func visitPost(_ node: FloatLiteralExprSyntax) { - if node.isMemberOfATestClass(testParentClasses) == false, node.floatingDigits.isMagicNumber { - violations.append(node.floatingDigits.positionAfterSkippingLeadingTrivia) + override func visitPost(_ node: ClassDeclSyntax) { + let className = node.identifier.text + if node.isXCTestCase(testParentClasses) { + testClasses.insert(className) + removeViolations(forClassName: className) + } else { + nonTestClasses.insert(className) } } - override func visitPost(_ node: IntegerLiteralExprSyntax) { - if node.isMemberOfATestClass(testParentClasses) == false, node.digits.isMagicNumber { - violations.append(node.digits.positionAfterSkippingLeadingTrivia) + override func visitPost(_ node: FloatLiteralExprSyntax) { + guard node.floatingDigits.isMagicNumber else { + return } + collectViolation(forNode: node) + } + + override func visitPost(_ node: IntegerLiteralExprSyntax) { + guard node.digits.isMagicNumber else { + return + } + collectViolation(forNode: node) + } + + private func collectViolation(forNode node: ExprSyntaxProtocol) { + if node.isMemberOfATestClass(testParentClasses) { + return + } + let violation = node.positionAfterSkippingLeadingTrivia + if let extendedTypeName = node.extendedTypeName() { + if !testClasses.contains(extendedTypeName) { + violations.append(violation) + if !nonTestClasses.contains(extendedTypeName) { + possibleViolations[extendedTypeName, default: []].insert(violation) + } + } + } else { + violations.append(violation) + } + } + + private func removeViolations(forClassName className: String) { + guard let possibleViolationsForClass = possibleViolations[className] else { + return + } + let violationsToRemove = Set(possibleViolationsForClass.map { ReasonedRuleViolation(position: $0) }) + violations.removeAll { violationsToRemove.contains($0) } + possibleViolations.removeValue(forKey: className) } } } @@ -126,4 +185,15 @@ private extension ExprSyntaxProtocol { } return false } + + func extendedTypeName() -> String? { + var parent = parent + while parent != nil { + if let extensionDecl = parent?.as(ExtensionDeclSyntax.self) { + return extensionDecl.extendedType.trimmedDescription + } + parent = parent?.parent + } + return nil + } } diff --git a/Source/SwiftLintCore/Protocols/SwiftSyntaxRule.swift b/Source/SwiftLintCore/Protocols/SwiftSyntaxRule.swift index 4d2259ef4..38576c50b 100644 --- a/Source/SwiftLintCore/Protocols/SwiftSyntaxRule.swift +++ b/Source/SwiftLintCore/Protocols/SwiftSyntaxRule.swift @@ -86,7 +86,7 @@ public extension SwiftSyntaxRule { } /// A violation produced by `ViolationsSyntaxVisitor`s. -public struct ReasonedRuleViolation: Comparable { +public struct ReasonedRuleViolation: Comparable, Hashable { /// The violation's position. public let position: AbsolutePosition /// A specific reason for the violation.