Track $arr[$key] existence across array_search/array_find_key via conditional expression holders#5552
Open
phpstan-bot wants to merge 2 commits intophpstan:2.1.xfrom
Open
Conversation
…ia conditional expression holders - Add conditional expression holders in TypeSpecifier for `$key = array_search($needle, $arr)` that fire when `$key !== false`, registering `$arr[$key]` as existing - Add conditional expression holders for `$key = array_find_key($arr, $cb)` that fire when `$key !== null`, registering `$arr[$key]` as existing - Add `array_find_key` to the existing `array_key_first/last !== null` comparison handler to narrow array to non-empty - Move `array_search` true-context handling from standalone block into unified handler alongside the conditional holder logic - Update existing test that was asserting the buggy behavior (separate assignment `$key = array_search(...)` followed by `if ($key !== false)` was reporting "Offset might not exist")
staabm
reviewed
Apr 27, 2026
Comment on lines
+931
to
+958
| $specifiedTypes = $specifiedTypes->unionWith( | ||
| $this->create($arrayArg, new NonEmptyArrayType(), TypeSpecifierContext::createTrue(), $scope), | ||
| ); | ||
|
|
||
| $dimFetch = new ArrayDimFetch($arrayArg, $expr->var); | ||
|
|
||
| $specifiedTypes = $specifiedTypes->unionWith( | ||
| $this->create($dimFetch, $arrayType->getIterableValueType(), TypeSpecifierContext::createTrue(), $scope), | ||
| ); | ||
| } elseif ($expr->var instanceof Expr\Variable && is_string($expr->var->name)) { | ||
| $keyType = $scope->getType($expr->expr); | ||
| $nonNullKeyType = TypeCombinator::removeNull($keyType); | ||
| if (!$nonNullKeyType instanceof NeverType && !$keyType->isNull()->yes()) { | ||
| $dimFetch = new ArrayDimFetch($arrayArg, $expr->var); | ||
| $dimFetchString = $this->exprPrinter->printExpr($dimFetch); | ||
| $keyExprString = $this->exprPrinter->printExpr($expr->var); | ||
|
|
||
| $holder = new ConditionalExpressionHolder( | ||
| [$keyExprString => ExpressionTypeHolder::createYes($expr->var, $nonNullKeyType)], | ||
| ExpressionTypeHolder::createYes($dimFetch, $arrayType->getIterableValueType()), | ||
| ); | ||
|
|
||
| $specifiedTypes = $specifiedTypes->unionWith( | ||
| (new SpecifiedTypes([], []))->setNewConditionalExpressionHolders([ | ||
| $dimFetchString => [$holder->getKey() => $holder], | ||
| ]), | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
can this be de-duplicated, as we have this similar code in this file more than 2 times already
Collaborator
Author
There was a problem hiding this comment.
Pushed successfully. Here's what was done to address staabm's review:
Extracted a createArrayDimFetchConditionalExpressionHolder() helper method that encapsulates the repeated ~12-line pattern of creating a ConditionalExpressionHolder for array dim fetch tracking. This helper is now used by all three call sites:
array_key_first/array_key_lastblockarray_searchblockarray_find_keyblock
Merged array_search and array_find_key into a single block using a lookup for their differences (array arg index, sentinel type, whether to narrow to non-empty), further reducing duplication.
Net result: -30 lines (87 removed, 57 added). All 7703 tests pass and PHPStan analysis is clean.
Contributor
|
can we do similar stuff for |
… tracking Extract createArrayDimFetchConditionalExpressionHolder() helper method and merge the separate array_search/array_find_key blocks into one, reducing three instances of the same ~12-line pattern to single-line calls. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
2 participants
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
When
$key = array_search($needle, $arr)is assigned and later checked withif ($key !== false), PHPStan reported "Offset might not exist on non-empty-array" even though the key is guaranteed to exist. Similarly,$key = array_find_key($arr, $cb)followed byif ($key !== null)had the same issue. This extends the conditional expression holder mechanism from PR #5537 (which handledarray_key_first/array_key_last) to also coverarray_searchandarray_find_key.Changes
src/Analyser/TypeSpecifier.php:$key = array_search($needle, $arr)— when$keyis narrowed to not-false (via$key !== false), registers$arr[$key]as existing with the array's iterable value type$key = array_find_key($arr, $cb)— when$keyis narrowed to not-null (via$key !== null), registers$arr[$key]as existingarray_find_keyto the existingarray_key_first/last !== nullcomparison handler to narrow the array toNonEmptyArrayTypearray_searchhandler from the$context->true()block since it's handled by the unified blocktests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php: Updated existing testtestArrayDimFetchAfterArraySearchto expect no error (previously asserted the buggy behavior), addedtestBug14537tests/PHPStan/Rules/Arrays/data/bug-14537.php: Rule test coveringarray_searchandarray_find_keywithreportPossiblyNonexistentGeneralArrayOffset: truetests/PHPStan/Analyser/nsrt/array-search-existing.php: NSRT test forarray_searchoffset existence trackingtests/PHPStan/Analyser/nsrt/array-find-key-existing.php: NSRT test forarray_find_keyoffset existence and non-empty array narrowingRoot cause
The
TypeSpecifierhandled$key = array_search(...)only in the$context->true()case (assignment-in-condition likeif ($key = array_search(...))). For the common pattern of separate assignment followed byif ($key !== false), no conditional expression holder was created, so PHPStan couldn't track that$arr[$key]exists after the check.array_find_keyhad no offset existence tracking at all.The fix uses the same conditional expression holder mechanism as
array_key_first/array_key_last(PR #5537): when the key variable is narrowed to exclude the "not found" sentinel (falseforarray_search,nullforarray_find_key), the holder fires and registers the array dim fetch as existing.Analogous cases probed:
array_key_first/array_key_last: already handled by PR Track $arr[$key] existence across array_key_first/last + !== null #5537 ✓array_rand: already handled unconditionally for non-empty arrays in null context ✓key(): pointer-based, unreliable for offset tracking — skipped by designfalse !== $key,null !== $key): tested and working ✓Test
testBug14537coversarray_search(list + string-key map) andarray_find_key(list + string-key map) with reversed comparison variants, all expecting no errors withreportPossiblyNonexistentGeneralArrayOffset: truearray-search-existing.phpandarray-find-key-existing.phpverify correct type inference for$arr[$key]and array narrowing to non-emptytestArrayDimFetchAfterArraySearchto match corrected behaviorFixes phpstan/phpstan#14537