Walk a while loop's exit condition instead of narrowing the body's scope by it

src/Analyser/StmtHandler/WhileHandler.php

@@ -15,7 +15,6 @@
 use PHPStan\Analyser\RecordingNodeCallback;
 use PHPStan\Analyser\StatementContext;
 use PHPStan\Analyser\StmtHandler;
-use PHPStan\Analyser\TypeSpecifierContext;
 use PHPStan\Analyser\VariableFlow;
 use PHPStan\Dependency\Dependencies;
 use PHPStan\DependencyInjection\AutowiredParameter;
@@ -196,12 +195,18 @@ public function processStmt(
 		$backEdgeScope = $finalScopeResult->getLoopBackEdgeScope();
 		$backEdgeDead = $backEdgeScope === null;
 		$finalScope = $backEdgeScope ?? $finalScopeResult->getScope();
-		// the loop condition narrows the post-loop scope to its falsey branch;
-		// $finalScope (after the body ran) is a different scope than the condition's
-		// own, so reprocess the condition there rather than re-running its result.
-		// The duplicate lets subresults whose state did not change in the body
-		// answer from the final pass instead of being re-priced.
-		$finalScope = $finalScope->applySpecifiedTypes($nodeScopeResolver->processExprOnDemand($stmt->cond, $finalScope, $storage->duplicate())->getSpecifiedTypesForScope($finalScope, TypeSpecifierContext::createFalsey()));
+		// the loop exits when the condition, evaluated again after the body ran,
+		// is falsey - walk it on $finalScope and take its falsey scope, so the
+		// assignments it makes (`while ($row = fetch())`) replace the body's
+		// values instead of the body's stale values being narrowed by them.
+		// The duplicate keeps this walk's results out of the final pass's storage.
+		$exitCondStorage = $storage->duplicate();
+		$scope->pushExpressionResultStorage($exitCondStorage);
+		try {
+			$finalScope = $nodeScopeResolver->processExprNode($stmt, $stmt->cond, $finalScope, $exitCondStorage, new NoopNodeCallback(), ExpressionContext::createDeep(resolveTemplateArguments: false))->getFalseyScope();
+		} finally {
+			$scope->popExpressionResultStorage();
+		}
 
 		$alwaysIterates = false;
 		$neverIterates = false;
@@ -236,7 +241,10 @@ public function processStmt(
 			if (!$this->polluteScopeWithLoopInitialAssignments) {
 				$condScope = $condScope->mergeWith($scope);
 			}
-			$finalScope = $finalScope === null ? $condScope : $finalScope->mergeWith($condScope);
+			// the loop either ran or exited on its first condition, so these are
+			// alternative paths like an if's branches: keep a conditional that only
+			// one path records when the other path makes it vacuously true
+			$finalScope = $finalScope === null ? $condScope : $finalScope->mergeWith($condScope, true);
 		}
 		$finalScope ??= $finalScopeResult->getScope();
 

tests/PHPStan/Analyser/Bug15722WithoutLoopPollutionTest.php

@@ -0,0 +1,36 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Analyser;
+
+use PHPStan\Testing\TypeInferenceTestCase;
+use PHPUnit\Framework\Attributes\DataProvider;
+
+class Bug15722WithoutLoopPollutionTest extends TypeInferenceTestCase
+{
+
+	public static function dataFileAsserts(): iterable
+	{
+		yield from self::gatherAssertTypes(__DIR__ . '/data/bug-15722-without-loop-pollution.php');
+	}
+
+	/**
+	 * @param mixed ...$args
+	 */
+	#[DataProvider('dataFileAsserts')]
+	public function testFileAsserts(
+		string $assertType,
+		string $file,
+		...$args,
+	): void
+	{
+		$this->assertFileAsserts($assertType, $file, ...$args);
+	}
+
+	public static function getAdditionalConfigFiles(): array
+	{
+		return [
+			__DIR__ . '/../Rules/without-loop-pollution.neon',
+		];
+	}
+
+}

tests/PHPStan/Analyser/data/bug-15722-without-loop-pollution.php

@@ -0,0 +1,55 @@
+<?php declare(strict_types = 1);
+
+namespace Bug15722WithoutLoopPollution;
+
+use function PHPStan\Testing\assertType;
+
+interface Source
+{
+	public function stop(): bool;
+	public function next(): int;
+}
+
+function loop(Source $s): void
+{
+	$rows = 0;
+	while (!($stop = $s->stop()) && ($rows = $s->next())) {
+	}
+
+	if ($rows !== 0) {
+		assertType('true', $stop);
+	}
+
+	if ($rows === 0) {
+		assertType('bool', $stop);
+	} else {
+		assertType('true', $stop);
+	}
+}
+
+function noCorrelation(): void
+{
+	$n = 0;
+	$flag = false;
+	while (rand(0, 1)) {
+		$n = 1;
+		$flag = (bool) rand(0, 1);
+	}
+	if ($n === 1) {
+		assertType('bool', $flag);
+	}
+}
+
+function guardOverlaps(): void
+{
+	$n = rand(0, 2);
+	$flag = false;
+	while (rand(0, 1)) {
+		$n = 1;
+		$flag = true;
+	}
+	if ($n === 1) {
+		// the loop may not have run, with $n = 1 and $flag still false
+		assertType('bool', $flag);
+	}
+}

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

@@ -0,0 +1,64 @@
+<?php declare(strict_types = 1);
+
+namespace Bug15726;
+
+use function PHPStan\Testing\assertType;
+
+/** @return array{mixed}|false */
+function fetchRow()
+{
+	return rand(0, 1) === 1 ? [rand(0, 1)] : false;
+}
+
+function foo(): void
+{
+	$prev = -1;
+	$printed = false;
+	while ($row = fetchRow()) {
+		if ($row[0]) {
+			$printed = true;
+		}
+		$prev = rand(0, 3);
+	}
+	assertType('false', $row);
+	if ($prev !== -1) {
+		assertType('bool', $printed); // the last row's [0] can be falsy
+	}
+	if ($prev != -1) {
+		assertType('bool', $printed);
+	}
+}
+
+function postDecrement(): void
+{
+	$i = 10;
+	while ($i-- > 0) {
+		assertType('int<0, 9>', $i);
+	}
+	assertType('-1', $i);
+}
+
+// https://github.com/phpstan/phpstan/issues/15722 Problem B
+
+interface Source
+{
+	public function stop(): bool;
+	public function next(): int;
+}
+
+function loop(Source $s): void
+{
+	$rows = 0;
+	while (!($stop = $s->stop()) && ($rows = $s->next())) {
+	}
+
+	if ($rows !== 0) {
+		assertType('true', $stop);
+	}
+
+	if ($rows === 0) {
+		assertType('bool', $stop);
+	} else {
+		assertType('true', $stop);
+	}
+}

tests/PHPStan/Analyser/nsrt/while-loop-variables.php

@@ -49,7 +49,7 @@ function () {
 		assertType('int<1, 10>', $i);
 	}
 
-	assertType('int<0, 10>', $i);
+	assertType('int<0, 11>', $i); // the condition increments $i to 11 when $i++ < 10 fails
 
 	assertType('LoopVariables\Bar|LoopVariables\Foo|LoopVariables\Lorem|null', $foo);
 

turbo-ext/src/WhileHandler.cpp

@@ -294,27 +294,31 @@ class WhileHandler
 		} else {
 			finalScope = std::move(backEdgeScope);
 		}
