Skip to content

Skip boolean conditional holders whose target was dropped as a no-op antecedent narrowing#5958

Merged
staabm merged 3 commits into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-x01hq09
Jul 1, 2026
Merged

Skip boolean conditional holders whose target was dropped as a no-op antecedent narrowing#5958
staabm merged 3 commits into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-x01hq09

Conversation

@phpstan-bot

Copy link
Copy Markdown
Collaborator

Summary

When a guard such as if ($a === $b && $a !== "" && $b !== "") { return; } was
followed by an (even empty) sibling if ($a !== "") {} block, PHPStan later
narrowed $a to '' and reported a bogus "Strict comparison using !== between
'' and '' will always evaluate to false". The narrowing was unsound: $a and
$b can each be any distinct non-empty string in the else branch.

The fix makes the boolean conditional-expression holder mechanism refuse to build
a holder whose consequent targets an expression that only entered the antecedent
through a relation it cannot represent.

Changes

  • src/Analyser/ExprHandler/Helper/ConditionalExpressionHolderHelper.php:
    • processBooleanConditionalTypes() now records every condition-side expression
      whose narrowing is a no-op and therefore gets dropped from the antecedent
      ($droppedNoOpConditions).
    • When projecting consequents, it skips any holder target that was dropped as a
      no-op condition, since the antecedent no longer constrains that target.
  • tests/PHPStan/Analyser/nsrt/bug-14891.php: regression test covering the
    reported === case plus the analogous cases fixed by the same change.

Root cause

The holder mechanism turns !(Left && Right) into implications like
"if Left is true then Right is false". The antecedent is built from type
narrowings of the operands. A relation like $a === $b between two variables of
the same broad type (string) narrows nothing, so it is a no-op and gets dropped
(this drop was already done by #5951). Dropping it leaves an antecedent that is
strictly weaker than "Left is true" — here just "$a is non-empty-string" —
while the consequent still claims something about $b ($b === ''). Because the
only thing that tied $b into the antecedent was the dropped $a === $b
relation, the resulting holder "if $a is non-empty-string then $b is ''"
fires whenever $a is non-empty, regardless of $b.

The pattern is: a consequent must not be projected onto an expression whose sole
link to the antecedent was a relational condition the type-guard mechanism can't
represent.
Detecting that the target itself was dropped as a no-op condition is
exactly this signal.

Test

  • tests/PHPStan/Analyser/nsrt/bug-14891.php reproduces the reported case with
    assertType() ($a stays string, and non-empty-string after if ($a !== "")).
    It fails before the fix ($a was '', then *NEVER*) and passes after.
  • It also covers three analogous cases found on the same axis, each independently
    verified to fail before the fix and pass after:
    • a loose == relational antecedent,
    • a relation between two property fetches ($this->a === $this->b),
    • a relation between two integers with integer-range arms
      ($a === $b && $a > 0 && $b > 0, where $a was wrongly narrowed to int<min, 0>).
  • The BooleanOr path shares the same helper, so it is covered by the same change.
  • The degenerate-antecedent regressions from Skip degenerate no-op antecedents when building boolean conditional expression holders #5951 (bug-14878,
    boolean-and-conditional-holders-mixed-context, bug-6202, bug-14874,
    bug-10644, negated-boolean-and-conditional-holders) still pass.

Fixes phpstan/phpstan#14891

@staabm staabm requested review from VincentLanglet and Copilot and removed request for Copilot July 1, 2026 05:07
Comment thread src/Analyser/ExprHandler/Helper/ConditionalExpressionHolderHelper.php Outdated
@VincentLanglet VincentLanglet requested a review from staabm July 1, 2026 07:47
phpstan-bot and others added 2 commits July 1, 2026 09:54
…antecedent narrowing

- In ConditionalExpressionHolderHelper::processBooleanConditionalTypes, track
  which condition-side expressions produced a no-op narrowing and were dropped
  from the antecedent (e.g. `$a === $b` between two variables of the same broad
  type, which couples the two without changing either type).
- When building a holder, skip any target expression that was dropped as a no-op
  condition. Its only link into the antecedent was the un-representable relation,
  so a holder projected onto it would fire even when that relation does not hold.
- This fixes unsound holders like "if $a is non-empty-string then $b is ''"
  generated from `!($a === $b && $a !== "" && $b !== "")`, which wrongly narrowed
  $a to '' in a later branch.
- Same defect (and same fix) covers the loose `==` relational antecedent, relations
  between property fetches, and integer-range narrowings; the shared helper is used
  by both BooleanAndHandler and BooleanOrHandler.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@staabm staabm force-pushed the create-pull-request/patch-x01hq09 branch from 948ed69 to ada32d3 Compare July 1, 2026 07:54
@staabm staabm merged commit f2d2d94 into phpstan:2.2.x Jul 1, 2026
669 of 671 checks passed
@staabm staabm deleted the create-pull-request/patch-x01hq09 branch July 1, 2026 08:34
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.

incorrect type narrowing with dependent types

3 participants