From 68d4ccf5e19355cf17b70a3a1a8afc4350406b96 Mon Sep 17 00:00:00 2001 From: staabm <120441+staabm@users.noreply.github.com> Date: Wed, 24 Jun 2026 06:10:05 +0000 Subject: [PATCH 1/5] Recover never-collapsed variables from the loop-condition falsey scope for side-effecting `while`/`for` conditions - `while`/`for` loops compute their after-loop scope by narrowing the end-of-body scope with `filterByFalseyValue($cond)`. That only narrows types, it never re-applies the condition's side effects, so a condition such as `while (--$x > 0)` narrows the not-yet-decremented `$x` against the spec computed for its decremented value and collapses `$x` to `*NEVER*`, producing a spurious `identical.alwaysFalse`. - Add `MutatingScope::restoreNeverTypesFrom()`: for every expression that became `NeverType` it takes the corrected type from another scope (where the value is not never), leaving everything else untouched. - In the `while` and `for` exit handling, when the condition contains a side effect (`++`/`--`/`=`/`+=`/`=&`), recover the never-collapsed variables from the falsey branch of the loop-condition evaluation, where the side effects were applied exactly once. Conditions without side effects keep the existing precise behaviour. - Covers pre/post increment and decrement as well as assignments inside the condition, for both `while` and `for`. `do-while` already evaluated the condition once at the bottom and was unaffected. --- src/Analyser/MutatingScope.php | 53 +++++++++++++++++ src/Analyser/NodeScopeResolver.php | 29 +++++++++- tests/PHPStan/Analyser/nsrt/bug-10109.php | 69 +++++++++++++++++++++++ 3 files changed, 149 insertions(+), 2 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-10109.php diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index e02c18b0da3..13e0bebb217 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -3325,6 +3325,59 @@ public function filterByFalseyValue(Expr $expr): self return $scope; } + /** + * Recovers expressions that were collapsed to never type because a side-effecting condition + * (e.g. the `--$x > 0` of a `while` loop) was narrowed by filterByFalseyValue() without + * re-applying its side effects. Their corrected type is taken from the loop-condition falsey + * scope, where the side effects were applied exactly once. + */ + public function restoreNeverTypesFrom(self $other): self + { + $expressionTypes = $this->expressionTypes; + $nativeExpressionTypes = $this->nativeExpressionTypes; + $changed = false; + foreach ($expressionTypes as $exprString => $holder) { + if (!$holder->getType() instanceof NeverType) { + continue; + } + if (!array_key_exists($exprString, $other->expressionTypes)) { + continue; + } + $otherHolder = $other->expressionTypes[$exprString]; + if ($otherHolder->getType() instanceof NeverType) { + continue; + } + $expressionTypes[$exprString] = $otherHolder; + if (array_key_exists($exprString, $other->nativeExpressionTypes)) { + $nativeExpressionTypes[$exprString] = $other->nativeExpressionTypes[$exprString]; + } + $changed = true; + } + + if (!$changed) { + return $this; + } + + return $this->scopeFactory->create( + $this->context, + $this->isDeclareStrictTypes(), + $this->getFunction(), + $this->getNamespace(), + $expressionTypes, + $nativeExpressionTypes, + $this->conditionalExpressions, + $this->inClosureBindScopeClasses, + $this->anonymousFunctionReflection, + $this->inFirstLevelStatement, + $this->currentlyAssignedExpressions, + $this->currentlyAllowedUndefinedExpressions, + [], + $this->afterExtractCall, + $this->parentScope, + $this->nativeTypesPromoted, + ); + } + /** * @return static */ diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 3f63434d03f..54452b23b80 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -1716,9 +1716,18 @@ public function processStmtNode( $bodyScope = $bodyScope->mergeWith($scope); $bodyScopeMaybeRan = $bodyScope; $storage = $originalStorage; - $bodyScope = $this->processExprNode($stmt, $stmt->cond, $bodyScope, $storage, $nodeCallback, ExpressionContext::createDeep())->getTruthyScope(); + $condExprResult = $this->processExprNode($stmt, $stmt->cond, $bodyScope, $storage, $nodeCallback, ExpressionContext::createDeep()); + $bodyScope = $condExprResult->getTruthyScope(); $finalScopeResult = $this->processStmtNodesInternal($stmt, $stmt->stmts, $bodyScope, $storage, $nodeCallback, $context)->filterOutLoopExitPoints(); $finalScope = $finalScopeResult->getScope()->filterByFalseyValue($stmt->cond); + if ($this->exprContainsSideEffect($stmt->cond)) { + // A condition with side effects (e.g. `while (--$x > 0)`) mutates its variables as + // part of being evaluated. filterByFalseyValue() only narrows, it does not re-apply + // those side effects, so it can collapse a variable to never type by contradicting + // its not-yet-mutated value. Recover such variables from the loop-condition falsey + // scope, which applied the side effects exactly once. + $finalScope = $finalScope->restoreNeverTypesFrom($condExprResult->getFalseyScope()); + } $alwaysIterates = false; $neverIterates = false; @@ -1941,9 +1950,11 @@ public function processStmtNode( $bodyScope = $bodyScope->mergeWith($initScope); $alwaysIterates = TrinaryLogic::createFromBoolean($context->isTopLevel()); + $lastCondExprResult = null; if ($lastCondExpr !== null) { $alwaysIterates = $alwaysIterates->and($bodyScope->getType($lastCondExpr)->toBoolean()->isTrue()); - $bodyScope = $this->processExprNode($stmt, $lastCondExpr, $bodyScope, $storage, $nodeCallback, ExpressionContext::createDeep())->getTruthyScope(); + $lastCondExprResult = $this->processExprNode($stmt, $lastCondExpr, $bodyScope, $storage, $nodeCallback, ExpressionContext::createDeep()); + $bodyScope = $lastCondExprResult->getTruthyScope(); $bodyScope = $this->inferForLoopExpressions($stmt, $lastCondExpr, $bodyScope); } @@ -1961,6 +1972,9 @@ public function processStmtNode( if ($lastCondExpr !== null) { $finalScope = $finalScope->filterByFalseyValue($lastCondExpr); + if ($this->exprContainsSideEffect($lastCondExpr)) { + $finalScope = $finalScope->restoreNeverTypesFrom($lastCondExprResult->getFalseyScope()); + } } $breakExitPoints = $finalScopeResult->getExitPointsByType(Break_::class); @@ -5119,6 +5133,17 @@ private function getNextUnreachableStatements(array $nodes, bool $earlyBinding): return $stmts; } + private function exprContainsSideEffect(Expr $expr): bool + { + return (new NodeFinder())->findFirst([$expr], static fn (Node $node): bool => $node instanceof Expr\PreInc + || $node instanceof Expr\PreDec + || $node instanceof Expr\PostInc + || $node instanceof Expr\PostDec + || $node instanceof Expr\Assign + || $node instanceof Expr\AssignOp + || $node instanceof Expr\AssignRef) !== null; + } + private function inferForLoopExpressions(For_ $stmt, Expr $lastCondExpr, MutatingScope $bodyScope): MutatingScope { // infer $items[$i] type from for ($i = 0; $i < count($items); $i++) {...} diff --git a/tests/PHPStan/Analyser/nsrt/bug-10109.php b/tests/PHPStan/Analyser/nsrt/bug-10109.php new file mode 100644 index 00000000000..9c55403496b --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-10109.php @@ -0,0 +1,69 @@ + 0) { + } + + assertType('int', $x); +} + +function withBody(): void +{ + $x = 5; + while (--$x > 0) { + echo $x; + } + + assertType('int', $x); +} + +function preIncrement(): void +{ + $x = 5; + while (++$x < 10) { + } + + assertType('int<10, max>', $x); +} + +function assignInCondition(): void +{ + $x = 5; + while (($x = $x - 1) > 0) { + } + + assertType('int', $x); +} + +function postDecrement(): void +{ + $x = 5; + while ($x-- > 0) { + } + + assertType('int', $x); +} + +function forLoop(): void +{ + for ($x = 5; --$x > 0;) { + } + + assertType('int', $x); +} + +function noSideEffectInCondition(): void +{ + $x = 5; + while ($x > 0) { + $x = $x - 1; + } + + assertType('0', $x); +} From ddf848b19505572425988f60e5338466ef1f1222 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sat, 27 Jun 2026 11:51:15 +0000 Subject: [PATCH 2/5] Add rule test covering the reported identical.alwaysFalse false positive Co-Authored-By: Claude Opus 4.8 --- ...rictComparisonOfDifferentTypesRuleTest.php | 5 ++++ .../Rules/Comparison/data/bug-10109.php | 30 +++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 tests/PHPStan/Rules/Comparison/data/bug-10109.php diff --git a/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php b/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php index 792d737e48f..34d92c510c0 100644 --- a/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php @@ -1258,4 +1258,9 @@ public function testBug14847(): void ]); } + public function testBug10109(): void + { + $this->analyse([__DIR__ . '/data/bug-10109.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Comparison/data/bug-10109.php b/tests/PHPStan/Rules/Comparison/data/bug-10109.php new file mode 100644 index 00000000000..8c54f0606e9 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-10109.php @@ -0,0 +1,30 @@ + 0) { + } + + if ($x === 0) { + echo 'zero'; + } +} + +function closerToRealCode(int $max): void +{ + $x = $max; + while (--$x > 0) { + doSomething($x); + } + + if ($x === 0) { + echo 'reached the end'; + } +} + +function doSomething(int $x): void +{ +} From ec7c6284ac2aa10a900905267447cf08fca7cd16 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 27 Jun 2026 13:56:30 +0200 Subject: [PATCH 3/5] fix name collision --- tests/PHPStan/Analyser/nsrt/{bug-10109.php => bug-10109b.php} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename tests/PHPStan/Analyser/nsrt/{bug-10109.php => bug-10109b.php} (97%) diff --git a/tests/PHPStan/Analyser/nsrt/bug-10109.php b/tests/PHPStan/Analyser/nsrt/bug-10109b.php similarity index 97% rename from tests/PHPStan/Analyser/nsrt/bug-10109.php rename to tests/PHPStan/Analyser/nsrt/bug-10109b.php index 9c55403496b..8322259102c 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-10109.php +++ b/tests/PHPStan/Analyser/nsrt/bug-10109b.php @@ -1,6 +1,6 @@ Date: Sat, 27 Jun 2026 14:00:55 +0200 Subject: [PATCH 4/5] use 1:1 repro --- .../Rules/Comparison/data/bug-10109.php | 35 +++++++------------ 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/tests/PHPStan/Rules/Comparison/data/bug-10109.php b/tests/PHPStan/Rules/Comparison/data/bug-10109.php index 8c54f0606e9..cf784974460 100644 --- a/tests/PHPStan/Rules/Comparison/data/bug-10109.php +++ b/tests/PHPStan/Rules/Comparison/data/bug-10109.php @@ -2,29 +2,20 @@ namespace Bug10109; -function simple(): void -{ - $x = 5; - while (--$x > 0) { - } - - if ($x === 0) { - echo 'zero'; - } +// simpler +$x = 5; +while (--$x > 0) { + echo "$x\n"; } - -function closerToRealCode(int $max): void -{ - $x = $max; - while (--$x > 0) { - doSomething($x); - } - - if ($x === 0) { - echo 'reached the end'; - } +if ($x === 0) { + echo "zero\n"; } -function doSomething(int $x): void -{ +// closer to real codebase +$x = 5; +while (mt_rand(0, 10) < 10 && --$x > 0) { + echo "$x\n"; +} +if ($x === 0) { + echo "zero\n"; } From c9a1912951a5d14606b592bb31efc543d800643c Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sat, 27 Jun 2026 12:27:37 +0000 Subject: [PATCH 5/5] Recover short-circuited side-effecting loop variables from the condition falsey scope The previous recovery only restored variables that filterByFalseyValue() collapsed to never. When the side effect sits behind a short-circuiting operator (e.g. `while (mt_rand(...) < 10 && --$x > 0)`) the variable is not collapsed to never but left at its in-loop value, which can exclude a reachable exit value (here `int<1, 5>`, missing `0`), reintroducing the `identical.alwaysFalse` false positive. Graft the mutated variables from the loop-condition falsey scope by unioning their corrected type into the after-loop scope. Pre-increments/decrements and assignments compare the same value the variable ends up with, so their falsey type is always grafted; post-increments/decrements compare the pre-mutation value, so they are only grafted to rescue a never-collapse, preserving the precise bound of counter idioms like `$i++ < 10`. Co-Authored-By: Claude Opus 4.8 --- src/Analyser/MutatingScope.php | 35 +++++++++----- src/Analyser/NodeScopeResolver.php | 53 ++++++++++++++++++---- tests/PHPStan/Analyser/nsrt/bug-10109b.php | 22 +++++++++ 3 files changed, 89 insertions(+), 21 deletions(-) diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 13e0bebb217..3e1a44314fb 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -3326,20 +3326,22 @@ public function filterByFalseyValue(Expr $expr): self } /** - * Recovers expressions that were collapsed to never type because a side-effecting condition - * (e.g. the `--$x > 0` of a `while` loop) was narrowed by filterByFalseyValue() without - * re-applying its side effects. Their corrected type is taken from the loop-condition falsey - * scope, where the side effects were applied exactly once. + * Recovers the loop variables mutated by a side-effecting condition (e.g. the `--$x` of a + * `while (--$x > 0)` loop). filterByFalseyValue() only narrows types, it does not re-apply + * the condition's side effects, so the after-loop type of such a variable can be left wrong + * (collapsed to never, or stuck at its in-body value that excludes a reachable exit value). + * Their corrected type is taken from the loop-condition falsey scope, where the side effects + * were applied exactly once, by unioning it into the current type. + * + * @param list $exprs */ - public function restoreNeverTypesFrom(self $other): self + public function restoreLoopConditionSideEffects(self $other, array $exprs): self { $expressionTypes = $this->expressionTypes; $nativeExpressionTypes = $this->nativeExpressionTypes; $changed = false; - foreach ($expressionTypes as $exprString => $holder) { - if (!$holder->getType() instanceof NeverType) { - continue; - } + foreach ($exprs as [$expr, $onlyWhenNever]) { + $exprString = $this->getNodeKey($expr); if (!array_key_exists($exprString, $other->expressionTypes)) { continue; } @@ -3347,9 +3349,20 @@ public function restoreNeverTypesFrom(self $other): self if ($otherHolder->getType() instanceof NeverType) { continue; } - $expressionTypes[$exprString] = $otherHolder; + if (array_key_exists($exprString, $expressionTypes)) { + if ($onlyWhenNever && !$expressionTypes[$exprString]->getType() instanceof NeverType) { + continue; + } + $expressionTypes[$exprString] = $expressionTypes[$exprString]->and($otherHolder); + } else { + $expressionTypes[$exprString] = $otherHolder; + } if (array_key_exists($exprString, $other->nativeExpressionTypes)) { - $nativeExpressionTypes[$exprString] = $other->nativeExpressionTypes[$exprString]; + if (array_key_exists($exprString, $nativeExpressionTypes)) { + $nativeExpressionTypes[$exprString] = $nativeExpressionTypes[$exprString]->and($other->nativeExpressionTypes[$exprString]); + } else { + $nativeExpressionTypes[$exprString] = $other->nativeExpressionTypes[$exprString]; + } } $changed = true; } diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 54452b23b80..7b2875e85ce 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -1720,13 +1720,14 @@ public function processStmtNode( $bodyScope = $condExprResult->getTruthyScope(); $finalScopeResult = $this->processStmtNodesInternal($stmt, $stmt->stmts, $bodyScope, $storage, $nodeCallback, $context)->filterOutLoopExitPoints(); $finalScope = $finalScopeResult->getScope()->filterByFalseyValue($stmt->cond); - if ($this->exprContainsSideEffect($stmt->cond)) { + $condSideEffectExprs = $this->getExprSideEffectTargets($stmt->cond); + if (count($condSideEffectExprs) > 0) { // A condition with side effects (e.g. `while (--$x > 0)`) mutates its variables as // part of being evaluated. filterByFalseyValue() only narrows, it does not re-apply - // those side effects, so it can collapse a variable to never type by contradicting - // its not-yet-mutated value. Recover such variables from the loop-condition falsey - // scope, which applied the side effects exactly once. - $finalScope = $finalScope->restoreNeverTypesFrom($condExprResult->getFalseyScope()); + // those side effects, so the after-loop type of such a variable can be left wrong. + // Recover the mutated variables from the loop-condition falsey scope, which applied + // the side effects exactly once. + $finalScope = $finalScope->restoreLoopConditionSideEffects($condExprResult->getFalseyScope(), $condSideEffectExprs); } $alwaysIterates = false; @@ -1972,8 +1973,9 @@ public function processStmtNode( if ($lastCondExpr !== null) { $finalScope = $finalScope->filterByFalseyValue($lastCondExpr); - if ($this->exprContainsSideEffect($lastCondExpr)) { - $finalScope = $finalScope->restoreNeverTypesFrom($lastCondExprResult->getFalseyScope()); + $condSideEffectExprs = $this->getExprSideEffectTargets($lastCondExpr); + if (count($condSideEffectExprs) > 0) { + $finalScope = $finalScope->restoreLoopConditionSideEffects($lastCondExprResult->getFalseyScope(), $condSideEffectExprs); } } @@ -5133,15 +5135,46 @@ private function getNextUnreachableStatements(array $nodes, bool $earlyBinding): return $stmts; } - private function exprContainsSideEffect(Expr $expr): bool + /** + * Returns the assignment targets of the side effects (increments, decrements, assignments) + * contained anywhere in the given expression, e.g. `[$x]` for `--$x > 0`. + * + * The boolean flags whether the variable's after-loop type may only be grafted from the + * loop-condition falsey scope when filterByFalseyValue() collapsed it to never. It is false + * for pre-increments/decrements and assignments — there the value compared by the condition + * equals the variable's resulting value, so the falsey scope is the correct after-loop type + * and is always grafted. It is true for post-increments/decrements, where the compared value + * is the pre-mutation one, so grafting only rescues a never-collapse and otherwise keeps the + * more precise in-loop type (e.g. the bound on a `$i++ < 10` counter). + * + * @return list + */ + private function getExprSideEffectTargets(Expr $expr): array { - return (new NodeFinder())->findFirst([$expr], static fn (Node $node): bool => $node instanceof Expr\PreInc + $sideEffects = (new NodeFinder())->find([$expr], static fn (Node $node): bool => $node instanceof Expr\PreInc || $node instanceof Expr\PreDec || $node instanceof Expr\PostInc || $node instanceof Expr\PostDec || $node instanceof Expr\Assign || $node instanceof Expr\AssignOp - || $node instanceof Expr\AssignRef) !== null; + || $node instanceof Expr\AssignRef); + + $targets = []; + foreach ($sideEffects as $sideEffect) { + if ($sideEffect instanceof Expr\PostInc || $sideEffect instanceof Expr\PostDec) { + $targets[] = [$sideEffect->var, true]; + } elseif ( + $sideEffect instanceof Expr\PreInc + || $sideEffect instanceof Expr\PreDec + || $sideEffect instanceof Expr\Assign + || $sideEffect instanceof Expr\AssignOp + || $sideEffect instanceof Expr\AssignRef + ) { + $targets[] = [$sideEffect->var, false]; + } + } + + return $targets; } private function inferForLoopExpressions(For_ $stmt, Expr $lastCondExpr, MutatingScope $bodyScope): MutatingScope diff --git a/tests/PHPStan/Analyser/nsrt/bug-10109b.php b/tests/PHPStan/Analyser/nsrt/bug-10109b.php index 8322259102c..8d262eb23e9 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-10109b.php +++ b/tests/PHPStan/Analyser/nsrt/bug-10109b.php @@ -58,6 +58,28 @@ function forLoop(): void assertType('int', $x); } +function shortCircuitedSideEffect(): void +{ + // The side effect is short-circuited, so the loop can also exit with $x unchanged. The + // after-loop type must still include the values produced by the decrement (notably 0). + $x = 5; + while (mt_rand(0, 10) < 10 && --$x > 0) { + } + + assertType('int', $x); +} + +function shortCircuitedCounter(): void +{ + // A post-increment counter keeps its precise in-loop bound instead of being widened by the + // loop-condition falsey scope. + $i = 0; + while (mt_rand(0, 10) < 10 && $i++ < 10) { + } + + assertType('int<0, 10>', $i); +} + function noSideEffectInCondition(): void { $x = 5;