Skip simple tuples in no_magic_numbers rule (#5327)

realm · Nov 15, 2023 · 33e3593 · 33e3593
1 parent 9fbc04e
commit 33e3593
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -23,6 +23,11 @@
   the surrounding declaration is nested in an extension.  
   [SimplyDanny](https://github.com/SimplyDanny)
 
+* Fixed false positives for the `no_magic_numbers` rule, when they
+  are defined in a tuple like `let (a, b) = (5, 10)` or `let a = (2, 3)`.  
+  [Martin Redington](https://github.com/mildm8nnered)
+  [#5305](https://github.com/realm/SwiftLint/pull/5305)
+
 ## 0.54.0: Macro-Economic Forces
 
 #### Breaking
@@ -87,17 +92,17 @@
   [BB9z](https://github.com/BB9z)
   [#5289](https://github.com/realm/SwiftLint/issues/5289)
 
-* Fix invalid corrections for opaque and existential optionals in 
+* Fix invalid corrections for opaque and existential optionals in
   `syntactic_sugar` rule.  
   [SimplyDanny](https://github.com/SimplyDanny)
   [#5277](https://github.com/realm/SwiftLint/issues/5277)
 
-* Fix false positive in `unused_import` rule that triggered on 
+* Fix false positive in `unused_import` rule that triggered on
   `@_exported` imports which could break downstream modules if removed.  
   [jszumski](https://github.com/jszumski)
   [#5242](https://github.com/realm/SwiftLint/pull/5242)
 
-* Fix false positive in `unused_import` rule when using a constructor 
+* Fix false positive in `unused_import` rule when using a constructor
   defined in a transitive module.  
   [jszumski](https://github.com/jszumski)
   [#5246](https://github.com/realm/SwiftLint/pull/5246)

diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/NoMagicNumbersRule.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/NoMagicNumbersRule.swift
@@ -74,7 +74,12 @@ struct NoMagicNumbersRule: OptInRule {
             Example("let foo = 1 >> 2"),
             Example("let foo = 2 >> 2"),
             Example("let foo = 2 << 2"),
-            Example("let a = b / 100.0")
+            Example("let a = b / 100.0"),
+            Example("let (lowerBound, upperBound) = (400, 599)"),
+            Example("let a = (5, 10)"),
+            Example("""
+                    let notFound = (statusCode: 404, description: "Not Found", isError: true)
+                    """)
         ],
         triggeringExamples: [
             Example("foo(↓321)"),
@@ -88,7 +93,14 @@ struct NoMagicNumbersRule: OptInRule {
                     extension NSObject {
                         let a = Int(↓3)
                     }
-            """)
+            """),
+            Example("""
+            if (fileSize > ↓1000000) {
+                return
+            }
+            """),
+            Example("let imageHeight = (width - ↓24)"),
+            Example("return (↓5, ↓10, ↓15)")
         ]
     )
 }
@@ -99,6 +111,10 @@ private extension NoMagicNumbersRule {
         private var nonTestClasses: Set<String> = []
         private var possibleViolations: [String: Set<AbsolutePosition>] = [:]
 
+        override func visit(_ node: PatternBindingSyntax) -> SyntaxVisitorContinueKind {
+            node.isSimpleTupleAssignment ? .skipChildren : .visitChildren
+        }
+
         override func visitPost(_ node: ClassDeclSyntax) {
             let className = node.name.text
             if node.isXCTestCase(configuration.testParentClasses) {
@@ -212,3 +228,14 @@ private extension ExprSyntaxProtocol {
         return false
     }
 }
+
+private extension PatternBindingSyntax {
+    var isSimpleTupleAssignment: Bool {
+        initializer?.value.as(TupleExprSyntax.self)?.elements.allSatisfy {
+            $0.expression.is(IntegerLiteralExprSyntax.self) ||
+            $0.expression.is(FloatLiteralExprSyntax.self) ||
+            $0.expression.is(StringLiteralExprSyntax.self) ||
+            $0.expression.is(BooleanLiteralExprSyntax.self)
+        } ?? false
+    }
+}