Scan numeric strings (#17)

* Detect numeric strings, enable by default, allow to switch on/off * Ignore uncatchable instanceof in AbstractMagicNumberRule * Update README.md
sidz · Aug 7, 2023 · 6646674 · 6646674
1 parent 51042a9
commit 6646674
Show file tree

Hide file tree

Showing 35 changed files with 224 additions and 33 deletions.
diff --git a/README.md b/README.md
@@ -26,6 +26,13 @@ parameters:
 	sidzIgnoreMagicNumbers: [0, 1, 100]
 ```
 
+Each rule by default detects numeric strings like `'12'` in source code. This behavior could be disabled via parameter:
+
+```neon
+parameters:
+	sidzIgnoreNumericStrings: true
+```
+
 ## Ignoring particular rules
 
 If you need to ignore particular rule, for example `NoMagicNumberInComparisonOperatorRule`, you can do so by using built-in `ignoreErrors` parameter:
@@ -58,6 +65,8 @@ parameters:
                 - tests/*
 ```
 
+##
+
 ## Rules
 
 This package provides the following rules for use with [`phpstan/phpstan`](https://github.com/phpstan/phpstan):

diff --git a/infection.json5 b/infection.json5
@@ -6,6 +6,11 @@
         ]
     },
     "mutators": {
-        "@default": true
+        "@default": true,
+        "InstanceOf_": {
+            "ignore": [
+                "Sid\\PHPStan\\Rules\\MagicNumber\\AbstractMagicNumberRule::isNumericString"
+            ]
+        }
     }
 }
diff --git a/rules.neon b/rules.neon
@@ -1,90 +1,104 @@
 parameters:
 	sidzIgnoreMagicNumbers: [0, 1]
+	sidzIgnoreNumericStrings: false
 
 parametersSchema:
 	sidzIgnoreMagicNumbers: arrayOf(number())
+	sidzIgnoreNumericStrings: boolean()
 
 services:
     NoMagicNumberAsFunctionArgumentRule:
         class: Sid\PHPStan\Rules\MagicNumber\NoMagicNumberAsFunctionArgumentRule
         arguments:
             ignoreMagicNumbers: %sidzIgnoreMagicNumbers%
+            ignoreNumericStrings: %sidzIgnoreNumericStrings%
         tags:
             - phpstan.rules.rule
 
     NoMagicNumberAssignedToPropertyRule:
         class: Sid\PHPStan\Rules\MagicNumber\NoMagicNumberAssignedToPropertyRule
         arguments:
             ignoreMagicNumbers: %sidzIgnoreMagicNumbers%
+            ignoreNumericStrings: %sidzIgnoreNumericStrings%
         tags:
             - phpstan.rules.rule
 
     NoMagicNumberInArithmeticOperatorRule:
         class: Sid\PHPStan\Rules\MagicNumber\NoMagicNumberInArithmeticOperatorRule
         arguments:
             ignoreMagicNumbers: %sidzIgnoreMagicNumbers%
+            ignoreNumericStrings: %sidzIgnoreNumericStrings%
         tags:
             - phpstan.rules.rule
 
     NoMagicNumberInBitwiseOperatorRule:
         class: Sid\PHPStan\Rules\MagicNumber\NoMagicNumberInBitwiseOperatorRule
         arguments:
             ignoreMagicNumbers: %sidzIgnoreMagicNumbers%
+            ignoreNumericStrings: %sidzIgnoreNumericStrings%
         tags:
             - phpstan.rules.rule
 
     NoMagicNumberInComparisonOperatorRule:
         class: Sid\PHPStan\Rules\MagicNumber\NoMagicNumberInComparisonOperatorRule
         arguments:
             ignoreMagicNumbers: %sidzIgnoreMagicNumbers%
+            ignoreNumericStrings: %sidzIgnoreNumericStrings%
         tags:
             - phpstan.rules.rule
 
     NoMagicNumberInDefaultParameterRule:
         class: Sid\PHPStan\Rules\MagicNumber\NoMagicNumberInDefaultParameterRule
         arguments:
             ignoreMagicNumbers: %sidzIgnoreMagicNumbers%
+            ignoreNumericStrings: %sidzIgnoreNumericStrings%
         tags:
             - phpstan.rules.rule
 
     NoMagicNumberInLogicalOperatorRule:
         class: Sid\PHPStan\Rules\MagicNumber\NoMagicNumberInLogicalOperatorRule
         arguments:
             ignoreMagicNumbers: %sidzIgnoreMagicNumbers%
+            ignoreNumericStrings: %sidzIgnoreNumericStrings%
         tags:
             - phpstan.rules.rule
 
     NoMagicNumberInMatchRule:
         class: Sid\PHPStan\Rules\MagicNumber\NoMagicNumberInMatchRule
         arguments:
             ignoreMagicNumbers: %sidzIgnoreMagicNumbers%
