Skip to content

Skip degenerate no-op antecedents when building boolean conditional expression holders#5951

Merged
staabm merged 2 commits into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-z32uxw4
Jun 29, 2026
Merged

Skip degenerate no-op antecedents when building boolean conditional expression holders#5951
staabm merged 2 commits into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-z32uxw4

Conversation

@phpstan-bot

Copy link
Copy Markdown
Collaborator

Summary

When a non-strict in_array() (or any condition that cannot actually narrow its
argument) was the left operand of && in an if, reaching the following
elseif whose condition touched that same argument made PHPStan wrongly narrow
an unrelated variable. In the reported example:

function test($a, $b) {
	if (in_array($a, [1, 2]) && $b === true) {
	} elseif ($a == 3) {
		// PHPStan inferred $b as mixed~true here, so it reported
		// "Result of && is always false" and
		// "=== between mixed and true will always evaluate to false"
	}
}

$b should stay mixed in the elseif. The fix stops PHPStan from creating a
degenerate, trivially-true conditional implication that fired the consequent
narrowing unconditionally.

Changes

  • src/Analyser/ExprHandler/Helper/ConditionalExpressionHolderHelper.php: in
    processBooleanConditionalTypes(), the sureNotTypes loop now skips an
    antecedent whose condition type (intersect(scopeType, $type)) equals the
    current scope type — i.e. a no-op narrowing. This mirrors the guard the
    sureTypes loop already had (if ($scopeType->equals($conditionType)) continue;).

The helper is shared by BooleanAndHandler and BooleanOrHandler, so both &&
and || are covered by the single change.

Root cause

processBooleanConditionalTypes() builds "conditional expression holders" that
encode implications like if the antecedent narrows expression A to type X, then
narrow the consequent expression B to type Y
. These holders are stored in the
scope and fired later (in MutatingScope) whenever A gets re-specified.

For !(in_array($a, [1, 2]) && $b === true) the intended holder is
if in_array($a, [1, 2]) is true then $b !== true. A non-strict in_array
on a mixed needle cannot narrow $a at all, so its falsey specification
produced a sureNotType for $a whose intersection with the scope type was just
mixed again. The sureNotTypes branch stored that as the antecedent
condition $a = mixed, which is trivially satisfied. The resulting holder was
effectively (always) then $b !== true, so as soon as the elseif ($a == 3)
re-specified $a, the consequent fired and narrowed $b to mixed~true.

The pattern is "a boolean operand that does not narrow its argument still
contributes a condition". The sureTypes branch already avoided it; the
sureNotTypes branch did not. The fix applies the same equality guard there.

This is not specific to in_array() — any loose comparison with the same
no-narrowing shape triggers it, e.g. ($a == 1 || $a == 2) && $b === true.

Test

tests/PHPStan/Analyser/nsrt/bug-14878.php covers:

  • the reported case with elseif ($a == 3)$b is mixed;
  • the strict-=== elseif variant — $b is mixed;
  • the plain else variant (was already correct) — $b is mixed;
  • an analogous degenerate condition without in_array,
    ($a == 1 || $a == 2) && $b === true$b is mixed;
  • a guard that a genuine conditional holder still fires: with strict
    in_array($a, [1, 2], true) and elseif ($a === 1), $b is correctly
    narrowed to mixed~true.

The first, second and loose-equality cases produced mixed~true before the fix
and mixed after it.

Fixes phpstan/phpstan#14878

…xpression holders

- `ConditionalExpressionHolderHelper::processBooleanConditionalTypes()` builds
  conditional holders ("if antecedent narrows to X then consequent narrows to Y")
  that project a boolean operand's narrowing into later scopes.
- The `sureNotTypes` branch added a condition even when the antecedent narrowing
  was a no-op (`intersect(scopeType, $type)` equal to the current scope type, e.g.
  `$a = mixed`). A trivially-true antecedent makes the implication fire
  unconditionally, so re-specifying the antecedent variable later (e.g. in an
  `elseif ($a == 3)`) wrongly applied the consequent and narrowed an unrelated
  variable (`$b` to `mixed~true`).
- Mirror the guard already present in the `sureTypes` branch: skip the condition
  when the computed condition type equals the existing scope type.
- This covers both `&&` and `||` since `BooleanAndHandler` and `BooleanOrHandler`
  share the helper. The fix is not specific to `in_array()`: the same degenerate
  holder was produced by plain loose comparisons such as `($a == 1 || $a == 2)`.
- Legitimate conditional holders (e.g. strict `in_array($a, [1, 2], true)` whose
  antecedent genuinely narrows `$a`) still fire as before.
@VincentLanglet VincentLanglet requested a review from staabm June 29, 2026 12:17
@@ -0,0 +1,71 @@
<?php declare(strict_types = 1);

namespace Bug14878;

@staabm staabm Jun 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should reference this file from *RuleTest classes to make sure no emited error like

Result of && is always false/true.

or

Strict comparison using === between ... and ... will always evaluate to ....

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. I addressed staabm's review.

What the reviewer asked

Reference bug-14878.php from *RuleTest classes so a regression is caught by the actual rules that emitted the wrong errors — "Result of && is always false/true" and "Strict comparison using === … will always evaluate to …".

What I changed

  1. BooleanAndConstantConditionRuleTest and StrictComparisonOfDifferentTypesRuleTest — added testBug14878() analysing nsrt/bug-14878.php and expecting no errors.

  2. nsrt/bug-14878.php — the original data file's branches only contained assertType() calls, so referencing it from the rule tests would have passed even with the fix reverted (no comparison in the branch ⇒ no rule error). I added the statements that actually regressed, inside each no-narrowing branch:

    $b === true;                   // StrictComparison guard
    $result = $b === true && $cond; // BooleanAnd guard

    I had to pick operands carefully so the guards aren't constant for the wrong reason: $a === 3 becomes 3 === 3 once $a is narrowed in the strict elseif, and the right operand of && sees $b narrowed to true from the left — so I used a separate mixed $cond parameter for the right side.

Verification

I reconstructed the reproducer locally (the playground wasn't reachable) to confirm the exact errors, then proved the tests are real guards by reverting the source fix:

  • With the fix: all three tests pass; full BooleanAndConstantConditionRuleTest + StrictComparisonOfDifferentTypesRuleTest (112 tests) pass; make phpstan clean.
  • With the fix reverted: the nsrt type test fails ($b is mixed~true), StrictComparison reports "will always evaluate to false", and BooleanAnd reports "Result of && is always false" — exactly the errors the reviewer wanted guarded. (The testPlainElse case correctly stays clean even with the bug, matching the PR's note that the plain-else variant was already correct.)

Committed as cf817daa8 and pushed.

…son rules

Reference the regression test from BooleanAndConstantConditionRuleTest and
StrictComparisonOfDifferentTypesRuleTest, and add the `$b === true` /
`$b === true && $cond` statements inside the branches that previously emitted
"Result of && is always false" and "Strict comparison ... will always evaluate
to false" so those rule tests genuinely guard the fix.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@staabm staabm merged commit 6cc244e into phpstan:2.2.x Jun 29, 2026
965 of 967 checks passed
@staabm staabm deleted the create-pull-request/patch-z32uxw4 branch June 29, 2026 13:52
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 or value inference when using non strict in_array() in conditionnal

3 participants