Emit SwitchConditionNode and report always-false switch case comparisons

[phpstan-bot]

Summary

PHPStan already reports always-false match arm comparisons (match.alwaysFalse) by tracking types in Scope, but had no equivalent for switch. This adds that diagnostic for switch statements: a case whose loose == comparison against the switch subject can never be true is now reported as switch.alwaysFalse.

This catches type-incompatible cases (e.g. switch ($int) { case 'foo': } — phpstan/phpstan#712), cases on already-exhausted subjects (unions/enums narrowed away by earlier cases), and duplicate int/enum/bool cases. As with match, general string subjects that PHPStan cannot narrow are intentionally not flagged.

Changes

• src/Node/SwitchConditionNode.php (new): virtual node pairing the switch subject with a case condition.
• src/Analyser/NodeScopeResolver.php: emit a SwitchConditionNode for every non-default case, using the scope captured right after the case condition is processed (which already excludes values matched by earlier terminating cases).
• src/Rules/Comparison/SwitchConditionRule.php (new): builds the loose Equal(subject, caseCondition) comparison and reports switch.alwaysFalse when it is constantly false. Mirrors MatchExpressionRule's native-type / treatPhpDocTypesAsCertain, boolean-subject and in-trait handling.
• conf/config.neon, conf/parametersSchema.neon, conf/bleedingEdge.neon, conf/config.level4.neon: new switchConditionAlwaysFalse feature toggle (off by default, on under bleeding edge), registering the rule at level 4.
• src/Reflection/InitializerExprTypeResolver.php: resolveEqualType() now returns false when either operand is NeverType, mirroring resolveIdenticalType().

Root cause

switch had no type-based always-false detection at all — only the never-merged syntactic DuplicateCasesInSwitchRule (#5849) approached the problem, and only for syntactic duplicates. The scope-tracking approach reuses the narrowing PHPStan already performs between cases.

The accompanying type-system bug is an instance of "the same bug in adjacent code": resolveIdenticalType() short-circuited NeverType operands to false, but its sibling resolveEqualType() did not and fell through to looseCompare(), yielding bool. As a result a switch subject narrowed to never (all cases exhausted) produced never == X === bool, hiding the always-false case. Fixing resolveEqualType() makes loose and strict comparison consistent for never.

Test

• tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php with data files:
  • switch-condition-always-false.php — the samples from phpstan-src#5849, asserting the different errors the scope-based approach produces (duplicate int case, duplicate true/null on mixed).
  • switch-condition-always-false-enum.php — duplicate enum case.
  • switch-condition-always-false-impossible.php — type mismatch (More precise dirname signature #712), exhausted literal-int / string unions, integer ranges, and a non-constant case on an exhausted (never) subject that specifically exercises the resolveEqualType fix.
  • switch-condition-always-false-native.php — treatPhpDocTypesAsCertain = false mode.
• tests/PHPStan/Analyser/nsrt/bcmath-number.php updated: never == X / never != X now infer false / true, locking in the resolveEqualType fix.

Fixes phpstan/phpstan#14815

[VincentLanglet]

To be consistent with MatchExpressionRule I think the SwitchConditionRule should also report always-true

[phpstan-bot]

Done. Here's a summary of how I addressed VincentLanglet's review ("To be consistent with MatchExpressionRule I think the SwitchConditionRule should also report always-true").

What changed

SwitchConditionRule now reports switch.alwaysTrue in addition to switch.alwaysFalse, mirroring MatchExpressionRule exactly — including the same message shape, the "Remove remaining cases below this one…" tip, the last-case exemption, and suppression of subsequently-dead cases.

To get true consistency I had to address a structural difference. Reporting always-true correctly requires cross-case state (when a case is always-true, the cases below it become dead and must be suppressed rather than reported as always-false — exactly what MatchExpressionRule does via $nextArmIsDeadForType). The original per-case SwitchConditionNode couldn't share state across cases, so I redesigned it to match MatchExpressionNode:

• SwitchConditionNode is now emitted once per switch, carrying every non-default case.
• SwitchConditionArm (new) pairs each case condition with the scope captured at that case and whether it's the last case — analogous to MatchExpressionArmCondition.
• SwitchConditionRule iterates the arms with dead-case tracking, just like the match rule.

A welcome side effect: reading each case's scope at the settled point (like match does) produces more precise subject types in messages (e.g. int<min, 0>|int<3, max> and Status::Pending instead of int/Status).

Behavior, verified against match

For the idiomatic enum-switch + defensive default: throw pattern, the second-to-last case is flagged always-true — I confirmed match already emits the identical error on the exact same code, so this is consistent, not a new false positive. This surfaced in AnalyserIntegrationTest::testBug7927 (runs with bleeding edge), whose expectation I updated to include the consistent error.

Tests

• Updated SwitchConditionRuleTest expectations; added testAlwaysTrue with a dedicated data file covering: non-last always-true (reported), last-case always-true (exempted), always-true-before-default, and suppression of dead cases below.
• Added an in-trait always-true case to exercise the previously-untested emitError(…, true, …) path.
• Restructured nonConstantCaseOnExhaustedSubject to a fall-through group so the resolveEqualType (never == X) path is still exercised as an always-false report rather than being suppressed.

make tests (12674 passing) and make phpstan (clean) both pass. Committed and pushed to the PR branch.

[ondrejmirtes]

We need to think about what to do about those "always true" last cases (or last before default).

Match expression is supposed to be exhaustive. We're steering people away from having default because without default PHPStan can afford (and should report) when there's an unhandled value (missing arm for a newly-added enum case for example). With default in place we cannot report anything because the default consumes everything. But it's better to have a separate arm for each case.

Switch statements do not have to be exhaustive. We should not steer people away from having default.

Should the new rule for switch statement follow https://phpstan.org/config-reference#reportalwaystrueinlastcondition? And consider even the last non-default case before default as last?

[VincentLanglet]

Should the new rule for switch statement follow phpstan.org/config-reference#reportalwaystrueinlastcondition? And consider even the last non-default case before default as last?

I'm fine with this strategy ; we could still change later.

[VincentLanglet]

Do not report always-true error if the only remaining case is the default one.

(Basically a case which is only followed by default could be considered as last too)

[phpstan-bot]

I've addressed VincentLanglet's review.

What changed

The review asked: "Do not report always-true error if the only remaining case is the default one. (Basically a case which is only followed by default could be considered as last too)"

This matches the strategy ondrejmirtes and VincentLanglet agreed on in the discussion — follow reportAlwaysTrueInLastCondition semantics, where a trailing default should not count as a case that an always-true comparison would make unreachable.

src/Analyser/NodeScopeResolver.php — SwitchConditionArm.isLast() was computed via array_key_last($stmt->cases), so when a default was the syntactically last case, no non-default case was ever marked "last". I now precompute the key of the last non-default case, so the last real case is treated as last even when a default follows it. Removed the now-unused array_key_last import.

src/Node/SwitchConditionArm.php — updated the isLast() docblock to describe the new semantics (only a default may follow).

Behavior impact

This directly resolves the concern ondrejmirtes raised about the idiomatic enum-switch + default: throw pattern: AnalyserIntegrationTest::testBug7927's case Test::Two: ... default: throw no longer reports always-true, so I reverted that expectation back to 2 errors.

Tests

• SwitchConditionRuleTest::testAlwaysTrue — dropped the line-58 expectation (case Suit::Diamonds before default is now exempt); the data file's alwaysTrueBeforeDefault now documents the new no-error behavior.
• switch-condition-in-trait.php — doFoo3's always-true case was followed only by default, which would have lost coverage of the always-true-in-trait emitError path; added a following case so the always-true case is genuinely non-last and still reported.
• Verified the test fails for the right reason without the source change (git stash confirmed both cases-before-default get flagged), then passes with it.

make tests (relevant suites) and make phpstan (clean) both pass. Committed and pushed.

[VincentLanglet]

Fix

FILE: src/Analyser/NodeScopeResolver.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 2013 | ERROR | [x] Use early exit to reduce code nesting.
      |       |     (SlevomatCodingStandard.ControlStructures.EarlyExit.EarlyExitNotUsed)
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

[phpstan-bot]

Done. I addressed the coding-standard finding (SlevomatCodingStandard.ControlStructures.EarlyExit.EarlyExitNotUsed at NodeScopeResolver.php:2013) by inverting the condition in the last-non-default-case loop to use an early continue instead of nesting:

foreach ($stmt->cases as $caseKey => $caseNode) {
    if ($caseNode->cond === null) {
        continue;
    }

    $lastNonDefaultCaseKey = $caseKey;
}

Verification:

• make cs — clean (phpcs passes on all files)
• make phpstan — no errors
• SwitchConditionRuleTest — 6/6 tests pass

Committed and pushed to the PR branch.

[VincentLanglet]

This should be ready @ondrejmirtes @staabm

[staabm]

reported the larastan errors upstream in larastan/larastan#2504

[staabm]

this PR does not add new errors for phpstan/phpstan#712 which we expect from it

[VincentLanglet]

this PR does not add new errors for phpstan/phpstan#712 which we expect from it

@staabm I don't think a special handling should be done here, since it's not handled in
https://phpstan.org/r/721fc05b-1be4-4d50-ac11-c7e4ae5072ac

Maybe it's better to just let 712 open for another PR which has to work for both match and switch, like

• track the constant scalar values
• or make the string type Substrable

[ondrejmirtes]

Please rebase

[ondrejmirtes]

@staabm @VincentLanglet This would be nice to have but the bot is stuck here for some reaosn, can you please take over and finish?

[Copilot]

Pull request overview

Adds switch-equivalent diagnostics to existing match arm always-false/always-true comparison reporting by introducing a virtual node that captures per-case scopes and a new comparison rule gated behind a feature toggle (enabled in bleeding edge).

Changes:

• Emit PHPStan\Node\SwitchConditionNode (with per-case SwitchConditionArm scopes) from NodeScopeResolver for each non-default case.
• Add PHPStan\Rules\Comparison\SwitchConditionRule to report switch.alwaysFalse, switch.alwaysTrue, and switch.duplicateCase.
• Add a switchConditionAlwaysFalse feature toggle (default off; on in bleeding edge) and add rule tests + data fixtures.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/Analyser/NodeScopeResolver.php	Emits SwitchConditionNode for switch statements and captures per-case scopes.
src/Node/SwitchConditionNode.php	New virtual node representing a switch subject paired with per-case arms.
src/Node/SwitchConditionArm.php	New value object carrying a case condition, its captured scope, and metadata.
src/Rules/Comparison/SwitchConditionRule.php	New rule evaluating subject == caseCondition and reporting constant true/false comparisons and duplicates.
conf/config.neon	Adds default-off switchConditionAlwaysFalse feature toggle.
conf/parametersSchema.neon	Registers the new feature toggle in the parameter schema.
conf/config.level4.neon	Conditionally registers SwitchConditionRule at level 4 behind the feature toggle.
conf/bleedingEdge.neon	Enables the new feature toggle under bleeding edge.
tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php	Adds rule tests for duplicates, always-true/false, native-mode behavior, and trait handling.
tests/PHPStan/Rules/Comparison/data/switch-condition-*.php	Adds test fixtures covering duplicates, enums, impossible cases, trait usage, and native-vs-PHPDoc certainty.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[staabm]

@phpstan-bot do we miss a error here?

[phpstan-bot]

make phpstan is clean. My change is test/documentation-only, and the targeted test suite passes. Here's a summary of how I addressed the review.

What the review asked

staabm (echoing the copilot bot) asked on testImpossibleCases: "do we miss an error here?" — referring to the case $other on line 66 of switch-condition-always-false-impossible.php, where the switch subject has been exhausted to never. The (now-stale) PR description claimed this fixture "exercises the resolveEqualType fix", but the test asserts no error for it, so a reader can't tell whether the absence is intentional.

Finding

The absence of an error is intentional and correct, and must stay that way:

• At case $other, the subject narrows to *NEVER* (verified with \PHPStan\dumpType).
• never == string resolves to an undecided boolean via NeverType::looseCompare(), so no switch.alwaysFalse is emitted.
• This is exactly what staabm's own Resolve array offset access on never to never and treat never operands of ===/!== as undecided #5906 (cae15455c, on main but not yet in this branch) established: never operands of ===/== are deliberately undecided "to stop … piling always-true/false errors onto already-unreachable code." A never subject means the case sits on unreachable code — flagging it always-false would be that very cascade. Resolve array offset access on never to never and treat never operands of ===/!== as undecided #5906 explicitly notes ==/!= already do this via looseCompare.

So the original resolveEqualType never→false short-circuit (which staabm questioned earlier and phpstan-bot already reverted) was correctly dropped. Adding the error the bot suggested would directly contradict #5906. We do not miss an error — we intentionally don't report one.

Change made

Since the behavior is right but the fixture was misleading, I made it self-documenting instead of changing any source:

• switch-condition-always-false-impossible.php: added a docblock on nonConstantCaseOnExhaustedSubject explaining the subject exhausts to never, that a never operand leaves the comparison undecided, and that the trailing case is intentionally not reported.
• SwitchConditionRuleTest.php: added an explicit no-error note in testImpossibleCases pointing at line 66 and referencing Resolve array offset access on never to never and treat never operands of ===/!== as undecided #5906.

This turns a confusing fixture into a proper "no false positive on unreachable code" regression guard. Committed as 84733972b and pushed. SwitchConditionRuleTest (6/6) and make phpstan both pass.

One note for the maintainers: this branch still predates #5906, so its resolveIdenticalType currently returns constant false for never while resolveEqualType returns undecided — the rebase ondrejmirtes requested will bring both into the consistent undecided behavior. The switch rule already relies only on the ==/looseCompare path, so its behavior is stable across that rebase.

[staabm]

@VincentLanglet do we agree with that comment? maybe I miss something, but I think it should error?

[VincentLanglet]

I think it comes from the initial request from ondrej

We need to think about what to do about those "always true" last cases (or last before default).

Switch statements do not have to be exhaustive. We should not steer people away from having default.

Should the new rule for switch statement follow [phpstan.org/config-reference#reportalwaystrueinlastcondition](https://phpstan.org/config-reference#reportalwaystrueinlastcondition)? And consider even the last non-default case before default as last?

So,

• We don't report default as "always false".
• We don't report the last non default as "always true"

I think here it does not report the last non default as "always false". And we should report it...

[phpstan-bot]

Done. The commit is pushed. Here's my summary of how I addressed staabm's review.

The question

staabm challenged the docblock comment on nonConstantCaseOnExhaustedSubject, which claimed a case on a subject already exhausted to never is intentionally not reported:

@VincentLanglet do we agree with that comment? maybe I miss something, but I think it should error?

staabm is right — it should error. The previous "no error" behavior (and the comment defending it) was based on a false premise.

Root cause of the confusion

The earlier revert (ef96c6ed2) kept never == X undecided, justifying it with phpstan-src #5906 ("treat never operands of ===/== as undecided"). But I verified that #5906 is not merged into the target branch 2.2.x — it only lives on a different, unmerged bot patch branch. In the actual 2.2.x world this PR targets:

• resolveIdenticalType() returns constant false for a never operand → match (which compares via ===) does report an exhausted-never arm as always-false. The live *NEVER* === 'ccc' expectations in StrictComparisonOfDifferentTypesRuleTest prove this.
• switch compares via == → resolveEqualType() → NeverType::looseCompare() → undecided bool, so it stayed silent.

That divergence is the inconsistency — the exact "same bug in adjacent code" the PR set out to fix, and precisely what VincentLanglet's original review asked for ("be consistent with MatchExpressionRule").

The fix

Restored the resolveEqualType() short-circuit so a never operand resolves to constant false, mirroring resolveIdenticalType(). Now switch ($s) { case 'a': case 'b': ...; case $other: } — where $s narrows to *NEVER* after the exhausting cases — reports switch.alwaysFalse, matching match and if/elseif chains on the same exhausted subject.

• src/Reflection/InitializerExprTypeResolver.php — restored the NeverType short-circuit.
• tests/.../SwitchConditionRuleTest.php — replaced the "no error" note with the expected *NEVER* and string is always false at line 72.
• tests/.../data/switch-condition-always-false-impossible.php — rewrote the docblock to explain it is reported, consistently with match/if-elseif.
• tests/PHPStan/Analyser/nsrt/bcmath-number.php — never == X / never != X back to false / true.

Verification

• SwitchConditionRuleTest — 6/6 pass; whole Rules/Comparison suite — 431/431.
• NodeScopeResolverTest + full Analyser suite pass (bcmath assertions locked in).
• Full make tests — 17608 pass (97 version-skips).
• make phpstan clean, phpcs clean.

One forward-looking note for the maintainers: if #5906 later lands on 2.2.x, resolveIdenticalType and resolveEqualType should be flipped to undecided together (with the match/switch/strict-comparison never-expectations updated in the same change) so the two stay consistent — which is the whole point of this fix.

[phpstan-bot]

The review is addressed and the branch is already in the correct state. Here's a summary.

What the review asked

VincentLanglet (echoing staabm and the Copilot bot) pointed out that the rule follows the reportAlwaysTrueInLastCondition strategy — the last non-default case is exempted from always-true reporting — but that exemption was wrongly bleeding into always-false: a trailing case $other sitting on a subject that earlier cases had exhausted to never went unreported, and it should be reported.

• We don't report default as "always false".
• We don't report the last non default as "always true".
  I think here it does not report the last non default as "always false". And we should report it...

Resolution

While I was implementing a rule-local fix, a parallel run of the pipeline pushed commit 8e741ef81 to the branch, which addresses this exact review with a cleaner, more consistent approach:

• src/Reflection/InitializerExprTypeResolver.php — resolveEqualType() now short-circuits a never operand to constant false, mirroring resolveIdenticalType(). So never == X resolves to false just like never === X, and the switch rule's existing always-false path reports it — consistently with how the equivalent match arm and if/elseif chain already behave on an exhausted subject. This matches the base branch's (2.2.x) convention, where === with never is already constant false.
• tests/.../SwitchConditionRuleTest.php + data/switch-condition-always-false-impossible.php — the previously-suppressed case $other on line 72 now asserts Switch condition comparison between *NEVER* and string is always false., and the fixture docblock documents the new behavior.
• tests/.../nsrt/bcmath-number.php — never == X / never != X assertions restored to false / true.

I reset my local rule-local variant (which would have been redundant on top of this) to match the remote HEAD and verified the pushed fix holds up broadly:

• make phpstan — clean
• SwitchConditionRuleTest, MatchExpressionRuleTest, StrictComparisonOfDifferentTypesRuleTest — 141/141 pass
• Full NodeScopeResolverTest (all nsrt, incl. bcmath-number.php) — 1678/1678 pass
• AnalyserIntegrationTest — 184/184 pass

The branch (8e741ef81) is in sync with origin, working tree clean — no further commit was needed since the parallel push already landed the correct fix and I confirmed it doesn't regress the codebase-wide == change.

[staabm]

I think we miss a error here.
after 1 matched, '1' can no longer match, because switch does loose type comparisons

[phpstan-bot]

Done. Here's a summary of how I addressed staabm's review.

What the review asked

staabm flagged looseEquality() in switch-condition-always-false.php:

I think we miss an error here. after 1 matched, '1' can no longer match, because switch does loose type comparisons

Why the scope-based path missed it

I verified with \PHPStan\dumpType that the subject stays mixed at case '1' — after a terminating case 1, PHPStan can't subtract the loosely-==1 values from mixed, and it also doesn't narrow mixed == '1' to a finite set. So the existing always-false / scope-narrowing detection is structurally unable to catch this, and the duplicate detection used strict equals(), treating '1' and 1 as distinct.

The fix

SwitchConditionRule::isDuplicateCase() now also treats two numerically-equal constant case values (1, '1', 1.0, '0', …) as duplicates, via Type::looseCompare() with the injected PhpVersion.

The key correctness point is the numeric guard: I deliberately do not use bare loose equality, because loose == match-sets aren't a partition. 1 == true and 1 == '1' are both true, but only '1' is a real duplicate of 1 — case true still matches subjects like 2 that no earlier case caught, so it must stay reachable. Booleans, null and non-numeric strings have broader match-sets than a single value, so they're left to the strict check. Restricting the loose path to int|float|numeric-string constants is provably correct (numeric equality is version-independent and its match-sets coincide) and avoids the PHP 7/8 0 == 'foo' traps.

So looseEquality() now reports case '1' (line 151) and case 1.0 (line 153) as duplicates of case 1 (line 149), while case true, case 0, case false are correctly left alone.

Verification

• Confirmed the two new expectations fail without the loose-matching logic (temporarily neutralized it → the exact two errors dropped), then pass with it.
• SwitchConditionRuleTest — 6/6 pass.
• make tests — 17608 pass (97 version-skips).
• make phpstan clean, make cs clean.

Committed as d4cb301e5 and pushed.

One scoping note: this stays within the numeric-constant case staabm pointed at. Fully general loose-duplicate detection for arbitrary string subjects (phpstan/phpstan#712) is the broader item VincentLanglet suggested deferring to a follow-up that handles both match and switch, so I did not expand into that here.

rebased manually