Do not split a compound-boolean holder side whose asserted truth value is a disjunction#5817
Merged
ondrejmirtes merged 1 commit intoJun 6, 2026
Conversation
…e is a disjunction
- `processBooleanConditionalTypes()` now decides whether a holder side may be
decomposed into per-expression holders from the holder-side expression's
logical structure instead of counting trackable targets in the sureNot list.
- A new `isUnsplittableCompoundHolderSide()` suppresses holder creation when the
side is a conjunction (`&&`/`and`) asserted false in the `BooleanAnd` false
context, or a disjunction (`||`/`or`) asserted true in the `BooleanOr` true
context. In both cases the asserted fact is itself a disjunction with no sound
per-expression narrowing.
- The holder-side expression (`$expr->left` / `$expr->right`) is now threaded
into `processBooleanConditionalTypes()` for both the `BooleanAnd` and
`BooleanOr` holder paths.
- Fixes the reported false positive: after `if ($a && $b && $c) { return; }`,
`$c` being true no longer narrowed `$a`/`$b` to `false`, so `$c && $b` was
wrongly reported as `Right side of && is always false`. The previous
count-based guard only covered the sureNot list, but a plain-boolean
conjunction's false narrowing lands (over-narrowed) in the sure list.
- Side effect / improvement: a disjunction operand asserted false (e.g.
`!(($p || $q) && $r)` with `$r` true) is now correctly narrowed, since the
negation of a disjunction is a conjunction and is safe to split.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
After a guard like
if ($a && $b && $c) { return; }, PHPStan wrongly narrowed theother operands when one was later tested. Entering
if ($c)narrowed both$aand$btofalse, soif ($c && $b)was reported asRight side of && is always false— while the logically equivalent
if ($b && $c)was not, hence the operand-orderdependence in the report. The only fact known after the guard is the disjunction
!$a || !$b || !$c; it does not make$bfalse when$cis true (e.g.$a = false, $b = true, $c = truereaches the branch).Changes
src/Analyser/TypeSpecifier.phpprocessBooleanConditionalTypes(): replaced the sureNot-only, count-basedsuppression with a structural check driven by the holder-side expression.
isUnsplittableCompoundHolderSide(): a holder side is left whole (noper-expression holders) when it is a conjunction (
&&/and) asserted false inthe
BooleanAndfalse context, or a disjunction (||/or) asserted true in theBooleanOrtrue context.$expr->left/$expr->right) intoprocessBooleanConditionalTypes()from both theBooleanAndandBooleanOrholder paths.
tests/PHPStan/Analyser/nsrt/bug-14787.php: regression test.Root cause
The
BooleanAnd/BooleanOrdecomposition creates conditional holders of the form"if one side is true, the other side is …". When the other (holder) side is itself a
compound boolean, splitting its narrowing into independent per-expression holders is
only sound when the asserted fact is a conjunction:
BooleanAndfalse context — the holder side is asserted false. A conjunction$a && $basserted false is!$a || !$b, a disjunction, which has noper-expression narrowing.
BooleanOrtrue context — the holder side is asserted true. A disjunction$a || $basserted true is itself a disjunction.The previous guard (added for #14780) only inspected the sureNot list and counted
trackable targets. For
=== null-style operands the over-narrowed disjunction landsin the sureNot list, so it was caught; but for plain boolean operands the
conjunction's false narrowing is computed via
intersectWithand lands in the surelist, which the old guard never inspected — so the holders were split and over-narrowed.
Deciding splittability from the holder-side expression's logical structure fixes both
list placements uniformly, and symmetrically covers the
BooleanOrtrue context.Test
tests/PHPStan/Analyser/nsrt/bug-14787.phpcovers:if ($a && $b && $c) { return; },$a/$bstayboolunder
if ($c), and both$c && $band$b && $cnarrow totrue(noalways false). Fails before the fix (the&&blocks collapse to*NEVER*).!($b && $c)with$ctrue keeps$basfalse.!(($p || $q) && $r)with$rtrue nowcorrectly narrows
$pand$qtofalse(negation of a disjunction is aconjunction, safe to split). This was previously a missed narrowing.
The
BooleanOrtrue-context counterpart was also probed: a disjunction operandasserted true already produces empty narrowings, so it never over-narrowed; the new
structural check covers it for robustness without changing behavior.
Fixes phpstan/phpstan#14787