-		// the loop condition narrows the post-loop scope to its falsey branch;
-		// $finalScope (after the body ran) is a different scope than the condition's
-		// own, so reprocess the condition there rather than re-running its result.
-		// The duplicate lets subresults whose state did not change in the body
-		// answer from the final pass instead of being re-priced.
+		// the loop exits when the condition, evaluated again after the body ran,
+		// is falsey - walk it on $finalScope and take its falsey scope, so the
+		// assignments it makes (`while ($row = fetch())`) replace the body's
+		// values instead of the body's stale values being narrowed by them.
+		// The duplicate keeps this walk's results out of the final pass's storage.
 		{
-			zval *cond = ptsh::readNodeProperty(pt_wh_cond_site, stmt, PT_LC("cond"));
-			if (UNEXPECTED(cond == NULL)) return zv::Val();
-			zv::Val condHold = zv::Val::copyOf(zv::Ref(cond));
-			zv::Val duplicate = pt_expression_result_storage_duplicate(finalStorage);
-			if (UNEXPECTED(duplicate.isUndef())) return zv::Val();
-			zv::Val onDemand = pt_node_scope_resolver_process_expr_on_demand(nodeScopeResolver, condHold.raw(), finalScope.raw(), duplicate.raw());
-			if (UNEXPECTED(onDemand.isUndef())) return zv::Val();
-			zend_object *falsey = pt_type_specifier_context_create_falsey();
-			if (UNEXPECTED(falsey == NULL)) return zv::Val();
-			zval falseyContext;
-			ZVAL_OBJ(&falseyContext, falsey);
-			zv::Val specifiedTypes = pt_expression_result_get_specified_types_for_scope(onDemand.raw(), finalScope.raw(), &falseyContext);
-			if (UNEXPECTED(specifiedTypes.isUndef())) return zv::Val();
-			finalScope = pt_mutating_scope_apply_specified_types(Z_OBJ_P(finalScope.raw()), specifiedTypes.raw());
-			if (UNEXPECTED(finalScope.isUndef())) return zv::Val();
+			zv::Val exitCondStorage = pt_expression_result_storage_duplicate(finalStorage);
+			if (UNEXPECTED(exitCondStorage.isUndef())) return zv::Val();
+			if (UNEXPECTED(!pt_mutating_scope_push_expression_result_storage(Z_OBJ_P(scope.raw()), exitCondStorage.raw()))) return zv::Val();
+			zv::Val falseyScope;
+			[&]() {
+				zval *cond = ptsh::readNodeProperty(pt_wh_cond_site, stmt, PT_LC("cond"));
+				if (UNEXPECTED(cond == NULL)) return;
+				zv::Val condHold = zv::Val::copyOf(zv::Ref(cond));
+				zv::Val noop = ptlh::newNoopNodeCallback();
+				if (UNEXPECTED(noop.isUndef())) return;
+				zv::Val expressionContext = pt_expression_context_create_deep(false);
+				if (UNEXPECTED(expressionContext.isUndef())) return;
+				zv::Val exitCondResult = pt_node_scope_resolver_process_expr_node(nodeScopeResolver, stmt, condHold.raw(), finalScope.raw(), exitCondStorage.raw(), noop.raw(), expressionContext.raw());
+				if (UNEXPECTED(exitCondResult.isUndef())) return;
+				falseyScope = pt_expression_result_get_falsey_scope(exitCondResult.raw());
+			}();
+			pt_finally([&]() { (void) pt_mutating_scope_pop_expression_result_storage(Z_OBJ_P(scope.raw())); });
+			if (UNEXPECTED(EG(exception))) return zv::Val();
+			finalScope = std::move(falseyScope);
 		}
 
 		bool alwaysIterates = false;
@@ -376,7 +380,10 @@ class WhileHandler
 			if (finalScope.isNull()) {
 				finalScope = zv::Val::copyOf(condScope.ref());
 			} else {
-				finalScope = pt_mutating_scope_merge_with(Z_OBJ_P(finalScope.raw()), condScope.raw());
+				// the loop either ran or exited on its first condition, so these are
+				// alternative paths like an if's branches: keep a conditional that only
+				// one path records when the other path makes it vacuously true
+				finalScope = pt_mutating_scope_merge_with(Z_OBJ_P(finalScope.raw()), condScope.raw(), true);
 				if (UNEXPECTED(finalScope.isUndef())) return zv::Val();
 			}
 		}