Skip to content

Commit 16f2934

Browse files
phpstan-botclaude
phpstan-bot
and
claude
committed
Converge loops nested in a top-level loop's convergence passes instead of walking them once
- StatementContext: add shouldConvergeLoops() and enterLoopPass(). A top-level loop's passes now run in a deep context whose loops still converge; the passes of those nested loops run in a plain deep context, where loops are walked once, so the cost stays polynomial in the nesting depth. - ForeachHandler, WhileHandler, DoWhileHandler, ForHandler: converge when shouldConvergeLoops() (was isTopLevel()) and walk passes with enterLoopPass() (was enterDeep()). Foreach unrolling over constant arrays stays top-level only. - Previously a nested loop inside an outer pass ran a single iteration, so a counter it increments stayed 0|1. A condition such as `$n > 1` was then never true in the outer loop's passes, and an array appended under it converged to array{}|array{string} instead of list<string>. - Analogous cases fixed by the same change and covered by tests: nested while/for/do-while inner loops, while/for/do-while outer loops, a flag set inside the inner body, and three levels of nesting. - turbo-ext: mirror StatementContext (new slot, methods, direct entries) and the four loop handlers; regenerate generated/StatementContext.h. - bug-10438: the expectation recorded the same bug; an inner foreach appends any number of strings, so list<string> is correct. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent acd0511 commit 16f2934

16 files changed

Lines changed: 394 additions & 80 deletions

‎src/Analyser/StatementContext.php‎

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ private function __construct(
2323
private bool $resolveTemplateArguments = true,
2424
private ?Type $expectedReturnType = null,
2525
private ?Type $nativeExpectedReturnType = null,
26+
private bool $convergesLoops = false,
2627
)
2728
{
2829
}
@@ -48,6 +49,35 @@ public function isTopLevel(): bool
4849
return $this->isTopLevel;
4950
}
5051

52+
/**
53+
* Whether loops walk their body until the types stabilize. Besides the
54+
* top level, the loops nested in a convergence pass of a top-level loop
55+
* do, and so do the loops nested in their final walks - otherwise a
56+
* variable a nested loop changes over several iterations keeps the type
57+
* of its first iteration in the outer loop's passes.
58+
*/
59+
public function shouldConvergeLoops(): bool
60+
{
61+
return $this->isTopLevel || $this->convergesLoops;
62+
}
63+
64+
/**
65+
* The context of a loop's convergence pass. The loops nested in a pass of
66+
* a top-level loop converge themselves, their passes' nested loops are
67+
* walked once.
68+
*/
69+
public function enterLoopPass(): self
70+
{
71+
if ($this->isTopLevel) {
72+
return new self(false, $this->foreachUnrollFactor, $this->resolveTemplateArguments, $this->expectedReturnType, $this->nativeExpectedReturnType, true);
73+
}
74+
if ($this->convergesLoops) {
75+
return new self(false, $this->foreachUnrollFactor, $this->resolveTemplateArguments, $this->expectedReturnType, $this->nativeExpectedReturnType);
76+
}
77+
78+
return $this;
79+
}
80+
5181
public function getForeachUnrollFactor(): int
5282
{
5383
return $this->foreachUnrollFactor;
@@ -64,7 +94,7 @@ public function withoutTemplateArgumentResolution(): self
6494
return $this;
6595
}
6696

67-
return new self($this->isTopLevel, $this->foreachUnrollFactor, false, $this->expectedReturnType, $this->nativeExpectedReturnType);
97+
return new self($this->isTopLevel, $this->foreachUnrollFactor, false, $this->expectedReturnType, $this->nativeExpectedReturnType, $this->convergesLoops);
6898
}
6999

