Consume only the nullsafe twin's receiver from stored results instead of its whole subtree

src/Analyser/ArgumentsHandler.php

@@ -370,18 +370,14 @@ public function processArgs(
 			if ($arg->value instanceof Expr\Closure) {
 
 				$storedClosureArgResult = null;
-				if ($nodeScopeResolver->isReturningStoredExpressionResults() || $nodeScopeResolver->isConsumingStoredExpressionResults()) {
+				if ($nodeScopeResolver->isReturningStoredExpressionResults()) {
 					// an on-demand re-walk of the enclosing call must not re-run the
 					// closure's whole by-ref convergence: consume the main walk's
 					// stored result, or (when the body release already dropped it)
 					// price the closure through getClosureType's per-node cache -
 					// a single body walk on miss, none on repeat asks
 					$storedClosureArgResult = $storage->findExpressionResult($arg->value);
-					// consume mode alone (the nullsafe call's plain-twin walk) is the
-					// FIRST and only walk of these arguments - nothing stored them, so
-					// a miss means "walk it", not "price it silently": pricing skips
-					// the body walk and never fires the closure's node callbacks
-					if ($storedClosureArgResult === null && $nodeScopeResolver->isReturningStoredExpressionResults()) {
+					if ($storedClosureArgResult === null) {
 						$storedClosureArgResult = $this->expressionResultFactory->create(
 							$scopeToPass,
 							beforeScope: $scopeToPass,
@@ -528,12 +524,10 @@ public function processArgs(
 			} elseif ($arg->value instanceof Expr\ArrowFunction) {
 
 				$storedClosureArgResult = null;
-				if ($nodeScopeResolver->isReturningStoredExpressionResults() || $nodeScopeResolver->isConsumingStoredExpressionResults()) {
+				if ($nodeScopeResolver->isReturningStoredExpressionResults()) {
 					// see the Closure branch above - consume or price via the cache
 					$storedClosureArgResult = $storage->findExpressionResult($arg->value);
-					// consume mode alone (the nullsafe call's plain-twin walk) is the
-					// first and only walk of the argument - a miss means walk it
-					if ($storedClosureArgResult === null && $nodeScopeResolver->isReturningStoredExpressionResults()) {
+					if ($storedClosureArgResult === null) {
 						$storedClosureArgResult = $this->expressionResultFactory->create(
 							$scopeToPass,
 							beforeScope: $scopeToPass,

src/Analyser/ExprHandler/NullsafeMethodCallHandler.php

@@ -92,6 +92,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
 		$exprResult = $nodeScopeResolver->processExprNodeConsumingStored(
 			$stmt,
 			$methodCall,
+			$expr->var,
 			$nonNullabilityResult->getScope(),
 			$storage,
 			$nodeCallback,

src/Analyser/ExprHandler/NullsafePropertyFetchHandler.php

@@ -85,7 +85,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
 			$expr->name,
 			$attributes,
 		);
-		$exprResult = $nodeScopeResolver->processExprNodeConsumingStored($stmt, $propertyFetch, $nonNullabilityResult->getScope(), $storage, $nodeCallback, $context);
+		$exprResult = $nodeScopeResolver->processExprNodeConsumingStored($stmt, $propertyFetch, $expr->var, $nonNullabilityResult->getScope(), $storage, $nodeCallback, $context);
 		$scope = $this->nonNullabilityHelper->revertNonNullability($exprResult->getScope(), $nonNullabilityResult->getSpecifiedExpressions());
 
 		// The `?->`'s own type on the asking scope. $receiverType is the receiver's

src/Analyser/NodeScopeResolver.php

@@ -80,10 +80,13 @@ class NodeScopeResolver
 	/**
 	 * Consume-stored mode: a walk that deliberately re-enters an
 	 * already-walked subtree (the nullsafe plain twin re-walking its
-	 * receiver) consumes stored results unconditionally instead of
-	 * re-processing - node callbacks fired during the original walk.
+	 * receiver) consumes this node's stored result unconditionally instead
+	 * of re-processing it - node callbacks fired during the original walk.
+	 * Only this exact node is consumed: the rest of the twin (arguments,
+	 * a dynamic name) is walked normally, even when the storage was seeded
+	 * with results of an earlier loop convergence pass.
 	 */
-	private bool $consumeStoredExpressionResults = false;
+	private ?Expr $consumeStoredExpression = null;
 
 	private ?NonNullabilityHelper $nonNullabilityHelper = null;
 
@@ -493,12 +496,6 @@ public function isReturningStoredExpressionResults(): bool
 		return $this->returnStoredExpressionResults;
 	}
 
-	/** Whether the walk consumes stored results unconditionally (see processExprNodeConsumingStored()). */
-	public function isConsumingStoredExpressionResults(): bool
-	{
-		return $this->consumeStoredExpressionResults;
-	}
-
 	public function lookForSetAllowedUndefinedExpressions(MutatingScope $scope, Expr $expr): MutatingScope
 	{
 		return $this->lookForExpressionCallback($scope, $expr, static fn (MutatingScope $scope, Expr $expr): MutatingScope => $scope->setAllowedUndefinedExpression($expr));
@@ -544,22 +541,22 @@ private function lookForExpressionCallback(MutatingScope $scope, Expr $expr, Clo
 	 * are stored into the given storage - pass a duplicate to keep them isolated.
 	 */
 	/**
-	 * Processes an expression whose already-walked subtrees must be CONSUMED
-	 * from their stored results instead of re-walked: the nullsafe handlers
+	 * Processes an expression whose already-walked receiver must be CONSUMED
+	 * from its stored result instead of re-walked: the nullsafe handlers
 	 * process the receiver once (real callbacks) and then walk the plain twin,
-	 * whose receiver subtree answers from storage, re-anchored to the twin's
+	 * whose receiver answers from storage, re-anchored to the twin's
 	 * (ensured) scope.
 	 *
 	 * @param callable(Node $node, Scope $scope): void $nodeCallback
 	 */
-	public function processExprNodeConsumingStored(Node\Stmt $stmt, Expr $expr, MutatingScope $scope, ExpressionResultStorage $storage, callable $nodeCallback, ExpressionContext $context): ExpressionResult
+	public function processExprNodeConsumingStored(Node\Stmt $stmt, Expr $expr, Expr $consumedExpr, MutatingScope $scope, ExpressionResultStorage $storage, callable $nodeCallback, ExpressionContext $context): ExpressionResult
 	{
-		$previous = $this->consumeStoredExpressionResults;
-		$this->consumeStoredExpressionResults = true;
+		$previous = $this->consumeStoredExpression;
+		$this->consumeStoredExpression = $consumedExpr;
 		try {
 			return $this->processExprNode($stmt, $expr, $scope, $storage, $nodeCallback, $context);
 		} finally {
-			$this->consumeStoredExpressionResults = $previous;
+			$this->consumeStoredExpression = $previous;
 		}
 	}
 
@@ -775,7 +772,8 @@ public function processExprNode(
 		ExpressionContext $context,
 	): ExpressionResult
 	{
-		if ($this->returnStoredExpressionResults || $this->consumeStoredExpressionResults) {
+		$consume = $this->consumeStoredExpression === $expr;
+		if ($this->returnStoredExpressionResults || $consume) {
 			$storedResult = $storage->findExpressionResult($expr);
 			// a stored result only answers when the current scope agrees with its
 			// evaluation position on the variables the expression reads - a
@@ -784,7 +782,7 @@ public function processExprNode(
 			// re-processes the node on its own scope instead. In CONSUME mode the
 			// divergence is intentional (an ensured-non-null device) and the
 			// stored result is consumed unconditionally, re-anchored below.
-			if ($storedResult !== null && ($this->consumeStoredExpressionResults || $storedResult->askScopeVariableStateMatches($scope, $scope->nativeTypesPromoted))) {
+			if ($storedResult !== null && ($consume || $storedResult->askScopeVariableStateMatches($scope, $scope->nativeTypesPromoted))) {
 				// a foreign-position answer must not thread its original walk
 				// scopes into THIS walk - re-anchor it to the asking position so
 				// subsequent operands keep evaluating on the asking scope
@@ -793,7 +791,7 @@ public function processExprNode(
 				}
 
 				$reanchored = $storedResult->atAskPosition($scope);
-				if ($this->consumeStoredExpressionResults) {
+				if ($consume) {
 					// the re-anchored view IS this walk's result for the node
 					// (the nullsafe twin's receiver at the ensured position) -
 					// store it so later asks (rules' storage reads) see the same

tests/PHPStan/Analyser/AnalyserIntegrationTest.php

@@ -16,6 +16,7 @@
 use function array_map;
 use function array_unique;
 use function array_values;
+use function count;
 use function extension_loaded;
 use function sprintf;
 use const PHP_VERSION_ID;
@@ -1807,6 +1808,31 @@ public function testBug15331(): void
 		$this->assertNoErrors($errors);
 	}
 
+	#[RequiresPhp('>= 8.0.0')]
+	public function testBug15520(): void
+	{
+		$errors = $this->runAnalyse(__DIR__ . '/data/bug-15520.php');
+		$expected = [
+			['Access to an undefined property Bug15520\\Presenter::$undefinedProperty.', 20],
+			['Binary operation "+" between 1 and \'x\' results in an error.', 20],
+			['Call to an undefined method Bug15520\\Presenter::undefinedMethod().', 20],
+			['Method Bug15520\\Category::imageFor() invoked with 2 parameters, 1 required.', 20],
+			['Call to an undefined method Bug15520\\Presenter::undefinedWhile().', 33],
+			['Call to an undefined method Bug15520\\Presenter::undefinedDoWhile().', 46],
+			['Call to an undefined method Bug15520\\Presenter::undefinedFor().', 59],
+			['Call to an undefined method Bug15520\\Presenter::undefinedInClosure().', 74],
+			['Call to an undefined method Bug15520\\Presenter::undefinedInArrowFunction().', 76],
+			['Call to an undefined method Bug15520\\Presenter::undefinedInPropertyName().', 90],
+			['Call to an undefined method Bug15520\\Presenter::undefinedInNested().', 104],
+			['Call to an undefined method Bug15520\\Presenter::undefinedInMethodName().', 105],
+		];
+		$this->assertCount(count($expected), $errors);
+		foreach ($expected as $i => [$message, $line]) {
+			$this->assertSame($message, $errors[$i]->getMessage());
+			$this->assertSame($line, $errors[$i]->getLine());
+		}
+	}
+
 	/**
 	 * @param string[]|null $allAnalysedFiles
 	 * @return list<Error>

tests/PHPStan/Analyser/data/bug-15520.php

@@ -0,0 +1,110 @@
+<?php declare(strict_types = 1);
+
+namespace Bug15520;
+
+interface Category
+{
+	public function imageFor(string $season): ?string;
+}
+
+final class Presenter
+{
+	/**
+	 * @param list<?Category> $categories
+	 * @return list<?string>
+	 */
+	public function images(array $categories): array
+	{
+		$images = [];
+		foreach ($categories as $category) {
+			$images[] = $category?->imageFor($this->undefinedMethod($this->undefinedProperty), 1 + 'x');
+		}
+
+		return $images;
+	}
+
+	/**
+	 * @return list<?string>
+	 */
+	public function whileLoop(?Category $category): array
+	{
+		$images = [];
+		while (count($images) < 10) {
+			$images[] = $category?->imageFor($this->undefinedWhile());
+		}
+
+		return $images;
+	}
+
+	/**
+	 * @return list<?string>
+	 */
+	public function doWhileLoop(?Category $category): array
+	{
+		$images = [];
+		do {
+			$images[] = $category?->imageFor($this->undefinedDoWhile());
+		} while (count($images) < 10);
+
+		return $images;
+	}
+
+	/**
+	 * @return list<?string>
+	 */
+	public function forLoop(?Category $category): array
+	{
+		$images = [];
+		for ($i = 0; $i < 10; $i++) {
+			$images[] = $category?->imageFor($this->undefinedFor());
+		}
+
+		return $images;
+	}
+
+	/**
+	 * @param list<?Category> $categories
+	 * @return list<?string>
+	 */
+	public function closureArgument(array $categories): array
+	{
+		$images = [];
+		foreach ($categories as $category) {
+			$images[] = $category?->imageFor((function (): string {
+				return $this->undefinedInClosure();
+			})());
+			$images[] = $category?->imageFor((fn (): string => $this->undefinedInArrowFunction())());
+		}
+
+		return $images;
+	}
+
+	/**
+	 * @param list<?object> $objects
+	 * @return list<mixed>
+	 */
+	public function nullsafePropertyFetch(array $objects, string $name): array
+	{
+		$values = [];
+		foreach ($objects as $object) {
+			$values[] = $object?->{$name . $this->undefinedInPropertyName()};
+		}
+
+		return $values;
+	}
+
+	/**
+	 * @param list<?Category> $categories
+	 * @return list<?string>
+	 */
+	public function nestedAndChained(array $categories, string $method): array
+	{
+		$images = [];
+		foreach ($categories as $category) {
+			$images[] = $category?->imageFor($category?->imageFor($this->undefinedInNested()) ?? '');
+			$images[] = $category?->{$method . $this->undefinedInMethodName()}();
+		}
+
+		return $images;
+	}
+}

tests/PHPStan/Rules/DeadCode/UnusedPrivatePropertyRuleTest.php

@@ -475,6 +475,15 @@ public function testBug14703(): void
 		$this->analyse([__DIR__ . '/data/bug-14703.php'], []);
 	}
 
+	#[RequiresPhp('>= 8.0.0')]
+	public function testBug15520(): void
+	{
+		$this->alwaysWrittenTags = [];
+		$this->alwaysReadTags = [];
+
+		$this->analyse([__DIR__ . '/data/bug-15520.php'], []);
+	}
+
 	public function testBug14880(): void
 	{
 		$this->alwaysWrittenTags = [];

tests/PHPStan/Rules/DeadCode/data/bug-15520.php

@@ -0,0 +1,81 @@
+<?php declare(strict_types = 1); // lint >= 8.0
+
+namespace Bug15520Property;
+
+interface SeasonResolver
+{
+	public function current(): string;
+}
+
+interface Category
+{
+	public function imageFor(string $season): ?string;
+}
+
+final class Presenter
+{
+	public function __construct(
+		private SeasonResolver $seasonResolver,
+	)
+	{
+	}
+
+	/**
+	 * @param list<?Category> $categories
+	 * @return list<?string>
+	 */
+	public function images(array $categories): array
+	{
+		$images = [];
+		foreach ($categories as $category) {
+			$images[] = $category?->imageFor($this->seasonResolver->current());
+		}
+
+		return $images;
+	}
+}
+
+final class WhilePresenter
+{
+	public function __construct(
+		private SeasonResolver $seasonResolver,
+	)
+	{
+	}
+
+	/**
+	 * @return list<?string>
+	 */
+	public function images(?Category $category): array
+	{
+		$images = [];
+		while (count($images) < 10) {
+			$images[] = $category?->imageFor($this->seasonResolver->current());
+		}
+
+		return $images;
+	}
+}
+
+final class PropertyNamePresenter
+{
+	public function __construct(
+		private string $name,
+	)
+	{
+	}
+
+	/**
+	 * @param list<?object> $objects
+	 * @return list<mixed>
+	 */
+	public function values(array $objects): array
+	{
+		$values = [];
+		foreach ($objects as $object) {
+			$values[] = $object?->{$this->name};
+		}
+
+		return $values;
+	}
+}