From ed4ebedcd76432bdc919045ba0546692bf9a4397 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 540119391e7..09e1f930684 100644 --- a/src/Analyser/ExprHandler/AssignHandler.php +++ b/src/Analyser/ExprHandler/AssignHandler.php @@ -540,6 +540,7 @@ public function processAssignVar( $var === $originalVar && $var->dim !== null && $scope->hasExpressionType($var)->yes() + && !$scope->isExpressionTrackingOnly($var) ) { $assignedPropertyExpr = new SetExistingOffsetValueTypeExpr( $varForSetOffsetValue, @@ -1408,7 +1409,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; @@ -1426,13 +1427,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 { @@ -1469,6 +1470,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 c81d35e98fd..db86e0a1ec8 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -980,6 +980,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(); } @@ -1310,6 +1311,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 */ @@ -4205,12 +4215,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; @@ -4227,6 +4260,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 3129028a6f1..27f205c390b 100644 --- a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php +++ b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php @@ -1348,6 +1348,13 @@ public function testArrayFindKeyExisting(): void ]); } + public function testBug14598(): void + { + $this->reportPossiblyNonexistentConstantArrayOffset = true; + + $this->analyse([__DIR__ . '/data/bug-14598.php'], []); + } + public function testBug13688(): void { $this->analyse([__DIR__ . '/data/bug-13688.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]); + } +}