From a5f0c724fe28f6a21f9b72e4edb54f163ca79a92 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Tue, 28 Apr 2026 10:47:34 +0200 Subject: [PATCH 1/4] Reapply "Track array value type overwrites in by-reference `foreach` without key variable (#5542)" This reverts commit 6c69c44f85257d0b92811c2dae9db77405cd526b. --- src/Analyser/NodeScopeResolver.php | 131 ++++++++---- tests/PHPStan/Analyser/nsrt/bug-1311.php | 27 +++ tests/PHPStan/Analyser/nsrt/bug-13809.php | 6 +- tests/PHPStan/Analyser/nsrt/bug-14083.php | 2 +- tests/PHPStan/Analyser/nsrt/bug-14084.php | 4 +- .../Analyser/nsrt/bug-1940-with-key.php | 59 ++++++ tests/PHPStan/Analyser/nsrt/bug-1940.php | 193 ++++++++++++++++++ .../Analyser/nsrt/overwritten-arrays.php | 2 +- .../Rules/Variables/data/bug-14124.php | 2 +- .../Rules/Variables/data/bug-14124b.php | 2 +- 10 files changed, 376 insertions(+), 52 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-1311.php create mode 100644 tests/PHPStan/Analyser/nsrt/bug-1940-with-key.php create mode 100644 tests/PHPStan/Analyser/nsrt/bug-1940.php diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 89a65fca313..7fffbc46317 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -1294,6 +1294,10 @@ public function processStmtNode( $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()) { @@ -1301,12 +1305,30 @@ public function processStmtNode( } 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 @@ public function processStmtNode( 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 ($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); } - $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; + } } + $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 { @@ -1395,7 +1441,6 @@ public function processStmtNode( $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); diff --git a/tests/PHPStan/Analyser/nsrt/bug-1311.php b/tests/PHPStan/Analyser/nsrt/bug-1311.php new file mode 100644 index 00000000000..d2d72eb437f --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-1311.php @@ -0,0 +1,27 @@ + + */ + private $list = []; + + /** + * @param array $temp + */ + public function convertList(array $temp): void + { + foreach ($temp as &$item) { + $item = (string) $item; + } + + assertType('array', $temp); + + $this->list = $temp; + } +} diff --git a/tests/PHPStan/Analyser/nsrt/bug-13809.php b/tests/PHPStan/Analyser/nsrt/bug-13809.php index 9cdb9d5fa5e..b91c64f7fdc 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-13809.php +++ b/tests/PHPStan/Analyser/nsrt/bug-13809.php @@ -13,7 +13,7 @@ function foo(array $list): void $value = 'foo'; } - assertType('list', $list); + assertType("list<'foo'>", $list); } /** @@ -56,7 +56,7 @@ function bar3(array $list): void } } - assertType("list", $list); // could be list<'foo'|'maybe'> + assertType("list<'foo'|'maybe'>", $list); } /** @@ -68,7 +68,7 @@ function baz(array $list): void $value = 'bar'; } - assertType('list', $list); + assertType("list<'bar'>", $list); } /** diff --git a/tests/PHPStan/Analyser/nsrt/bug-14083.php b/tests/PHPStan/Analyser/nsrt/bug-14083.php index 8da92d43d36..14fcbef77a3 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-14083.php +++ b/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', $convert); + assertType('list', $convert); } /** diff --git a/tests/PHPStan/Analyser/nsrt/bug-14084.php b/tests/PHPStan/Analyser/nsrt/bug-14084.php index 3f044aa559a..006510dea47 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-14084.php +++ b/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>', $convert); + assertType('array>', $convert); } /** @@ -29,7 +29,7 @@ function example2(array $convert): void $inner[$key] = strtoupper($val); } } - assertType('array>', $convert); + assertType('array>', $convert); } /** diff --git a/tests/PHPStan/Analyser/nsrt/bug-1940-with-key.php b/tests/PHPStan/Analyser/nsrt/bug-1940-with-key.php new file mode 100644 index 00000000000..0ba6d79cfc9 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-1940-with-key.php @@ -0,0 +1,59 @@ + $a + */ +function byRefWithKeyModifySubElement(array $a): void +{ + foreach ($a as $k => &$testArray) { + $testArray['two'] = 'two'; + } + + assertType("list", $a); +} + +/** + * By-ref WITHOUT key, modifying sub-element. + * Parallel case to the reviewer's example but without key variable. + * @param list $a + */ +function byRefWithoutKeyModifySubElement(array $a): void +{ + foreach ($a as &$testArray) { + $testArray['two'] = 'two'; + } + + assertType("list", $a); +} + +/** + * By-ref with key, direct overwrite (already worked before this PR) + * @param array $arr + */ +function byRefWithKeyDirectOverwrite(array $arr): void +{ + foreach ($arr as $k => &$v) { + $v = 1; + } + + assertType('array', $arr); +} + +/** + * By-ref without key, direct overwrite (this PR's main fix) + * @param array $arr + */ +function byRefWithoutKeyDirectOverwrite(array $arr): void +{ + foreach ($arr as &$v) { + $v = 1; + } + + assertType('array', $arr); +} diff --git a/tests/PHPStan/Analyser/nsrt/bug-1940.php b/tests/PHPStan/Analyser/nsrt/bug-1940.php new file mode 100644 index 00000000000..67a17e38fed --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-1940.php @@ -0,0 +1,193 @@ + $arr + */ +function byRefWithoutKey(array $arr): void +{ + foreach ($arr as &$v) { + $v = 1; + } + + assertType('array', $arr); +} + +/** + * @param array $arr + */ +function byRefWithoutKeyConditional(array $arr): void +{ + foreach ($arr as &$v) { + if (rand(0, 1)) { + $v = 1; + } + } + + assertType('array', $arr); +} + +/** + * @param array $arr + */ +function byRefWithoutKeyAlwaysOverwritten(array $arr): void +{ + foreach ($arr as &$v) { + if (rand(0, 1)) { + $v = 1; + } else { + $v = 2; + } + } + + assertType('array', $arr); +} + +/** + * @param list $arr + */ +function byRefWithoutKeyList(array $arr): void +{ + foreach ($arr as &$v) { + $v = 'replaced'; + } + + assertType("list<'replaced'>", $arr); +} + +/** + * @param array $arr + */ +function byRefWithoutKeyStringKeys(array $arr): void +{ + foreach ($arr as &$v) { + $v = 'hello'; + } + + assertType("array", $arr); +} + +/** + * @param non-empty-array $arr + */ +function byRefWithoutKeyNonEmpty(array $arr): void +{ + foreach ($arr as &$v) { + $v = 42; + } + + assertType('non-empty-array', $arr); +} + +/** + * By-ref without key with break — should NOT rewrite since not all elements may be visited. + * @param array $arr + */ +function byRefWithoutKeyBreak(array $arr): void +{ + foreach ($arr as &$v) { + $v = 1; + if (rand(0, 1)) { + break; + } + } + + assertType('array', $arr); +} + +/** + * By-ref without key with continue — should still rewrite since all elements are visited. + * @param array $arr + */ +function byRefWithoutKeyContinue(array $arr): void +{ + foreach ($arr as &$v) { + $v = 1; + if (rand(0, 1)) { + continue; + } + } + + assertType('array', $arr); +} + +/** + * By-ref without key with continue where value is overwritten in all branches. + * @param array $arr + */ +function byRefWithoutKeyContinueBranches(array $arr): void +{ + foreach ($arr as &$v) { + if (rand(0, 1)) { + $v = 1; + continue; + } + $v = 2; + } + + assertType('array', $arr); +} + +/** + * By-ref without key with continue where value is NOT overwritten in the continue branch. + * @param array $arr + */ +function byRefWithoutKeyContinuePartial(array $arr): void +{ + foreach ($arr as &$v) { + if (rand(0, 1)) { + continue; + } + $v = 1; + } + + assertType('array', $arr); +} + +class Foo +{ + /** @var array */ + private array $prop; + + /** + * By-ref without key on a property. + */ + public function byRefWithoutKeyProperty(): void + { + foreach ($this->prop as &$v) { + $v = 'hello'; + } + + assertType("array", $this->prop); + } +} + +/** + * By-ref without key with intval transformation. + * @param array $arr + */ +function byRefWithoutKeyTransform(array $arr): void +{ + foreach ($arr as &$v) { + $v = intval($v); + } + + assertType('array', $arr); +} + +/** + * By-ref without key — value not overwritten at all. + * @param array $arr + */ +function byRefWithoutKeyNoOverwrite(array $arr): void +{ + foreach ($arr as &$v) { + // just read, don't write + echo $v; + } + + assertType('array', $arr); +} diff --git a/tests/PHPStan/Analyser/nsrt/overwritten-arrays.php b/tests/PHPStan/Analyser/nsrt/overwritten-arrays.php index 878ae9d082c..1ec5dd8b9ec 100644 --- a/tests/PHPStan/Analyser/nsrt/overwritten-arrays.php +++ b/tests/PHPStan/Analyser/nsrt/overwritten-arrays.php @@ -127,7 +127,7 @@ public function doFoo7(array $a): void $v = 1; } - assertType('array', $a); // could be array + assertType('array', $a); } /** diff --git a/tests/PHPStan/Rules/Variables/data/bug-14124.php b/tests/PHPStan/Rules/Variables/data/bug-14124.php index be3ad1a8ded..3a7303fb85b 100644 --- a/tests/PHPStan/Rules/Variables/data/bug-14124.php +++ b/tests/PHPStan/Rules/Variables/data/bug-14124.php @@ -15,7 +15,7 @@ function example3a(array &$convert): void $val = strtoupper($val); } } - assertType('array>', $convert); + assertType('array>', $convert); } /** diff --git a/tests/PHPStan/Rules/Variables/data/bug-14124b.php b/tests/PHPStan/Rules/Variables/data/bug-14124b.php index 0ad84eb7a3e..822e9f9ac68 100644 --- a/tests/PHPStan/Rules/Variables/data/bug-14124b.php +++ b/tests/PHPStan/Rules/Variables/data/bug-14124b.php @@ -17,7 +17,7 @@ function example3a(array &$convert): void } } } - assertType('array>>', $convert); + assertType('array>>', $convert); } /** From c1924d6539dd861eebc19250f90435b1f6f1e9a4 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Tue, 28 Apr 2026 10:48:33 +0200 Subject: [PATCH 2/4] Fix on constant array --- src/Analyser/NodeScopeResolver.php | 20 ++-- .../Analyser/nsrt/bug-1311-constant-array.php | 99 +++++++++++++++++++ 2 files changed, 112 insertions(+), 7 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-1311-constant-array.php diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 7fffbc46317..585192d6705 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -1352,7 +1352,7 @@ public function processStmtNode( && ($stmt->keyVar !== null || $byRefWithoutKey) && (!$hasExpr->no() || !$stmt->expr instanceof Variable) && $exprType->isArray()->yes() - && $exprType->isConstantArray()->no() + && ($exprType->isConstantArray()->no() || $stmt->byRef) ) { $nativeExprType = $scope->getNativeType($stmt->expr); $arrayDimFetchLoopType = $exprType->getIterableValueType(); @@ -1372,17 +1372,13 @@ public function processStmtNode( $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; } + } elseif ($stmt->byRef && $originalValueExpr !== null) { + $dimFetchType = $scopeWithIterableValueType->getType($stmt->valueVar); } $arrayDimFetchLoopTypes[] = $dimFetchType; $keyLoopTypes[] = $scopeWithIterableValueType->getType($stmt->keyVar); @@ -1400,6 +1396,8 @@ public function processStmtNode( if ($dimFetchNativeType->isSuperTypeOf($valueVarNativeType)->yes()) { $dimFetchNativeType = $valueVarNativeType; } + } elseif ($stmt->byRef && $originalValueExpr !== null) { + $dimFetchNativeType = $scopeWithIterableValueType->getNativeType($stmt->valueVar); } $arrayDimFetchLoopNativeTypes[] = $dimFetchNativeType; $keyLoopNativeTypes[] = $scopeWithIterableValueType->getType($stmt->keyVar); @@ -1432,6 +1430,10 @@ public function processStmtNode( return $traverse($type); } + if ($type->isConstantArray()->yes() && $valueTypeChanged && !$keyTypeChanged) { + return $type->traverse(static fn () => $arrayDimFetchLoopType); + } + if (!$type instanceof ArrayType) { return $type; } @@ -1446,6 +1448,10 @@ public function processStmtNode( return $traverse($type); } + if ($type->isConstantArray()->yes() && $valueTypeChanged && !$keyTypeChanged) { + return $type->traverse(static fn () => $arrayDimFetchLoopNativeType); + } + if (!$type instanceof ArrayType) { return $type; } diff --git a/tests/PHPStan/Analyser/nsrt/bug-1311-constant-array.php b/tests/PHPStan/Analyser/nsrt/bug-1311-constant-array.php new file mode 100644 index 00000000000..df4e3ca6b1f --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-1311-constant-array.php @@ -0,0 +1,99 @@ + + */ + 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); + } +} From 2576141516e1e0c47a5d1276d2634b44eb498af0 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Tue, 28 Apr 2026 11:50:49 +0000 Subject: [PATCH 3/4] Merge test files and add reviewer's constant array test case - Merge bug-1311-constant-array.php into bug-1311.php (single file, single namespace) - Merge bug-1940-with-key.php into bug-1940.php (single file, single namespace) - Add test for constant array by-ref foreach with sub-element modification to verify no false positive about missing offset after mutations Co-Authored-By: Claude Opus 4.6 --- .../Analyser/nsrt/bug-1311-constant-array.php | 99 --------------- tests/PHPStan/Analyser/nsrt/bug-1311.php | 120 ++++++++++++++++++ .../Analyser/nsrt/bug-1940-with-key.php | 59 --------- tests/PHPStan/Analyser/nsrt/bug-1940.php | 58 +++++++-- 4 files changed, 168 insertions(+), 168 deletions(-) delete mode 100644 tests/PHPStan/Analyser/nsrt/bug-1311-constant-array.php delete mode 100644 tests/PHPStan/Analyser/nsrt/bug-1940-with-key.php diff --git a/tests/PHPStan/Analyser/nsrt/bug-1311-constant-array.php b/tests/PHPStan/Analyser/nsrt/bug-1311-constant-array.php deleted file mode 100644 index df4e3ca6b1f..00000000000 --- a/tests/PHPStan/Analyser/nsrt/bug-1311-constant-array.php +++ /dev/null @@ -1,99 +0,0 @@ - - */ - 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); - } -} diff --git a/tests/PHPStan/Analyser/nsrt/bug-1311.php b/tests/PHPStan/Analyser/nsrt/bug-1311.php index d2d72eb437f..257d4baee74 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-1311.php +++ b/tests/PHPStan/Analyser/nsrt/bug-1311.php @@ -24,4 +24,124 @@ public function convertList(array $temp): void $this->list = $temp; } + + 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); + } + + public function constantArrayByRefSubElement(): void + { + $a = [ + [ + 'one' => 'one', + 'two' => 'two', + ], + [ + 'one' => 'one', + ], + ]; + + foreach ($a as &$testArray) { + $testArray['two'] = 'two'; + } + unset($testArray); + + assertType("array{array{one: 'one', two: 'two'}, array{one: 'one', two: 'two'}}", $a); + + $key = 'three'; + + foreach ($a as $offset => $testArray) { + $a[$offset][$key] = $key; + } + + assertType("array{array{one: 'one', two: 'two', three: 'three'}, array{one: 'one', two: 'two', three: 'three'}}", $a); + + foreach ($a as $testArray) { + assertType("array{one: 'one', two: 'two', three: 'three'}", $testArray); + $testArray['two']; + $testArray['three']; + } + } } diff --git a/tests/PHPStan/Analyser/nsrt/bug-1940-with-key.php b/tests/PHPStan/Analyser/nsrt/bug-1940-with-key.php deleted file mode 100644 index 0ba6d79cfc9..00000000000 --- a/tests/PHPStan/Analyser/nsrt/bug-1940-with-key.php +++ /dev/null @@ -1,59 +0,0 @@ - $a - */ -function byRefWithKeyModifySubElement(array $a): void -{ - foreach ($a as $k => &$testArray) { - $testArray['two'] = 'two'; - } - - assertType("list", $a); -} - -/** - * By-ref WITHOUT key, modifying sub-element. - * Parallel case to the reviewer's example but without key variable. - * @param list $a - */ -function byRefWithoutKeyModifySubElement(array $a): void -{ - foreach ($a as &$testArray) { - $testArray['two'] = 'two'; - } - - assertType("list", $a); -} - -/** - * By-ref with key, direct overwrite (already worked before this PR) - * @param array $arr - */ -function byRefWithKeyDirectOverwrite(array $arr): void -{ - foreach ($arr as $k => &$v) { - $v = 1; - } - - assertType('array', $arr); -} - -/** - * By-ref without key, direct overwrite (this PR's main fix) - * @param array $arr - */ -function byRefWithoutKeyDirectOverwrite(array $arr): void -{ - foreach ($arr as &$v) { - $v = 1; - } - - assertType('array', $arr); -} diff --git a/tests/PHPStan/Analyser/nsrt/bug-1940.php b/tests/PHPStan/Analyser/nsrt/bug-1940.php index 67a17e38fed..1e220e9a4cd 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-1940.php +++ b/tests/PHPStan/Analyser/nsrt/bug-1940.php @@ -83,7 +83,6 @@ function byRefWithoutKeyNonEmpty(array $arr): void } /** - * By-ref without key with break — should NOT rewrite since not all elements may be visited. * @param array $arr */ function byRefWithoutKeyBreak(array $arr): void @@ -99,7 +98,6 @@ function byRefWithoutKeyBreak(array $arr): void } /** - * By-ref without key with continue — should still rewrite since all elements are visited. * @param array $arr */ function byRefWithoutKeyContinue(array $arr): void @@ -115,7 +113,6 @@ function byRefWithoutKeyContinue(array $arr): void } /** - * By-ref without key with continue where value is overwritten in all branches. * @param array $arr */ function byRefWithoutKeyContinueBranches(array $arr): void @@ -132,7 +129,6 @@ function byRefWithoutKeyContinueBranches(array $arr): void } /** - * By-ref without key with continue where value is NOT overwritten in the continue branch. * @param array $arr */ function byRefWithoutKeyContinuePartial(array $arr): void @@ -152,9 +148,6 @@ class Foo /** @var array */ private array $prop; - /** - * By-ref without key on a property. - */ public function byRefWithoutKeyProperty(): void { foreach ($this->prop as &$v) { @@ -166,7 +159,6 @@ public function byRefWithoutKeyProperty(): void } /** - * By-ref without key with intval transformation. * @param array $arr */ function byRefWithoutKeyTransform(array $arr): void @@ -179,15 +171,61 @@ function byRefWithoutKeyTransform(array $arr): void } /** - * By-ref without key — value not overwritten at all. * @param array $arr */ function byRefWithoutKeyNoOverwrite(array $arr): void { foreach ($arr as &$v) { - // just read, don't write echo $v; } assertType('array', $arr); } + +/** + * @param list $a + */ +function byRefWithKeyModifySubElement(array $a): void +{ + foreach ($a as $k => &$testArray) { + $testArray['two'] = 'two'; + } + + assertType("list", $a); +} + +/** + * @param list $a + */ +function byRefWithoutKeyModifySubElement(array $a): void +{ + foreach ($a as &$testArray) { + $testArray['two'] = 'two'; + } + + assertType("list", $a); +} + +/** + * @param array $arr + */ +function byRefWithKeyDirectOverwrite(array $arr): void +{ + foreach ($arr as $k => &$v) { + $v = 1; + } + + assertType('array', $arr); +} + +/** + * @param array $arr + */ +function byRefWithoutKeyDirectOverwrite(array $arr): void +{ + foreach ($arr as &$v) { + $v = 1; + } + + assertType('array', $arr); +} From 2377075ad7cf7bbb394029adb7970f04f9e46824 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Tue, 28 Apr 2026 12:03:49 +0000 Subject: [PATCH 4/4] Simplify by-ref constant array handling via MutatingScope::enterForeach Remove the isConstantArray()->no() guard from the with-key IntertwinedVariableByReferenceWithExpr setup in enterForeach. The ArrayDimFetch-based intertwined expression correctly targets individual elements via the key variable, so it works for constant arrays (unlike the SetExistingOffsetValueTypeExpr path which updates all values at once and must remain guarded). This lets the intertwined mechanism automatically propagate by-ref type changes to the dim fetch for constant arrays, removing the need for the elseif ($stmt->byRef) fallback branches in the post-loop rewrite in NodeScopeResolver. Co-Authored-By: Claude Opus 4.6 --- src/Analyser/MutatingScope.php | 2 +- src/Analyser/NodeScopeResolver.php | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 79d4be017bf..6d4f467c468 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -2373,7 +2373,7 @@ public function enterForeach(self $originalScope, Expr $iteratee, string $valueN if ($keyName !== null) { $scope = $scope->enterForeachKey($originalScope, $iteratee, $keyName); - if ($valueByRef && $iterateeType->isArray()->yes() && $iterateeType->isConstantArray()->no()) { + if ($valueByRef && $iterateeType->isArray()->yes()) { $scope = $scope->assignExpression( new IntertwinedVariableByReferenceWithExpr($valueName, new Expr\ArrayDimFetch($iteratee, new Variable($keyName)), new Variable($valueName)), $valueType, diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 585192d6705..6abcd0fb7eb 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -1377,8 +1377,6 @@ public function processStmtNode( if ($dimFetchType->isSuperTypeOf($valueVarType)->yes()) { $dimFetchType = $valueVarType; } - } elseif ($stmt->byRef && $originalValueExpr !== null) { - $dimFetchType = $scopeWithIterableValueType->getType($stmt->valueVar); } $arrayDimFetchLoopTypes[] = $dimFetchType; $keyLoopTypes[] = $scopeWithIterableValueType->getType($stmt->keyVar); @@ -1396,8 +1394,6 @@ public function processStmtNode( if ($dimFetchNativeType->isSuperTypeOf($valueVarNativeType)->yes()) { $dimFetchNativeType = $valueVarNativeType; } - } elseif ($stmt->byRef && $originalValueExpr !== null) { - $dimFetchNativeType = $scopeWithIterableValueType->getNativeType($stmt->valueVar); } $arrayDimFetchLoopNativeTypes[] = $dimFetchNativeType; $keyLoopNativeTypes[] = $scopeWithIterableValueType->getType($stmt->keyVar);