From b3f4336b81f9535e69fdc92d3ec4bd3f876c0a65 Mon Sep 17 00:00:00 2001 From: phpstan-bot <79867460+phpstan-bot@users.noreply.github.com> Date: Thu, 14 May 2026 09:23:40 +0000 Subject: [PATCH] Preserve ArrayDimFetch expression tracking through loop generalization When a parent expression is generalized during loop fixpoint iteration, child ArrayDimFetch expressions are now preserved with a trackingOnly flag instead of being removed. This prevents false positive "nonexistent offset" errors on array dim fetches that were explicitly assigned earlier in the same code path. The trackingOnly flag decouples expression tracking (hasExpressionType returns Yes, so the NonexistentOffsetInArrayDimFetchRule skips the check) from type resolution (resolveType falls through to the ExprHandler to compute the type fresh from the parent, avoiding stale stored types). Closes https://github.com/phpstan/phpstan/issues/14598 --- src/Analyser/ExprHandler/AssignHandler.php | 8 ++- src/Analyser/ExpressionTypeHolder.php | 21 +++++- src/Analyser/MutatingScope.php | 36 ++++++++++- ...nexistentOffsetInArrayDimFetchRuleTest.php | 7 ++ tests/PHPStan/Rules/Arrays/data/bug-14598.php | 64 +++++++++++++++++++ 5 files changed, 129 insertions(+), 7 deletions(-) create mode 100644 tests/PHPStan/Rules/Arrays/data/bug-14598.php diff --git a/src/Analyser/ExprHandler/AssignHandler.php b/src/Analyser/ExprHandler/AssignHandler.php index 5d2a084ec04..a6e1401ce7b 100644 --- a/src/Analyser/ExprHandler/AssignHandler.php +++ b/src/Analyser/ExprHandler/AssignHandler.php @@ -347,6 +347,7 @@ public function processAssignVar( $var === $originalVar && $var->dim !== null && $scope->hasExpressionType($var)->yes() + && !$scope->isExpressionTrackingOnly($var) ) { $assignedPropertyExpr = new SetExistingOffsetValueTypeExpr( $varForSetOffsetValue, @@ -1067,7 +1068,7 @@ private function processArrayByRefItems(MutatingScope $scope, string $rootVarNam * * @return array{Type, list} */ - private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, array $offsetTypes, Type $offsetValueType, Type $valueToWrite, Scope $scope): array + private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, array $offsetTypes, Type $offsetValueType, Type $valueToWrite, MutatingScope $scope): array { $originalValueToWrite = $valueToWrite; @@ -1080,13 +1081,13 @@ private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, ar } else { $has = $offsetValueType->hasOffsetValueType($offsetType); if ($has->yes()) { - if ($scope->hasExpressionType($dimFetch)->yes()) { + if ($scope->hasExpressionType($dimFetch)->yes() && !$scope->isExpressionTrackingOnly($dimFetch)) { $offsetValueType = $scope->getType($dimFetch); } else { $offsetValueType = $offsetValueType->getOffsetValueType($offsetType); } } elseif ($has->maybe()) { - if ($scope->hasExpressionType($dimFetch)->yes()) { + if ($scope->hasExpressionType($dimFetch)->yes() && !$scope->isExpressionTrackingOnly($dimFetch)) { $generalizeOnWrite = false; $offsetValueType = $scope->getType($dimFetch); } else { @@ -1122,6 +1123,7 @@ private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, ar $offsetType !== null && $arrayDimFetch !== null && $scope->hasExpressionType($arrayDimFetch)->yes() + && !$scope->isExpressionTrackingOnly($arrayDimFetch) && !$offsetValueType->hasOffsetValueType($offsetType)->no() ) { $hasOffsetType = null; diff --git a/src/Analyser/ExpressionTypeHolder.php b/src/Analyser/ExpressionTypeHolder.php index 7477dbf3dcd..9d44db8621a 100644 --- a/src/Analyser/ExpressionTypeHolder.php +++ b/src/Analyser/ExpressionTypeHolder.php @@ -14,6 +14,7 @@ public function __construct( private readonly Expr $expr, private readonly Type $type, private readonly TrinaryLogic $certainty, + private readonly bool $trackingOnly = false, ) { } @@ -52,22 +53,31 @@ public function equals(self $other): bool public function and(self $other): self { + $newTrackingOnly = $this->trackingOnly || $other->trackingOnly; + if ($this->type === $other->type || $this->type->equals($other->type)) { - if ($this->certainty->and($other->certainty)->yes()) { + $newCertainty = $this->certainty->and($other->certainty); + if ($newCertainty->yes() && !$newTrackingOnly) { return $this; } - if ($this->certainty->maybe()) { + if ($this->certainty->maybe() && $this->trackingOnly === $newTrackingOnly) { return $this; } - return $other; + return new self( + $this->expr, + $this->type, + $newCertainty, + $newTrackingOnly, + ); } return new self( $this->expr, TypeCombinator::union($this->type, $other->type), $this->certainty->and($other->certainty), + $newTrackingOnly, ); } @@ -86,4 +96,9 @@ public function getCertainty(): TrinaryLogic return $this->certainty; } + public function isTrackingOnly(): bool + { + return $this->trackingOnly; + } + } diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 12cf17a9bf4..ef86d9dbc93 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -972,6 +972,7 @@ private function resolveType(string $exprString, Expr $node): Type && !$node instanceof Expr\Closure && !$node instanceof Expr\ArrowFunction && $this->hasExpressionType($node)->yes() + && !$this->expressionTypes[$exprString]->isTrackingOnly() ) { return $this->expressionTypes[$exprString]->getType(); } @@ -1302,6 +1303,15 @@ public function hasExpressionType(Expr $node): TrinaryLogic return $this->expressionTypes[$exprString]->getCertainty(); } + public function isExpressionTrackingOnly(Expr $node): bool + { + $exprString = $this->getNodeKey($node); + if (!isset($this->expressionTypes[$exprString])) { + return false; + } + return $this->expressionTypes[$exprString]->isTrackingOnly(); + } + /** * @param MethodReflection|FunctionReflection|null $reflection */ @@ -4087,12 +4097,35 @@ private function generalizeVariableTypeHolders( $generalizedExpressions = []; $newVariableTypeHolders = []; foreach ($variableTypeHolders as $variableExprString => $variableTypeHolder) { + $invalidatedByGeneralization = false; foreach ($generalizedExpressions as $generalizedExprString => $generalizedExpr) { if (!$this->shouldInvalidateExpression($generalizedExprString, $generalizedExpr, $variableTypeHolder->getExpr(), $variableExprString)) { continue; } - continue 2; + $invalidatedByGeneralization = true; + break; + } + + if ($invalidatedByGeneralization) { + $expr = $variableTypeHolder->getExpr(); + if ( + $expr instanceof Expr\ArrayDimFetch + && $expr->dim !== null + && $variableTypeHolder->getCertainty()->yes() + && isset($otherVariableTypeHolders[$variableExprString]) + && $otherVariableTypeHolders[$variableExprString]->getCertainty()->yes() + ) { + $newVariableTypeHolders[$variableExprString] = new ExpressionTypeHolder( + $expr, + $variableTypeHolder->getType(), + $variableTypeHolder->getCertainty(), + true, + ); + continue; + } + + continue; } if (!isset($otherVariableTypeHolders[$variableExprString])) { $newVariableTypeHolders[$variableExprString] = $variableTypeHolder; @@ -4109,6 +4142,7 @@ private function generalizeVariableTypeHolders( $variableTypeHolder->getExpr(), $generalizedType, $variableTypeHolder->getCertainty(), + $variableTypeHolder->isTrackingOnly(), ); } diff --git a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php index 024f71bbcb9..8d0f7c4db3e 100644 --- a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php +++ b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php @@ -1343,4 +1343,11 @@ public function testArrayFindKeyExisting(): void ]); } + public function testBug14598(): void + { + $this->reportPossiblyNonexistentConstantArrayOffset = true; + + $this->analyse([__DIR__ . '/data/bug-14598.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Arrays/data/bug-14598.php b/tests/PHPStan/Rules/Arrays/data/bug-14598.php new file mode 100644 index 00000000000..0e7f9b01542 --- /dev/null +++ b/tests/PHPStan/Rules/Arrays/data/bug-14598.php @@ -0,0 +1,64 @@ +, 'off'|'on'>> $raw + * @return array<'A'|'B'|'C', string> + */ +function buildData(array $raw): array +{ + $return = []; + foreach ($raw as $id => $sensors) { + $tmp[$id] = []; + $last = "off"; + foreach ($sensors as $i => $stat) { + if ($last !== $stat) { + $tmp[$id][] = sprintf("%02d", $i); + $last = $stat; + } + } + $return[$id] = count($tmp[$id]) + ? implode(",", $tmp[$id]) + : "invalid"; + } + + return $return; +} + +/** + * @param array<'A'|'B'|'C', array> $raw + */ +function simpleNestedForeach(array $raw): void +{ + $tmp = []; + foreach ($raw as $id => $sensors) { + $tmp[$id] = []; + foreach ($sensors as $i => $stat) { + if ($i > 0) { + $tmp[$id][] = $stat; + } + } + echo count($tmp[$id]); + } +} + +/** + * @param list<'A'|'B'|'C'> $keys + * @param array $values + */ +function nestedWhileLoop(array $keys, array $values): void +{ + $tmp = []; + foreach ($keys as $id) { + $tmp[$id] = []; + $i = 0; + while ($i < count($values)) { + if ($values[$i] > 0) { + $tmp[$id][] = $values[$i]; + } + $i++; + } + echo count($tmp[$id]); + } +}