Fix phpstan/phpstan#14215: Nonexistent offset detection on lists with count() -1#5116
Open
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
Open
Fix phpstan/phpstan#14215: Nonexistent offset detection on lists with count() -1#5116phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
count() -1#5116phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
Conversation
… `count() - 1` - Extended TypeSpecifier to detect `count($list) - K` patterns in comparisons, not just plain `count($list)` - Adjusted sizeType computation to account for the subtraction value - List offset inference now works with both `$index < count($list) - K` and `$index <= count($list) - K` (when K >= 1) - Non-empty array narrowing threshold adjusted for subtraction - New regression test in tests/PHPStan/Rules/Arrays/data/bug-14215.php
Contributor
VincentLanglet
left a comment
There was a problem hiding this comment.
Seems interesting, dunno if you plan/want to work on it @staabm ?
| $sizeType = $leftType->shift($offset); | ||
| $sizeType = $leftType->shift($offset + $subtraction); | ||
| } else { | ||
| $sizeType = $leftType; |
Contributor
There was a problem hiding this comment.
Shouldn't we add $substraction here too ?
| $subtraction = $expr->right->right->value; | ||
| } | ||
|
|
||
| if ( |
Contributor
There was a problem hiding this comment.
I think the bot is trying to support thing like
if (3 < count($array) - 1)
Contributor
|
I can have a look. If you are interested feel free to take it |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PHPStan was reporting false positive "Offset might not exist" errors when accessing a list element after a
$index < count($array) - 1or$index <= count($array) - 1guard. This extends the fix from #5090 (which handled plaincount($array)) to also handlecount($array) - Kpatterns.Changes
src/Analyser/TypeSpecifier.phpto extractcount()/sizeof()calls fromBinaryOp\Minusexpressions (e.g.count($list) - 1)sizeTypecomputation to account for the subtraction value K when narrowing the count type$index <= count($list) - Kis safe when K >= 1 (equivalent to$index < count($list) - (K-1))Root cause
The
TypeSpecifieronly looked forFuncCallnodes matchingcount/sizeofon the right side of comparisons. When the right side wasBinaryOp\Minus(FuncCall(count), Int_(K)), it was not recognized, so no offset validity inference was made. The fix extracts the count call from the minus expression and adjusts the comparison semantics by the subtraction amount.Test
Added
tests/PHPStan/Rules/Arrays/data/bug-14215.phpwith test cases for:$index < count($array) - 1with positive-int (no error)$index <= count($array) - 1with positive-int (no error)$index <= count($array) - 1with plain int (still reports - could be negative)$index < count($array) - 1withint<0, max>(no error)$index <= count($array) - 1withint<0, max>(no error)$index < count($array) - 2/$index <= count($array) - 2with positive-int (no error)sizeof()variant (no error)count($array) - 1 > $index(no error)Fixes phpstan/phpstan#14215