Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -58,6 +65,8 @@ parameters:
- tests/*
```

##
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably not needed?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Cheers!


## Rules

This package provides the following rules for use with [`phpstan/phpstan`](https://github.com/phpstan/phpstan):
Expand Down
7 changes: 6 additions & 1 deletion infection.json5
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@
]
},
"mutators": {
"@default": true
"@default": true,
"InstanceOf_": {
"ignore": [
"Sid\\PHPStan\\Rules\\MagicNumber\\AbstractMagicNumberRule::isNumericString"
]
}
}
}
14 changes: 14 additions & 0 deletions rules.neon
Original file line number Diff line number Diff line change
@@ -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
28 changes: 17 additions & 11 deletions src/Rules/MagicNumber/AbstractMagicNumberRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 [];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 [];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 [];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 [];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 [];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 [];
}

Expand Down
4 changes: 2 additions & 2 deletions src/Rules/MagicNumber/NoMagicNumberInMatchRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 [];
}

Expand Down
4 changes: 2 additions & 2 deletions src/Rules/MagicNumber/NoMagicNumberInSwitchCaseRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
4 changes: 2 additions & 2 deletions src/Rules/MagicNumber/NoMagicNumberInTernaryOperatorRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 [];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 [];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
],
]
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ public function test_rule(): void
NoMagicNumberAssignedToPropertyRule::ERROR_MESSAGE,
9,
],
[
NoMagicNumberAssignedToPropertyRule::ERROR_MESSAGE,
13,
],
]
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
],
]
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ public function test_rule(): void
NoMagicNumberInComparisonOperatorRule::ERROR_MESSAGE,
57,
],
[
NoMagicNumberInComparisonOperatorRule::ERROR_MESSAGE,
59,
],
]
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ public function test_rule(): void
NoMagicNumberInDefaultParameterRule::ERROR_MESSAGE,
11,
],
[
NoMagicNumberInDefaultParameterRule::ERROR_MESSAGE,
77,
],
]
);
}
Expand Down
Loading