Skip to content

Fix phpstan/phpstan#13591: False positive: Type narrowing not working with conditional exception validation#5385

Open
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-q95kasv
Open

Fix phpstan/phpstan#13591: False positive: Type narrowing not working with conditional exception validation#5385
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-q95kasv

Conversation

@phpstan-bot
Copy link
Copy Markdown
Collaborator

Summary

When a compound condition like if ($hotelId === null && ($action === 'get_rooms' || $action === 'update')) { throw ...; } guards subsequent code, PHPStan should narrow $hotelId to non-null when $action === 'get_rooms'. This wasn't happening because the conditional expression holders created union condition types that couldn't be matched by more specific type narrowings.

Changes

  • Modified processBooleanSureConditionalTypes and processBooleanNotSureConditionalTypes in src/Analyser/TypeSpecifier.php to expand union condition types into separate holders
  • Added expandUnionConditions helper method that takes a condition set and produces additional holder variants for each member of any union types in the conditions
  • The original union-typed holder is preserved for exact matches, and individual member holders are added for subtype matches

Root cause

When $hotelId === null && ($action === 'get_rooms' || $action === 'update') is evaluated in the falsey context (after throwing), the processBooleanNotSureConditionalTypes method creates a conditional expression holder with condition {$action: 'get_rooms'|'update'} and result {$hotelId: int}. This means "if $action is exactly 'get_rooms'|'update', then $hotelId is int".

However, when entering if ($action === 'get_rooms'), the scope narrows $action to 'get_rooms' — a single constant type, not the union 'get_rooms'|'update'. The equals check in filterBySpecifiedTypes requires exact type equality, so the holder never matches.

The fix expands the union condition into separate holders: one for {$action: 'get_rooms'} and one for {$action: 'update'}, each pointing to the same result. This way, entering if ($action === 'get_rooms') matches the first expanded holder exactly.

Test

Added tests/PHPStan/Analyser/nsrt/bug-13591.php — an NSRT test that verifies $hotelId is narrowed to int (not int|null) inside if ($action === 'get_rooms') after the throwing guard.

Fixes phpstan/phpstan#13591

…l exception validation

- When a compound condition like `$x === null && ($y === 'a' || $y === 'b')` throws,
  the negation creates conditional holders with union condition types (e.g. $y: 'a'|'b')
- These union conditions could never be matched exactly by later narrowing like `$y === 'a'`
- Fix: expand union condition types in conditional holders into separate holders for each
  union member, while keeping the original union holder for exact matches
- New regression test in tests/PHPStan/Analyser/nsrt/bug-13591.php
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.

2 participants