From 1635b5b549f2eec189ef108159c9f4150c08202e Mon Sep 17 00:00:00 2001 From: phpstan-bot <79867460+phpstan-bot@users.noreply.github.com> Date: Mon, 17 Aug 2026 07:29:02 +0000 Subject: [PATCH 1/3] Keep list type when writing to `$list[count($list)]` and compare the offset's array through its printed expression - `AssignHandler::shouldKeepList()` now recognizes `$list[count($list)]` / `$list[sizeof($list)]` as a write right behind the last element, so the list type survives the assignment - Replaced `isSameVariable()` with `isSameArrayExpr()`, which compares side-effect-free expressions (variables, property fetches, static property fetches and dim fetches over them) through `ExprPrinter`. All existing heuristics - `count($list) - n`, `array_key_last()`/`array_key_first()`, `array_search()` - now work on `$this->list`, `self::$list` and `$data['x']` too, not just on plain variables - Added `$list[array_key_last($list) + 1]` as another append idiom, guarded by non-emptiness because `array_key_last()` returns `null` on an empty list - Function arguments are now read through a helper that rejects unpacked and named args, so `count(...$list)` no longer looks like `count($list)` - Fixed a crash: `$list[array_search($list)]` (too few arguments) read `getArgs()[1]` unconditionally and blew up with a `TypeError` - `shouldKeepList()` is now handed the dim fetch the written offset actually belongs to; the reversed loop indexed `$dimFetchStack` the other way round, which broke every heuristic for nested writes like `$data['x'][count($data['x'])]` - Additional expression types are no longer registered when the offset expression resolves differently after the write. `$list = [1, 2, 3]; $list[count($list)] = 37;` re-evaluated `count($list)` against the already-updated array and intersected the result with `hasOffsetValue(4, 37)`, collapsing the whole array to `*NEVER*` --- src/Analyser/ExprHandler/AssignHandler.php | 178 ++++++++++++----- tests/PHPStan/Analyser/nsrt/bug-15080.php | 186 ++++++++++++++++++ .../Rules/Functions/ReturnTypeRuleTest.php | 7 + .../Rules/Functions/data/bug-15080.php | 12 ++ 4 files changed, 338 insertions(+), 45 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-15080.php create mode 100644 tests/PHPStan/Rules/Functions/data/bug-15080.php diff --git a/src/Analyser/ExprHandler/AssignHandler.php b/src/Analyser/ExprHandler/AssignHandler.php index c0dade166a0..c9ed5c5040c 100644 --- a/src/Analyser/ExprHandler/AssignHandler.php +++ b/src/Analyser/ExprHandler/AssignHandler.php @@ -1067,7 +1067,17 @@ public function applyWrite( } foreach ($additionalExpressions as $k => $additionalExpression) { - [$expr, $type] = $additionalExpression; + [$expr, $type, $offsetType] = $additionalExpression; + if ( + $offsetType !== null + && $expr->dim !== null + && !$scope->getType($expr->dim)->equals($offsetType) + ) { + // the offset expression - e.g. count($list) - no longer points at the + // offset that was just written now that the container has changed + continue; + } + $nativeType = $type; if (isset($additionalNativeExpressions[$k])) { [, $nativeType] = $additionalNativeExpressions[$k]; @@ -1731,7 +1741,7 @@ private function processArrayByRefItems(MutatingScope $scope, string $rootVarNam * @param non-empty-list $dimFetchStack * @param non-empty-list $offsetTypes * - * @return array{Type, list} + * @return array{Type, list} */ private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, array $offsetTypes, Type $offsetValueType, Type $valueToWrite, Scope $scope): array { @@ -1775,7 +1785,7 @@ private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, ar $lastDimKey = array_key_last($dimFetchStack); $computedContainerValues = []; - foreach (array_reverse($offsetTypes) as $i => [$offsetType]) { + foreach (array_reverse($offsetTypes) as $i => [$offsetType, $writtenDimFetch]) { /** @var Type $offsetValueType */ $offsetValueType = array_pop($offsetValueTypeStack); if ( @@ -1837,7 +1847,9 @@ private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, ar $valueToWrite = $offsetValueType->setOffsetValueType($offsetType, $valueToWrite, $unionValues); } - if ($arrayDimFetch !== null && $offsetValueType->isList()->yes() && $this->shouldKeepList($arrayDimFetch, $scope, $offsetValueType)) { + // $writtenDimFetch is the dim fetch $offsetType belongs to - unlike + // $dimFetchStack[$i] above, which this reversed loop indexes the other way round + if ($offsetValueType->isList()->yes() && $this->shouldKeepList($writtenDimFetch, $scope, $offsetValueType)) { $valueToWrite = TypeCombinator::intersect($valueToWrite, new AccessoryArrayListType()); } @@ -1864,7 +1876,7 @@ private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, ar $additionalValueType = $valueToWrite->getOffsetValueType($offsetType); } - $additionalExpressions[] = [$dimFetch, $additionalValueType]; + $additionalExpressions[] = [$dimFetch, $additionalValueType, $offsetTypes[$key][0] ?? null]; } return [$valueToWrite, $additionalExpressions]; @@ -1872,59 +1884,135 @@ private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, ar private function shouldKeepList(ArrayDimFetch $arrayDimFetch, Scope $scope, Type $offsetValueType): bool { - if ($arrayDimFetch->dim instanceof Expr\BinaryOp\Plus) { - if ( // keep list for $list[$index + 1] assignments - $arrayDimFetch->dim->right instanceof Variable - && $arrayDimFetch->dim->left instanceof Node\Scalar\Int_ - && $arrayDimFetch->dim->left->value === 1 - && $scope->hasExpressionType(new ArrayDimFetch($arrayDimFetch->var, $arrayDimFetch->dim->right))->yes() - ) { - return true; - } elseif ( // keep list for $list[1 + $index] assignments - $arrayDimFetch->dim->left instanceof Variable - && $arrayDimFetch->dim->right instanceof Node\Scalar\Int_ - && $arrayDimFetch->dim->right->value === 1 - && $scope->hasExpressionType(new ArrayDimFetch($arrayDimFetch->var, $arrayDimFetch->dim->left))->yes() + $dim = $arrayDimFetch->dim; + if ($dim === null) { + return false; + } + + $array = $arrayDimFetch->var; + + if ($dim instanceof Expr\BinaryOp\Plus) { + [$plusOperand, $plusIncrement] = $dim->left instanceof Node\Scalar\Int_ + ? [$dim->right, $dim->left] + : [$dim->left, $dim->right]; + + if (!$plusIncrement instanceof Node\Scalar\Int_ || $plusIncrement->value !== 1) { + return false; + } + + if ( // keep list for $list[$index + 1] and $list[1 + $index] assignments + $plusOperand instanceof Variable + && $scope->hasExpressionType(new ArrayDimFetch($array, $plusOperand))->yes() ) { return true; } - } elseif ( // keep list for $list[count($list) - n] assignments - $arrayDimFetch->dim instanceof Expr\BinaryOp\Minus - && $arrayDimFetch->dim->right instanceof Node\Scalar\Int_ - && $arrayDimFetch->dim->left instanceof Expr\FuncCall - && $arrayDimFetch->dim->left->name instanceof Name - && in_array($arrayDimFetch->dim->left->name->toLowerString(), ['count', 'sizeof'], true) - && count($arrayDimFetch->dim->left->getArgs()) === 1 // could support COUNT_RECURSIVE, COUNT_NORMAL - && $this->isSameVariable($arrayDimFetch->var, $arrayDimFetch->dim->left->getArgs()[0]->value) - && IntegerRangeType::fromInterval(0, null)->isSuperTypeOf($scope->getType($arrayDimFetch->dim))->yes() - && $offsetValueType->isIterableAtLeastOnce()->yes() + + // keep list for $list[array_key_last($list) + 1] assignments; + // on an empty list array_key_last() returns null, so 0 + 1 would leave a hole + return $this->isFuncCallOnSameArray($plusOperand, ['array_key_last'], 0, $array) + && $offsetValueType->isIterableAtLeastOnce()->yes(); + } + + if ( // keep list for $list[count($list)] assignments - writes right behind the last element + $this->isCountOfSameArray($dim, $array) ) { return true; - } elseif ( // keep list for $list[array_key_last($list)] and $list[array_key_first($list)] assignments - $arrayDimFetch->dim instanceof Expr\FuncCall - && $arrayDimFetch->dim->name instanceof Name - && in_array($arrayDimFetch->dim->name->toLowerString(), ['array_key_last', 'array_key_first'], true) - && count($arrayDimFetch->dim->getArgs()) >= 1 - && $this->isSameVariable($arrayDimFetch->var, $arrayDimFetch->dim->getArgs()[0]->value) + } + + if ( // keep list for $list[count($list) - n] assignments + $dim instanceof Expr\BinaryOp\Minus + && $dim->right instanceof Node\Scalar\Int_ + && $this->isCountOfSameArray($dim->left, $array) + && IntegerRangeType::fromInterval(0, null)->isSuperTypeOf($scope->getType($dim))->yes() + && ($offsetValueType->isIterableAtLeastOnce()->yes() || $dim->right->value === 0) ) { return true; - } elseif ( // keep list for $list[array_search($needle, $list)] assignments - $arrayDimFetch->dim instanceof Expr\FuncCall - && $arrayDimFetch->dim->name instanceof Name - && $arrayDimFetch->dim->name->toLowerString() === 'array_search' - && count($arrayDimFetch->dim->getArgs()) >= 1 - && $this->isSameVariable($arrayDimFetch->var, $arrayDimFetch->dim->getArgs()[1]->value) - ) { + } + + // keep list for $list[array_key_last($list)] and $list[array_key_first($list)] assignments + if ($this->isFuncCallOnSameArray($dim, ['array_key_last', 'array_key_first'], 0, $array)) { return true; } - return false; + // keep list for $list[array_search($needle, $list)] assignments + return $this->isFuncCallOnSameArray($dim, ['array_search'], 1, $array); + } + + private function isCountOfSameArray(Expr $expr, Expr $array): bool + { + // a second argument could be COUNT_RECURSIVE, which no longer describes the offset behind the last element + return $expr instanceof Expr\FuncCall + && count($expr->getArgs()) === 1 + && $this->isFuncCallOnSameArray($expr, ['count', 'sizeof'], 0, $array); + } + + /** + * @param non-empty-list $functionNames + */ + private function isFuncCallOnSameArray(Expr $expr, array $functionNames, int $argPosition, Expr $array): bool + { + if (!$expr instanceof Expr\FuncCall || !$expr->name instanceof Name) { + return false; + } + + if (!in_array($expr->name->toLowerString(), $functionNames, true)) { + return false; + } + + $args = $expr->getArgs(); + if (!isset($args[$argPosition])) { + return false; + } + + $arg = $args[$argPosition]; + if ($arg->unpack || $arg->name !== null) { + return false; + } + + return $this->isSameArrayExpr($array, $arg->value); + } + + private function isSameArrayExpr(Expr $a, Expr $b): bool + { + if (!$this->isStableExpr($a) || !$this->isStableExpr($b)) { + return false; + } + + return $this->exprPrinter->printExpr($a) === $this->exprPrinter->printExpr($b); } - private function isSameVariable(Expr $a, Expr $b): bool + /** + * Only expressions that are side-effect free and denote the same container + * on every evaluation may be compared through their printed form. + */ + private function isStableExpr(Expr $expr): bool { - if ($a instanceof Variable && $b instanceof Variable && is_string($a->name) && is_string($b->name)) { - return $a->name === $b->name; + if ($expr instanceof Variable) { + return is_string($expr->name); + } + + if ($expr instanceof PropertyFetch) { + return $expr->name instanceof Node\Identifier && $this->isStableExpr($expr->var); + } + + if ($expr instanceof StaticPropertyFetch) { + return $expr->class instanceof Name && $expr->name instanceof Node\Identifier; + } + + if ($expr instanceof ArrayDimFetch) { + if ($expr->dim === null) { + return false; + } + + if ( + !$expr->dim instanceof Node\Scalar\Int_ + && !$expr->dim instanceof Node\Scalar\String_ + && !$this->isStableExpr($expr->dim) + ) { + return false; + } + + return $this->isStableExpr($expr->var); } return false; diff --git a/tests/PHPStan/Analyser/nsrt/bug-15080.php b/tests/PHPStan/Analyser/nsrt/bug-15080.php new file mode 100644 index 00000000000..6f1508ee169 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-15080.php @@ -0,0 +1,186 @@ + $list + * @return list + */ +function appendToList(array $list, int $value): array { + $list[count($list)] = $value; + assertType('non-empty-list', $list); + return $list; +} + +/** @return list */ +function foo(): array { return []; } + +function appendWithSizeof(): void { + $list = foo(); + $list[sizeof($list)] = 37; + assertType('non-empty-list', $list); +} + +function appendToKnownSizeList(): void { + $list = foo(); + if (count($list) === 3) { + assertType('array{int, int, int}', $list); + $list[count($list)] = 37; + assertType('array{int, int, int, 37}', $list); + } +} + +function appendToConstantArray(): void { + $list = [1, 2, 3]; + $list[count($list)] = 37; + assertType('array{1, 2, 3, 37}', $list); +} + +function writeBehindConstantArray(): void { + $list = [1, 2, 3]; + $list[count($list) + 1] = 37; + assertType('array{0: 1, 1: 2, 2: 3, 4: 37}', $list); +} + +function appendInLoop(): void { + $list = foo(); + for ($i = 0; $i < 10; $i++) { + $list[count($list)] = $i; + } + assertType('non-empty-list', $list); +} + +function appendWithCoalesceAssign(): void { + $list = foo(); + $list[count($list)] ??= 37; + assertType('non-empty-list', $list); +} + +function appendWithCountMinusZero(): void { + $list = foo(); + $list[count($list) - 0] = 37; + assertType('non-empty-list', $list); +} + +function appendAfterKeyLast(): void { + $list = foo(); + if (count($list) > 0) { + $list[array_key_last($list) + 1] = 37; + assertType('non-empty-list', $list); + $list[1 + array_key_last($list)] = 38; + assertType('non-empty-list', $list); + } +} + +function appendAfterKeyLastOfPossiblyEmptyList(): void { + $list = foo(); + // array_key_last() returns null on an empty list, so null + 1 leaves a hole + $list[array_key_last($list) + 1] = 37; + assertType('non-empty-array, int>', $list); +} + +function countOfDifferentArray(array $other): void { + $list = foo(); + $list[count($other)] = 37; + assertType('non-empty-array, int>', $list); +} + +function countRecursive(): void { + $list = foo(); + $list[count($list, COUNT_RECURSIVE)] = 37; + assertType('non-empty-array, int>', $list); +} + +function unpackedCountArgs(): void { + $list = foo(); + $list[count(...$list)] = 37; + assertType('non-empty-array, int>', $list); +} + +function arraySearchWithSingleArg(): void { + $list = foo(); + // no crash even though array_search() is called with too few arguments + $list[array_search($list)] = 37; + assertType('non-empty-array', $list); +} + +function nestedList(): void { + /** @var array{x: list} $data */ + $data = ['x' => []]; + $data['x'][count($data['x'])] = 37; + assertType('non-empty-list', $data['x']); +} + +function nestedListWithVariableKey(string $key): void { + /** @var array> $data */ + $data = []; + $data[$key][count($data[$key])] = 37; + assertType('non-empty-list', $data[$key]); +} + +class HelloWorld +{ + + /** @var list */ + public array $list = []; + + /** @var list */ + public static array $staticList = []; + + /** @var array{x: list} */ + public array $nested = ['x' => []]; + + public function appendToProperty(): void + { + $this->list[count($this->list)] = 37; + assertType('non-empty-list', $this->list); + } + + public function appendToStaticProperty(): void + { + self::$staticList[count(self::$staticList)] = 37; + assertType('non-empty-list', self::$staticList); + } + + public function appendToNestedProperty(): void + { + $this->nested['x'][count($this->nested['x'])] = 37; + assertType('non-empty-list', $this->nested['x']); + } + + public function overwriteKeyLastOfProperty(): void + { + if (count($this->list) > 0) { + $this->list[array_key_last($this->list)] = 37; + assertType('non-empty-list', $this->list); + } + } + + public function overwriteCountMinusOneOfProperty(): void + { + if (count($this->list) > 0) { + $this->list[count($this->list) - 1] = 37; + assertType('non-empty-list', $this->list); + } + } + + public function overwriteArraySearchOfProperty(int $needle): void + { + $this->list[array_search($needle, $this->list)] = 37; + assertType('non-empty-list', $this->list); + } + + public function countOfDifferentArray(array $other): void + { + $this->list[count($other)] = 37; + assertType('non-empty-array, int>', $this->list); + } + +} diff --git a/tests/PHPStan/Rules/Functions/ReturnTypeRuleTest.php b/tests/PHPStan/Rules/Functions/ReturnTypeRuleTest.php index 88a4b5562cf..75b891723ca 100644 --- a/tests/PHPStan/Rules/Functions/ReturnTypeRuleTest.php +++ b/tests/PHPStan/Rules/Functions/ReturnTypeRuleTest.php @@ -481,6 +481,13 @@ public function testBug13190(): void ]); } + public function testBug15080(): void + { + $this->checkNullables = true; + $this->checkExplicitMixed = true; + $this->analyse([__DIR__ . '/data/bug-15080.php'], []); + } + public function testBug13114(): void { $this->checkNullables = true; diff --git a/tests/PHPStan/Rules/Functions/data/bug-15080.php b/tests/PHPStan/Rules/Functions/data/bug-15080.php new file mode 100644 index 00000000000..551f8cd5fe4 --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/bug-15080.php @@ -0,0 +1,12 @@ + $list + * @return list + */ +function appendToList(array $list, int $value): array { + $list[count($list)] = $value; + return $list; +} From 69412bc21e0b74e7eb03a51e4737a42ee706c7ff Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Tue, 18 Aug 2026 07:00:57 +0000 Subject: [PATCH 2/3] Describe both reasons the re-read offset is skipped after a dim fetch write The comment named only the case where the offset is derived from the container it is written into, e.g. $list[count($list)]. The condition is an equality on the offset type, so it also fires when the offset expression has a side effect ($list[$i++]) and when the offset merely widened because the container changed (end($a); $a[key($a)] = ...), where the expression still designates the same element. That equality is the only cheap evidence the expression still points at what was written, so all of them are skipped alike - say so. Co-Authored-By: Claude Opus 5 --- src/Analyser/ExprHandler/AssignHandler.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/Analyser/ExprHandler/AssignHandler.php b/src/Analyser/ExprHandler/AssignHandler.php index c9ed5c5040c..1d4bc6489a7 100644 --- a/src/Analyser/ExprHandler/AssignHandler.php +++ b/src/Analyser/ExprHandler/AssignHandler.php @@ -1073,8 +1073,14 @@ public function applyWrite( && $expr->dim !== null && !$scope->getType($expr->dim)->equals($offsetType) ) { - // the offset expression - e.g. count($list) - no longer points at the - // offset that was just written now that the container has changed + // Registering $expr would make the scope remember the written value under + // an offset re-read from the *post-assignment* scope. Only an offset that + // still resolves to exactly what was written may be registered, so anything + // else is skipped: the offset moved because it was derived from the container + // ($list[count($list)]) or from a side effect ($list[$i++]), or it merely + // widened because the container changed ($list[key($list)]). The equality is + // the only cheap evidence that the expression still designates the element + // that was just written, so both cases are treated the same way. continue; } From c0974275f56ea122071ae63c89ac71485189b372 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Tue, 18 Aug 2026 07:01:08 +0000 Subject: [PATCH 3/3] Name the mirrored dim fetch and record why the two indexes disagree The two adjacent lines index $dimFetchStack differently, and only one of them can be the link $offsetType belongs to. Reconstructing the intent: the index was introduced for the list-keeping check alone (9efcdf565) and later reused for the setExistingOffsetValueType() branch, both written for the non-nested case where the mirrored and the written index coincide. Handing the written dim fetch to that branch as well is not a no-op. At the outermost level of a nested write the mirror is the whole left-hand side, which is untracked before the assignment, so the container's value type is replaced; the written dim fetch is tracked there, so it would be union'ed instead and every nested write widens back to what it was before - measured as failures in assign-nested-arrays.php, pr-4390.php, bug-13637.php, bug-13786.php and bug-14084.php. Indexing the container instead trades those for the non-nested list writes in bug-12274.php, so no index alone settles it: the branch has to tell replacing a known element from widening a maybe-existing one first. Rename $arrayDimFetch to $mirroredDimFetch and write that down where the next person in this loop will read it. Co-Authored-By: Claude Opus 5 --- src/Analyser/ExprHandler/AssignHandler.php | 27 ++++++++++++++++++---- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/src/Analyser/ExprHandler/AssignHandler.php b/src/Analyser/ExprHandler/AssignHandler.php index 1d4bc6489a7..97e81a72c92 100644 --- a/src/Analyser/ExprHandler/AssignHandler.php +++ b/src/Analyser/ExprHandler/AssignHandler.php @@ -1805,11 +1805,27 @@ private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, ar } } - $arrayDimFetch = $dimFetchStack[$i] ?? null; + // This loop walks the chain from the innermost write outwards: $offsetType is the + // offset written by $writtenDimFetch, and $offsetValueType is the type of the + // container it is written into ($writtenDimFetch->var). $dimFetchStack is indexed + // the other way round, so $dimFetchStack[$i] is the mirror image of + // $writtenDimFetch - the two only coincide for a non-nested write. + // + // Swapping the mirror for $writtenDimFetch here is not a no-op. At the outermost + // level of a nested write the mirror is the whole left-hand side, which is untracked + // before the assignment, so the write takes the setOffsetValueType() branch below and + // *replaces* the container's value type. $writtenDimFetch is tracked there, so the + // write would take the setExistingOffsetValueType() branch and *union* the old value + // type into the new one, widening every nested write back to what it was before + // (assign-nested-arrays.php, pr-4390.php, bug-13637.php, bug-13786.php, + // bug-14084.php). Making the index consistent therefore means teaching this branch to + // tell replacing a known element from widening a maybe-existing one, which no index + // on its own can do. + $mirroredDimFetch = $dimFetchStack[$i] ?? null; if ( $offsetType !== null - && $arrayDimFetch !== null - && $scope->hasExpressionType($arrayDimFetch)->yes() + && $mirroredDimFetch !== null + && $scope->hasExpressionType($mirroredDimFetch)->yes() && !$offsetValueType->hasOffsetValueType($offsetType)->no() ) { $hasOffsetType = null; @@ -1853,8 +1869,9 @@ private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, ar $valueToWrite = $offsetValueType->setOffsetValueType($offsetType, $valueToWrite, $unionValues); } - // $writtenDimFetch is the dim fetch $offsetType belongs to - unlike - // $dimFetchStack[$i] above, which this reversed loop indexes the other way round + // $writtenDimFetch, not the mirror above: shouldKeepList() reads the offset + // expression together with the array it indexes, so it has to be handed the link + // of the chain that $offsetType belongs to if ($offsetValueType->isList()->yes() && $this->shouldKeepList($writtenDimFetch, $scope, $offsetValueType)) { $valueToWrite = TypeCombinator::intersect($valueToWrite, new AccessoryArrayListType()); }