From 98cf40b17174029a7e48889ece5a6d4d2a0f4500 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Fri, 7 Nov 2025 20:45:02 +0100 Subject: [PATCH] Understand always-overwritten arrays in foreach --- src/Analyser/MutatingScope.php | 15 +- src/Analyser/NodeScopeResolver.php | 85 +++++++++++- src/Node/Expr/OriginalForeachKeyExpr.php | 37 +++++ src/Node/Printer/Printer.php | 6 + tests/PHPStan/Analyser/nsrt/bug-12274.php | 2 +- tests/PHPStan/Analyser/nsrt/bug-13730.php | 24 ++++ tests/PHPStan/Analyser/nsrt/bug-2273.php | 22 +++ .../Analyser/nsrt/overwritten-arrays.php | 131 ++++++++++++++++++ 8 files changed, 317 insertions(+), 5 deletions(-) create mode 100644 src/Node/Expr/OriginalForeachKeyExpr.php create mode 100644 tests/PHPStan/Analyser/nsrt/bug-13730.php create mode 100644 tests/PHPStan/Analyser/nsrt/bug-2273.php create mode 100644 tests/PHPStan/Analyser/nsrt/overwritten-arrays.php diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 65a2969f90..5aa8f26bc5 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -34,6 +34,7 @@ use PHPStan\Node\Expr\GetIterableValueTypeExpr; use PHPStan\Node\Expr\GetOffsetValueTypeExpr; use PHPStan\Node\Expr\NativeTypeExpr; +use PHPStan\Node\Expr\OriginalForeachKeyExpr; use PHPStan\Node\Expr\OriginalPropertyTypeExpr; use PHPStan\Node\Expr\ParameterVariableOriginalValueExpr; use PHPStan\Node\Expr\PropertyInitializationExpr; @@ -3949,13 +3950,19 @@ public function enterForeachKey(self $originalScope, Expr $iteratee, string $key { $iterateeType = $originalScope->getType($iteratee); $nativeIterateeType = $originalScope->getNativeType($iteratee); + + $keyType = $originalScope->getIterableKeyType($iterateeType); + $nativeKeyType = $originalScope->getIterableKeyType($nativeIterateeType); + $scope = $this->assignVariable( $keyName, - $originalScope->getIterableKeyType($iterateeType), - $originalScope->getIterableKeyType($nativeIterateeType), + $keyType, + $nativeKeyType, TrinaryLogic::createYes(), ); + $originalForeachKeyExpr = new OriginalForeachKeyExpr($keyName); + $scope = $scope->assignExpression($originalForeachKeyExpr, $keyType, $nativeKeyType); if ($iterateeType->isArray()->yes()) { $scope = $scope->assignExpression( new Expr\ArrayDimFetch($iteratee, new Variable($keyName)), @@ -4139,6 +4146,10 @@ public function assignVariable(string $variableName, Type $type, Type $nativeTyp unset($scope->expressionTypes[$parameterOriginalValueExprString]); unset($scope->nativeExpressionTypes[$parameterOriginalValueExprString]); + $originalForeachKeyExpr = $this->getNodeKey(new OriginalForeachKeyExpr($variableName)); + unset($scope->expressionTypes[$originalForeachKeyExpr]); + unset($scope->nativeExpressionTypes[$originalForeachKeyExpr]); + return $scope; } diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index f37a7fae1d..fbef630d61 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -91,6 +91,7 @@ use PHPStan\Node\Expr\GetIterableValueTypeExpr; use PHPStan\Node\Expr\GetOffsetValueTypeExpr; use PHPStan\Node\Expr\NativeTypeExpr; +use PHPStan\Node\Expr\OriginalForeachKeyExpr; use PHPStan\Node\Expr\OriginalPropertyTypeExpr; use PHPStan\Node\Expr\PropertyInitializationExpr; use PHPStan\Node\Expr\SetExistingOffsetValueTypeExpr; @@ -1247,14 +1248,94 @@ private function processStmtNode( $bodyScope = $this->enterForeach($bodyScope, $originalScope, $stmt, $nodeCallback); $finalScopeResult = $this->processStmtNodes($stmt, $stmt->stmts, $bodyScope, $nodeCallback, $context)->filterOutLoopExitPoints(); $finalScope = $finalScopeResult->getScope(); + $scopesWithIterableValueType = []; + + $originalKeyVarExpr = null; + $continueExitPointHasUnoriginalKeyType = false; + 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; + } + } + foreach ($finalScopeResult->getExitPointsByType(Continue_::class) as $continueExitPoint) { - $finalScope = $continueExitPoint->getScope()->mergeWith($finalScope); + $continueScope = $continueExitPoint->getScope(); + $finalScope = $continueScope->mergeWith($finalScope); + if ($originalKeyVarExpr === null || !$continueScope->hasExpressionType($originalKeyVarExpr)->yes()) { + $continueExitPointHasUnoriginalKeyType = true; + continue; + } + $scopesWithIterableValueType[] = $continueScope; } - foreach ($finalScopeResult->getExitPointsByType(Break_::class) as $breakExitPoint) { + $breakExitPoints = $finalScopeResult->getExitPointsByType(Break_::class); + foreach ($breakExitPoints as $breakExitPoint) { $finalScope = $breakExitPoint->getScope()->mergeWith($finalScope); } $exprType = $scope->getType($stmt->expr); + $hasExpr = $scope->hasExpressionType($stmt->expr); + if ( + count($breakExitPoints) === 0 + && count($scopesWithIterableValueType) > 0 + && !$continueExitPointHasUnoriginalKeyType + && $stmt->keyVar !== null + && $exprType->isArray()->yes() + && $exprType->isConstantArray()->no() + && !$hasExpr->no() + ) { + $arrayExprDimFetch = new ArrayDimFetch($stmt->expr, $stmt->keyVar); + $arrayDimFetchLoopTypes = []; + foreach ($scopesWithIterableValueType as $scopeWithIterableValueType) { + $arrayDimFetchLoopTypes[] = $scopeWithIterableValueType->getType($arrayExprDimFetch); + } + + $arrayDimFetchLoopType = TypeCombinator::union(...$arrayDimFetchLoopTypes); + + $arrayDimFetchLoopNativeTypes = []; + foreach ($scopesWithIterableValueType as $scopeWithIterableValueType) { + $arrayDimFetchLoopNativeTypes[] = $scopeWithIterableValueType->getNativeType($arrayExprDimFetch); + } + + $arrayDimFetchLoopNativeType = TypeCombinator::union(...$arrayDimFetchLoopNativeTypes); + + if (!$arrayDimFetchLoopType->equals($exprType->getIterableValueType())) { + $newExprType = TypeTraverser::map($exprType, static function (Type $type, callable $traverse) use ($arrayDimFetchLoopType): Type { + if ($type instanceof UnionType || $type instanceof IntersectionType) { + return $traverse($type); + } + + if (!$type instanceof ArrayType) { + return $type; + } + + return new ArrayType($type->getKeyType(), $arrayDimFetchLoopType); + }); + $newExprNativeType = TypeTraverser::map($scope->getNativeType($stmt->expr), static function (Type $type, callable $traverse) use ($arrayDimFetchLoopNativeType): Type { + if ($type instanceof UnionType || $type instanceof IntersectionType) { + return $traverse($type); + } + + if (!$type instanceof ArrayType) { + return $type; + } + + return new ArrayType($type->getKeyType(), $arrayDimFetchLoopNativeType); + }); + + if ($stmt->expr instanceof Variable && is_string($stmt->expr->name)) { + $finalScope = $finalScope->assignVariable( + $stmt->expr->name, + $newExprType, + $newExprNativeType, + $hasExpr, + ); + } + } + } + $isIterableAtLeastOnce = $exprType->isIterableAtLeastOnce(); if ($exprType->isIterable()->no() || $isIterableAtLeastOnce->maybe()) { $finalScope = $finalScope->mergeWith($scope->filterByTruthyValue(new BooleanOr( diff --git a/src/Node/Expr/OriginalForeachKeyExpr.php b/src/Node/Expr/OriginalForeachKeyExpr.php new file mode 100644 index 0000000000..44d135d2d8 --- /dev/null +++ b/src/Node/Expr/OriginalForeachKeyExpr.php @@ -0,0 +1,37 @@ +variableName; + } + + #[Override] + public function getType(): string + { + return 'PHPStan_Node_OriginalForeachKeyExpr'; + } + + /** + * @return string[] + */ + #[Override] + public function getSubNodeNames(): array + { + return []; + } + +} diff --git a/src/Node/Printer/Printer.php b/src/Node/Printer/Printer.php index df71b09035..2a9da55e28 100644 --- a/src/Node/Printer/Printer.php +++ b/src/Node/Printer/Printer.php @@ -11,6 +11,7 @@ use PHPStan\Node\Expr\GetIterableValueTypeExpr; use PHPStan\Node\Expr\GetOffsetValueTypeExpr; use PHPStan\Node\Expr\NativeTypeExpr; +use PHPStan\Node\Expr\OriginalForeachKeyExpr; use PHPStan\Node\Expr\OriginalPropertyTypeExpr; use PHPStan\Node\Expr\ParameterVariableOriginalValueExpr; use PHPStan\Node\Expr\PropertyInitializationExpr; @@ -99,6 +100,11 @@ protected function pPHPStan_Node_ParameterVariableOriginalValueExpr(ParameterVar return sprintf('__phpstanParameterVariableOriginalValue(%s)', $expr->getVariableName()); } + protected function pPHPStan_Node_OriginalForeachKeyExpr(OriginalForeachKeyExpr $expr): string // phpcs:ignore + { + return sprintf('__phpstanOriginalForeachKey(%s)', $expr->getVariableName()); + } + protected function pPHPStan_Node_IssetExpr(IssetExpr $expr): string // phpcs:ignore { return sprintf('__phpstanIssetExpr(%s)', $this->p($expr->getExpr())); diff --git a/tests/PHPStan/Analyser/nsrt/bug-12274.php b/tests/PHPStan/Analyser/nsrt/bug-12274.php index b17b11aed8..d4ad2e302d 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-12274.php +++ b/tests/PHPStan/Analyser/nsrt/bug-12274.php @@ -15,7 +15,7 @@ function getItems(array $items): array $items[$index] = 1; } - assertType('non-empty-list', $items); + assertType('non-empty-list<1>', $items); return $items; } diff --git a/tests/PHPStan/Analyser/nsrt/bug-13730.php b/tests/PHPStan/Analyser/nsrt/bug-13730.php new file mode 100644 index 0000000000..38faa4b370 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-13730.php @@ -0,0 +1,24 @@ + $arr + * @return array + */ + public function sayHello(array $arr): array + { + foreach($arr as $k => $v) { + $arr[$k] = $v ?? ''; + } + + assertType('array', $arr); + + return $arr; + } +} diff --git a/tests/PHPStan/Analyser/nsrt/bug-2273.php b/tests/PHPStan/Analyser/nsrt/bug-2273.php new file mode 100644 index 0000000000..2a900b7fe7 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-2273.php @@ -0,0 +1,22 @@ + $v) { + $x[$k] = \realpath($v); + if ($x[$k] === false) { + throw new \Exception(); + } + } + + assertType('array', $x); + } +} diff --git a/tests/PHPStan/Analyser/nsrt/overwritten-arrays.php b/tests/PHPStan/Analyser/nsrt/overwritten-arrays.php new file mode 100644 index 0000000000..5ba65bcfa9 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/overwritten-arrays.php @@ -0,0 +1,131 @@ + $a + */ + public function doFoo(array $a): void + { + foreach ($a as $k => $v) { + $a[$k] = 1; + } + + assertType('array', $a); + } + + /** + * @param array $a + */ + public function doFoo2(array $a): void + { + foreach ($a as $k => $v) { + if (rand(0, 1)) { + $a[$k] = 2; + continue; + } + $a[$k] = 1; + } + + assertType('array', $a); + } + + /** + * @param array $a + */ + public function doFoo3(array $a): void + { + foreach ($a as $k => $v) { + if (rand(0, 1)) { + break; + } + if (rand(0, 1)) { + $a[$k] = 2; + continue; + } + $a[$k] = 1; + } + + assertType('array', $a); + } + + /** + * @param array $a + */ + public function doFoo4(array $a): void + { + foreach ($a as $k => $v) { + $k++; + $a[$k] = 1; + } + + assertType('array', $a); + } + + /** + * @param array $a + */ + public function doFoo5(array $a): void + { + foreach ($a as $k => $v) { + if (rand(0, 1)) { + $k++; + $a[$k] = 2; + continue; + } + $a[$k] = 1; + } + + assertType('array', $a); + } + + /** + * @param array $a + */ + public function doFoo6(array $a): void + { + foreach ($a as $k => $v) { + if (rand(0, 1)) { + $a[$k] = 2; + continue; + } + $k++; + $a[$k] = 1; + } + + assertType('array', $a); + } + + /** + * @param array $a + */ + public function doFoo7(array $a): void + { + foreach ($a as &$v) { + $v = 1; + } + + assertType('array', $a); + } + + /** + * @param array $a + */ + public function doFoo8(array $a): void + { + foreach ($a as &$v) { + if (rand(0, 1)) { + $v = 1; + } + } + + assertType('array', $a); + } + +}