More precise tracking $value in foreach

Author: ondrejmirtes

src/Analyser/MutatingScope.php

@@ -28,6 +28,7 @@
 use PHPStan\Node\Expr\GetIterableKeyTypeExpr;
 use PHPStan\Node\Expr\IntertwinedVariableByReferenceWithExpr;
 use PHPStan\Node\Expr\OriginalForeachKeyExpr;
+use PHPStan\Node\Expr\OriginalForeachValueExpr;
 use PHPStan\Node\Expr\ParameterVariableOriginalValueExpr;
 use PHPStan\Node\Expr\PossiblyImpureCallExpr;
 use PHPStan\Node\Expr\PropertyInitializationExpr;
@@ -2353,6 +2354,11 @@ public function enterForeach(self $originalScope, Expr $iteratee, string $valueN
 			$nativeValueType,
 			TrinaryLogic::createYes(),
 		);
+		// Track the original foreach value so narrowings applied to the value
+		// variable (e.g. is_string($type)) can later be projected back onto the
+		// corresponding array dim fetch without being confused by a reassignment
+		// ($type = 'foo' invalidates this expression, same as OriginalForeachKeyExpr).
+		$scope = $scope->assignExpression(new OriginalForeachValueExpr($valueName), $valueType, $nativeValueType);
 		if ($valueByRef && $iterateeType->isArray()->yes() && $iterateeType->isConstantArray()->no()) {
 			$scope = $scope->assignExpression(
 				new IntertwinedVariableByReferenceWithExpr($valueName, $iteratee, new SetExistingOffsetValueTypeExpr(

src/Analyser/NodeScopeResolver.php

@@ -79,6 +79,7 @@
 use PHPStan\Node\Expr\GetIterableKeyTypeExpr;
 use PHPStan\Node\Expr\GetIterableValueTypeExpr;
 use PHPStan\Node\Expr\OriginalForeachKeyExpr;
+use PHPStan\Node\Expr\OriginalForeachValueExpr;
 use PHPStan\Node\Expr\PropertyInitializationExpr;
 use PHPStan\Node\Expr\TypeExpr;
 use PHPStan\Node\Expr\UnsetOffsetExpr;
@@ -1330,10 +1331,27 @@ public function processStmtNode(
 				&& $exprType->isConstantArray()->no()
 			) {
 				$arrayExprDimFetch = new ArrayDimFetch($stmt->expr, $stmt->keyVar);
+				$originalValueExpr = null;
+				if ($stmt->valueVar instanceof Variable && is_string($stmt->valueVar->name)) {
+					$originalValueExpr = new OriginalForeachValueExpr($stmt->valueVar->name);
+				}
 				$arrayDimFetchLoopTypes = [];
 				$keyLoopTypes = [];
 				foreach ($scopesWithIterableValueType as $scopeWithIterableValueType) {
-					$arrayDimFetchLoopTypes[] = $scopeWithIterableValueType->getType($arrayExprDimFetch);
+					$dimFetchType = $scopeWithIterableValueType->getType($arrayExprDimFetch);
+					// Condition-based narrowings like `is_string($type)` apply to the value
+					// variable but not automatically to the array dim fetch, even though the
+					// two describe the same element for a given iteration. If the value var
+					// hasn't been reassigned (OriginalForeachValueExpr still tracked) we use
+					// the narrowed value-var type in place of the broader dim fetch type so
+					// the loop's final array rewrite below picks up the sharper element type.
+					if ($originalValueExpr !== null && $scopeWithIterableValueType->hasExpressionType($originalValueExpr)->yes()) {
+						$valueVarType = $scopeWithIterableValueType->getType($stmt->valueVar);
+						if ($dimFetchType->isSuperTypeOf($valueVarType)->yes()) {
+							$dimFetchType = $valueVarType;
+						}
+					}
+					$arrayDimFetchLoopTypes[] = $dimFetchType;
 					$keyLoopTypes[] = $scopeWithIterableValueType->getType($stmt->keyVar);
 				}
 
@@ -1343,8 +1361,15 @@ public function processStmtNode(
 				$arrayDimFetchLoopNativeTypes = [];
 				$keyLoopNativeTypes = [];
 				foreach ($scopesWithIterableValueType as $scopeWithIterableValueType) {
-					$arrayDimFetchLoopNativeTypes[] = $scopeWithIterableValueType->getNativeType($arrayExprDimFetch);
-					$keyLoopNativeTypes[] = $scopeWithIterableValueType->getNativeType($stmt->keyVar);
+					$dimFetchNativeType = $scopeWithIterableValueType->getNativeType($arrayExprDimFetch);
+					if ($originalValueExpr !== null && $scopeWithIterableValueType->hasExpressionType($originalValueExpr)->yes()) {
+						$valueVarNativeType = $scopeWithIterableValueType->getNativeType($stmt->valueVar);
+						if ($dimFetchNativeType->isSuperTypeOf($valueVarNativeType)->yes()) {
+							$dimFetchNativeType = $valueVarNativeType;
+						}
+					}
+					$arrayDimFetchLoopNativeTypes[] = $dimFetchNativeType;
+					$keyLoopNativeTypes[] = $scopeWithIterableValueType->getType($stmt->keyVar);
 				}
 
 				$arrayDimFetchLoopNativeType = TypeCombinator::union(...$arrayDimFetchLoopNativeTypes);
@@ -3911,6 +3936,11 @@ private function tryProcessUnrolledConstantArrayForeach(
 				$nativeValueType,
 				TrinaryLogic::createYes(),
 			);
+			$iterScope = $iterScope->assignExpression(
+				new OriginalForeachValueExpr($valueVarName),
+				$valueType,
+				$nativeValueType,
+			);
 			if ($keyVarName !== null) {
 				$iterScope = $iterScope->assignVariable(
 					$keyVarName,

src/Node/Expr/OriginalForeachValueExpr.php

@@ -0,0 +1,40 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Node\Expr;
+
+use Override;
+use PhpParser\Node\Expr;
+use PHPStan\Node\VirtualNode;
+
+final class OriginalForeachValueExpr extends Expr implements VirtualNode
+{
+
+	public Expr\Variable $var;
+
+	public function __construct(private string $variableName)
+	{
+		parent::__construct([]);
+		$this->var = new Expr\Variable($this->variableName);
+	}
+
+	public function getVariableName(): string
+	{
+		return $this->variableName;
+	}
+
+	#[Override]
+	public function getType(): string
+	{
+		return 'PHPStan_Node_OriginalForeachValueExpr';
+	}
+
+	/**
+	 * @return string[]
+	 */
+	#[Override]
+	public function getSubNodeNames(): array
+	{
+		return ['var'];
+	}
+
+}

src/Node/Printer/Printer.php

@@ -15,6 +15,7 @@
 use PHPStan\Node\Expr\IntertwinedVariableByReferenceWithExpr;
 use PHPStan\Node\Expr\NativeTypeExpr;
 use PHPStan\Node\Expr\OriginalForeachKeyExpr;
+use PHPStan\Node\Expr\OriginalForeachValueExpr;
 use PHPStan\Node\Expr\OriginalPropertyTypeExpr;
 use PHPStan\Node\Expr\ParameterVariableOriginalValueExpr;
 use PHPStan\Node\Expr\PossiblyImpureCallExpr;
@@ -118,6 +119,11 @@ protected function pPHPStan_Node_OriginalForeachKeyExpr(OriginalForeachKeyExpr $
 		return sprintf('__phpstanOriginalForeachKey(%s)', $expr->getVariableName());
 	}
 
+	protected function pPHPStan_Node_OriginalForeachValueExpr(OriginalForeachValueExpr $expr): string // phpcs:ignore
+	{
+		return sprintf('__phpstanOriginalForeachValue(%s)', $expr->getVariableName());
+	}
+
 	protected function pPHPStan_Node_IntertwinedVariableByReferenceWithExpr(IntertwinedVariableByReferenceWithExpr $expr): string // phpcs:ignore
 	{
 		return sprintf('__phpstanIntertwinedVariableByReference(%s, %s, %s)', $expr->getVariableName(), $this->p($expr->getExpr()), $this->p($expr->getAssignedExpr()));

tests/PHPStan/Analyser/nsrt/foreach-array-rewrite.php

@@ -0,0 +1,70 @@
+<?php
+
+namespace ForeachArrayRewrite;
+
+use function PHPStan\Testing\assertType;
+
+interface Thing
+{
+}
+
+class Resolver
+{
+	public function resolve(string $s): Thing
+	{
+		throw new \Exception();
+	}
+}
+
+class Tester
+{
+
+	/**
+	 * @param list<Thing>|list<string> $types
+	 */
+	public function narrowAndReplace(array $types, Resolver $resolver): void
+	{
+		foreach ($types as $i => $type) {
+			if (!is_string($type)) {
+				continue;
+			}
+
+			$types[$i] = $resolver->resolve($type);
+		}
+
+		// Every iteration ends with $types[$i] being a Thing — either the continue
+		// branch (where $type was already a Thing, so $types[$i] was too) or the
+		// assignment branch (where $types[$i] was overwritten with a Thing).
+		assertType('list<ForeachArrayRewrite\\Thing>', $types);
+	}
+
+	/**
+	 * @param list<Thing|string> $types
+	 */
+	public function reassignValueVarIsNotAliased(array $types): void
+	{
+		foreach ($types as $i => $type) {
+			// Reassigning $type does not modify $types[$i], so the array's element
+			// type must be preserved (not narrowed to 'foo').
+			$type = 'foo';
+		}
+
+		assertType('list<ForeachArrayRewrite\\Thing|string>', $types);
+	}
+
+	/**
+	 * @param list<Thing|string> $types
+	 */
+	public function plainNarrowingFlowsThrough(array $types): void
+	{
+		foreach ($types as $i => $type) {
+			if (is_string($type)) {
+				continue;
+			}
+		}
+
+		// The loop didn't modify $types, so its shape is unchanged.
+		assertType('list<ForeachArrayRewrite\\Thing|string>', $types);
+	}
+
+}