Skip to content

Commit c55aa05

Browse files
herndlmherndlm
authored
Improve for loop initial statement scope pollution
1 parent ccfb4ab commit c55aa05

8 files changed

+356
-1
lines changed

‎src/Analyser/MutatingScope.php

+62
Original file line numberDiff line numberDiff line change
@@ -4900,6 +4900,68 @@ public function processAlwaysIterableForeachScopeWithoutPollute(self $finalScope
49004900
);
49014901
}
49024902

4903+
public function processAlwaysIterableForScopeWithoutPollute(self $finalScope, self $initScope): self
4904+
{
4905+
$expressionTypes = $this->expressionTypes;
4906+
$initScopeExpressionTypes = $initScope->expressionTypes;
4907+
foreach ($finalScope->expressionTypes as $variableExprString => $variableTypeHolder) {
4908+
if (!isset($expressionTypes[$variableExprString])) {
4909+
if (isset($initScopeExpressionTypes[$variableExprString])) {
4910+
$expressionTypes[$variableExprString] = ExpressionTypeHolder::createMaybe($variableTypeHolder->getExpr(), $variableTypeHolder->getType());
4911+
continue;
4912+
}
4913+
4914+
$expressionTypes[$variableExprString] = $variableTypeHolder;
4915+
continue;
4916+
}
4917+
4918+
$expressionTypes[$variableExprString] = new ExpressionTypeHolder(
4919+
$variableTypeHolder->getExpr(),
4920+
$variableTypeHolder->getType(),
4921+
$variableTypeHolder->getCertainty()->and($expressionTypes[$variableExprString]->getCertainty()),
4922+
);
4923+
}
4924+
4925+
$nativeTypes = $this->nativeExpressionTypes;
4926+
$initScopeNativeExpressionTypes = $initScope->nativeExpressionTypes;
4927+
foreach ($finalScope->nativeExpressionTypes as $variableExprString => $variableTypeHolder) {
4928+
if (!isset($nativeTypes[$variableExprString])) {
4929+
if (isset($initScopeNativeExpressionTypes[$variableExprString])) {
4930+
$nativeTypes[$variableExprString] = ExpressionTypeHolder::createMaybe($variableTypeHolder->getExpr(), $variableTypeHolder->getType());
4931+
continue;
4932+
}
4933+
4934+
$nativeTypes[$variableExprString] = $variableTypeHolder;
4935+
continue;
4936+
}
4937+
4938+
$nativeTypes[$variableExprString] = new ExpressionTypeHolder(
4939+
$variableTypeHolder->getExpr(),
4940+
$variableTypeHolder->getType(),
4941+
$variableTypeHolder->getCertainty()->and($nativeTypes[$variableExprString]->getCertainty()),
4942+
);
4943+
}
4944+
4945+
return $this->scopeFactory->create(
4946+
$this->context,
4947+
$this->isDeclareStrictTypes(),
4948+
$this->getFunction(),
4949+
$this->getNamespace(),
4950+
$expressionTypes,
4951+
$nativeTypes,
4952+
$this->conditionalExpressions,
4953+
$this->inClosureBindScopeClasses,
4954+
$this->anonymousFunctionReflection,
4955+
$this->inFirstLevelStatement,
4956+
[],
4957+
[],
4958+
[],
4959+
$this->afterExtractCall,
4960+
$this->parentScope,
4961+
$this->nativeTypesPromoted,
4962+
);
4963+
}
4964+
49034965
public function generalizeWith(self $otherScope): self
49044966
{
49054967
$variableTypeHolders = $this->generalizeVariableTypeHolders(

‎src/Analyser/NodeScopeResolver.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -1410,7 +1410,7 @@ private function processStmtNode(
14101410
}
14111411
} else {
14121412
if (!$this->polluteScopeWithLoopInitialAssignments) {
1413-
$finalScope = $finalScope->mergeWith($scope);
1413+
$finalScope = $scope->processAlwaysIterableForScopeWithoutPollute($finalScope, $initScope);
14141414
}
14151415
}
14161416

‎tests/PHPStan/Analyser/ForLoopNoScopePollutionTest.php

+36
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 ForLoopNoScopePollutionTest extends TypeInferenceTestCase
8+
{
9+
10+
/** @return iterable<array<string, mixed[]>> */
11+
public function dataFileAsserts(): iterable
12+
{
13+
yield from $this->gatherAssertTypes(__DIR__ . '/data/for-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__ . '/forLoopNoScopePollution.neon',
33+
];
34+
}
35+
36+
}

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

+107
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace ForLoopNoScopePollution;
4+
5+
6+
use PHPStan\TrinaryLogic;
7+
use function PHPStan\Testing\assertNativeType;
8+
use function PHPStan\Testing\assertType;
9+
use function PHPStan\Testing\assertVariableCertainty;
10+
11+
class ForLoop
12+
{
13+
14+
/** @param int $b */
15+
public function loopThatIteratesAtLeastOnce(int $a, $b): void
16+
{
17+
$j = 0;
18+
for ($i = 0, $j = 10; $i < 10; $i++, $j--) {
19+
$a = rand(0, 1);
20+
$b = rand(0, 1);
21+
$c = rand(0, 1);
22+
}
23+
24+
assertType('int<10, max>', $i);
25+
assertNativeType('int<10, max>', $i);
26+
assertVariableCertainty(TrinaryLogic::createMaybe(), $i);
27+
28+
assertType('int<min, 10>', $j);
29+
assertNativeType('int<min, 10>', $j);
30+
assertVariableCertainty(TrinaryLogic::createYes(), $j);
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+
$j = 0;
49+
for ($i = 0, $j = 10; $i < rand(0, 1); $i++, $j--) {
50+
$a = rand(0, 1);
51+
$b = rand(0, 1);
52+
$c = rand(0, 1);
53+
}
54+
55+
assertType('int<0, max>', $i);
56+
assertNativeType('int<0, max>', $i);
57+
assertVariableCertainty(TrinaryLogic::createMaybe(), $i);
58+
59+
assertType('int<min, 10>', $j);
60+
assertNativeType('int<min, 10>', $j);
61+
assertVariableCertainty(TrinaryLogic::createYes(), $j);
62+
63+
assertType('int', $a);
64+
assertNativeType('int', $a);
65+
assertVariableCertainty(TrinaryLogic::createYes(), $a);
66+
67+
assertType('int', $b);
68+
assertNativeType('mixed', $b);
69+
assertVariableCertainty(TrinaryLogic::createYes(), $b);
70+
71+
assertType('int<0, 1>', $c);
72+
assertNativeType('int', $c);
73+
assertVariableCertainty(TrinaryLogic::createMaybe(), $c);
74+
}
75+
76+
/** @param int $b */
77+
public function loopThatNeverIterates(int $a, $b): void
78+
{
79+
$j = 0;
80+
for ($i = 0, $j = 10; $i > 10; $i++, $j--) {
81+
$a = rand(0, 1);
82+
$b = rand(0, 1);
83+
$c = rand(0, 1);
84+
}
85+
86+
assertType('*ERROR*', $i);
87+
assertNativeType('*ERROR*', $i);
88+
assertVariableCertainty(TrinaryLogic::createNo(), $i);
89+
90+
assertType('0', $j);
91+
assertNativeType('0', $j);
92+
assertVariableCertainty(TrinaryLogic::createYes(), $j);
93+
94+
assertType('int', $a);
95+
assertNativeType('int', $a);
96+
assertVariableCertainty(TrinaryLogic::createYes(), $a);
97+
98+
assertType('int', $b);
99+
assertNativeType('mixed', $b);
100+
assertVariableCertainty(TrinaryLogic::createYes(), $b);
101+
102+
assertType('*ERROR*', $c);
103+
assertNativeType('*ERROR*', $c);
104+
assertVariableCertainty(TrinaryLogic::createNo(), $c);
105+
}
106+
107+
}

‎tests/PHPStan/Analyser/forLoopNoScopePollution.neon

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
parameters:
2+
polluteScopeWithLoopInitialAssignments: false

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

+107
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace ForLoop;
4+
5+
6+
use PHPStan\TrinaryLogic;
7+
use function PHPStan\Testing\assertNativeType;
8+
use function PHPStan\Testing\assertType;
9+
use function PHPStan\Testing\assertVariableCertainty;
10+
11+
class ForLoop
12+
{
13+
14+
/** @param int $b */
15+
public function loopThatIteratesAtLeastOnce(int $a, $b): void
16+
{
17+
$j = 0;
18+
for ($i = 0, $j = 10; $i < 10; $i++, $j--) {
19+
$a = rand(0, 1);
20+
$b = rand(0, 1);
21+
$c = rand(0, 1);
22+
}
23+
24+
assertType('int<10, max>', $i);
25+
assertNativeType('int<10, max>', $i);
26+
assertVariableCertainty(TrinaryLogic::createYes(), $i);
27+
28+
assertType('int<min, 10>', $j);
29+
assertNativeType('int<min, 10>', $j);
30+
assertVariableCertainty(TrinaryLogic::createYes(), $j);
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+
$j = 0;
49+
for ($i = 0, $j = 10; $i < rand(0, 1); $i++, $j--) {
50+
$a = rand(0, 1);
51+
$b = rand(0, 1);
52+
$c = rand(0, 1);
53+
}
54+
55+
assertType('int<0, max>', $i);
56+
assertNativeType('int<0, max>', $i);
57+
assertVariableCertainty(TrinaryLogic::createYes(), $i);
58+
59+
assertType('int<min, 10>', $j);
60+
assertNativeType('int<min, 10>', $j);
61+
assertVariableCertainty(TrinaryLogic::createYes(), $j);
62+
63+
assertType('int', $a);
64+
assertNativeType('int', $a);
65+
assertVariableCertainty(TrinaryLogic::createYes(), $a);
66+
67+
assertType('int', $b);
68+
assertNativeType('mixed', $b);
69+
assertVariableCertainty(TrinaryLogic::createYes(), $b);
70+
71+
assertType('int<0, 1>', $c);
72+
assertNativeType('int', $c);
73+
assertVariableCertainty(TrinaryLogic::createMaybe(), $c);
74+
}
75+
76+
/** @param int $b */
77+
public function loopThatNeverIterates(int $a, $b): void
78+
{
79+
$j = 0;
80+
for ($i = 0, $j = 10; $i > 10; $i++, $j--) {
81+
$a = rand(0, 1);
82+
$b = rand(0, 1);
83+
$c = rand(0, 1);
84+
}
85+
86+
assertType('0', $i);
87+
assertNativeType('0', $i);
88+
assertVariableCertainty(TrinaryLogic::createYes(), $i);
89+
90+
assertType('10', $j);
91+
assertNativeType('10', $j);
92+
assertVariableCertainty(TrinaryLogic::createYes(), $j);
93+
94+
assertType('int', $a);
95+
assertNativeType('int', $a);
96+
assertVariableCertainty(TrinaryLogic::createYes(), $a);
97+
98+
assertType('int', $b);
99+
assertNativeType('mixed', $b);
100+
assertVariableCertainty(TrinaryLogic::createYes(), $b);
101+
102+
assertType('*ERROR*', $c);
103+
assertNativeType('*ERROR*', $c);
104+
assertVariableCertainty(TrinaryLogic::createNo(), $c);
105+
}
106+
107+
}

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

+9
Original file line numberDiff line numberDiff line change
@@ -1068,4 +1068,13 @@ public function testBug10228(): void
10681068
$this->analyse([__DIR__ . '/data/bug-10228.php'], []);
10691069
}
10701070

