Skip to content

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

Merged
staabm merged 2 commits into
phpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-f2mw098
May 23, 2026
Merged

Do not create conditional expression when holder and guard have the same type and guard overlaps with other branch#5724
staabm merged 2 commits 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

VincentLanglet and others added 2 commits May 23, 2026 13:04
…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
@staabm staabm force-pushed the create-pull-request/patch-f2mw098 branch from c353bed to 418f59a Compare May 23, 2026 11:04
@staabm
Copy link
Copy Markdown
Contributor

staabm commented May 23, 2026

(the diff got less readable with the last commit, but the source readability itself improved)

@staabm staabm merged commit 583dc65 into phpstan:2.1.x May 23, 2026
878 of 888 checks passed
@staabm
Copy link
Copy Markdown
Contributor

staabm commented May 23, 2026

thanks!

@staabm staabm deleted the create-pull-request/patch-f2mw098 branch May 23, 2026 11:09
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.

3 participants