Skip to content

Emit SwitchConditionNode and report always-false switch case comparisons#5854

Open
phpstan-bot wants to merge 3 commits into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-pndxujx
Open

Emit SwitchConditionNode and report always-false switch case comparisons#5854
phpstan-bot wants to merge 3 commits into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-pndxujx

Conversation

@phpstan-bot

Copy link
Copy Markdown
Collaborator

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.phptreatPhpDocTypesAsCertain = 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

- Add `SwitchConditionNode` virtual node, emitted by `NodeScopeResolver` for
  every non-default `case`, pairing the switch subject with the case condition.
- Add `SwitchConditionRule` (level 4, gated behind the bleeding-edge
  `switchConditionAlwaysFalse` feature toggle) which inspects the loose `==`
  comparison the `switch` performs in the scope captured at the case condition.
  That scope already excludes the values matched by earlier (terminating) cases,
  so the rule reports type-incompatible cases (phpstan/phpstan#712), exhausted
  unions/enums, integer-range/literal cases and duplicate int/enum/bool cases as
  `switch.alwaysFalse`. This mirrors how `MatchExpressionRule` reports
  `match.alwaysFalse`.
- Fix `InitializerExprTypeResolver::resolveEqualType()` to return
  `false` when either operand is `NeverType`, mirroring the existing handling in
  `resolveIdenticalType()`. Without this, `never == X` returned `bool`, so
  exhausted switch subjects (narrowed to `never`) were not detected.
- Update `nsrt/bcmath-number.php` expectations: `never == X` / `never != X` are
  now `false` / `true` (previously the inconsistent `bool`), matching `===`/`!==`.
Comment on lines +44 to +47
if (!$this->isConstantBoolean($conditionType)) {
$this->constantConditionInTraitHelper->emitNoError(self::class, $scope, $conditionExpr);
return [];
}

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.

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

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.

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 VincentLanglet left a comment

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.

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

phpstan-bot and others added 2 commits June 12, 2026 09:46
Mirror MatchExpressionRule's trait handling test: combine the rule with
ConstantConditionInTraitRule via CompositeRule and assert that an
always-false switch case inside a trait is reported once when constant in
every using class, while a case that is constant in only some contexts is
not reported.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Mirror MatchExpressionRule by also reporting switch cases whose loose `==`
comparison against the subject can never be false. A case is flagged
`switch.alwaysTrue` when the subject is narrowed (by earlier terminating
cases) to exactly match it, unless it is the last case of the switch — in
which case the always-true comparison makes nothing unreachable. As with
match, once a case is always-true the subsequent (now dead) cases are
suppressed instead of being reported as always-false.

To track this across cases, SwitchConditionNode is now emitted once per
switch carrying every non-default case together with the scope captured at
that case (like MatchExpressionNode), and SwitchConditionRule iterates them
with cross-case dead-case tracking.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@phpstan-bot

Copy link
Copy Markdown
Collaborator Author

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

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Contributor

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 VincentLanglet left a comment

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.

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)

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.

Detect always false switch case conditions

4 participants