Skip to content

Do not create conditional expression when holder and guard have the same type and guard overlaps with other branch#5724

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

Do not create conditional expression when holder and guard have the same type and guard overlaps with other branch#5724
phpstan-bot wants to merge 1 commit into
phpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-f2mw098

Conversation

@phpstan-bot
Copy link
Copy Markdown
Collaborator

Summary

Fixes phpstan/phpstan#14469

When merging if/elseif scopes, createConditionalExpressions() could create a spurious conditional expression (CE) linking two variables that happened to have the same type in one branch. For example, in:

if ($var1) {
    $aa = $user->id === 10 ? 2 : null;
} elseif ($R['aa']) {
    $aa = $R['aa'];
}

In the elseif branch, both $aa and $R['aa'] have type mixed~falsy. The merge created a CE saying "when $aa is mixed~falsy, $R['aa'] is mixed~falsy". This CE later fired when if ($aa) narrowed $aa, incorrectly narrowing $R['aa'] to truthy — making !$R['aa'] a false positive "always false".

The fix adds a third skip condition in the inner loop of createConditionalExpressions(): when the expression does not exist in the other scope, the holder type equals the guard type, and the guard type overlaps with the other scope's guard type (isSuperTypeOf returns Maybe), the CE is skipped as unreliable. This correctly identifies cases where two variables were correlated only within a single branch, not causally linked.

Test plan

  • Added regression test testBug14469 covering the original report and analogous patterns (if-constant-condition, falsey branch, with-else, multiple-elseif)
  • Added NSRT test asserting $R['aa'] retains type mixed inside if ($aa)
  • Verified existing regression tests pass: bug-14411-regression, bug-1306, full BooleanNotConstantConditionRuleTest
  • Full test suite: 12131 tests, 0 failures
  • make phpstan: no errors
  • make cs-fix: no violations

…ame type and guard overlaps with other branch

When merging if/elseif scopes, createConditionalExpressions could create
a spurious CE linking two variables that happened to have the same type
in one branch (e.g. $aa = $R['aa'] in an elseif branch). This CE would
later fire incorrectly when the guard variable was narrowed, falsely
narrowing the holder variable's type too.

The fix adds a third skip path: when the expression does not exist in
the other scope, the holder type equals the guard type, and the guard
type overlaps with the other scope's guard type (isSuperTypeOf is not
No), the CE is redundant and should be skipped.

Closes phpstan/phpstan#14469
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