Detect numeric strings, enable by default, allow to switch on/off

sidz · sidz · commit 00324a515f61 · 2023-08-07T21:04:47.000+03:00
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,
+                ],
             ]
         );
     }
diff --git a/tests/Rules/MagicNumber/NoMagicNumberInMatchCaseRuleTest.php b/tests/Rules/MagicNumber/NoMagicNumberInMatchCaseRuleTest.php
@@ -34,6 +34,10 @@ public function test_rule(): void
                     NoMagicNumberInMatchRule::MATCH_ARM_COND_MESSAGE,
                     8,
                 ],
+                [
+                    NoMagicNumberInMatchRule::MATCH_ARM_COND_MESSAGE,
+                    8,
+                ],
             ]
         );
     }
diff --git a/tests/Rules/MagicNumber/NoMagicNumberInReturnStatementRuleTest.php b/tests/Rules/MagicNumber/NoMagicNumberInReturnStatementRuleTest.php
@@ -38,6 +38,10 @@ public function test_rule(): void
                     NoMagicNumberInReturnStatementRule::ERROR_MESSAGE,
                     47,
                 ],
+                [
+                    NoMagicNumberInReturnStatementRule::ERROR_MESSAGE,
+                    72,
+                ],
             ]
         );
     }
diff --git a/tests/Rules/MagicNumber/NoMagicNumberInSwitchCaseRuleTest.php b/tests/Rules/MagicNumber/NoMagicNumberInSwitchCaseRuleTest.php
@@ -22,6 +22,14 @@ public function test_rule(): void
                     NoMagicNumberInSwitchCaseRule::ERROR_MESSAGE,
                     4,
                 ],
+                [
+                    NoMagicNumberInSwitchCaseRule::ERROR_MESSAGE,
+                    6,
+                ],
+                [
+                    NoMagicNumberInSwitchCaseRule::ERROR_CONDITION_MESSAGE,
+                    11,
+                ],
             ]
         );
     }
diff --git a/tests/Rules/MagicNumber/NoMagicNumberInTernaryOperatorRuleTest.php b/tests/Rules/MagicNumber/NoMagicNumberInTernaryOperatorRuleTest.php
diff --git a/tests/Rules/MagicNumber/NoMagicNumberVariableAssignmentRuleTest.php b/tests/Rules/MagicNumber/NoMagicNumberVariableAssignmentRuleTest.php
diff --git a/tests/Rules/MagicNumber/data/arithmetic-cases.php b/tests/Rules/MagicNumber/data/arithmetic-cases.php
diff --git a/tests/Rules/MagicNumber/data/assignment-case.php b/tests/Rules/MagicNumber/data/assignment-case.php
diff --git a/tests/Rules/MagicNumber/data/class-cases.php b/tests/Rules/MagicNumber/data/class-cases.php
diff --git a/tests/Rules/MagicNumber/data/class-property-cases.php b/tests/Rules/MagicNumber/data/class-property-cases.php
diff --git a/tests/Rules/MagicNumber/data/comparison-cases.php b/tests/Rules/MagicNumber/data/comparison-cases.php
diff --git a/tests/Rules/MagicNumber/data/function-argument.php b/tests/Rules/MagicNumber/data/function-argument.php
diff --git a/tests/Rules/MagicNumber/data/switch-case.php b/tests/Rules/MagicNumber/data/switch-case.php
diff --git a/tests/Rules/MagicNumber/data/ternary-cases.php b/tests/Rules/MagicNumber/data/ternary-cases.php

Original file line number	Diff line number	Diff line change
`@@ -26,7 +26,7 @@ public function getNodeType(): string`
`26`	`26`	`*/`
`27`	`27`	`public function processNode(Node $node, Scope $scope): array`
`28`	`28`	`{`
`29`		`- if ($this->isNumber($node->value)) {`
	`29`	`+ if ($this->isNumeric($node->value)) {`
`30`	`30`	`return [`
`31`	`31`	`RuleErrorBuilder::message(self::ERROR_MESSAGE)->line($node->getLine())->build(),`
`32`	`32`	`];`
Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,7 @@ public function processNode(Node $node, Scope $scope): array`
`31`	`31`	`return [];`
`32`	`32`	`}`
`33`	`33`
`34`		`- if (!$this->isNumber($node->left) && !$this->isNumber($node->right)) {`
	`34`	`+ if (!$this->isNumeric($node->left) && !$this->isNumeric($node->right)) {`
`35`	`35`	`return [];`
`36`	`36`	`}`
`37`	`37`
Original file line number	Diff line number	Diff line change
`@@ -28,7 +28,7 @@ public function processNode(Node $node, Scope $scope): array`
`28`	`28`	`return [];`
`29`	`29`	`}`
`30`	`30`
`31`		`- if (!$this->isNumber($node->left) && !$this->isNumber($node->right)) {`
	`31`	`+ if (!$this->isNumeric($node->left) && !$this->isNumeric($node->right)) {`
`32`	`32`	`return [];`
`33`	`33`	`}`
`34`	`34`
Original file line number	Diff line number	Diff line change
`@@ -27,7 +27,7 @@ public function processNode(Node $node, Scope $scope): array`
`27`	`27`	`return [];`
`28`	`28`	`}`
`29`	`29`
`30`		`- if (!$this->isNumber($node->left) && !$this->isNumber($node->right)) {`
	`30`	`+ if (!$this->isNumeric($node->left) && !$this->isNumeric($node->right)) {`
`31`	`31`	`return [];`
`32`	`32`	`}`
`33`	`33`
Original file line number	Diff line number	Diff line change
`@@ -29,13 +29,13 @@ public function processNode(Node $node, Scope $scope): array`
`29`	`29`	`{`
`30`	`30`	`$messages = [];`
`31`	`31`
`32`		`- if ($this->isNumber($node->cond)) {`
	`32`	`+ if ($this->isNumeric($node->cond)) {`
`33`	`33`	`$messages[] = RuleErrorBuilder::message(self::MATCH_MESSAGE)->line($node->cond->getLine())->build();`
`34`	`34`	`}`
`35`	`35`
`36`	`36`	`foreach ($node->arms as $arm) {`
`37`	`37`	`foreach ($arm->conds as $case) {`
`38`		`- if (!$this->isNumber($case)) {`
	`38`	`+ if (!$this->isNumeric($case)) {`
`39`	`39`	`continue;`
`40`	`40`	`}`
`41`	`41`