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
Closes phpstan/phpstan#712

[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

[staabm]

not sure about this change, because in #5906 we seem to change to an undecided boolean instead (for ===)

[phpstan-bot]

The #712 example now correctly reports the 2×catchall and 2×unknown duplicates. Both review concerns are addressed.

Summary

I addressed staabm's two review points on PR #5854:

1. "This PR does not add new errors for #712 which we expect from it"

The scope-based always-false detection cannot flag a duplicate case when the switch subject is a general, non-narrowable type like string — subtracting a single literal from string isn't representable, so switch ($string) { case 'catchall'; ... case 'catchall'; } went unreported (I confirmed match has the same limitation, so this is inherent to the narrowing approach).

Fix (commit "Report duplicate switch cases"): SwitchConditionRule now tracks the constant scalar / enum-case value of each case condition (reusing the proven logic from the original #5849 rule) and reports a later case carrying an already-seen value as switch.duplicateCase, pointing at the original case line. This takes precedence over always-false/always-true for that arm, so every exact-duplicate case — string, int, enum, bool, null — is now reported consistently, regardless of whether the subject narrows. The #712 example now emits the expected 2×catchall / 2×unknown errors.

2. resolveEqualType never change conflicts with #5906's direction

#5906 changes strict comparison (===) with a never operand to an undecided boolean rather than constant false, on the basis that never denotes unreachable code carrying no comparable value.

Fix (commit "Keep loose comparison with a never operand undecided"): Reverted the resolveEqualType() short-circuit so never == X / never != X stay an undecided bool (via NeverType::looseCompare()), consistent with #5906. Reverted the bcmath-number.php assertions back to bool, and dropped the now-obsolete *NEVER*-subject expectation from the rule test (that case is no longer flagged — consistent with how match behaves under #5906).

Verification

• Full suite: 17498 tests pass (make tests)
• make phpstan clean, make cs clean
• Both changes committed separately and pushed to the PR branch.

One design note worth flagging for the reviewers: I folded duplicate detection into SwitchConditionRule rather than restoring #5849's separate rule, to avoid double-reporting (int/enum duplicates would otherwise be flagged as both switch.duplicateCase and switch.alwaysFalse). A side effect is that existing int/enum/bool/null duplicate cases now report as switch.duplicateCase instead of switch.alwaysFalse — more informative since the message points to the original case line, and coherent across all subject types.

[staabm]

why do we need case-key matching? we should be able to identify already subtracted values based on type

[phpstan-bot]

Done. I addressed staabm's review.

What changed

staabm asked: "why do we need case-key matching? we should be able to identify already subtracted values based on type"

The duplicate detection in SwitchConditionRule was building a hand-rolled identity key for each case value — ['scalar', $value] for scalars and ['enum', $class, $caseName] for enums — then comparing those arrays with ===. That reimplemented type identity by hand, which the codebase convention explicitly steers away from (compare types via equals(), don't reconstruct identity).

src/Rules/Comparison/SwitchConditionRule.php:

• A case carries a single definite value exactly when getFiniteTypes() returns one element — this unifies scalars, enum cases, bool and null into a single type-based check, replacing both branches of the old getCaseKey() helper.
• Two such values are duplicates when their Types equals() each other, so seenCases now stores the Type rather than a synthetic key.
• Removed the now-dead getCaseKey() method, and reused the already-computed $caseConditionType in the always-false describe() call instead of re-fetching it.

This is a behavior-preserving refactor: loose-but-distinct literals (1 vs '1', 0 vs false) still aren't duplicates because equals() is strict, and a backed enum case isn't a duplicate of its backing string — exactly what the existing tests assert.

Verification

• SwitchConditionRuleTest — 6/6 pass
• make tests — 17498 pass
• make phpstan — clean
• make cs — clean

Committed as "Identify duplicate switch cases by type instead of a hand-rolled key" and pushed to the PR branch.

[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