Recover never-collapsed variables from the loop-condition falsey scope for side-effecting while/for conditions

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
 	 */

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++) {...}

tests/PHPStan/Analyser/nsrt/bug-10109.php

@@ -0,0 +1,69 @@
+<?php
+
+namespace Bug10109;
+
+use function PHPStan\Testing\assertType;
+
+function simple(): void
+{
+	$x = 5;
+	while (--$x > 0) {
+	}
+
+	assertType('int<min, 0>', $x);
+}
+
+function withBody(): void
+{
+	$x = 5;
+	while (--$x > 0) {
+		echo $x;
+	}
+
+	assertType('int<min, 0>', $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<min, 0>', $x);
+}
+
+function postDecrement(): void
+{
+	$x = 5;
+	while ($x-- > 0) {
+	}
+
+	assertType('int<min, -1>', $x);
+}
+
+function forLoop(): void
+{
+	for ($x = 5; --$x > 0;) {
+	}
+
+	assertType('int<min, 0>', $x);
+}
+
+function noSideEffectInCondition(): void
+{
+	$x = 5;
+	while ($x > 0) {
+		$x = $x - 1;
+	}
+
+	assertType('0', $x);
+}