Skip unit tests in no_magic_numbers rule (realm#4897)

hiltonc · Apr 16, 2023 · 97fd216 · 97fd216
1 parent f127ba1
commit 97fd216
Show file tree

Hide file tree

Showing 5 changed files with 57 additions and 14 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -31,6 +31,11 @@
   [Martin Redington](https://github.com/mildm8nnered)
   [#4819](https://github.com/realm/SwiftLint/issues/4819)
 
+* Adds `test_parent_classes` option to the `no_magic_numbers` rule.
+  Violations within test classes will now be ignored by default.  
+  [Martin Redington](https://github.com/mildm8nnered)
+  [#4896](https://github.com/realm/SwiftLint/issues/4896)
+
 #### Bug Fixes
 
 * Fix `lower_acl_than_parent` rule rewriter by preserving leading whitespace.  

diff --git a/Source/SwiftLintFramework/Rules/Idiomatic/NoMagicNumbersRule.swift b/Source/SwiftLintFramework/Rules/Idiomatic/NoMagicNumbersRule.swift
@@ -3,7 +3,7 @@ import SwiftSyntax
 struct NoMagicNumbersRule: SwiftSyntaxRule, OptInRule, ConfigurationProviderRule {
     init() {}
 
-    var configuration = SeverityConfiguration(.warning)
+    var configuration = NoMagicNumbersRuleConfiguration()
 
     static let description = RuleDescription(
         identifier: "no_magic_numbers",
@@ -44,6 +44,20 @@ struct NoMagicNumbersRule: SwiftSyntaxRule, OptInRule, ConfigurationProviderRule
                 case positive = 2
                 case negative = -2
             }
+            """),
+            Example("""
+            class FooTests: XCTestCase {
+                let array: [Int] = []
+                let bar = array[42]
+            }
+            """),
+            Example("""
+            class FooTests: XCTestCase {
+                class Bar {
+                    let array: [Int] = []
+                    let bar = array[42]
+                }
+            }
             """)
         ],
         triggeringExamples: [
@@ -57,20 +71,27 @@ struct NoMagicNumbersRule: SwiftSyntaxRule, OptInRule, ConfigurationProviderRule
     )
 
     func makeVisitor(file: SwiftLintFile) -> ViolationsSyntaxVisitor {
-        Visitor(viewMode: .sourceAccurate)
+        Visitor(viewMode: .sourceAccurate, testParentClasses: configuration.testParentClasses)
     }
 }
 
 private extension NoMagicNumbersRule {
     final class Visitor: ViolationsSyntaxVisitor {
+        private let testParentClasses: Set<String>
+
+        init(viewMode: SyntaxTreeViewMode, testParentClasses: Set<String>) {
+            self.testParentClasses = testParentClasses
+            super.init(viewMode: viewMode)
+        }
+
         override func visitPost(_ node: FloatLiteralExprSyntax) {
-            if node.floatingDigits.isMagicNumber {
+            if node.isMemberOfATestClass(testParentClasses) == false, node.floatingDigits.isMagicNumber {
                 violations.append(node.floatingDigits.positionAfterSkippingLeadingTrivia)
             }
         }
 
         override func visitPost(_ node: IntegerLiteralExprSyntax) {
-            if node.digits.isMagicNumber {
+            if node.isMemberOfATestClass(testParentClasses) == false, node.digits.isMagicNumber {
                 violations.append(node.digits.positionAfterSkippingLeadingTrivia)
             }
         }
@@ -92,3 +113,19 @@ private extension TokenSyntax {
             && grandparent.as(PrefixOperatorExprSyntax.self)?.parent?.is(InitializerClauseSyntax.self) != true
     }
 }
+
+private extension ExprSyntaxProtocol {
+    func isMemberOfATestClass(_ testParentClasses: Set<String>) -> Bool {
+        var parent = parent
+        while parent != nil {
+            if
+                let classDecl = parent?.as(ClassDeclSyntax.self),
+                classDecl.isXCTestCase(testParentClasses)
+            {
+                return true
+            }
+            parent = parent?.parent
+        }
+        return false
+    }
+}
diff --git a/Source/SwiftLintFramework/Rules/Lint/BalancedXCTestLifecycleRule.swift b/Source/SwiftLintFramework/Rules/Lint/BalancedXCTestLifecycleRule.swift
@@ -113,22 +113,22 @@ struct BalancedXCTestLifecycleRule: SwiftSyntaxRule, OptInRule, ConfigurationPro
     // MARK: - Public
 
     func makeVisitor(file: SwiftLintFile) -> ViolationsSyntaxVisitor {
-        Visitor(viewMode: .sourceAccurate, testClasses: configuration.testParentClasses)
+        Visitor(viewMode: .sourceAccurate, testParentClasses: configuration.testParentClasses)
     }
 }
 
 private extension BalancedXCTestLifecycleRule {
     final class Visitor: ViolationsSyntaxVisitor {
-        private let testClasses: Set<String>
+        private let testParentClasses: Set<String>
         override var skippableDeclarations: [DeclSyntaxProtocol.Type] { .all }
 
-        init(viewMode: SyntaxTreeViewMode, testClasses: Set<String>) {
-            self.testClasses = testClasses
+        init(viewMode: SyntaxTreeViewMode, testParentClasses: Set<String>) {
+            self.testParentClasses = testParentClasses
             super.init(viewMode: viewMode)
         }
 
         override func visitPost(_ node: ClassDeclSyntax) {
-            guard node.isXCTestCase(testClasses) else {
+            guard node.isXCTestCase(testParentClasses) else {
                 return
             }
 

diff --git a/Source/SwiftLintFramework/Rules/RuleConfigurations/UnitTestRuleConfiguration.swift b/Source/SwiftLintFramework/Rules/RuleConfigurations/UnitTestRuleConfiguration.swift
@@ -1,6 +1,7 @@
 public typealias BalancedXCTestLifecycleConfiguration = UnitTestRuleConfiguration
 public typealias EmptyXCTestMethodConfiguration = UnitTestRuleConfiguration
 public typealias SingleTestClassConfiguration = UnitTestRuleConfiguration
+public typealias NoMagicNumbersRuleConfiguration = UnitTestRuleConfiguration
 
 public struct UnitTestRuleConfiguration: SeverityBasedRuleConfiguration, Equatable {
     public private(set) var severityConfiguration = SeverityConfiguration(.warning)

diff --git a/Source/SwiftLintFramework/Rules/Style/SingleTestClassRule.swift b/Source/SwiftLintFramework/Rules/Style/SingleTestClassRule.swift
@@ -52,7 +52,7 @@ struct SingleTestClassRule: SourceKitFreeRule, OptInRule, ConfigurationProviderR
     init() {}
 
     func validate(file: SwiftLintFile) -> [StyleViolation] {
-        let classes = TestClassVisitor(viewMode: .sourceAccurate, testClasses: configuration.testParentClasses)
+        let classes = TestClassVisitor(viewMode: .sourceAccurate, testParentClasses: configuration.testParentClasses)
             .walk(tree: file.syntaxTree, handler: \.violations)
 
         guard classes.count > 1 else { return [] }
@@ -67,16 +67,16 @@ struct SingleTestClassRule: SourceKitFreeRule, OptInRule, ConfigurationProviderR
 }
 
 private class TestClassVisitor: ViolationsSyntaxVisitor {
-    private let testClasses: Set<String>
+    private let testParentClasses: Set<String>
     override var skippableDeclarations: [DeclSyntaxProtocol.Type] { .all }
 
-    init(viewMode: SyntaxTreeViewMode, testClasses: Set<String>) {
-        self.testClasses = testClasses
+    init(viewMode: SyntaxTreeViewMode, testParentClasses: Set<String>) {
+        self.testParentClasses = testParentClasses
         super.init(viewMode: viewMode)
     }
 
     override func visitPost(_ node: ClassDeclSyntax) {
-        guard node.inheritanceClause.containsInheritedType(inheritedTypes: testClasses) else {
+        guard node.inheritanceClause.containsInheritedType(inheritedTypes: testParentClasses) else {
             return
         }