From 3f319a6a64396462e5fa4acd4e663cde6b7ce610 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] 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); +}