Fix phpstan/phpstan#14245: list lost after overwriting last element#5169
Conversation
src/Type/IntersectionType.php
Outdated
| return $this->intersectTypes(static fn (Type $type): Type => $type->setExistingOffsetValueType($offsetType, $valueType)); | ||
| $result = $this->intersectTypes(static fn (Type $type): Type => $type->setExistingOffsetValueType($offsetType, $valueType)); | ||
|
|
||
| if ($this->isList()->yes() && !$result->isList()->yes()) { |
There was a problem hiding this comment.
It's unclear to me why we need this extra check.
I feel like the fix should be somewhere else (in another type ?).
Cause Array&List should already stay the same
Or ConstantArray&List should already stay the same
There was a problem hiding this comment.
the mutations tell the same story. thats a AI leftover. I will remove it.
good catch.
| $valueToWrite = TypeCombinator::intersect($valueToWrite, new AccessoryArrayListType()); | ||
| } elseif ( // keep list for $list[1 + $index] assignments | ||
| $arrayDimFetch->dim->left instanceof Variable | ||
| $keepList = false; |
There was a problem hiding this comment.
Would it make sens for visibility to have a private shouldKeepList() method ?
…array_key_last, array_key_first - Extended list type preservation in AssignHandler to recognize count($list) - n, array_key_last($list), and array_key_first($list) as existing-key offset patterns - Added list preservation in IntersectionType::setExistingOffsetValueType - New regression test in tests/PHPStan/Analyser/nsrt/bug-14245.php Closes phpstan/phpstan#14245
staabm
force-pushed
the
create-pull-request/patch-nva5u2r
branch
from
7427ed6 to
9a5f88d
Compare
| ) { | ||
| return true; | ||
| } | ||
| } elseif ( // keep list for $list[count($list) - n] assignments |
There was a problem hiding this comment.
Cant we risk having n too big and set the offset -1 ?
There was a problem hiding this comment.
since the logic here will only add non-empty-list to a pre-existing maybe already degraded because too huge array, I don't see how a too big "n" value can be a problem here.
added a test with a huge array and a big offset
| && $offsetValueType->isIterableAtLeastOnce()->yes() | ||
| ) { | ||
| return true; | ||
| } elseif ( // keep list for $list[array_key_last($list)] and $list[array_key_first($list)] assignments |
There was a problem hiding this comment.
Array_search works too
|
thank you @VincentLanglet 👍 |
3 participants
Summary
When a
list<int>had its last element overwritten via$list[count($list) - 1] = x,$list[array_key_last($list)] = x, or$list[array_key_first($list)] = x, PHPStan lost thelisttype and degraded toarray<int<0, max>, int>. This fix preserves the list type for these common patterns.Changes
src/Analyser/ExprHandler/AssignHandler.phpto recognize three new syntactic patterns:$list[count($list) - n](BinaryOp\Minus with count() on the same variable)$list[array_key_last($list)](function call on the same variable)$list[array_key_first($list)](function call on the same variable)isSameVariable()helper to compare whether two expressions refer to the same variablesrc/Type/IntersectionType.phpsetExistingOffsetValueType()— when the original type is a list, the result preserves the list accessory typeRoot cause
The list type preservation logic in
AssignHandler::produceArrayDimFetchAssignValueToWrite()only handledBinaryOp\Pluspatterns (for$list[$index + 1]style assignments). Assignments usingcount($list) - 1,array_key_last($list), orarray_key_first($list)as the offset fell through without restoring theAccessoryArrayListType, causing the list type to be lost aftersetOffsetValueType()was called.Test
Added
tests/PHPStan/Analyser/nsrt/bug-14245.phpwith three test functions covering all three patterns (count() - 1,array_key_last(),array_key_first()), verifying both thenon-empty-list<int>type inside theifblock and thelist<int>type after the block.Fixes phpstan/phpstan#14245