70100
/**
@@ -78,7 +108,7 @@ public function withExpectedReturnType(?Type $expectedReturnType, ?Type $nativeE
78108
return $this;
79109
}
80110

81-
return new self($this->isTopLevel, $this->foreachUnrollFactor, $this->resolveTemplateArguments, $expectedReturnType, $nativeExpectedReturnType);
111+
return new self($this->isTopLevel, $this->foreachUnrollFactor, $this->resolveTemplateArguments, $expectedReturnType, $nativeExpectedReturnType, $this->convergesLoops);
82112
}
83113

84114
public function getExpectedReturnType(): ?Type
@@ -102,7 +132,7 @@ public function enterDeep(): self
102132

103133
public function enterUnrolledForeach(int $totalKeys): self
104134
{
105-
return new self($this->isTopLevel, $this->foreachUnrollFactor * $totalKeys, $this->resolveTemplateArguments, $this->expectedReturnType, $this->nativeExpectedReturnType);
135+
return new self($this->isTopLevel, $this->foreachUnrollFactor * $totalKeys, $this->resolveTemplateArguments, $this->expectedReturnType, $this->nativeExpectedReturnType, $this->convergesLoops);
106136
}
107137

108138
}

‎src/Analyser/StmtHandler/DoWhileHandler.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public function processStmt(
6767
$prevEntryScope = null;
6868
$previousPass = null;
6969
$previousPassStorage = null;
70-
if ($context->isTopLevel()) {
70+
if ($context->shouldConvergeLoops()) {
7171
$bodyIsReplayable = $nodeScopeResolver->isReplayableConvergenceBody($stmt, $stmt->stmts);
7272
// each pass after the first re-walks only the statements whose
7373
// entry state differs from the previous pass's (StatementsHandler::processLoopPassStmtNodes())
@@ -91,7 +91,7 @@ public function processStmt(
9191
$scope->pushExpressionResultStorage($storage);
9292
try {
9393
$passRecord = null;
94-
$bodyScopeResult = $nodeScopeResolver->processLoopPassStmtNodes($stmt, $stmt->stmts, $bodyScope, $storage, $bodyRecording, $context->enterDeep()->withoutTemplateArgumentResolution(), $previousPass, $passRecord)->filterOutLoopExitPoints();
94+
$bodyScopeResult = $nodeScopeResolver->processLoopPassStmtNodes($stmt, $stmt->stmts, $bodyScope, $storage, $bodyRecording, $context->enterLoopPass()->withoutTemplateArgumentResolution(), $previousPass, $passRecord)->filterOutLoopExitPoints();
9595
$previousPass = $passRecord;
9696
$previousPassStorage = $storage;
9797
$backEdgeScope = $bodyScopeResult->getLoopBackEdgeScope();

‎src/Analyser/StmtHandler/ForHandler.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ public function processStmt(
195195
}
196196
}
197197

198-
if ($context->isTopLevel()) {
198+
if ($context->shouldConvergeLoops()) {
199199
$count = 0;
200200
$prevEntryScope = null;
201201
// each pass after the first re-walks only the statements whose
@@ -226,7 +226,7 @@ public function processStmt(
226226
$passFlows[] = $passCondResult->getVariableFlow();
227227
}
228228
$passRecord = null;
229-
$bodyScopeResult = $nodeScopeResolver->processLoopPassStmtNodes($stmt, $stmt->stmts, $bodyScope, $storage, new NoopNodeCallback(), $context->enterDeep()->withoutTemplateArgumentResolution(), $previousPass, $passRecord)->filterOutLoopExitPoints();
229+
$bodyScopeResult = $nodeScopeResolver->processLoopPassStmtNodes($stmt, $stmt->stmts, $bodyScope, $storage, new NoopNodeCallback(), $context->enterLoopPass()->withoutTemplateArgumentResolution(), $previousPass, $passRecord)->filterOutLoopExitPoints();
230230
$previousPass = $passRecord;
231231
$previousPassStorage = $storage;
232232
$passFlows[] = $bodyScopeResult->getVariableFlow();

‎src/Analyser/StmtHandler/ForeachHandler.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ static function () use ($condResult, $emptyArrayType): Type {
195195
$replayEntryScope = null;
196196
$unrolledEndScope = null;
197197
$unrolledTotalKeys = null;
198-
if ($context->isTopLevel()) {
198+
if ($context->shouldConvergeLoops()) {
199199
$storage = $originalStorage->duplicate();
200200

201201
$originalScope = $nonEmptyIterateeScope;
@@ -213,7 +213,7 @@ static function () use ($condResult, $emptyArrayType): Type {
213213
$foreachIterateeType = $iterateeResult->getType();
214214
$foreachNativeIterateeType = $iterateeResult->getNativeType();
215215
}
216-
$unrolledResult = $this->tryProcessUnrolledConstantArrayForeach($nodeScopeResolver, $stmt, $originalScope, $originalStorage, $context, $foreachIterateeType, $foreachNativeIterateeType);
216+
$unrolledResult = $context->isTopLevel() ? $this->tryProcessUnrolledConstantArrayForeach($nodeScopeResolver, $stmt, $originalScope, $originalStorage, $context, $foreachIterateeType, $foreachNativeIterateeType) : null;
217217
if ($unrolledResult !== null) {
218218
$bodyScope = $unrolledResult['bodyScope'];
219219
$unrolledEndScope = $unrolledResult['endScope'];
@@ -250,7 +250,7 @@ static function () use ($condResult, $emptyArrayType): Type {
250250
try {
251251
$bodyScope = $this->enterForeach($nodeScopeResolver, $bodyScope, $storage, $originalScope, $stmt, $foreachIterateeType, $foreachNativeIterateeType, $nodeCallback);
252252
$passRecord = null;
253-
$bodyScopeResult = $nodeScopeResolver->processLoopPassStmtNodes($stmt, $stmt->stmts, $bodyScope, $storage, $bodyRecording, $context->enterDeep()->withoutTemplateArgumentResolution(), $previousPass, $passRecord)->filterOutLoopExitPoints();
253+
$bodyScopeResult = $nodeScopeResolver->processLoopPassStmtNodes($stmt, $stmt->stmts, $bodyScope, $storage, $bodyRecording, $context->enterLoopPass()->withoutTemplateArgumentResolution(), $previousPass, $passRecord)->filterOutLoopExitPoints();
254254
$previousPass = $passRecord;
255255
$previousPassStorage = $storage;
256256
$backEdgeScope = $bodyScopeResult->getLoopBackEdgeScope();

‎src/Analyser/StmtHandler/WhileHandler.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ public function processStmt(
9494
$prevEntryScope = null;
9595
$previousPass = null;
9696
$previousPassStorage = null;
97-
if ($context->isTopLevel()) {
97+
if ($context->shouldConvergeLoops()) {
9898
$count = 0;
9999
$bodyIsReplayable = $nodeScopeResolver->isReplayableConvergenceBody($stmt, $stmt->stmts);
100100
do {
@@ -119,7 +119,7 @@ public function processStmt(
119119
$passCondResult = $nodeScopeResolver->processExprNode($stmt, $stmt->cond, $bodyScope, $storage, $condRecording, ExpressionContext::createDeep(resolveTemplateArguments: false));
120120
$bodyScope = $passCondResult->getTruthyScope();
121121
$passRecord = null;
122-
$bodyScopeResult = $nodeScopeResolver->processLoopPassStmtNodes($stmt, $stmt->stmts, $bodyScope, $storage, $bodyRecording, $context->enterDeep()->withoutTemplateArgumentResolution(), $previousPass, $passRecord)->filterOutLoopExitPoints();
122+
$bodyScopeResult = $nodeScopeResolver->processLoopPassStmtNodes($stmt, $stmt->stmts, $bodyScope, $storage, $bodyRecording, $context->enterLoopPass()->withoutTemplateArgumentResolution(), $previousPass, $passRecord)->filterOutLoopExitPoints();
123123
$previousPass = $passRecord;
124124
$previousPassStorage = $storage;
125125
$backEdgeScope = $bodyScopeResult->getLoopBackEdgeScope();

‎tests/PHPStan/Analyser/nsrt/bug-10438.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ public function extract(SimpleXMLElement $data, string $type = 'Meta'): array
2121
$meta[$key] = (string)$tag->{$valueName};
2222
continue;
2323
}
24-
assertType('array<string, array{}|array{string}|string>', $meta);
24+
assertType('array<string, list<string>|string>', $meta);
2525
$meta[$key] = [];
2626
assertType('array{}', $meta[$key]);
2727
foreach ($tag->{$valueName} as $value) {

‎tests/PHPStan/Analyser/nsrt/bug-15435.php‎

Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace Bug15435;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
/**
8+
* @param list<array{bucket: string, key: string}> $facts
9+
* @param list<string> $months
10+
*/
11+
function nestedLoop(array $facts, array $months): void
12+
{
13+
$keys = [];
14+
foreach ($facts as $fact) {
15+
$n = 0;
16+
foreach ($months as $m) {
17+
if ('' !== $m) {
18+
++$n;
19+
}
20+
}
21+
if ($n > 1) {
22+
$keys[] = $fact['key'];
23+
}
24+
}
25+
assertType('list<string>', $keys);
26+
sort($keys);
27+
}
28+
29+
/**
30+
* @param list<array{bucket: string, key: string}> $facts
31+
* @param list<string> $months
32+
*/
33+
function flagInInner(array $facts, array $months): void
34+
{
35+
$keys = [];
36+
foreach ($facts as $fact) {
37+
$n = 0;
38+
$found = false;
39+
foreach ($months as $m) {
40+
$n++;
41+
if ($n > 1) {
42+
$found = true;
43+
}
44+
}
45+
if ($found) {
46+
$keys[] = $fact['key'];
47+
}
48+
}
49+
assertType('list<string>', $keys);
50+
}
51+
52+
/**
53+
* @param list<array{bucket: string, key: string}> $facts
54+
* @param list<string> $months
55+
*/
56+
function whileInner(array $facts, array $months): void
57+
{
58+
$keys = [];
59+
foreach ($facts as $fact) {
60+
$n = 0;
61+
while ($n < count($months)) {
62+
$n++;
63+
}
64+
if ($n > 1) {
65+
$keys[] = $fact['key'];
66+
}
67+
}
68+
assertType('list<string>', $keys);
69+
}
70+
71+
/**
72+
* @param list<array{bucket: string, key: string}> $facts
73+
* @param list<string> $months
74+
*/
75+
function forInner(array $facts, array $months): void
76+
{
77+
$keys = [];
78+
foreach ($facts as $fact) {
79+
$n = 0;
80+
for ($i = 0; $i < count($months); $i++) {
81+
$n++;
82+
}
83+
if ($n > 1) {
84+
$keys[] = $fact['key'];
85+
}
86+
}
87+
assertType('list<string>', $keys);
88+
}
89+
90+
/**
91+
* @param list<array{bucket: string, key: string}> $facts
92+
* @param list<string> $months
93+
*/
94+
function doWhileInner(array $facts, array $months): void
95+
{
96+
$keys = [];
97+
foreach ($facts as $fact) {
98+
$n = 0;
99+
do {
100+
$n++;
101+
} while ($n < count($months));
102+
if ($n > 1) {
103+
$keys[] = $fact['key'];
104+
}
105+
}
106+
assertType('list<string>', $keys);
107+
}
108+
109+
/**
110+
* @param list<array{bucket: string, key: string}> $facts
111+
* @param list<string> $months
112+
*/
113+
function whileOuter(array $facts, array $months): void
114+
{
115+
$keys = [];
116+
$j = 0;
117+
while ($j < count($facts)) {
118+
$n = 0;
119+
foreach ($months as $m) {
120+
if ('' !== $m) {
121+
++$n;
122+
}
123+
}
124+
if ($n > 1) {
125+
$keys[] = $facts[$j]['key'];
126+
}
127+
$j++;
128+
}
129+
assertType('list<string>', $keys);
130+
}
131+
132+
/**
133+
* @param list<array{bucket: string, key: string}> $facts
134+
* @param list<string> $months
135+
*/
136+
function forOuter(array $facts, array $months): void
137+
{
138+
$keys = [];
139+
for ($j = 0; $j < count($facts); $j++) {
140+
$n = 0;
141+
foreach ($months as $m) {
142+
if ('' !== $m) {
143+
++$n;
144+
}
145+
}
146+
if ($n > 1) {
147+
$keys[] = $facts[$j]['key'];
148+
}
149+
}
150+
assertType('list<string>', $keys);
151+
}
152+
153+
/**
154+
* @param list<array{bucket: string, key: string}> $facts
155+
* @param list<string> $months
156+
*/
157+
function doWhileOuter(array $facts, array $months): void
158+
{
159+
$keys = [];
160+
$j = 0;
161+
do {
162+
$n = 0;
163+
foreach ($months as $m) {
164+
if ('' !== $m) {
165+
++$n;
166+
}
167+
}
168+
if ($n > 1) {
169+
$keys[] = 'x';
170+
}
171+
$j++;
172+
} while ($j < count($facts));
173+
assertType("list<'x'>", $keys);
174+
}
175+
176+
/**
177+
* @param list<list<string>> $groups
178+
* @param list<string> $months
179+
*/
180+
function threeLevels(array $groups, array $months): void
181+
{
182+
$keys = [];
183+
foreach ($groups as $group) {
184+
foreach ($group as $item) {
185+
$n = 0;
186+
foreach ($months as $m) {
187+
++$n;
188+
}
189+
if ($n > 1) {
190+
$keys[] = $item;
191+
}
192+
}
193+
}
194+
assertType('list<string>', $keys);
195+
}

‎tests/PHPStan/Rules/Functions/SortWithoutEffectRuleTest.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,4 +185,9 @@ public function testNamedArguments(): void
185185
]);
186186
}
187187

188+
public function testBug15435(): void
189+
{
190+
$this->analyse([__DIR__ . '/data/bug-15435.php'], []);
191+
}
192+
188193
}

‎tests/PHPStan/Rules/Functions/data/bug-15435.php‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace Bug15435Rule;
4+
5+
/**
6+
* @param list<array{bucket: string, key: string}> $facts
7+
* @param list<string> $months
8+
*/
9+
function nestedLoop(array $facts, array $months): void
10+
{
11+
$keys = [];
12+
foreach ($facts as $fact) {
13+
$n = 0;
14+
foreach ($months as $m) {
15+
if ('' !== $m) {
16+
++$n;
17+
}
18+
}
19+
if ($n > 1) {
20+
$keys[] = $fact['key'];
21+
}
22+
}
23+
sort($keys);
24+
}

0 commit comments

Comments
 (0)