Skip to content

Commit 56ccf25

Browse files
ondrejmirtesclaude
ondrejmirtes
and
claude
committed
Widen only the variables a loop can write when its scope converges
A variable the loop never writes enters every iteration with its value from before the loop, so its type differs between convergence passes only through narrowing by the loop's conditions. Generalizing it lost its type for nothing: `while ($xi < $xn) { $xi += 0.1; }` widened an `int<1, max>` $xn to `int`. The convergence of while, do-while, for and foreach loops now unions the types of such variables instead. The variables a loop can write are collected from its AST, from the arguments the pass's variable flow marks as passed by reference, and from references created before the loop. Variable variables, extract(), parse_str(), eval and include keep widening every variable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014LVEGd9G9w8j64EZQ7rysC
1 parent 248723e commit 56ccf25

8 files changed

Lines changed: 591 additions & 17 deletions

File tree

‎src/Analyser/LoopWrittenVariableNames.php‎

Lines changed: 226 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,226 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Analyser;
4+
5+
use PhpParser\Node;
6+
use PhpParser\Node\Expr;
7+
use PhpParser\Node\Stmt;
8+
use function array_pop;
9+
use function in_array;
10+
use function is_array;
11+
use function is_string;
12+
13+
/**
14+
* The variables a loop can write while its scope converges.
15+
*
16+
* A variable the loop never writes enters every iteration with its value from
17+
* before the loop, so its type differs between convergence passes only through
18+
* narrowing by the loop's conditions. Widening such a variable loses its type
19+
* for nothing.
20+
*/
21+
final class LoopWrittenVariableNames
22+
{
23+
24+
private const SYNTACTIC_NAMES_ATTRIBUTE = 'phpstanLoopWrittenVariableNames';
25+
26+
/**
27+
* Assignments, increments, destructuring, foreach bindings, catch, static,
28+
* global, unset and by-reference closure uses are found in the loop's AST.
29+
* Whether an argument is passed by reference is known only from the called
30+
* function's reflection, so those writes are read off the variable flow of
31+
* the convergence pass.
32+
*
33+
* @return array<string, true>|null null when the loop can write variables whose names are not known
34+
*/
35+
public static function collect(Node $loop, ?VariableFlow $passFlow): ?array
36+
{
37+
$names = self::getSyntacticNames($loop);
38+
if ($names === null) {
39+
return null;
40+
}
41+
42+
$flows = [$passFlow];
43+
while ($flows !== []) {
44+
$flow = array_pop($flows);
45+
if ($flow instanceof VariableAccessFlow) {
46+
if (in_array($flow->kind, [VariableFlow::WRITE, VariableFlow::DEFINE, VariableFlow::DISCARD, VariableFlow::ESCAPE], true)) {
47+
$names[$flow->name] = true;
48+
}
49+
continue;
50+
}
51+
if ($flow instanceof VariableSequenceFlow) {
52+
foreach ($flow->children as $child) {
53+
$flows[] = $child;
54+
}
55+
continue;
56+
}
57+
if (!$flow instanceof VariableControlFlow) {
58+
continue;
59+
}
60+
61+
foreach ($flow->children as $child) {
62+
$flows[] = $child;
63+
}
64+
foreach ($flow->catches as [, $catchFlow]) {
65+
$flows[] = $catchFlow;
66+
}
67+
foreach ($flow->cases as [$caseCondition, $caseBody]) {
68+
$flows[] = $caseCondition;
69+
$flows[] = $caseBody;
70+
}
71+
foreach ($flow->bindings as $write) {
72+
$names[$write->getVariableName()] = true;
73+
}
74+
foreach ($flow->ownWrites as $write) {
75+
$names[$write->getVariableName()] = true;
76+
}
77+
}
78+
79+
return $names;
80+
}
81+
82+
/**
83+
* @return array<string, true>|null
84+
*/
85+
private static function getSyntacticNames(Node $loop): ?array
86+
{
87+
$cached = $loop->getAttribute(self::SYNTACTIC_NAMES_ATTRIBUTE);
88+
if (is_array($cached)) {
89+
return $cached;
90+
}
91+
if ($cached === false) {
92+
return null;
93+
}
94+
95+
$names = self::findSyntacticNames($loop);
96+
$loop->setAttribute(self::SYNTACTIC_NAMES_ATTRIBUTE, $names ?? false);
97+
98+
return $names;
99+
}
100+
101+
/**
102+
* @return array<string, true>|null
103+
*/
104+
private static function findSyntacticNames(Node $loop): ?array
105+
{
106+
$names = [];
107+
$nodes = [$loop];
108+
while ($nodes !== []) {
109+
$node = array_pop($nodes);
110+
if ($node instanceof Stmt\Function_ || $node instanceof Stmt\ClassLike) {
111+
continue;
112+
}
113+
if (
114+
($node instanceof Expr\Variable && !is_string($node->name))
115+
|| $node instanceof Expr\Include_
116+
|| $node instanceof Expr\Eval_
117+
|| (
118+
$node instanceof Expr\FuncCall
119+
&& $node->name instanceof Node\Name
120+
&& in_array($node->name->toLowerString(), ['extract', 'parse_str'], true)
121+
)
122+
) {
123+
return null;
124+
}
125+
126+
$targets = [];
127+
if ($node instanceof Expr\Assign || $node instanceof Expr\AssignOp) {
128+
$targets[] = $node->var;
129+
} elseif ($node instanceof Expr\AssignRef) {
130+
$targets[] = $node->var;
131+
$targets[] = $node->expr;
132+
} elseif ($node instanceof Expr\PreInc || $node instanceof Expr\PreDec || $node instanceof Expr\PostInc || $node instanceof Expr\PostDec) {
133+
$targets[] = $node->var;
134+
} elseif ($node instanceof Stmt\Foreach_) {
135+
if ($node->byRef) {
136+
$targets[] = $node->expr;
137+
}
138+
if ($node->keyVar !== null) {
139+
$targets[] = $node->keyVar;
140+
}
141+
$targets[] = $node->valueVar;
142+
} elseif ($node instanceof Stmt\Catch_) {
143+
if ($node->var !== null) {
144+
$targets[] = $node->var;
145+
}
146+
} elseif ($node instanceof Stmt\Static_) {
147+
foreach ($node->vars as $staticVar) {
148+
$targets[] = $staticVar->var;
149+
}
150+
} elseif ($node instanceof Stmt\Global_ || $node instanceof Stmt\Unset_) {
151+
foreach ($node->vars as $var) {
152+
$targets[] = $var;
153+
}
154+
} elseif ($node instanceof Expr\Closure) {
155+
foreach ($node->uses as $use) {
156+
if (!$use->byRef) {
157+
continue;
158+
}
159+
$targets[] = $use->var;
160+
}
161+
}
162+
163+
foreach ($targets as $target) {
164+
$targetNames = self::getTargetNames($target);
165+
if ($targetNames === null) {
166+
return null;
167+
}
168+
foreach ($targetNames as $targetName) {
169+
$names[$targetName] = true;
170+
}
171+
}
172+
173+
foreach ($node->getSubNodeNames() as $subNodeName) {
174+
$subNode = $node->$subNodeName;
175+
if ($subNode instanceof Node) {
176+
$nodes[] = $subNode;
177+
continue;
178+
}
179+
if (!is_array($subNode)) {
180+
continue;
181+
}
182+
foreach ($subNode as $subNodeItem) {
183+
if (!$subNodeItem instanceof Node) {
184+
continue;
185+
}
186+
$nodes[] = $subNodeItem;
187+
}
188+
}
189+
}
190+
191+
return $names;
192+
}
193+
194+
/**
195+
* @return list<string>|null
196+
*/
197+
private static function getTargetNames(Expr $target): ?array
198+
{
199+
while ($target instanceof Expr\ArrayDimFetch || $target instanceof Expr\PropertyFetch || $target instanceof Expr\NullsafePropertyFetch) {
200+
$target = $target->var;
201+
}
202+
if ($target instanceof Expr\Variable) {
203+
return is_string($target->name) ? [$target->name] : null;
204+
}
205+
if (!$target instanceof Expr\List_ && !$target instanceof Expr\Array_) {
206+
return [];
207+
}
208+
209+
$names = [];
210+
foreach ($target->items as $item) {
211+
if ($item === null) {
212+
continue;
213+
}
214+
$itemNames = self::getTargetNames($item->value);
215+
if ($itemNames === null) {
216+
return null;
217+
}
218+
foreach ($itemNames as $itemName) {
219+
$names[] = $itemName;
220+
}
221+
}
222+
223+
return $names;
224+
}
225+
226+
}

