diff --git a/src/Analyser/ExprHandler/MatchHandler.php b/src/Analyser/ExprHandler/MatchHandler.php index c83532f62d..90d4714a5a 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 ea7cfee5d1..3f63434d03 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 0000000000..d2ef1e2c7d --- /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 c05ead15b4..de23368b94 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); } /**