Skip to content

Re-add complement of a dropped self-condition to boolean-decomposition holder types#5943

Open
phpstan-bot wants to merge 1 commit into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-p6jom0y
Open

Re-add complement of a dropped self-condition to boolean-decomposition holder types#5943
phpstan-bot wants to merge 1 commit into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-p6jom0y

Conversation

@phpstan-bot

Copy link
Copy Markdown
Collaborator

Summary

After if (isset($a['foo']) && !is_array($a['foo'])) return false;, PHPStan believed that once array_key_exists('foo', $a) is true, $a['foo'] must be an array. But isset() is false both when the key is missing and when its value is null, so the value can still be null. This produced a false identical.alwaysFalse error on $a['foo'] === null. The fix makes PHPStan infer array|null there.

Changes

  • src/Analyser/ExprHandler/Helper/ConditionalExpressionHolderHelper.php: when processBooleanConditionalTypes() drops a condition that targets the holder's own expression, the complement of that condition's type (within the target's scope type) is now unioned into the holder type.
  • Probed analogous constructs:
    • is_int (and other is_* narrowing functions): same bug, fixed by the same change — the is_* arm is not special-cased. Covered by a test.
    • || (BooleanOr): shares the same helper. Its truthy form (!isset(...) || is_array(...)) already yielded the sound mixed (no false positive), so it needed no separate fix.
    • Typed-property isset($o->p): no array_key_exists-style existence re-check exists for typed properties, so there is no parallel unsound path.

Root cause

Boolean conditions are decomposed into "conditional expression holders" of the form if [conditions on other expressions] then [target] is [type]. A condition that narrows the target expression itself cannot be tracked as an antecedent (holders fire by matching other expressions), so it is dropped. For isset($a['foo']) the narrowing splits into $a having offset 'foo' (kept) and $a['foo'] being non-null (dropped, same expression as the target). The remaining holder — if $a has offset 'foo' then $a['foo'] is array — then fired on array_key_exists() (which only proves key existence, not non-null), wrongly forcing array and dropping null.

The fix recognises that a dropped self-condition restricted the target to a subset of its values; without it the holder must also allow the excluded values. It unions the complement (target scope type minus dropped condition type, here null) back into the holder type, yielding array|null.

Test

  • tests/PHPStan/Analyser/nsrt/bug-14874.php: the reported reproducer asserts $a['foo'] is array<mixed, mixed>|null inside the array_key_exists branch, plus an is_int mirror asserting int|null. Both fail without the fix (inferring array / int) and pass with it.

Fixes phpstan/phpstan#14874

…n holder types

- In `ConditionalExpressionHolderHelper::processBooleanConditionalTypes()`, a condition narrowing the holder's *target* expression itself is dropped (holders fire by matching other expressions). That dropped condition restricted the target to a subset of its values, so the resulting holder over-narrowed the target when only the remaining conditions later hold.
- Now, when such a self-condition is dropped, the values it excluded (the complement of its type within the target's scope type) are unioned back into the holder type, so the holder stays sound.
- Fixes `!(isset($a['foo']) && !is_array($a['foo']))` followed by `array_key_exists('foo', $a)` inferring `$a['foo']` as `array` instead of the correct `array|null` (the `isset` non-null self-condition was silently dropped).
- The same helper backs both `BooleanAndHandler` and `BooleanOrHandler`, and the `is_*` arm is not special-cased, so the mirror `is_int`/`||` cases are fixed by the same change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect narrowing of $a['foo'] after excluding isset($a['foo']) && !is_array($a['foo'])

1 participant