Skip to content

Fix #4173: Consequest scope with the same condition#5056

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

Fix #4173: Consequest scope with the same condition#5056
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-p4k2z20

Conversation

@phpstan-bot
Copy link
Collaborator

This PR fixes phpstan/phpstan#4173

When merging scopes from three or more branches (e.g., if/elseif/elseif/else), MutatingScope::mergeWith() performs pairwise merges. During each merge, intersectConditionalExpressions() preserves only conditional expressions that exist in both scopes. However, a conditional expression whose guard condition is impossible in the other scope is vacuously true there and should be preserved.

For example, with branches count($row) === 3 (defines $value), count($row) === 2 (no $value), and count($row) === 1 (no $value):

  • After merging branches 1+2, a conditional expression "if count($row)=3 then $value is defined" is created
  • When merging that result with branch 3 (count=1), intersectConditionalExpressions() discards this conditional expression because branch 3 doesn't have it
  • But in branch 3, count($row) is 1, so the guard count($row)=3 can never be satisfied — the conditional expression is vacuously true
  • Later, when count($row) === 1 is eliminated by an early return, the conditional expression is needed to know that $value is defined when count($row) is 2 or 3

The fix adds preserveVacuousConditionalExpressions() which recovers conditional expressions from either scope whose guard conditions are provably impossible in the other scope (using isSuperTypeOf()->no()).

Additionally fixes two pre-existing issues in PHPStan's own code that are now detected due to improved analysis precision:

  • Removed a redundant $parametersAcceptor !== null check in NodeScopeResolver (always true within $methodReflection !== null block)
  • Stored $operand->getMax() in a local variable in InitializerExprTypeResolver to allow proper null-narrowing

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