Skip to content

Commit 334898a

Browse files
authored
implemented a rule to detect overwriting parent scope vars in for loop init
1 parent 9b86e1e commit 334898a

File tree

4 files changed

+212
-0
lines changed

4 files changed

+212
-0
lines changed

Diff for: rules.neon

+7
Original file line numberDiff line numberDiff line change
@@ -60,3 +60,10 @@ services:
6060
universalObjectCratesClasses: %universalObjectCratesClasses%
6161
tags:
6262
- phpstan.rules.rule
63+
64+
-
65+
class: PHPStan\Rules\ForLoop\OverwriteVariablesWithForLoopInitRule
66+
67+
conditionalTags:
68+
PHPStan\Rules\ForLoop\OverwriteVariablesWithForLoopInitRule:
69+
phpstan.rules.rule: %featureToggles.bleedingEdge%
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\ForLoop;
4+
5+
use PhpParser\Node;
6+
use PhpParser\Node\Expr;
7+
use PhpParser\Node\Expr\Assign;
8+
use PhpParser\Node\Stmt\For_;
9+
use PHPStan\Analyser\Scope;
10+
use PHPStan\Rules\Rule;
11+
12+
/**
13+
* @implements Rule<For_>
14+
*/
15+
class OverwriteVariablesWithForLoopInitRule implements Rule
16+
{
17+
18+
public function getNodeType(): string
19+
{
20+
return For_::class;
21+
}
22+
23+
/**
24+
* @param For_ $node
25+
* @param Scope $scope
26+
* @return string[]
27+
*/
28+
public function processNode(Node $node, Scope $scope): array
29+
{
30+
$errors = [];
31+
foreach ($node->init as $expr) {
32+
if (!($expr instanceof Assign)) {
33+
continue;
34+
}
35+
36+
foreach ($this->checkValueVar($scope, $expr->var) as $error) {
37+
$errors[] = $error;
38+
}
39+
}
40+
41+
return $errors;
42+
}
43+
44+
/**
45+
* @param Scope $scope
46+
* @param Expr $expr
47+
* @return string[]
48+
*/
49+
private function checkValueVar(Scope $scope, Expr $expr): array
50+
{
51+
$errors = [];
52+
if (
53+
$expr instanceof Node\Expr\Variable
54+
&& is_string($expr->name)
55+
&& $scope->hasVariableType($expr->name)->yes()
56+
) {
57+
$errors[] = sprintf('For loop initial assignment overwrites variable $%s.', $expr->name);
58+
}
59+
60+
if (
61+
$expr instanceof Node\Expr\List_
62+
|| $expr instanceof Node\Expr\Array_
63+
) {
64+
foreach ($expr->items as $item) {
65+
if ($item === null) {
66+
continue;
67+
}
68+
69+
foreach ($this->checkValueVar($scope, $item->value) as $error) {
70+
$errors[] = $error;
71+
}
72+
}
73+
}
74+
75+
return $errors;
76+
}
77+
78+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\ForLoop;
4+
5+
use PHPStan\Rules\Rule;
6+
use PHPStan\Testing\RuleTestCase;
7+
8+
/**
9+
* @template-extends RuleTestCase<OverwriteVariablesWithForLoopInitRule>
10+
*/
11+
class OverwriteVariablesWithForLoopInitRuleTest extends RuleTestCase
12+
{
13+
14+
protected function getRule(): Rule
15+
{
16+
return new OverwriteVariablesWithForLoopInitRule();
17+
}
18+
19+
public function testRule(): void
20+
{
21+
$this->analyse([__DIR__ . '/data/data.php'], [
22+
[
23+
'For loop initial assignment overwrites variable $i.',
24+
9,
25+
],
26+
[
27+
'For loop initial assignment overwrites variable $i.',
28+
20,
29+
],
30+
[
31+
'For loop initial assignment overwrites variable $j.',
32+
20,
33+
],
34+
[
35+
'For loop initial assignment overwrites variable $i.',
36+
24,
37+
],
38+
[
39+
'For loop initial assignment overwrites variable $i.',
40+
35,
41+
],
42+
[
43+
'For loop initial assignment overwrites variable $j.',
44+
35,
45+
],
46+
[
47+
'For loop initial assignment overwrites variable $i.',
48+
39,
49+
],
50+
[
51+
'For loop initial assignment overwrites variable $j.',
52+
39,
53+
],
54+
[
55+
'For loop initial assignment overwrites variable $i.',
56+
50,
57+
],
58+
[
59+
'For loop initial assignment overwrites variable $i.',
60+
54,
61+
],
62+
]);
63+
}
64+
65+
}

Diff for: tests/Rules/ForLoop/data/data.php

+62
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
<?php
2+
3+
namespace OverwriteVariablesWithForLoopInit;
4+
5+
class Foo{
6+
7+
public function simple(int $i): void
8+
{
9+
for($i = 0; $i < 10; ++$i){
10+
11+
}
12+
13+
for($j = 0; $j < 10; ++$j){
14+
15+
}
16+
}
17+
18+
public function multi(int $i, int $j): void
19+
{
20+
for($i = 0, $j = 0; $i < 10; ++$i){
21+
22+
}
23+
24+
for($i = 0, $k = 0; $i < 10; ++$i){
25+
26+
}
27+
28+
for($k = 0, $l = 0; $k < 10; ++$k){
29+
30+
}
31+
}
32+
33+
public function list(int $i, int $j, array $b): void
34+
{
35+
for(list($i, $j) = $b; $i < 10; ++$i){
36+
37+
}
38+
39+
for(list($i, list($j, $k)) = $b; $i < 10; ++$i){
40+
41+
}
42+
43+
for(list($k, list($l, $m)) = $b; $k < 10; ++$k){
44+
45+
}
46+
}
47+
48+
public function array(int $i, array $b): void
49+
{
50+
for([$i, $j] = $b; $i < 10; ++$i){
51+
52+
}
53+
54+
for([$i, [$j, $k]] = $b; $i < 10; ++$i){
55+
56+
}
57+
58+
for([$k, [$l, $m]] = $b; $k < 10; ++$k){
59+
60+
}
61+
}
62+
}

0 commit comments

Comments
 (0)