From 39da6fcdec1bbe9145942c2c18102a6595eb2ee3 Mon Sep 17 00:00:00 2001 From: ondrejmirtes <104888+ondrejmirtes@users.noreply.github.com> Date: Mon, 15 Jun 2026 10:53:04 +0000 Subject: [PATCH 1/3] Narrow array element type after a keyless `foreach` whose body guards each element with an early exit - Generalize the post-loop array rewrite in NodeScopeResolver::processStmtNode() so it also fires for `foreach ($a as $v)` (no key variable). Previously the rewrite required `$stmt->keyVar !== null`, so type guards like `if (!is_string($v)) return;` never narrowed the iterated array. - Track iterations through the original value expression (OriginalForeachValueExpr) when no key variable is present, instead of the original key expression, and read the narrowed element type directly from the value variable rather than from an ArrayDimFetch (which requires a key). - Reassigning the value variable inside the loop still invalidates the tracking expression, so narrowing is correctly skipped in that case; loops that `break` out are still excluded. - This also covers the analogous keyless constructs that share the same code path: `instanceof` guards, `throw`, by-reference value variables, list vs non-list arrays, and `continue` (which correctly does not narrow). - Update tests/PHPStan/Analyser/nsrt/bug-7076.php (noKeyVar): the value type is now narrowed even without a key variable. - Remove a now-provably-dead defensive `instanceof ClassConstFetch` re-check in MatchHandler: the pre-validation loop already guarantees it via `break 2`, which the sharper inference now understands. --- src/Analyser/ExprHandler/MatchHandler.php | 5 +- src/Analyser/NodeScopeResolver.php | 94 ++++++++------ tests/PHPStan/Analyser/nsrt/bug-5755.php | 145 ++++++++++++++++++++++ tests/PHPStan/Analyser/nsrt/bug-7076.php | 4 +- 4 files changed, 205 insertions(+), 43 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-5755.php diff --git a/src/Analyser/ExprHandler/MatchHandler.php b/src/Analyser/ExprHandler/MatchHandler.php index c83532f62dc..90d4714a5af 100644 --- a/src/Analyser/ExprHandler/MatchHandler.php +++ b/src/Analyser/ExprHandler/MatchHandler.php @@ -271,9 +271,8 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $conditionCases = []; $conditionExprs = []; foreach ($arm->conds as $j => $cond) { - if (!$cond instanceof Expr\ClassConstFetch) { - throw new ShouldNotHappenException(); - } + // The pre-validation loop above already guaranteed (via break 2) + // that every reached condition is an enum-case ClassConstFetch. if (!$cond->class instanceof Name) { throw new ShouldNotHappenException(); } diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index ea7cfee5d17..3f63434d03f 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -1491,11 +1491,26 @@ public function processStmtNode( $finalScope = $finalScopeResult->getScope(); $scopesWithIterableValueType = []; + $keyVarExpr = null; $originalKeyVarExpr = null; - $continueExitPointHasUnoriginalKeyType = false; if ($stmt->keyVar instanceof Variable && is_string($stmt->keyVar->name)) { + $keyVarExpr = $stmt->keyVar; $originalKeyVarExpr = new OriginalForeachKeyExpr($stmt->keyVar->name); - if ($finalScope->hasExpressionType($originalKeyVarExpr)->yes()) { + } + $originalValueExpr = null; + if ($stmt->valueVar instanceof Variable && is_string($stmt->valueVar->name)) { + $originalValueExpr = new OriginalForeachValueExpr($stmt->valueVar->name); + } + + // With a key variable, each iteration is tracked through the original key + // expression and the narrowed element is projected onto the array dim fetch. + // Without one (`foreach ($a as $v)`) we instead track the original value + // expression and rewrite the array value type directly from the value var. + $trackingExpr = $originalKeyVarExpr ?? $originalValueExpr; + + $continueExitPointHasUnoriginalKeyType = false; + if ($trackingExpr !== null) { + if ($finalScope->hasExpressionType($trackingExpr)->yes()) { $scopesWithIterableValueType[] = $finalScope; } else { $continueExitPointHasUnoriginalKeyType = true; @@ -1505,7 +1520,7 @@ public function processStmtNode( foreach ($finalScopeResult->getExitPointsByType(Continue_::class) as $continueExitPoint) { $continueScope = $continueExitPoint->getScope(); $finalScope = $continueScope->mergeWith($finalScope); - if ($originalKeyVarExpr === null || !$continueScope->hasExpressionType($originalKeyVarExpr)->yes()) { + if ($trackingExpr === null || !$continueScope->hasExpressionType($trackingExpr)->yes()) { $continueExitPointHasUnoriginalKeyType = true; continue; } @@ -1526,58 +1541,59 @@ public function processStmtNode( count($breakExitPoints) === 0 && count($scopesWithIterableValueType) > 0 && !$continueExitPointHasUnoriginalKeyType - && $stmt->keyVar !== null + && ($keyVarExpr !== null || $originalValueExpr !== null) && (!$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; - } - } - $arrayDimFetchLoopTypes[] = $dimFetchType; - $keyLoopTypes[] = $scopeWithIterableValueType->getType($stmt->keyVar); - } - - $arrayDimFetchLoopType = TypeCombinator::union(...$arrayDimFetchLoopTypes); - $keyLoopType = TypeCombinator::union(...$keyLoopTypes); - $arrayDimFetchLoopNativeTypes = []; + $keyLoopTypes = []; $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; + if ($keyVarExpr !== null) { + $arrayExprDimFetch = new ArrayDimFetch($stmt->expr, $keyVarExpr); + $dimFetchType = $scopeWithIterableValueType->getType($arrayExprDimFetch); + $dimFetchNativeType = $scopeWithIterableValueType->getNativeType($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; + } + $valueVarNativeType = $scopeWithIterableValueType->getNativeType($stmt->valueVar); + if ($dimFetchNativeType->isSuperTypeOf($valueVarNativeType)->yes()) { + $dimFetchNativeType = $valueVarNativeType; + } } + $keyLoopTypes[] = $scopeWithIterableValueType->getType($keyVarExpr); + $keyLoopNativeTypes[] = $scopeWithIterableValueType->getType($keyVarExpr); + } else { + // No key variable: the narrowed value var is the array element type directly. + $dimFetchType = $scopeWithIterableValueType->getType($stmt->valueVar); + $dimFetchNativeType = $scopeWithIterableValueType->getNativeType($stmt->valueVar); } + $arrayDimFetchLoopTypes[] = $dimFetchType; $arrayDimFetchLoopNativeTypes[] = $dimFetchNativeType; - $keyLoopNativeTypes[] = $scopeWithIterableValueType->getType($stmt->keyVar); } + $arrayDimFetchLoopType = TypeCombinator::union(...$arrayDimFetchLoopTypes); $arrayDimFetchLoopNativeType = TypeCombinator::union(...$arrayDimFetchLoopNativeTypes); - $keyLoopNativeType = TypeCombinator::union(...$keyLoopNativeTypes); $valueTypeChanged = !$arrayDimFetchLoopType->equals($exprType->getIterableValueType()); - $keyTypeChanged = !$keyLoopType->equals($exprType->getIterableKeyType()); + $keyTypeChanged = false; + $keyLoopType = $exprType->getIterableKeyType(); + $keyLoopNativeType = $scope->getNativeType($stmt->expr)->getIterableKeyType(); + if ($keyVarExpr !== null) { + $keyLoopType = TypeCombinator::union(...$keyLoopTypes); + $keyLoopNativeType = TypeCombinator::union(...$keyLoopNativeTypes); + $keyTypeChanged = !$keyLoopType->equals($exprType->getIterableKeyType()); + } if ($valueTypeChanged || $keyTypeChanged) { $newExprType = $exprType; diff --git a/tests/PHPStan/Analyser/nsrt/bug-5755.php b/tests/PHPStan/Analyser/nsrt/bug-5755.php new file mode 100644 index 00000000000..d2ef1e2c7d2 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-5755.php @@ -0,0 +1,145 @@ + $ids + * @return list|null + */ +function validate(array $ids): array|null +{ + foreach ($ids as $id) { + if (!is_string($id)) { + return null; + } + } + + assertType('list', $ids); + + return $ids; +} + +class Test +{ + + /** + * @return array|float|int|string|false|null + */ + private function value(): mixed + { + $values = ['a', 1, false, null, 3.4, [], [2, 3, 4], ['a', 'b', 'c']]; + $index = rand(0, 7); + return $values[$index]; + } + + /** + * @return array|null + */ + public function strings(): ?array + { + $values = $this->value(); + if (!is_array($values)) { + return null; + } + foreach ($values as $value) { + if (!is_string($value)) { + return null; + } + } + + assertType('array', $values); + + return $values; + } + +} + +/** + * @param array $map + */ +function withGuardContinue(array $map): void +{ + foreach ($map as $value) { + if (is_int($value)) { + continue; + } + } + + // The continue keeps int around, no narrowing happens. + assertType('array', $map); +} + +/** + * @param array $map + */ +function narrowKeyless(array $map): void +{ + foreach ($map as $value) { + if (!is_string($value)) { + return; + } + } + + assertType('array', $map); +} + +/** + * @param list $list + */ +function reassignValueVarKeyless(array $list): void +{ + foreach ($list as $value) { + // Reassigning the value variable must not narrow the array element type. + $value = 'foo'; + } + + assertType('list', $list); +} + +interface Foo +{ +} + +/** + * @param list $list + */ +function instanceofKeyless(array $list): void +{ + foreach ($list as $value) { + if (!$value instanceof Foo) { + return; + } + } + + assertType('list', $list); +} + +/** + * @param list $list + */ +function throwKeyless(array $list): void +{ + foreach ($list as $value) { + if (!is_string($value)) { + throw new \Exception(); + } + } + + assertType('list', $list); +} + +/** + * @param list $list + */ +function byRefKeyless(array $list): void +{ + foreach ($list as &$value) { + if (!is_string($value)) { + return; + } + } + + assertType('list', $list); +} diff --git a/tests/PHPStan/Analyser/nsrt/bug-7076.php b/tests/PHPStan/Analyser/nsrt/bug-7076.php index c05ead15b45..de23368b94e 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-7076.php +++ b/tests/PHPStan/Analyser/nsrt/bug-7076.php @@ -123,7 +123,9 @@ function noKeyVar(array $arguments): void } } - assertType('array', $arguments); + // Even without a key variable, every element is guaranteed to be a string + // after the loop, so the value type is narrowed. + assertType('array', $arguments); } /** From 75a2297f93f829810559f9da7645ea2c76fc0b51 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Mon, 15 Jun 2026 20:07:09 +0000 Subject: [PATCH 2/3] Add lint comment for PHP 8.0 union types in bug-5755 fixture Co-Authored-By: Claude Opus 4.8 --- tests/PHPStan/Analyser/nsrt/bug-5755.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/PHPStan/Analyser/nsrt/bug-5755.php b/tests/PHPStan/Analyser/nsrt/bug-5755.php index d2ef1e2c7d2..44b06ba653a 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-5755.php +++ b/tests/PHPStan/Analyser/nsrt/bug-5755.php @@ -1,4 +1,6 @@ -= 8.0 + +declare(strict_types = 1); namespace Bug5755; From 1764782e4555b2461ad37e30acbf8a59e9dd4297 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Mon, 15 Jun 2026 21:14:13 +0000 Subject: [PATCH 3/3] Cover keyed foreach with destructured value in foreach-narrowing test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Infection flagged an escaped mutant on the foreach element-narrowing condition: the `$keyVarExpr !== null` operand of `($keyVarExpr !== null || $originalValueExpr !== null)` could be mutated without any test failing. The two operands only diverge when a key variable is present but the value is destructured (so the value var is not a simple `Variable` and `$originalValueExpr` is null) — a case no existing test exercised, since key narrowing then flows purely through the key path. Add a `foreach ($a as $k => [$v])` case that narrows the key via an early-exit guard, asserting the array is narrowed to `array`. The assertion fails when that operand is dropped, killing the mutant. Co-Authored-By: Claude Opus 4.8 --- tests/PHPStan/Analyser/nsrt/bug-5755.php | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/PHPStan/Analyser/nsrt/bug-5755.php b/tests/PHPStan/Analyser/nsrt/bug-5755.php index 44b06ba653a..a9a3c5de200 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-5755.php +++ b/tests/PHPStan/Analyser/nsrt/bug-5755.php @@ -145,3 +145,19 @@ function byRefKeyless(array $list): void assertType('list', $list); } + +/** + * @param array $a + */ +function keyedWithDestructuredValue(array $a): void +{ + // The value is destructured, so there is no value variable to track, but a key + // variable is still present. Key narrowing must keep working through the key path. + foreach ($a as $k => [$v]) { + if (!is_int($k)) { + return; + } + } + + assertType('array', $a); +}