+            ignoreNumericStrings: %sidzIgnoreNumericStrings%
         tags:
             - phpstan.rules.rule
 
     NoMagicNumberInReturnStatementRule:
         class: Sid\PHPStan\Rules\MagicNumber\NoMagicNumberInReturnStatementRule
         arguments:
             ignoreMagicNumbers: %sidzIgnoreMagicNumbers%
+            ignoreNumericStrings: %sidzIgnoreNumericStrings%
         tags:
             - phpstan.rules.rule
 
     NoMagicNumberInSwitchCaseRule:
         class: Sid\PHPStan\Rules\MagicNumber\NoMagicNumberInSwitchCaseRule
         arguments:
             ignoreMagicNumbers: %sidzIgnoreMagicNumbers%
+            ignoreNumericStrings: %sidzIgnoreNumericStrings%
         tags:
             - phpstan.rules.rule
 
     NoMagicNumberInTernaryOperatorRule:
         class: Sid\PHPStan\Rules\MagicNumber\NoMagicNumberInTernaryOperatorRule
         arguments:
             ignoreMagicNumbers: %sidzIgnoreMagicNumbers%
+            ignoreNumericStrings: %sidzIgnoreNumericStrings%
         tags:
             - phpstan.rules.rule
 
     NoMagicNumberVariableAssignmentRule:
         class: Sid\PHPStan\Rules\MagicNumber\NoMagicNumberVariableAssignmentRule
         arguments:
             ignoreMagicNumbers: %sidzIgnoreMagicNumbers%
+            ignoreNumericStrings: %sidzIgnoreNumericStrings%
         tags:
             - phpstan.rules.rule
diff --git a/src/Rules/MagicNumber/AbstractMagicNumberRule.php b/src/Rules/MagicNumber/AbstractMagicNumberRule.php
@@ -8,29 +8,28 @@
 use PhpParser\Node\Expr;
 use PhpParser\Node\Scalar\DNumber;
 use PhpParser\Node\Scalar\LNumber;
