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

[staabm]

this rule is doing some trait magic, and it seems to miss tests for it

[phpstan-bot]

I've addressed the reviewer's comment. The SwitchConditionRule delegated in-trait handling to ConstantConditionInTraitHelper (emitting collected data instead of reporting directly when $scope->isInTrait()), but that path had no test.

What I did, mirroring MatchExpressionRuleTest:

1. tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php — wrapped the rule in CompositeRule together with ConstantConditionInTraitRule (the CollectedDataNode rule that consumes the emitted trait data), and added a testInTrait() method.

2. tests/PHPStan/Rules/Comparison/data/switch-condition-in-trait.php (new) — a FooTrait used by two classes:

   • doFoo() — case $this->doBar() where doBar() returns false in one class but bool in another. The case is always-false in only one context, so it's correctly not reported.
   • doFoo2() — case $this->doBar2() where doBar2() returns false in both classes, so it's always-false everywhere and is reported once at the case line.

This exercises both branches of the trait magic: the emitNoError suppression (inconsistent across contexts) and the emitError dedup-and-report-once path. Tests pass (5 tests, 5 assertions) and make phpstan is clean. The commit has been pushed.

[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]

Use mixed as phpdoc to be compatible with php 7.4

[phpstan-bot]

I've addressed VincentLanglet's review.

What changed

The reviewer flagged that switch-condition-always-false.php used the native mixed type hint (boolAndNullCases(mixed $m) and looseEquality(mixed $m)), which is only available on PHP 8.0+. The data file has no // lint >= 8.x marker, so it must lint on PHP 7.4.

tests/PHPStan/Rules/Comparison/data/switch-condition-always-false.php — replaced both native mixed $m parameter type hints with untyped $m plus a @param mixed $m PHPDoc.

tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php — the two added PHPDoc blocks shifted the duplicate case true / case null lines in boolAndNullCases down by 3, so I updated the testRule() expectations from lines 107/109 to 110/112.

Verification

• SwitchConditionRuleTest — 6/6 pass.
• make phpstan — clean.

Committed and pushed.

[VincentLanglet]

Same here