Propagate ArrayDimFetch type narrowing to parent array in BooleanAnd/BooleanOr compound conditions#5592
Closed
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
Closed
Conversation
…nd`/`BooleanOr` compound conditions - Add `propagateArrayDimFetchToParentArray()` in TypeSpecifier that enriches normalized SpecifiedTypes with parent array narrowing when a dim fetch on a variable is present but the variable itself has no specifier - Apply the propagation before `intersectWith()` in BooleanAnd falsy and BooleanOr truthy paths, where the intersection previously dropped non-overlapping dim-fetch vs variable specifiers - Use `HasOffsetValueType` to compute the narrowed parent array type - Restrict propagation to cases where the parent is a simple variable (not a nested dim fetch) to avoid cascading side effects - Covers `is_string`, `is_int`, `is_array`, `instanceof`, and strict comparisons on array offsets combined with `isset` via `&&`
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
When
isset($arr['key']) && is_string($arr['key'])(or similar compound conditions) was used with an early return, PHPStan failed to narrow the parent array type in the else/falsy branch. For example, with$test: array{}|array{hi: 'hello'}|array{hi: array{0: 42, 1?: 42}}, afterif (isset($test['hi']) && is_string($test['hi'])) { return; }, PHPStan still inferred$testas the full union instead ofarray{}|array{hi: array{0: 42, 1?: 42}}.Changes
propagateArrayDimFetchToParentArray()method insrc/Analyser/TypeSpecifier.phpthat enriches normalizedSpecifiedTypeswith parent array narrowing usingHasOffsetValueTypewhen a dim fetch specifier exists but its parent variable has no specifierintersectWith()in two locations:BooleanAndfalsy path (line ~737): handlesisset($x['k']) && is_type($x['k'])in else branchesBooleanOrtruthy path (line ~797): handles!isset($x['k']) || !is_type($x['k'])in if branchesExpr\Variable(not anotherArrayDimFetch) to avoid cascading side effects with nested dim fetchesRoot cause
In the
BooleanAndfalsy context (!(A && B)=!A || !B),TypeSpecifiernormalizes left and right specifiers then intersects them. The intersection (SpecifiedTypes::intersectWith()) only retains specifiers for expressions present in both sides. When the left side (isset) narrows$testand the right side (is_string) narrows$test['hi'], these are different expression keys, so the intersection produces empty specifiers — no narrowing occurs.The fix propagates the dim-fetch narrowing to the parent array (via
HasOffsetValueType) before the intersection, ensuring both sides have specifiers for the same variable ($test). The intersection then correctly computes the union of the two path types.Test
tests/PHPStan/Analyser/nsrt/bug-14566.php:isset($test['hi']) && is_string($test['hi'])with early returnelsebranch variantis_arrayvariantis_intvariantinstanceofvariant=== 'hello') variantBooleanOrequivalent (!isset || !is_string) variantProbed and found already correct:
is_string($test['hi'])withoutisset(existing propagation inspecifyExpressionTypehandles this)Fixes phpstan/phpstan#14566