Skip to content

Commit 9ba75bb

Browse files
herndlmherndlm
committed
Improve foreach scope pollution
1 parent c55aa05 commit 9ba75bb

File tree

7 files changed

+289
-9
lines changed

7 files changed

+289
-9
lines changed

‎src/Analyser/MutatingScope.php‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4851,12 +4851,24 @@ public function processClosureScope(
48514851
);
48524852
}
48534853

4854-
public function processAlwaysIterableForeachScopeWithoutPollute(self $finalScope): self
4854+
public function processAlwaysIterableForeachScopeWithoutPollute(self $finalScope, Node\Stmt\Foreach_ $stmt): self
48554855
{
4856+
$keyVarExprString = $stmt->keyVar instanceof Variable && is_string($stmt->keyVar->name)
4857+
? '$' . $stmt->keyVar->name
4858+
: null;
4859+
$valueVarExprString = $stmt->valueVar instanceof Variable && is_string($stmt->valueVar->name)
4860+
? '$' . $stmt->valueVar->name
4861+
: null;
4862+
48564863
$expressionTypes = $this->expressionTypes;
48574864
foreach ($finalScope->expressionTypes as $variableExprString => $variableTypeHolder) {
48584865
if (!isset($expressionTypes[$variableExprString])) {
4859-
$expressionTypes[$variableExprString] = ExpressionTypeHolder::createMaybe($variableTypeHolder->getExpr(), $variableTypeHolder->getType());
4866+
if ($variableExprString === $keyVarExprString || $variableExprString === $valueVarExprString) {
4867+
$expressionTypes[$variableExprString] = ExpressionTypeHolder::createMaybe($variableTypeHolder->getExpr(), $variableTypeHolder->getType());
4868+
continue;
4869+
}
4870+
4871+
$expressionTypes[$variableExprString] = $variableTypeHolder;
48604872
continue;
48614873
}
48624874

@@ -4869,7 +4881,12 @@ public function processAlwaysIterableForeachScopeWithoutPollute(self $finalScope
48694881
$nativeTypes = $this->nativeExpressionTypes;
48704882
foreach ($finalScope->nativeExpressionTypes as $variableExprString => $variableTypeHolder) {
48714883
if (!isset($nativeTypes[$variableExprString])) {
4872-
$nativeTypes[$variableExprString] = ExpressionTypeHolder::createMaybe($variableTypeHolder->getExpr(), $variableTypeHolder->getType());
4884+
if ($variableExprString === $keyVarExprString || $variableExprString === $valueVarExprString) {
4885+
$nativeTypes[$variableExprString] = ExpressionTypeHolder::createMaybe($variableTypeHolder->getExpr(), $variableTypeHolder->getType());
4886+
continue;
4887+
}
4888+
4889+
$nativeTypes[$variableExprString] = $variableTypeHolder;
48734890
continue;
48744891
}
48754892

‎src/Analyser/NodeScopeResolver.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1119,8 +1119,7 @@ private function processStmtNode(
11191119
} elseif ($isIterableAtLeastOnce->no() || $finalScopeResult->isAlwaysTerminating()) {
11201120
$finalScope = $scope;
11211121
} elseif (!$this->polluteScopeWithAlwaysIterableForeach) {
1122-
$finalScope = $scope->processAlwaysIterableForeachScopeWithoutPollute($finalScope);
1123-
// get types from finalScope, but don't create new variables
1122+
$finalScope = $scope->processAlwaysIterableForeachScopeWithoutPollute($finalScope, $stmt);
11241123
}
11251124

11261125
if (!$isIterableAtLeastOnce->no()) {

‎tests/PHPStan/Analyser/ForeachLoopNoScopePollutionTest.php‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Analyser;
4+
5+
use PHPStan\Testing\TypeInferenceTestCase;
6+
7+
class ForeachLoopNoScopePollutionTest extends TypeInferenceTestCase
8+
{
9+
10+
/** @return iterable<array<string, mixed[]>> */
11+
public function dataFileAsserts(): iterable
12+
{
13+
yield from $this->gatherAssertTypes(__DIR__ . '/data/foreach-loop-no-scope-pollution.php');
14+
}
15+
16+
/**
17+
* @dataProvider dataFileAsserts
18+
* @param mixed ...$args
19+
*/
20+
public function testFileAsserts(
21+
string $assertType,
22+
string $file,
23+
...$args,
24+
): void
25+
{
26+
$this->assertFileAsserts($assertType, $file, ...$args);
27+
}
28+
29+
public static function getAdditionalConfigFiles(): array
30+
{
31+
return [
32+
__DIR__ . '/foreachLoopNoScopePollution.neon',
33+
];
34+
}
35+
36+
}

‎tests/PHPStan/Analyser/data/foreach-loop-no-scope-pollution.php‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace ForEachLoopNoScopePollution;
4+
5+
use PHPStan\TrinaryLogic;
6+
use function PHPStan\Testing\assertNativeType;
7+
use function PHPStan\Testing\assertType;
8+
use function PHPStan\Testing\assertVariableCertainty;
9+
10+
class ForEachLoopNoScopePollution
11+
{
12+
13+
/** @param int $b */
14+
public function loopThatIteratesAtLeastOnce(int $a, $b): void
15+
{
16+
$items = [17 => 'foo', 'bar' => 19];
17+
18+
foreach ($items as $key => $item) {
19+
$a = rand(0, 1);
20+
$b = rand(0, 1);
21+
$c = rand(0, 1);
22+
}
23+
24+
assertType("17|'bar'", $key);
25+
assertNativeType("17|'bar'", $key);
26+
assertVariableCertainty(TrinaryLogic::createMaybe(), $key);
27+
28+
assertType("19|'foo'", $item);
29+
assertNativeType("19|'foo'", $item);
30+
assertVariableCertainty(TrinaryLogic::createMaybe(), $item);
31+
32+
assertType('int<0, 1>', $a);
33+
assertNativeType('int', $a);
34+
assertVariableCertainty(TrinaryLogic::createYes(), $a);
35+
36+
assertType('int<0, 1>', $b);
37+
assertNativeType('int', $b);
38+
assertVariableCertainty(TrinaryLogic::createYes(), $b);
39+
40+
assertType('int<0, 1>', $c);
41+
assertNativeType('int', $c);
42+
assertVariableCertainty(TrinaryLogic::createYes(), $c);
43+
}
44+
45+
/** @param int $b */
46+
public function loopThatMightIterateAtLeastOnce(int $a, $b): void
47+
{
48+
$items = [];
49+
if (rand(0, 1)) {
50+
$items[17] = 'foo';
51+
}
52+
if (rand(0, 1)) {
53+
$items['bar'] = 19;
54+
}
55+
56+
foreach ($items as $key => $item) {
57+
$a = rand(0, 1);
58+
$b = rand(0, 1);
59+
$c = rand(0, 1);
60+
}
61+
62+
assertType("17|'bar'", $key);
63+
assertNativeType("17|'bar'", $key);
64+
assertVariableCertainty(TrinaryLogic::createMaybe(), $key);
65+
66+
assertType("19|'foo'", $item);
67+
assertNativeType("19|'foo'", $item);
68+
assertVariableCertainty(TrinaryLogic::createMaybe(), $item);
69+
70+
assertType('int', $a);
71+
assertNativeType('int', $a);
72+
assertVariableCertainty(TrinaryLogic::createYes(), $a);
73+
74+
assertType('int', $b);
75+
assertNativeType('mixed', $b);
76+
assertVariableCertainty(TrinaryLogic::createYes(), $b);
77+
78+
assertType('int<0, 1>', $c);
79+
assertNativeType('int', $c);
80+
assertVariableCertainty(TrinaryLogic::createMaybe(), $c);
81+
}
82+
83+
/** @param int $b */
84+
public function loopThatNeverIterates(int $a, $b): void
85+
{
86+
$items = [];
87+
88+
foreach ($items as $key => $item) {
89+
$a = rand(0, 1);
90+
$b = rand(0, 1);
91+
$c = rand(0, 1);
92+
}
93+
94+
assertType('*ERROR*', $key);
95+
assertNativeType('*ERROR*', $key);
96+
assertVariableCertainty(TrinaryLogic::createNo(), $key);
97+
98+
assertType('*ERROR*', $item);
99+
assertNativeType('*ERROR*', $item);
100+
assertVariableCertainty(TrinaryLogic::createNo(), $item);
101+
102+
assertType('int', $a);
103+
assertNativeType('int', $a);
104+
assertVariableCertainty(TrinaryLogic::createYes(), $a);
105+
106+
assertType('int', $b);
107+
assertNativeType('mixed', $b);
108+
assertVariableCertainty(TrinaryLogic::createYes(), $b);
109+
110+
assertType('*ERROR*', $c);
111+
assertNativeType('*ERROR*', $c);
112+
assertVariableCertainty(TrinaryLogic::createNo(), $c);
113+
}
114+
115+
}

‎tests/PHPStan/Analyser/foreachLoopNoScopePollution.neon‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
parameters:
2+
polluteScopeWithAlwaysIterableForeach: false

‎tests/PHPStan/Analyser/nsrt/foreach-loop.php‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace ForEachLoop;
4+
5+
use PHPStan\TrinaryLogic;
6+
use function PHPStan\Testing\assertNativeType;
7+
use function PHPStan\Testing\assertType;
8+
use function PHPStan\Testing\assertVariableCertainty;
9+
10+
class ForEachLoop
11+
{
12+
13+
/** @param int $b */
14+
public function loopThatIteratesAtLeastOnce(int $a, $b): void
15+
{
16+
$items = [17 => 'foo', 'bar' => 19];
17+
18+
foreach ($items as $key => $item) {
19+
$a = rand(0, 1);
20+
$b = rand(0, 1);
21+
$c = rand(0, 1);
22+
}
23+
24+
assertType("17|'bar'", $key);
25+
assertNativeType("17|'bar'", $key);
26+
assertVariableCertainty(TrinaryLogic::createYes(), $key);
27+
28+
assertType("19|'foo'", $item);
29+
assertNativeType("19|'foo'", $item);
30+
assertVariableCertainty(TrinaryLogic::createYes(), $item);
31+
32+
assertType('int<0, 1>', $a);
33+
assertNativeType('int', $a);
34+
assertVariableCertainty(TrinaryLogic::createYes(), $a);
35+
36+
assertType('int<0, 1>', $b);
37+
assertNativeType('int', $b);
38+
assertVariableCertainty(TrinaryLogic::createYes(), $b);
39+
40+
assertType('int<0, 1>', $c);
41+
assertNativeType('int', $c);
42+
assertVariableCertainty(TrinaryLogic::createYes(), $c);
43+
}
44+
45+
/** @param int $b */
46+
public function loopThatMightIterateAtLeastOnce(int $a, $b): void
47+
{
48+
$items = [];
49+
if (rand(0, 1)) {
50+
$items[17] = 'foo';
51+
}
52+
if (rand(0, 1)) {
53+
$items['bar'] = 19;
54+
}
55+
56+
foreach ($items as $key => $item) {
57+
$a = rand(0, 1);
58+
$b = rand(0, 1);
59+
$c = rand(0, 1);
60+
}
61+
62+
assertType("17|'bar'", $key);
63+
assertNativeType("17|'bar'", $key);
64+
assertVariableCertainty(TrinaryLogic::createMaybe(), $key);
65+
66+
assertType("19|'foo'", $item);
67+
assertNativeType("19|'foo'", $item);
68+
assertVariableCertainty(TrinaryLogic::createMaybe(), $item);
69+
70+
assertType('int', $a);
71+
assertNativeType('int', $a);
72+
assertVariableCertainty(TrinaryLogic::createYes(), $a);
73+
74+
assertType('int', $b);
75+
assertNativeType('mixed', $b);
76+
assertVariableCertainty(TrinaryLogic::createYes(), $b);
77+
78+
assertType('int<0, 1>', $c);
79+
assertNativeType('int', $c);
80+
assertVariableCertainty(TrinaryLogic::createMaybe(), $c);
81+
}
82+
83+
/** @param int $b */
84+
public function loopThatNeverIterates(int $a, $b): void
85+
{
86+
$items = [];
87+
88+
foreach ($items as $key => $item) {
89+
$a = rand(0, 1);
90+
$b = rand(0, 1);
91+
$c = rand(0, 1);
92+
}
93+
94+
assertType('*ERROR*', $key);
95+
assertNativeType('*ERROR*', $key);
96+
assertVariableCertainty(TrinaryLogic::createNo(), $key);
97+
98+
assertType('*ERROR*', $item);
99+
assertNativeType('*ERROR*', $item);
100+
assertVariableCertainty(TrinaryLogic::createNo(), $item);
101+
102+
assertType('int', $a);
103+
assertNativeType('int', $a);
104+
assertVariableCertainty(TrinaryLogic::createYes(), $a);
105+
106+
assertType('int', $b);
107+
assertNativeType('mixed', $b);
108+
assertVariableCertainty(TrinaryLogic::createYes(), $b);
109+
110+
assertType('*ERROR*', $c);
111+
assertNativeType('*ERROR*', $c);
112+
assertVariableCertainty(TrinaryLogic::createNo(), $c);
113+
}
114+
115+
}

‎tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -525,10 +525,6 @@ public function dataForeachPolluteScopeWithAlwaysIterableForeach(): array
525525
'Variable $val might not be defined.',
526526
20,
527527
],
528-
[
529-
'Variable $test might not be defined.',
530-
21,
531-
],
532528
[
533529
'Variable $key might not be defined.',
534530
32,

0 commit comments

Comments
 (0)