‎src/Analyser/MutatingScope.php‎

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4773,20 +4773,47 @@ public function processAlwaysIterableForeachScopeWithoutPollute(self $finalScope
47734773
);
47744774
}
47754775

4776-
public function generalizeWith(self $otherScope): self
4776+
/**
4777+
* @param array<string, true>|null $writableVariableNames variables the loop can write, null when unknown
4778+
*/
4779+
public function generalizeWith(self $otherScope, ?array $writableVariableNames = null): self
47774780
{
4778-
return $this->generalizeWithVariableState($otherScope)->addTemplateArgumentConstraints($otherScope->getTemplateArgumentConstraints());
4781+
return $this->generalizeWithVariableState($otherScope, $writableVariableNames)->addTemplateArgumentConstraints($otherScope->getTemplateArgumentConstraints());
47794782
}
47804783

4781-
private function generalizeWithVariableState(self $otherScope): self
4782-
{
4784+
/**
4785+
* @param array<string, true>|null $writableVariableNames
4786+
*/
4787+
private function generalizeWithVariableState(self $otherScope, ?array $writableVariableNames): self
4788+
{
4789+
if ($writableVariableNames !== null) {
4790+
// a reference created before the loop lets the loop write a variable it does not name
4791+
foreach ([$this->expressionTypes, $otherScope->expressionTypes] as $expressionTypes) {
4792+
foreach ($expressionTypes as $expressionTypeHolder) {
4793+
$intertwinedExpr = $expressionTypeHolder->getExpr();
4794+
if (!$intertwinedExpr instanceof IntertwinedVariableByReferenceWithExpr) {
4795+
continue;
4796+
}
4797+
$writableVariableNames[$intertwinedExpr->getVariableName()] = true;
4798+
foreach ([$intertwinedExpr->getExpr(), $intertwinedExpr->getAssignedExpr()] as $aliasedExpr) {
4799+
$aliasedVariableName = ScopeOps::getIntertwinedRefRootVariableName($aliasedExpr);
4800+
if ($aliasedVariableName === null) {
4801+
continue;
4802+
}
4803+
$writableVariableNames[$aliasedVariableName] = true;
4804+
}
4805+
}
4806+
}
4807+
}
47834808
$variableTypeHolders = $this->generalizeVariableTypeHolders(
47844809
$this->expressionTypes,
47854810
$otherScope->expressionTypes,
4811+
$writableVariableNames,
47864812
);
47874813
$nativeTypes = $this->generalizeVariableTypeHolders(
47884814
$this->nativeExpressionTypes,
47894815
$otherScope->nativeExpressionTypes,
4816+
$writableVariableNames,
47904817
);
47914818

47924819
return $this->scopeFactory->create(
@@ -4814,11 +4841,13 @@ private function generalizeWithVariableState(self $otherScope): self
48144841
/**
48154842
* @param array<string, ExpressionTypeHolder> $variableTypeHolders
48164843
* @param array<string, ExpressionTypeHolder> $otherVariableTypeHolders
4844+
* @param array<string, true>|null $writableVariableNames
48174845
* @return array<string, ExpressionTypeHolder>
48184846
*/
48194847
private function generalizeVariableTypeHolders(
48204848
array $variableTypeHolders,
48214849
array $otherVariableTypeHolders,
4850+
?array $writableVariableNames,
48224851
): array
48234852
{
48244853
uksort($variableTypeHolders, static fn (string $exprA, string $exprB): int => strlen($exprA) <=> strlen($exprB));
@@ -4838,7 +4867,18 @@ private function generalizeVariableTypeHolders(
48384867
continue;
48394868
}
48404869

4841-
$generalizedType = $this->generalizeType($variableTypeHolder->getType(), $otherVariableTypeHolders[$variableExprString]->getType(), 0);
4870+
$variableExpr = $variableTypeHolder->getExpr();
4871+
if (
4872+
$writableVariableNames !== null
4873+
&& $variableExpr instanceof Variable
4874+
&& is_string($variableExpr->name)
4875+
&& !isset($writableVariableNames[$variableExpr->name])
4876+
) {
4877+
// the loop does not write this variable, its types differ between passes only by narrowing
4878+
$generalizedType = TypeCombinator::union($variableTypeHolder->getType(), $otherVariableTypeHolders[$variableExprString]->getType());
4879+
} else {
4880+
$generalizedType = $this->generalizeType($variableTypeHolder->getType(), $otherVariableTypeHolders[$variableExprString]->getType(), 0);
4881+
}
48424882
if (
48434883
!$generalizedType->equals($variableTypeHolder->getType())
48444884
) {

‎src/Analyser/StmtHandler/DoWhileHandler.php‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use PHPStan\Analyser\ExpressionContext;
99
use PHPStan\Analyser\ExpressionResultStorage;
1010
use PHPStan\Analyser\InternalStatementResult;
11+
use PHPStan\Analyser\LoopWrittenVariableNames;
1112
use PHPStan\Analyser\MutatingScope;
1213
use PHPStan\Analyser\NodeScopeResolver;
1314
use PHPStan\Analyser\NoopNodeCallback;
@@ -83,22 +84,24 @@ public function processStmt(
8384
$replayPassStorage = $storage;
8485
$replayPassResult = $bodyScopeResult;
8586
}
86-
if ($backEdgeScope !== null) {
87-
$bodyScope = $nodeScopeResolver->processExprNode($stmt, $stmt->cond, $backEdgeScope, $storage, new NoopNodeCallback(), ExpressionContext::createDeep(resolveTemplateArguments: false))->getTruthyScope();
87+
if ($backEdgeScope === null) {
88+
$bodyScope = $prevScope;
89+
break;
8890
}
91+
$passCondResult = $nodeScopeResolver->processExprNode($stmt, $stmt->cond, $backEdgeScope, $storage, new NoopNodeCallback(), ExpressionContext::createDeep(resolveTemplateArguments: false));
92+
$bodyScope = $passCondResult->getTruthyScope();
8993
} finally {
9094
$scope->popExpressionResultStorage();
9195
}
92-
if ($backEdgeScope === null) {
93-
$bodyScope = $prevScope;
94-
break;
95-
}
9696
if ($bodyScope->equals($prevScope)) {
9797
break;
9898
}
9999

100100
if ($count >= NodeScopeResolver::GENERALIZE_AFTER_ITERATION) {
101-
$bodyScope = $prevScope->generalizeWith($bodyScope);
101+
$bodyScope = $prevScope->generalizeWith(
102+
$bodyScope,
103+
LoopWrittenVariableNames::collect($stmt, VariableFlow::sequence($bodyScopeResult->getVariableFlow(), $passCondResult->getVariableFlow())),
104+
);
102105
}
103106
$count++;
104107
} while ($count < NodeScopeResolver::LOOP_SCOPE_ITERATIONS);

‎src/Analyser/StmtHandler/ForHandler.php‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use PHPStan\Analyser\ExpressionContext;
1717
use PHPStan\Analyser\ExpressionResultStorage;
1818
use PHPStan\Analyser\InternalStatementResult;
19+
use PHPStan\Analyser\LoopWrittenVariableNames;
1920
use PHPStan\Analyser\MutatingScope;
2021
use PHPStan\Analyser\NodeScopeResolver;
2122
use PHPStan\Analyser\NoopNodeCallback;
@@ -195,10 +196,14 @@ public function processStmt(
195196
$prevEntryScope = $bodyScope;
196197
$scope->pushExpressionResultStorage($storage);
197198
try {
199+
$passFlows = [];
198200
if ($lastCondExpr !== null) {
199-
$bodyScope = $nodeScopeResolver->processExprNode($stmt, $lastCondExpr, $bodyScope, $storage, new NoopNodeCallback(), ExpressionContext::createDeep(resolveTemplateArguments: false))->getTruthyScope();
201+
$passCondResult = $nodeScopeResolver->processExprNode($stmt, $lastCondExpr, $bodyScope, $storage, new NoopNodeCallback(), ExpressionContext::createDeep(resolveTemplateArguments: false));
202+
$bodyScope = $passCondResult->getTruthyScope();
203+
$passFlows[] = $passCondResult->getVariableFlow();
200204
}
201205
$bodyScopeResult = $nodeScopeResolver->processStmtNodesInternal($stmt, $stmt->stmts, $bodyScope, $storage, new NoopNodeCallback(), $context->enterDeep()->withoutTemplateArgumentResolution())->filterOutLoopExitPoints();
206+
$passFlows[] = $bodyScopeResult->getVariableFlow();
202207
$backEdgeScope = $bodyScopeResult->getLoopBackEdgeScope();
203208
if ($backEdgeScope === null) {
204209
$bodyScope = $prevScope;
@@ -209,6 +214,7 @@ public function processStmt(
209214
foreach ($stmt->loop as $loopExpr) {
210215
$exprResult = $nodeScopeResolver->processExprNode($stmt, $loopExpr, $bodyScope, $storage, new NoopNodeCallback(), ExpressionContext::createTopLevel(resolveTemplateArguments: false));
211216
$bodyScope = $exprResult->getScope();
217+
$passFlows[] = $exprResult->getVariableFlow();
212218
$hasYield = $hasYield || $exprResult->hasYield();
213219
$throwPoints = array_merge($throwPoints, $exprResult->getThrowPoints());
214220
$impurePoints = array_merge($impurePoints, $exprResult->getImpurePoints());
@@ -222,7 +228,7 @@ public function processStmt(
222228
}
223229

224230
if ($count >= NodeScopeResolver::GENERALIZE_AFTER_ITERATION) {
225-
$bodyScope = $prevScope->generalizeWith($bodyScope);
231+
$bodyScope = $prevScope->generalizeWith($bodyScope, LoopWrittenVariableNames::collect($stmt, VariableFlow::sequence(...$passFlows)));
226232
}
227233
$count++;
228234
} while ($count < NodeScopeResolver::LOOP_SCOPE_ITERATIONS);

0 commit comments

Comments
 (0)