Track array value type overwrites in by-reference foreach on constant arrays

src/Analyser/NodeScopeResolver.php

@@ -1294,19 +1294,41 @@
 
 			$originalKeyVarExpr = null;
 			$continueExitPointHasUnoriginalKeyType = false;
+			$byRefWithoutKey = $stmt->byRef
+				&& $stmt->keyVar === null
+				&& $stmt->valueVar instanceof Variable && is_string($stmt->valueVar->name);
+
 			if ($stmt->keyVar instanceof Variable && is_string($stmt->keyVar->name)) {
 				$originalKeyVarExpr = new OriginalForeachKeyExpr($stmt->keyVar->name);
 				if ($finalScope->hasExpressionType($originalKeyVarExpr)->yes()) {
 					$scopesWithIterableValueType[] = $finalScope;
 				} else {
 					$continueExitPointHasUnoriginalKeyType = true;
 				}
+			} elseif ($byRefWithoutKey) {
+				$originalValueExpr = new OriginalForeachValueExpr($stmt->valueVar->name);
+				if (!$finalScope->hasExpressionType($originalValueExpr)->yes()) {
+					$scopesWithIterableValueType[] = $finalScope;
+				} else {
+					$continueExitPointHasUnoriginalKeyType = true;
+				}
 			}
 
 			foreach ($finalScopeResult->getExitPointsByType(Continue_::class) as $continueExitPoint) {
 				$continueScope = $continueExitPoint->getScope();
 				$finalScope = $continueScope->mergeWith($finalScope);
-				if ($originalKeyVarExpr === null || !$continueScope->hasExpressionType($originalKeyVarExpr)->yes()) {
+				if ($originalKeyVarExpr !== null) {
+					if (!$continueScope->hasExpressionType($originalKeyVarExpr)->yes()) {
+						$continueExitPointHasUnoriginalKeyType = true;
+						continue;
+					}
+				} elseif ($byRefWithoutKey) {
+					$originalValueExpr = new OriginalForeachValueExpr($stmt->valueVar->name);
+					if ($continueScope->hasExpressionType($originalValueExpr)->yes()) {
+						$continueExitPointHasUnoriginalKeyType = true;
+						continue;
+					}
+				} else {
 					$continueExitPointHasUnoriginalKeyType = true;
 					continue;
 				}
@@ -1327,65 +1349,91 @@
 				count($breakExitPoints) === 0
 				&& count($scopesWithIterableValueType) > 0
 				&& !$continueExitPointHasUnoriginalKeyType
-				&& $stmt->keyVar !== null
+				&& ($stmt->keyVar !== null || $byRefWithoutKey)
 				&& (!$hasExpr->no() || !$stmt->expr instanceof Variable)
 				&& $exprType->isArray()->yes()
-				&& $exprType->isConstantArray()->no()
+				&& ($exprType->isConstantArray()->no() || $stmt->byRef)
 			) {
-				$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) {
-					$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;
+				$nativeExprType = $scope->getNativeType($stmt->expr);
+				$arrayDimFetchLoopType = $exprType->getIterableValueType();
+				$arrayDimFetchLoopNativeType = $nativeExprType->getIterableValueType();
+				$keyLoopType = $exprType->getIterableKeyType();
+				$keyLoopNativeType = $nativeExprType->getIterableKeyType();
+				$valueTypeChanged = false;
+				$keyTypeChanged = false;
+
+				if ($stmt->keyVar !== null) {
+					$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) {
+						$dimFetchType = $scopeWithIterableValueType->getType($arrayExprDimFetch);
+						if ($originalValueExpr !== null && $scopeWithIterableValueType->hasExpressionType($originalValueExpr)->yes()) {
+							$valueVarType = $scopeWithIterableValueType->getType($stmt->valueVar);
+							if ($dimFetchType->isSuperTypeOf($valueVarType)->yes()) {
+								$dimFetchType = $valueVarType;
+							}
+						} elseif ($stmt->byRef && $originalValueExpr !== null) {
+							$dimFetchType = $scopeWithIterableValueType->getType($stmt->valueVar);
 						}
+						$arrayDimFetchLoopTypes[] = $dimFetchType;
+						$keyLoopTypes[] = $scopeWithIterableValueType->getType($stmt->keyVar);
 					}
-					$arrayDimFetchLoopTypes[] = $dimFetchType;
-					$keyLoopTypes[] = $scopeWithIterableValueType->getType($stmt->keyVar);
-				}
 
-				$arrayDimFetchLoopType = TypeCombinator::union(...$arrayDimFetchLoopTypes);
-				$keyLoopType = TypeCombinator::union(...$keyLoopTypes);
-
-				$arrayDimFetchLoopNativeTypes = [];
-				$keyLoopNativeTypes = [];
-				foreach ($scopesWithIterableValueType as $scopeWithIterableValueType) {
-					$dimFetchNativeType = $scopeWithIterableValueType->getNativeType($arrayExprDimFetch);
-					if ($originalValueExpr !== null && $scopeWithIterableValueType->hasExpressionType($originalValueExpr)->yes()) {
-						$valueVarNativeType = $scopeWithIterableValueType->getNativeType($stmt->valueVar);
-						if ($dimFetchNativeType->isSuperTypeOf($valueVarNativeType)->yes()) {
-							$dimFetchNativeType = $valueVarNativeType;
+					$arrayDimFetchLoopType = TypeCombinator::union(...$arrayDimFetchLoopTypes);
+					$keyLoopType = TypeCombinator::union(...$keyLoopTypes);
+
+					$arrayDimFetchLoopNativeTypes = [];
+					$keyLoopNativeTypes = [];
+					foreach ($scopesWithIterableValueType as $scopeWithIterableValueType) {
+						$dimFetchNativeType = $scopeWithIterableValueType->getNativeType($arrayExprDimFetch);
+						if ($originalValueExpr !== null && $scopeWithIterableValueType->hasExpressionType($originalValueExpr)->yes()) {
+							$valueVarNativeType = $scopeWithIterableValueType->getNativeType($stmt->valueVar);
+							if ($dimFetchNativeType->isSuperTypeOf($valueVarNativeType)->yes()) {
+								$dimFetchNativeType = $valueVarNativeType;
+							}
+						} elseif ($stmt->byRef && $originalValueExpr !== null) {
+							$dimFetchNativeType = $scopeWithIterableValueType->getNativeType($stmt->valueVar);
 						}
+						$arrayDimFetchLoopNativeTypes[] = $dimFetchNativeType;
+						$keyLoopNativeTypes[] = $scopeWithIterableValueType->getType($stmt->keyVar);
 					}
-					$arrayDimFetchLoopNativeTypes[] = $dimFetchNativeType;
-					$keyLoopNativeTypes[] = $scopeWithIterableValueType->getType($stmt->keyVar);
-				}
 
-				$arrayDimFetchLoopNativeType = TypeCombinator::union(...$arrayDimFetchLoopNativeTypes);
-				$keyLoopNativeType = TypeCombinator::union(...$keyLoopNativeTypes);
+					$arrayDimFetchLoopNativeType = TypeCombinator::union(...$arrayDimFetchLoopNativeTypes);
+					$keyLoopNativeType = TypeCombinator::union(...$keyLoopNativeTypes);
+
+					$valueTypeChanged = !$arrayDimFetchLoopType->equals($exprType->getIterableValueType());
+					$keyTypeChanged = !$keyLoopType->equals($exprType->getIterableKeyType());
+				} elseif ($byRefWithoutKey) {
+					$arrayDimFetchLoopTypes = [];
+					foreach ($scopesWithIterableValueType as $scopeWithIterableValueType) {
+						$arrayDimFetchLoopTypes[] = $scopeWithIterableValueType->getType($stmt->valueVar);
+					}
+					$arrayDimFetchLoopType = TypeCombinator::union(...$arrayDimFetchLoopTypes);
+
+					$arrayDimFetchLoopNativeTypes = [];
+					foreach ($scopesWithIterableValueType as $scopeWithIterableValueType) {
+						$arrayDimFetchLoopNativeTypes[] = $scopeWithIterableValueType->getNativeType($stmt->valueVar);
+					}
+					$arrayDimFetchLoopNativeType = TypeCombinator::union(...$arrayDimFetchLoopNativeTypes);
 
-				$valueTypeChanged = !$arrayDimFetchLoopType->equals($exprType->getIterableValueType());
-				$keyTypeChanged = !$keyLoopType->equals($exprType->getIterableKeyType());
+					$valueTypeChanged = !$arrayDimFetchLoopType->equals($exprType->getIterableValueType());
+				}
 
 				if ($valueTypeChanged || $keyTypeChanged) {
 					$newExprType = TypeTraverser::map($exprType, static function (Type $type, callable $traverse) use ($arrayDimFetchLoopType, $keyLoopType, $valueTypeChanged, $keyTypeChanged): Type {
 						if ($type instanceof UnionType || $type instanceof IntersectionType) {
 							return $traverse($type);
 						}
 
+						if ($type->isConstantArray()->yes() && $valueTypeChanged && !$keyTypeChanged) {
+							return $type->traverse(static fn () => $arrayDimFetchLoopType);
+						}
+
 						if (!$type instanceof ArrayType) {
 							return $type;
 						}
@@ -1395,12 +1443,15 @@
 							$valueTypeChanged ? $arrayDimFetchLoopType : $type->getIterableValueType(),
 						);
 					});
-					$nativeExprType = $scope->getNativeType($stmt->expr);
 					$newExprNativeType = TypeTraverser::map($nativeExprType, static function (Type $type, callable $traverse) use ($arrayDimFetchLoopNativeType, $keyLoopNativeType, $valueTypeChanged, $keyTypeChanged): Type {
 						if ($type instanceof UnionType || $type instanceof IntersectionType) {
 							return $traverse($type);
 						}
 
+						if ($type->isConstantArray()->yes() && $valueTypeChanged && !$keyTypeChanged) {
+							return $type->traverse(static fn () => $arrayDimFetchLoopNativeType);
+						}
+
 						if (!$type instanceof ArrayType) {
 							return $type;
 						}

tests/PHPStan/Analyser/nsrt/bug-1311-constant-array.php

@@ -0,0 +1,99 @@
+<?php
+
+namespace Bug1311ConstantArray;
+
+use function PHPStan\Testing\assertType;
+
+class HelloWorld
+{
+	/**
+	 * @var array<int, string>
+	 */
+	private $list = [];
+
+	public function convertListByRefWithoutKey(): void
+	{
+		$temp = [1, 2, 3];
+
+		foreach ($temp as &$item) {
+			$item = (string) $item;
+		}
+
+		assertType("array{'1'|'2'|'3', '1'|'2'|'3', '1'|'2'|'3'}", $temp);
+
+		$this->list = $temp;
+	}
+
+	public function convertListByRefWithKey(): void
+	{
+		$temp = [1, 2, 3];
+
+		foreach ($temp as $k => &$item) {
+			$item = (string) $item;
+		}
+
+		assertType("array{'1'|'2'|'3', '1'|'2'|'3', '1'|'2'|'3'}", $temp);
+
+		$this->list = $temp;
+	}
+
+	public function byRefConstantArrayConditional(): void
+	{
+		$temp = [1, 2, 3];
+
+		foreach ($temp as &$item) {
+			if (rand(0, 1)) {
+				$item = (string) $item;
+			}
+		}
+
+		assertType("array{1|2|3|'1'|'2'|'3', 1|2|3|'1'|'2'|'3', 1|2|3|'1'|'2'|'3'}", $temp);
+	}
+
+	public function byRefConstantArrayWithBreak(): void
+	{
+		$temp = [1, 2, 3];
+
+		foreach ($temp as &$item) {
+			$item = (string) $item;
+			if (rand(0, 1)) {
+				break;
+			}
+		}
+
+		assertType('array{1, 2, 3}', $temp);
+	}
+
+	public function byRefConstantArrayIntval(): void
+	{
+		$temp = ['a', 'b', 'c'];
+
+		foreach ($temp as &$item) {
+			$item = strlen($item);
+		}
+
+		assertType('array{1, 1, 1}', $temp);
+	}
+
+	public function byRefConstantArrayStringKeys(): void
+	{
+		$temp = ['x' => 1, 'y' => 2];
+
+		foreach ($temp as &$v) {
+			$v = (string) $v;
+		}
+
+		assertType("array{x: '1'|'2', y: '1'|'2'}", $temp);
+	}
+
+	public function byRefConstantArrayNoOverwrite(): void
+	{
+		$temp = [1, 2, 3];
+
+		foreach ($temp as &$item) {
+			echo $item;
+		}
+
+		assertType('array{1, 2, 3}', $temp);
+	}
+}

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

@@ -0,0 +1,27 @@
+<?php
+
+namespace Bug1311;
+
+use function PHPStan\Testing\assertType;
+
+class HelloWorld
+{
+	/**
+	 * @var array<int, string>
+	 */
+	private $list = [];
+
+	/**
+	 * @param array<int, int> $temp
+	 */
+	public function convertList(array $temp): void
+	{
+		foreach ($temp as &$item) {
+			$item = (string) $item;
+		}
+
+		assertType('array<int, lowercase-string&numeric-string&uppercase-string>', $temp);
+
+		$this->list = $temp;
+	}
+}

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

@@ -13,7 +13,7 @@ function foo(array $list): void
 		$value = 'foo';
 	}
 
-	assertType('list<mixed>', $list);
+	assertType("list<'foo'>", $list);
 }
 
 /**
@@ -56,7 +56,7 @@ function bar3(array $list): void
 		}
 	}
 
-	assertType("list<mixed>", $list); // could be list<'foo'|'maybe'>
+	assertType("list<'foo'|'maybe'>", $list);
 }
 
 /**
@@ -68,7 +68,7 @@ function baz(array $list): void
 		$value = 'bar';
 	}
 
-	assertType('list<string>', $list);
+	assertType("list<'bar'>", $list);
 }
 
 /**

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

@@ -13,7 +13,7 @@ function example(array $convert): void {
 	foreach ($convert as &$item) {
 		$item = strtoupper($item);
 	}
-	assertType('list<string>', $convert);
+	assertType('list<uppercase-string>', $convert);
 }
 
 /**

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

@@ -16,7 +16,7 @@ function example(array $convert): void
 			$val = strtoupper($val); // https://github.com/phpstan/phpstan/issues/14083
 		}
 	}
-	assertType('array<string, list<string>>', $convert);
+	assertType('array<string, list<uppercase-string>>', $convert);
 }
 
 /**
@@ -29,7 +29,7 @@ function example2(array $convert): void
 			$inner[$key] = strtoupper($val);
 		}
 	}
-	assertType('array<string, list<string>>', $convert);
+	assertType('array<string, list<uppercase-string>>', $convert);
 }
 
 /**