1071+
public function testBug9550(): void
1072+
{
1073+
$this->cliArgumentsVariablesRegistered = true;
1074+
$this->polluteScopeWithLoopInitialAssignments = false;
1075+
$this->checkMaybeUndefinedVariables = true;
1076+
$this->polluteScopeWithAlwaysIterableForeach = true;
1077+
$this->analyse([__DIR__ . '/data/bug-9550.php'], []);
1078+
}
1079+
10711080
}

‎tests/PHPStan/Rules/Variables/data/bug-9550.php

+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug9550;
4+
5+
class Foo
6+
{
7+
protected static function makeShortTextFromScalar(string $value, int $maxUtf8Length = 50): string
8+
{
9+
$maxUtf8Length = max(20, min($maxUtf8Length, 100));
10+
11+
$vStrLonger = mb_substr($value, 0, $maxUtf8Length + 1000);
12+
13+
$withThreeDots = false;
14+
\PHPStan\dumpType($maxUtf8Length);
15+
for ($l = $maxUtf8Length; $l > 0; --$l) {
16+
$vStr = mb_substr($vStrLonger, 0, $l);
17+
if ($vStr !== $vStrLonger) {
18+
$vStrLonger = $vStr;
19+
$vStr = mb_substr($vStr, 0, $l - 3);
20+
$withThreeDots = true;
21+
} else {
22+
$vStrLonger = $vStr;
23+
}
24+
$vStr = str_replace(["\0", "\t", "\n", "\r"], ['\0', '\t', '\n', '\r'], $vStr);
25+
if (mb_strlen($vStr) <= $maxUtf8Length - ($withThreeDots ? 3 : 0)) {
26+
break;
27+
}
28+
}
29+
30+
return '\'' . $vStr . '\'' . ($withThreeDots ? '...' : '');
31+
}
32+
}

0 commit comments

Comments
 (0)