Track array value type overwrites in by-reference foreach without key variable

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,58 +1349,82 @@
 				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()
 			) {
-				$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 ($byRefWithoutKey) {
+					$arrayDimFetchLoopTypes = [];
+					foreach ($scopesWithIterableValueType as $scopeWithIterableValueType) {
+						$arrayDimFetchLoopTypes[] = $scopeWithIterableValueType->getType($stmt->valueVar);
 					}
-					$arrayDimFetchLoopTypes[] = $dimFetchType;
-					$keyLoopTypes[] = $scopeWithIterableValueType->getType($stmt->keyVar);
-				}
+					$arrayDimFetchLoopType = TypeCombinator::union(...$arrayDimFetchLoopTypes);
 
-				$arrayDimFetchLoopType = TypeCombinator::union(...$arrayDimFetchLoopTypes);
-				$keyLoopType = TypeCombinator::union(...$keyLoopTypes);
+					$arrayDimFetchLoopNativeTypes = [];
+					foreach ($scopesWithIterableValueType as $scopeWithIterableValueType) {
+						$arrayDimFetchLoopNativeTypes[] = $scopeWithIterableValueType->getNativeType($stmt->valueVar);
+					}
+					$arrayDimFetchLoopNativeType = TypeCombinator::union(...$arrayDimFetchLoopNativeTypes);
+
+					$valueTypeChanged = !$arrayDimFetchLoopType->equals($exprType->getIterableValueType());
+				} elseif ($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);
+						// 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);
+					}
 
-				$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;
+							}
 						}
+						$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());
+					$valueTypeChanged = !$arrayDimFetchLoopType->equals($exprType->getIterableValueType());
+					$keyTypeChanged = !$keyLoopType->equals($exprType->getIterableKeyType());
+				}
 
 				if ($valueTypeChanged || $keyTypeChanged) {
 					$newExprType = TypeTraverser::map($exprType, static function (Type $type, callable $traverse) use ($arrayDimFetchLoopType, $keyLoopType, $valueTypeChanged, $keyTypeChanged): Type {
@@ -1395,7 +1441,6 @@
 							$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);

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);
 }
 
 /**

tests/PHPStan/Analyser/nsrt/bug-1940-with-key.php

@@ -0,0 +1,59 @@
+<?php
+
+namespace Bug1940WithKey;
+
+use function PHPStan\Testing\assertType;
+
+/**
+ * Reviewer's exact example: by-ref with key, modifying sub-element.
+ * This already worked before this PR.
+ * @param list<array{one: string}> $a
+ */
+function byRefWithKeyModifySubElement(array $a): void
+{
+	foreach ($a as $k => &$testArray) {
+		$testArray['two'] = 'two';
+	}
+
+	assertType("list<array{one: string, two: 'two'}>", $a);
+}
+
+/**
+ * By-ref WITHOUT key, modifying sub-element.
+ * Parallel case to the reviewer's example but without key variable.
+ * @param list<array{one: string}> $a
+ */
+function byRefWithoutKeyModifySubElement(array $a): void
+{
+	foreach ($a as &$testArray) {
+		$testArray['two'] = 'two';
+	}
+
+	assertType("list<array{one: string, two: 'two'}>", $a);
+}
+
+/**
+ * By-ref with key, direct overwrite (already worked before this PR)
+ * @param array<int, string> $arr
+ */
+function byRefWithKeyDirectOverwrite(array $arr): void
+{
+	foreach ($arr as $k => &$v) {
+		$v = 1;
+	}
+
+	assertType('array<int, 1>', $arr);
+}
+
+/**
+ * By-ref without key, direct overwrite (this PR's main fix)
+ * @param array<int, string> $arr
+ */
+function byRefWithoutKeyDirectOverwrite(array $arr): void
+{
+	foreach ($arr as &$v) {
+		$v = 1;
+	}
+
+	assertType('array<int, 1>', $arr);
+}