+use PhpParser\Node\Scalar\String_;
 use PHPStan\Rules\Rule;
 
 abstract class AbstractMagicNumberRule implements Rule
 {
-    /**
-     * @var list<numeric>
-     */
-    private $ignoreMagicNumbers;
-
     /**
      * @param list<numeric> $ignoreMagicNumbers
      */
-    public function __construct(array $ignoreMagicNumbers = [])
-    {
-        $this->ignoreMagicNumbers = $ignoreMagicNumbers;
+    public function __construct(
+        private readonly array $ignoreMagicNumbers = [],
+        private readonly bool $ignoreNumericStrings = false,
+    ) {
     }
 
-    protected function isNumber(?Expr $expr): bool
+    protected function isNumeric(?Expr $expr): bool
     {
         $isNumber = $expr instanceof LNumber
             || $expr instanceof DNumber
-            || ($expr instanceof Expr\UnaryMinus && $this->isNumber($expr->expr))
-            || ($expr instanceof Expr\UnaryPlus && $this->isNumber($expr->expr));
+            || ($expr instanceof Expr\UnaryMinus && $this->isNumeric($expr->expr))
+            || ($expr instanceof Expr\UnaryPlus && $this->isNumeric($expr->expr))
+            || ($expr instanceof Expr\Cast\String_ && $this->isNumeric($expr->expr))
+            || $this->isNumericString($expr);
 
         return $isNumber && !$this->ignoreNumber($expr);
     }
@@ -42,4 +41,11 @@ private function ignoreNumber(Expr $scalar): bool
     {
         return in_array($scalar->value, $this->ignoreMagicNumbers, true);
     }
+
+    private function isNumericString(?Expr $expr): bool
+    {
+        return !$this->ignoreNumericStrings
+            && $expr instanceof String_
+            && is_numeric($expr->value);
+    }
 }
diff --git a/src/Rules/MagicNumber/NoMagicNumberAsFunctionArgumentRule.php b/src/Rules/MagicNumber/NoMagicNumberAsFunctionArgumentRule.php
@@ -26,7 +26,7 @@ public function getNodeType(): string
      */
     public function processNode(Node $node, Scope $scope): array
     {
-        if ($this->isNumber($node->value)) {
+        if ($this->isNumeric($node->value)) {
             return [
                 RuleErrorBuilder::message(self::ERROR_MESSAGE)->line($node->getLine())->build(),
             ];

diff --git a/src/Rules/MagicNumber/NoMagicNumberAssignedToPropertyRule.php b/src/Rules/MagicNumber/NoMagicNumberAssignedToPropertyRule.php
@@ -26,7 +26,7 @@ public function getNodeType(): string
      */
     public function processNode(Node $node, Scope $scope): array
     {
-        if ($node->default === null || !$this->isNumber($node->default)) {
+        if ($node->default === null || !$this->isNumeric($node->default)) {
             return [];
         }
 

diff --git a/src/Rules/MagicNumber/NoMagicNumberInArithmeticOperatorRule.php b/src/Rules/MagicNumber/NoMagicNumberInArithmeticOperatorRule.php
@@ -31,7 +31,7 @@ public function processNode(Node $node, Scope $scope): array
             return [];
         }
 
-        if (!$this->isNumber($node->left) && !$this->isNumber($node->right)) {
+        if (!$this->isNumeric($node->left) && !$this->isNumeric($node->right)) {
             return [];
         }
 

diff --git a/src/Rules/MagicNumber/NoMagicNumberInBitwiseOperatorRule.php b/src/Rules/MagicNumber/NoMagicNumberInBitwiseOperatorRule.php
@@ -28,7 +28,7 @@ public function processNode(Node $node, Scope $scope): array
             return [];
         }
 
-        if (!$this->isNumber($node->left) && !$this->isNumber($node->right)) {
+        if (!$this->isNumeric($node->left) && !$this->isNumeric($node->right)) {
             return [];
         }
 

diff --git a/src/Rules/MagicNumber/NoMagicNumberInComparisonOperatorRule.php b/src/Rules/MagicNumber/NoMagicNumberInComparisonOperatorRule.php
@@ -31,7 +31,7 @@ public function processNode(Node $node, Scope $scope): array
             return [];
         }
 
-        if (!$this->isNumber($node->left) && !$this->isNumber($node->right)) {
+        if (!$this->isNumeric($node->left) && !$this->isNumeric($node->right)) {
             return [];
         }
 

diff --git a/src/Rules/MagicNumber/NoMagicNumberInDefaultParameterRule.php b/src/Rules/MagicNumber/NoMagicNumberInDefaultParameterRule.php
@@ -26,7 +26,7 @@ public function getNodeType(): string
      */
     public function processNode(Node $node, Scope $scope): array
     {
-        if ($node->default === null || !$this->isNumber($node->default)) {
+        if ($node->default === null || !$this->isNumeric($node->default)) {
             return [];
         }
 

diff --git a/src/Rules/MagicNumber/NoMagicNumberInLogicalOperatorRule.php b/src/Rules/MagicNumber/NoMagicNumberInLogicalOperatorRule.php
@@ -27,7 +27,7 @@ public function processNode(Node $node, Scope $scope): array
             return [];
         }
 
-        if (!$this->isNumber($node->left) && !$this->isNumber($node->right)) {
+        if (!$this->isNumeric($node->left) && !$this->isNumeric($node->right)) {
             return [];
         }
 

diff --git a/src/Rules/MagicNumber/NoMagicNumberInMatchRule.php b/src/Rules/MagicNumber/NoMagicNumberInMatchRule.php
@@ -29,13 +29,13 @@ public function processNode(Node $node, Scope $scope): array
     {
         $messages = [];
 
-        if ($this->isNumber($node->cond)) {
+        if ($this->isNumeric($node->cond)) {
             $messages[] = RuleErrorBuilder::message(self::MATCH_MESSAGE)->line($node->cond->getLine())->build();
         }
 
         foreach ($node->arms as $arm) {
             foreach ($arm->conds as $case) {
-                if (!$this->isNumber($case)) {
+                if (!$this->isNumeric($case)) {
                     continue;
                 }
 

diff --git a/src/Rules/MagicNumber/NoMagicNumberInReturnStatementRule.php b/src/Rules/MagicNumber/NoMagicNumberInReturnStatementRule.php
@@ -26,7 +26,7 @@ public function getNodeType(): string
      */
     public function processNode(Node $node, Scope $scope): array
     {
-        if (!$this->isNumber($node->expr)) {
+        if (!$this->isNumeric($node->expr)) {
             return [];
         }
 

diff --git a/src/Rules/MagicNumber/NoMagicNumberInSwitchCaseRule.php b/src/Rules/MagicNumber/NoMagicNumberInSwitchCaseRule.php
@@ -29,12 +29,12 @@ public function processNode(Node $node, Scope $scope): array
     {
         $messages = [];
 
-        if ($this->isNumber($node->cond)) {
+        if ($this->isNumeric($node->cond)) {
             $messages[] = RuleErrorBuilder::message(self::ERROR_CONDITION_MESSAGE)->line($node->cond->getLine())->build();
         }
 
         foreach ($node->cases as $case) {
-            if (!$this->isNumber($case->cond)) {
+            if (!$this->isNumeric($case->cond)) {
                 continue;
             }
 

diff --git a/src/Rules/MagicNumber/NoMagicNumberInTernaryOperatorRule.php b/src/Rules/MagicNumber/NoMagicNumberInTernaryOperatorRule.php
@@ -26,11 +26,11 @@ public function getNodeType(): string
      */
     public function processNode(Node $node, Scope $scope): array
     {
-        if ($node->if !== null && !$this->isNumber($node->else) && !$this->isNumber($node->if)) {
+        if ($node->if !== null && !$this->isNumeric($node->else) && !$this->isNumeric($node->if)) {
             return [];
         }
 
-        if ($node->if === null && !$this->isNumber($node->else) && !$this->isNumber($node->if)) {
+        if ($node->if === null && !$this->isNumeric($node->else) && !$this->isNumeric($node->if)) {
             return [];
         }
 

diff --git a/src/Rules/MagicNumber/NoMagicNumberVariableAssignmentRule.php b/src/Rules/MagicNumber/NoMagicNumberVariableAssignmentRule.php
@@ -26,7 +26,7 @@ public function getNodeType(): string
      */
     public function processNode(Node $node, Scope $scope): array
     {
-        if (!$this->isNumber($node->expr)) {
+        if (!$this->isNumeric($node->expr)) {
             return [];
         }
 

diff --git a/tests/Rules/MagicNumber/NoMagicNumberAsFunctionArgumentRuleTest.php b/tests/Rules/MagicNumber/NoMagicNumberAsFunctionArgumentRuleTest.php
@@ -20,11 +20,19 @@ public function test_rule(): void
                 ],
                 [
                     NoMagicNumberAsFunctionArgumentRule::ERROR_MESSAGE,
-                    14,
+                    18,
                 ],
                 [
                     NoMagicNumberAsFunctionArgumentRule::ERROR_MESSAGE,
-                    25,
+                    29,
+                ],
+                [
+                    NoMagicNumberAsFunctionArgumentRule::ERROR_MESSAGE,
+                    31,
+                ],
+                [
+                    NoMagicNumberAsFunctionArgumentRule::ERROR_MESSAGE,
+                    33,
                 ],
             ]
         );

diff --git a/tests/Rules/MagicNumber/NoMagicNumberAssignedToPropertyRuleTest.php b/tests/Rules/MagicNumber/NoMagicNumberAssignedToPropertyRuleTest.php
@@ -22,6 +22,10 @@ public function test_rule(): void
                     NoMagicNumberAssignedToPropertyRule::ERROR_MESSAGE,
                     9,
                 ],
+                [
+                    NoMagicNumberAssignedToPropertyRule::ERROR_MESSAGE,
+                    13,
+                ],
             ]
         );
     }

diff --git a/tests/Rules/MagicNumber/NoMagicNumberInArithmeticOperatorRuleTest.php b/tests/Rules/MagicNumber/NoMagicNumberInArithmeticOperatorRuleTest.php
@@ -62,6 +62,18 @@ public function test_rule(): void
                     NoMagicNumberInArithmeticOperatorRule::ERROR_MESSAGE,
                     27,
                 ],
+                [
+                    NoMagicNumberInArithmeticOperatorRule::ERROR_MESSAGE,
+                    40,
+                ],
+                [
+                    NoMagicNumberInArithmeticOperatorRule::ERROR_MESSAGE,
+                    42,
+                ],
+                [
+                    NoMagicNumberInArithmeticOperatorRule::ERROR_MESSAGE,
+                    44,
+                ],
             ]
         );
     }

diff --git a/tests/Rules/MagicNumber/NoMagicNumberInComparisonOperatorRuleTest.php b/tests/Rules/MagicNumber/NoMagicNumberInComparisonOperatorRuleTest.php
@@ -66,6 +66,10 @@ public function test_rule(): void
                     NoMagicNumberInComparisonOperatorRule::ERROR_MESSAGE,
                     57,
                 ],
+                [
+                    NoMagicNumberInComparisonOperatorRule::ERROR_MESSAGE,
+                    59,
+                ],
             ]
         );
     }

diff --git a/tests/Rules/MagicNumber/NoMagicNumberInDefaultParameterRuleTest.php b/tests/Rules/MagicNumber/NoMagicNumberInDefaultParameterRuleTest.php
@@ -18,6 +18,10 @@ public function test_rule(): void
                     NoMagicNumberInDefaultParameterRule::ERROR_MESSAGE,
                     11,
                 ],
+                [
+                    NoMagicNumberInDefaultParameterRule::ERROR_MESSAGE,
+                    77,
+                ],
             ]
         );
     }
-Original file line number
+Diff line change
@@ Expand Up / @@ -22,6 +22,10 @@ public function test_rule(): void @@
                         NoMagicNumberAssignedToPropertyRule::ERROR_MESSAGE,
 ,
                     ],
+                    [
+                        NoMagicNumberAssignedToPropertyRule::ERROR_MESSAGE,
+,
+                    ],
                 ]
             );
         }
@@ Expand Down @@