Skip to content

Commit abae9f1

Browse files
janedbaljanedbal
authored
forbidArithmeticOperationOnNonNumber (#180)
1 parent 79172a7 commit abae9f1

File tree

7 files changed

+419
-1
lines changed

7 files changed

+419
-1
lines changed

‎README.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ parameters:
4040
enabled: true
4141
enforceReadonlyPublicProperty:
4242
enabled: true
43+
forbidArithmeticOperationOnNonNumber:
44+
enabled: true
45+
allowNumericString: false
4346
forbidAssignmentNotMatchingVarDoc:
4447
enabled: true
4548
allowNarrowing: false
@@ -305,6 +308,17 @@ class EnforceReadonlyPublicPropertyRule {
305308
}
306309
```
307310

311+
### forbidArithmeticOperationOnNonNumber
312+
- Allows using [arithmetic operators](https://www.php.net/manual/en/language.operators.arithmetic.php) with non-numeric types (only float and int is allowed)
313+
- You can allow even numeric-string by using `allowNumericString: true` configuration
314+
- Modulo operator (`%`) allows only integers as it [emits deprecation otherwise](https://3v4l.org/VpVoq)
315+
- Plus operator is allowed for merging arrays
316+
317+
```php
318+
function add(string $a, string $b) {
319+
return $a + $b; // denied, non-numeric types are allowed
320+
}
321+
```
308322

309323
### forbidAssignmentNotMatchingVarDoc
310324
- Verifies if defined type in `@var` phpdoc accepts the assigned type during assignment

‎rules.neon

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@ parameters:
1919
enabled: true
2020
enforceReadonlyPublicProperty:
2121
enabled: true
22+
forbidArithmeticOperationOnNonNumber:
23+
enabled: true
24+
allowNumericString: false
2225
forbidAssignmentNotMatchingVarDoc:
2326
enabled: true
2427
allowNarrowing: false
@@ -128,6 +131,10 @@ parametersSchema:
128131
enforceReadonlyPublicProperty: structure([
129132
enabled: bool()
130133
])
134+
forbidArithmeticOperationOnNonNumber: structure([
135+
enabled: bool()
136+
allowNumericString: bool()
137+
])
131138
forbidAssignmentNotMatchingVarDoc: structure([
132139
enabled: bool()
133140
allowNarrowing: bool()
@@ -234,6 +241,8 @@ conditionalTags:
234241
phpstan.rules.rule: %shipmonkRules.enforceListReturn.enabled%
235242
ShipMonk\PHPStan\Rule\EnforceReadonlyPublicPropertyRule:
236243
phpstan.rules.rule: %shipmonkRules.enforceReadonlyPublicProperty.enabled%
244+
ShipMonk\PHPStan\Rule\ForbidArithmeticOperationOnNonNumberRule:
245+
phpstan.rules.rule: %shipmonkRules.forbidArithmeticOperationOnNonNumber.enabled%
237246
ShipMonk\PHPStan\Rule\ForbidAssignmentNotMatchingVarDocRule:
238247
phpstan.rules.rule: %shipmonkRules.forbidAssignmentNotMatchingVarDoc.enabled%
239248
ShipMonk\PHPStan\Rule\ForbidCastRule:
@@ -328,6 +337,10 @@ services:
328337
treatPhpDocTypesAsCertain: %treatPhpDocTypesAsCertain%
329338
-
330339
class: ShipMonk\PHPStan\Rule\EnforceReadonlyPublicPropertyRule
340+
-
341+
class: ShipMonk\PHPStan\Rule\ForbidArithmeticOperationOnNonNumberRule
342+
arguments:
343+
allowNumericString: %shipmonkRules.forbidArithmeticOperationOnNonNumber.allowNumericString%
331344
-
332345
class: ShipMonk\PHPStan\Rule\ForbidAssignmentNotMatchingVarDocRule
333346
arguments:

‎src/Rule/ForbidArithmeticOperationOnNonNumberRule.php

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace ShipMonk\PHPStan\Rule;
4+
5+
use PhpParser\Node;
6+
use PhpParser\Node\Expr;
7+
use PhpParser\Node\Expr\BinaryOp\Div;
8+
use PhpParser\Node\Expr\BinaryOp\Minus;
9+
use PhpParser\Node\Expr\BinaryOp\Mod;
10+
use PhpParser\Node\Expr\BinaryOp\Mul;
11+
use PhpParser\Node\Expr\BinaryOp\Plus;
12+
use PhpParser\Node\Expr\BinaryOp\Pow;
13+
use PhpParser\Node\Expr\UnaryMinus;
14+
use PhpParser\Node\Expr\UnaryPlus;
15+
use PHPStan\Analyser\Scope;
16+
use PHPStan\Rules\Rule;
17+
use PHPStan\Rules\RuleError;
18+
use PHPStan\Rules\RuleErrorBuilder;
19+
use PHPStan\Type\FloatType;
20+
use PHPStan\Type\IntegerType;
21+
use PHPStan\Type\Type;
22+
use PHPStan\Type\UnionType;
23+
use PHPStan\Type\VerbosityLevel;
24+
use function sprintf;
25+
26+
/**
27+
* @implements Rule<Node>
28+
*/
29+
class ForbidArithmeticOperationOnNonNumberRule implements Rule
30+
{
31+
32+
private bool $allowNumericString;
33+
34+
public function __construct(bool $allowNumericString)
35+
{
36+
$this->allowNumericString = $allowNumericString;
37+
}
38+
39+
public function getNodeType(): string
40+
{
41+
return Node::class;
42+
}
43+
44+
/**
45+
* @return list<RuleError>
46+
*/
47+
public function processNode(Node $node, Scope $scope): array
48+
{
49+
if (
50+
$node instanceof UnaryPlus
51+
|| $node instanceof UnaryMinus
52+
) {
53+
return $this->processUnary($node->expr, $scope, $node instanceof UnaryMinus ? '-' : '+');
54+
}
55+
56+
if (
57+
$node instanceof Plus
58+
|| $node instanceof Minus
59+
|| $node instanceof Div
60+
|| $node instanceof Mul
61+
|| $node instanceof Mod
62+
|| $node instanceof Pow
63+
) {
64+
return $this->processBinary($node->left, $node->right, $scope, $node->getOperatorSigil());
65+
}
66+
67+
return [];
68+
}
69+
70+
/**
71+
* @return list<RuleError>
72+
*/
73+
private function processUnary(Expr $expr, Scope $scope, string $operator): array
74+
{
75+
$exprType = $scope->getType($expr);
76+
77+
if (!$this->isNumeric($exprType)) {
78+
$errorMessage = sprintf(
79+
'Using %s over non-number (%s)',
80+
$operator,
81+
$exprType->describe(VerbosityLevel::typeOnly()),
82+
);
83+
$error = RuleErrorBuilder::message($errorMessage)
84+
->identifier('shipmonk.arithmeticOnNonNumber')
85+
->build();
86+
return [$error];
87+
}
88+
89+
return [];
90+
}
91+
92+
/**
93+
* @return list<RuleError>
94+
*/
95+
private function processBinary(Expr $left, Expr $right, Scope $scope, string $operator): array
96+
{
97+
$leftType = $scope->getType($left);
98+
$rightType = $scope->getType($right);
99+
100+
if ($operator === '+' && $leftType->isArray()->yes() && $rightType->isArray()->yes()) {
101+
return []; // array merge syntax
102+
}
103+
104+
if (
105+
$operator === '%' &&
106+
(!$leftType->isInteger()->yes() || !$rightType->isInteger()->yes())
107+
) {
108+
return $this->buildBinaryErrors($operator, 'non-integer', $leftType, $rightType);
109+
}
110+
111+
if (!$this->isNumeric($leftType) || !$this->isNumeric($rightType)) {
112+
return $this->buildBinaryErrors($operator, 'non-number', $leftType, $rightType);
113+
}
114+
115+
return [];
116+
}
117+
118+
private function isNumeric(Type $type): bool
119+
{
120+
$int = new IntegerType();
121+
$float = new FloatType();
122+
$intOrFloat = new UnionType([$int, $float]);
123+
124+
return $int->isSuperTypeOf($type)->yes()
125+
|| $float->isSuperTypeOf($type)->yes()
126+
|| $intOrFloat->isSuperTypeOf($type)->yes()
127+
|| ($this->allowNumericString && $type->isNumericString()->yes());
128+
}
129+
130+
/**
131+
* @return list<RuleError>
132+
*/
133+
private function buildBinaryErrors(string $operator, string $type, Type $leftType, Type $rightType): array
134+
{
135+
$errorMessage = sprintf(
136+
'Using %s over %s (%s %s %s)',
137+
$operator,
138+
$type,
139+
$leftType->describe(VerbosityLevel::typeOnly()),
140+
$operator,
141+
$rightType->describe(VerbosityLevel::typeOnly()),
142+
);
143+
$error = RuleErrorBuilder::message($errorMessage)
144+
->identifier('shipmonk.arithmeticOnNonNumber')
145+
->build();
146+
147+
return [$error];
148+
}
149+
150+
}

‎src/Rule/ForbidEnumInFunctionArgumentsRule.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ public function processNode(Node $node, Scope $scope): array
9797
$argumentType = $scope->getType($argument->value);
9898

9999
if ($this->containsEnum($argumentType)) {
100-
$wrongArguments[] = $position + 1;
100+
$wrongArguments[] = (int) $position + 1;
101101
}
102102
}
103103

‎tests/Rule/ForbidArithmeticOperationOnNonNumberRuleTest.php

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace ShipMonk\PHPStan\Rule;
4+
5+
use LogicException;
6+
use PHPStan\Rules\Rule;
7+
use ShipMonk\PHPStan\RuleTestCase;
8+
9+
/**
10+
* @extends RuleTestCase<ForbidArithmeticOperationOnNonNumberRule>
11+
*/
12+
class ForbidArithmeticOperationOnNonNumberRuleTest extends RuleTestCase
13+
{
14+
15+
private ?bool $allowNumericString = null;
16+
17+
protected function getRule(): Rule
18+
{
19+
if ($this->allowNumericString === null) {
20+
throw new LogicException('allowNumericString must be set');
21+
}
22+
23+
return new ForbidArithmeticOperationOnNonNumberRule($this->allowNumericString);
24+
}
25+
26+
public function test(): void
27+
{
28+
$this->allowNumericString = true;
29+
$this->analyseFile(__DIR__ . '/data/ForbidArithmeticOperationOnNonNumberRule/code.php');
30+
}
31+
32+
public function testNoNumericString(): void
33+
{
34+
$this->allowNumericString = false;
35+
$this->analyseFile(__DIR__ . '/data/ForbidArithmeticOperationOnNonNumberRule/no-numeric-string.php');
36+
}
37+
38+
}

0 commit comments

Comments
 (0)