Skip boolean conditional holders whose target was dropped as a no-op antecedent narrowing#5958
Merged
staabm merged 3 commits intoJul 1, 2026
Conversation
VincentLanglet
approved these changes
Jul 1, 2026
…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>
948ed69 to
ada32d3
Compare
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
When a guard such as
if ($a === $b && $a !== "" && $b !== "") { return; }wasfollowed by an (even empty) sibling
if ($a !== "") {}block, PHPStan laternarrowed
$ato''and reported a bogus "Strict comparison using!==between''and''will always evaluate to false". The narrowing was unsound:$aand$bcan 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 expressionwhose narrowing is a no-op and therefore gets dropped from the antecedent
(
$droppedNoOpConditions).no-op condition, since the antecedent no longer constrains that target.
tests/PHPStan/Analyser/nsrt/bug-14891.php: regression test covering thereported
===case plus the analogous cases fixed by the same change.Root cause
The holder mechanism turns
!(Left && Right)into implications like"if
Leftis true thenRightis false". The antecedent is built from typenarrowings of the operands. A relation like
$a === $bbetween two variables ofthe 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 "
Leftis true" — here just "$ais non-empty-string" —while the consequent still claims something about
$b($b === ''). Because theonly thing that tied
$binto the antecedent was the dropped$a === $brelation, the resulting holder "if
$ais non-empty-string then$bis''"fires whenever
$ais 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.phpreproduces the reported case withassertType()($astaysstring, andnon-empty-stringafterif ($a !== "")).It fails before the fix (
$awas'', then*NEVER*) and passes after.verified to fail before the fix and pass after:
==relational antecedent,$this->a === $this->b),(
$a === $b && $a > 0 && $b > 0, where$awas wrongly narrowed toint<min, 0>).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