From 440cee59682566c09344eb1c4537e8604064ea32 Mon Sep 17 00:00:00 2001 From: ondrejmirtes <104888+ondrejmirtes@users.noreply.github.com> Date: Fri, 12 Jun 2026 09:27:02 +0000 Subject: [PATCH 01/11] Emit SwitchConditionNode and report always-false switch case comparisons - 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 `===`/`!==`. --- conf/bleedingEdge.neon | 1 + conf/config.level4.neon | 7 + conf/config.neon | 1 + conf/parametersSchema.neon | 1 + src/Analyser/NodeScopeResolver.php | 2 + src/Node/SwitchConditionNode.php | 55 ++++++ .../InitializerExprTypeResolver.php | 4 + src/Rules/Comparison/SwitchConditionRule.php | 92 ++++++++++ tests/PHPStan/Analyser/nsrt/bcmath-number.php | 4 +- .../Comparison/SwitchConditionRuleTest.php | 109 +++++++++++ .../switch-condition-always-false-enum.php | 55 ++++++ ...itch-condition-always-false-impossible.php | 72 ++++++++ .../switch-condition-always-false-native.php | 27 +++ .../data/switch-condition-always-false.php | 171 ++++++++++++++++++ 14 files changed, 599 insertions(+), 2 deletions(-) create mode 100644 src/Node/SwitchConditionNode.php create mode 100644 src/Rules/Comparison/SwitchConditionRule.php create mode 100644 tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php create mode 100644 tests/PHPStan/Rules/Comparison/data/switch-condition-always-false-enum.php create mode 100644 tests/PHPStan/Rules/Comparison/data/switch-condition-always-false-impossible.php create mode 100644 tests/PHPStan/Rules/Comparison/data/switch-condition-always-false-native.php create mode 100644 tests/PHPStan/Rules/Comparison/data/switch-condition-always-false.php diff --git a/conf/bleedingEdge.neon b/conf/bleedingEdge.neon index 2ad962c8d32..b723d52f133 100644 --- a/conf/bleedingEdge.neon +++ b/conf/bleedingEdge.neon @@ -22,3 +22,4 @@ parameters: checkDynamicConstantNameValues: true unusedLabel: true newOnNonObject: true + switchConditionAlwaysFalse: true diff --git a/conf/config.level4.neon b/conf/config.level4.neon index cd0b60d4c16..1d2198e3b4a 100644 --- a/conf/config.level4.neon +++ b/conf/config.level4.neon @@ -14,6 +14,8 @@ conditionalTags: phpstan.rules.rule: %exceptions.check.tooWideThrowType% PHPStan\Rules\Keywords\UnusedLabelRule: phpstan.rules.rule: %featureToggles.unusedLabel% + PHPStan\Rules\Comparison\SwitchConditionRule: + phpstan.rules.rule: %featureToggles.switchConditionAlwaysFalse% parameters: checkAdvancedIsset: true @@ -35,3 +37,8 @@ services: - class: PHPStan\Rules\Keywords\UnusedLabelRule + + - + class: PHPStan\Rules\Comparison\SwitchConditionRule + arguments: + treatPhpDocTypesAsCertain: %treatPhpDocTypesAsCertain% diff --git a/conf/config.neon b/conf/config.neon index 0af76250814..b6917ab2d69 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -49,6 +49,7 @@ parameters: checkDynamicConstantNameValues: false unusedLabel: false newOnNonObject: false + switchConditionAlwaysFalse: false fileExtensions: - php checkAdvancedIsset: false diff --git a/conf/parametersSchema.neon b/conf/parametersSchema.neon index 7d1ed9047b5..668765b0d5f 100644 --- a/conf/parametersSchema.neon +++ b/conf/parametersSchema.neon @@ -51,6 +51,7 @@ parametersSchema: checkDynamicConstantNameValues: bool() unusedLabel: bool() newOnNonObject: bool() + switchConditionAlwaysFalse: bool() ]) fileExtensions: listOf(string()) checkAdvancedIsset: bool() diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 3f63434d03f..43f712411ee 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -106,6 +106,7 @@ use PHPStan\Node\PropertyHookStatementNode; use PHPStan\Node\ReturnStatement; use PHPStan\Node\StaticMethodCallableNode; +use PHPStan\Node\SwitchConditionNode; use PHPStan\Node\UnreachableStatementNode; use PHPStan\Node\VariableAssignNode; use PHPStan\Node\VarTagChangedExpressionTypeNode; @@ -2029,6 +2030,7 @@ public function processStmtNode( $hasYield = $hasYield || $caseResult->hasYield(); $throwPoints = array_merge($throwPoints, $caseResult->getThrowPoints()); $impurePoints = array_merge($impurePoints, $caseResult->getImpurePoints()); + $this->callNodeCallback($nodeCallback, new SwitchConditionNode($stmt->cond, $caseNode->cond, $caseNode), $scopeForBranches, $storage); $branchScope = $caseResult->getScope()->filterByTruthyValue($condExpr); } else { $hasDefaultCase = true; diff --git a/src/Node/SwitchConditionNode.php b/src/Node/SwitchConditionNode.php new file mode 100644 index 00000000000..428e21743d3 --- /dev/null +++ b/src/Node/SwitchConditionNode.php @@ -0,0 +1,55 @@ +getAttributes()); + } + + public function getSubject(): Expr + { + return $this->subject; + } + + public function getCaseCondition(): Expr + { + return $this->caseCondition; + } + + #[Override] + public function getType(): string + { + return 'PHPStan_Node_SwitchCondition'; + } + + /** + * @return string[] + */ + #[Override] + public function getSubNodeNames(): array + { + return []; + } + +} diff --git a/src/Reflection/InitializerExprTypeResolver.php b/src/Reflection/InitializerExprTypeResolver.php index 5599286bb8a..ddc3751d1f9 100644 --- a/src/Reflection/InitializerExprTypeResolver.php +++ b/src/Reflection/InitializerExprTypeResolver.php @@ -1978,6 +1978,10 @@ public function resolveIdenticalType(Type $leftType, Type $rightType): TypeResul */ public function resolveEqualType(Type $leftType, Type $rightType): TypeResult { + if ($leftType instanceof NeverType || $rightType instanceof NeverType) { + return new TypeResult(new ConstantBooleanType(false), []); + } + if ( ($leftType->isEnum()->yes() && $rightType->isTrue()->no()) || ($rightType->isEnum()->yes() && $leftType->isTrue()->no()) diff --git a/src/Rules/Comparison/SwitchConditionRule.php b/src/Rules/Comparison/SwitchConditionRule.php new file mode 100644 index 00000000000..94fda45cff5 --- /dev/null +++ b/src/Rules/Comparison/SwitchConditionRule.php @@ -0,0 +1,92 @@ + + */ +final class SwitchConditionRule implements Rule +{ + + public function __construct( + private ConstantConditionRuleHelper $constantConditionRuleHelper, + private PossiblyImpureTipHelper $possiblyImpureTipHelper, + private ConstantConditionInTraitHelper $constantConditionInTraitHelper, + private bool $treatPhpDocTypesAsCertain, + ) + { + } + + public function getNodeType(): string + { + return SwitchConditionNode::class; + } + + public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataEmitter $scope): array + { + $subject = $node->getSubject(); + $caseCondition = $node->getCaseCondition(); + $conditionExpr = new Equal($subject, $caseCondition); + + $conditionType = $scope->getType($conditionExpr); + if (!$this->isConstantBoolean($conditionType)) { + $this->constantConditionInTraitHelper->emitNoError(self::class, $scope, $conditionExpr); + return []; + } + + if (!$this->treatPhpDocTypesAsCertain) { + $conditionNativeType = $scope->getNativeType($conditionExpr); + if (!$this->isConstantBoolean($conditionNativeType)) { + $this->constantConditionInTraitHelper->emitNoError(self::class, $scope, $conditionExpr); + return []; + } + } + + $subjectType = $scope->getType($subject); + if ($this->isConstantBoolean($subjectType)) { + $caseConditionStandaloneType = $this->constantConditionRuleHelper->getBooleanType($scope, $caseCondition); + if (!$this->isConstantBoolean($caseConditionStandaloneType)) { + $this->constantConditionInTraitHelper->emitNoError(self::class, $scope, $conditionExpr); + return []; + } + } + + if (!$conditionType->isFalse()->yes()) { + $this->constantConditionInTraitHelper->emitNoError(self::class, $scope, $conditionExpr); + return []; + } + + $errorBuilder = RuleErrorBuilder::message(sprintf( + 'Switch condition comparison between %s and %s is always false.', + $subjectType->describe(VerbosityLevel::value()), + $scope->getType($caseCondition)->describe(VerbosityLevel::value()), + ))->line($caseCondition->getStartLine())->identifier('switch.alwaysFalse'); + $this->possiblyImpureTipHelper->addTip($scope, $conditionExpr, $errorBuilder); + $ruleError = $errorBuilder->build(); + + if ($scope->isInTrait()) { + $this->constantConditionInTraitHelper->emitError(self::class, $scope, $conditionExpr, false, $ruleError); + return []; + } + + return [$ruleError]; + } + + private function isConstantBoolean(Type $type): bool + { + return $type->isTrue()->yes() || $type->isFalse()->yes(); + } + +} diff --git a/tests/PHPStan/Analyser/nsrt/bcmath-number.php b/tests/PHPStan/Analyser/nsrt/bcmath-number.php index d78887e4efb..e16c6819209 100644 --- a/tests/PHPStan/Analyser/nsrt/bcmath-number.php +++ b/tests/PHPStan/Analyser/nsrt/bcmath-number.php @@ -393,8 +393,8 @@ public function bcVsNever(Number $a): void assertType('bool', $a > $b); assertType('bool', $a >= $b); assertType('*NEVER*', $a <=> $b); - assertType('bool', $a == $b); - assertType('bool', $a != $b); + assertType('false', $a == $b); + assertType('true', $a != $b); assertType('*NEVER*', $a & $b); assertType('*NEVER*', $a ^ $b); assertType('*NEVER*', $a | $b); diff --git a/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php new file mode 100644 index 00000000000..d8176e1b773 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php @@ -0,0 +1,109 @@ + + */ +class SwitchConditionRuleTest extends RuleTestCase +{ + + private bool $treatPhpDocTypesAsCertain = true; + + protected function getRule(): Rule + { + return new SwitchConditionRule( + new ConstantConditionRuleHelper( + new ImpossibleCheckTypeHelper( + self::createReflectionProvider(), + $this->getTypeSpecifier(), + $this->treatPhpDocTypesAsCertain, + ), + $this->treatPhpDocTypesAsCertain, + ), + new PossiblyImpureTipHelper(true), + self::getContainer()->getByType(ConstantConditionInTraitHelper::class), + $this->treatPhpDocTypesAsCertain, + ); + } + + protected function shouldTreatPhpDocTypesAsCertain(): bool + { + return $this->treatPhpDocTypesAsCertain; + } + + public function testRule(): void + { + if (!defined('DUPLICATE_SWITCH_CASE_CONST')) { + define('DUPLICATE_SWITCH_CASE_CONST', 'unknown'); + } + + $this->analyse([__DIR__ . '/data/switch-condition-always-false.php'], [ + [ + 'Switch condition comparison between int and 1 is always false.', + 46, + ], + [ + 'Switch condition comparison between mixed and true is always false.', + 107, + ], + [ + 'Switch condition comparison between mixed and null is always false.', + 109, + ], + ]); + } + + public function testRuleEnum(): void + { + $this->analyse([__DIR__ . '/data/switch-condition-always-false-enum.php'], [ + [ + 'Switch condition comparison between SwitchConditionAlwaysFalseEnum\Status and SwitchConditionAlwaysFalseEnum\Status::Active is always false.', + 24, + ], + ]); + } + + public function testImpossibleCases(): void + { + $this->analyse([__DIR__ . '/data/switch-condition-always-false-impossible.php'], [ + [ + 'Switch condition comparison between int and \'foo\' is always false.', + 11, + ], + [ + 'Switch condition comparison between 1|2|3 and 4 is always false.', + 22, + ], + [ + 'Switch condition comparison between \'a\'|\'b\' and \'c\' is always false.', + 39, + ], + [ + 'Switch condition comparison between int<5, max> and 1 is always false.', + 50, + ], + [ + 'Switch condition comparison between \'a\'|\'b\' and string is always false.', + 67, + ], + ]); + } + + public function testDoNotTreatPhpDocTypesAsCertain(): void + { + $this->treatPhpDocTypesAsCertain = false; + $this->analyse([__DIR__ . '/data/switch-condition-always-false-native.php'], [ + [ + 'Switch condition comparison between int and \'foo\' is always false.', + 11, + ], + ]); + } + +} diff --git a/tests/PHPStan/Rules/Comparison/data/switch-condition-always-false-enum.php b/tests/PHPStan/Rules/Comparison/data/switch-condition-always-false-enum.php new file mode 100644 index 00000000000..0646b061ae1 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/switch-condition-always-false-enum.php @@ -0,0 +1,55 @@ += 8.1 + +namespace SwitchConditionAlwaysFalseEnum; + +enum Status: string +{ + + case Active = 'active'; + case Inactive = 'inactive'; + case Pending = 'pending'; + +} + +class Foo +{ + + public function doFoo(Status $status): void + { + switch ($status) { + case Status::Active: + break; + case Status::Inactive: + break; + case Status::Active: + break; + } + } + + public function noDuplicates(Status $status): void + { + switch ($status) { + case Status::Active: + break; + case Status::Inactive: + break; + case Status::Pending: + break; + } + } + + public function backedEnumCaseIsNotADuplicateOfItsBackingValue(Status|string $value): void + { + switch ($value) { + case 'active': + break; + case Status::Active: + break; + case Status::Inactive: + break; + case 'inactive': + break; + } + } + +} diff --git a/tests/PHPStan/Rules/Comparison/data/switch-condition-always-false-impossible.php b/tests/PHPStan/Rules/Comparison/data/switch-condition-always-false-impossible.php new file mode 100644 index 00000000000..0a3eecbca87 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/switch-condition-always-false-impossible.php @@ -0,0 +1,72 @@ + $i + */ + public function integerRange(int $i): void + { + switch ($i) { + case 1: + break; + case 10: + break; + } + } + + /** + * @param 'a'|'b' $s + */ + public function nonConstantCaseOnExhaustedSubject(string $s, string $other): void + { + switch ($s) { + case 'a': + break; + case 'b': + break; + case $other: + break; + } + } + +} diff --git a/tests/PHPStan/Rules/Comparison/data/switch-condition-always-false-native.php b/tests/PHPStan/Rules/Comparison/data/switch-condition-always-false-native.php new file mode 100644 index 00000000000..03c1a99830e --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/switch-condition-always-false-native.php @@ -0,0 +1,27 @@ + $i + */ + public function phpDocOnly(int $i): void + { + switch ($i) { + case 1: + break; + } + } + +} diff --git a/tests/PHPStan/Rules/Comparison/data/switch-condition-always-false.php b/tests/PHPStan/Rules/Comparison/data/switch-condition-always-false.php new file mode 100644 index 00000000000..e4fb7ade6e0 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/switch-condition-always-false.php @@ -0,0 +1,171 @@ +weightInGrams = 1; + break; + case 'kg': + $this->weightInGrams = 1000; + break; + case 'mg': + $this->weightInGrams = 0; + break; + case 'lb': + $this->weightInGrams = 454; + break; + case 'oz': + $this->weightInGrams = 28; + break; + case 'lb': + $this->weightInGrams = 453; + break; + case 'oz': + $this->weightInGrams = 29; + break; + } + } + + public function intCases(int $i): void + { + switch ($i) { + case 1: + break; + case 2: + break; + case 1: + break; + } + } + + public function tripleDuplicate(string $s): void + { + switch ($s) { + case 'x': + break; + case 'y': + break; + case 'x': + break; + case 'x': + break; + } + } + + public function classConstant(string $operator): void + { + switch ($operator) { + case '=': + break; + case '<': + break; + case self::EQ: + break; + } + } + + public function globalConstant(string $s): void + { + switch ($s) { + case 'unknown': + break; + case DUPLICATE_SWITCH_CASE_CONST: + break; + } + } + + public function fallthroughGroups(string $s): void + { + switch ($s) { + case 'a': + case 'b': + doFoo(); + break; + case 'a': + doBar(); + break; + } + } + + public function boolAndNullCases(mixed $m): void + { + switch ($m) { + case true: + break; + case null: + break; + case true: + break; + case null: + break; + } + } + + public function defaultIsNotADuplicate(string $s): void + { + switch ($s) { + case 'a': + break; + default: + break; + case 'b': + break; + } + } + + public function nonConstantConditions(string $s, string $foo): void + { + switch ($s) { + case $foo: + break; + case $foo: + break; + case rand() === 1 ? 'a' : 'b': + break; + case rand() === 1 ? 'a' : 'b': + break; + } + } + + public function looseEquality(mixed $m): void + { + switch ($m) { + case 1: + break; + case '1': + break; + case 1.0: + break; + case true: + break; + case 0: + break; + case false: + break; + } + } + + public function separateSwitches(string $s): void + { + switch ($s) { + case 'a': + break; + } + + switch ($s) { + case 'a': + break; + } + } + +} From 4216a2b4e150cd8ff9cd4bff08889feaefe2f1ab Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Fri, 12 Jun 2026 09:46:41 +0000 Subject: [PATCH 02/11] Add in-trait test coverage for SwitchConditionRule 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 --- .../Comparison/SwitchConditionRuleTest.php | 36 +++++++---- .../data/switch-condition-in-trait.php | 60 +++++++++++++++++++ 2 files changed, 86 insertions(+), 10 deletions(-) create mode 100644 tests/PHPStan/Rules/Comparison/data/switch-condition-in-trait.php diff --git a/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php index d8176e1b773..0df59afe029 100644 --- a/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php @@ -3,12 +3,13 @@ namespace PHPStan\Rules\Comparison; use PHPStan\Rules\Rule; +use PHPStan\Testing\CompositeRule; use PHPStan\Testing\RuleTestCase; use function define; use function defined; /** - * @extends RuleTestCase + * @extends RuleTestCase */ class SwitchConditionRuleTest extends RuleTestCase { @@ -17,19 +18,23 @@ class SwitchConditionRuleTest extends RuleTestCase protected function getRule(): Rule { - return new SwitchConditionRule( - new ConstantConditionRuleHelper( - new ImpossibleCheckTypeHelper( - self::createReflectionProvider(), - $this->getTypeSpecifier(), + // @phpstan-ignore argument.type + return new CompositeRule([ + new SwitchConditionRule( + new ConstantConditionRuleHelper( + new ImpossibleCheckTypeHelper( + self::createReflectionProvider(), + $this->getTypeSpecifier(), + $this->treatPhpDocTypesAsCertain, + ), $this->treatPhpDocTypesAsCertain, ), + new PossiblyImpureTipHelper(true), + self::getContainer()->getByType(ConstantConditionInTraitHelper::class), $this->treatPhpDocTypesAsCertain, ), - new PossiblyImpureTipHelper(true), - self::getContainer()->getByType(ConstantConditionInTraitHelper::class), - $this->treatPhpDocTypesAsCertain, - ); + new ConstantConditionInTraitRule(), + ]); } protected function shouldTreatPhpDocTypesAsCertain(): bool @@ -106,4 +111,15 @@ public function testDoNotTreatPhpDocTypesAsCertain(): void ]); } + public function testInTrait(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/switch-condition-in-trait.php'], [ + [ + 'Switch condition comparison between true and false is always false.', + 21, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Comparison/data/switch-condition-in-trait.php b/tests/PHPStan/Rules/Comparison/data/switch-condition-in-trait.php new file mode 100644 index 00000000000..05f363d3b04 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/switch-condition-in-trait.php @@ -0,0 +1,60 @@ +doBar(): + break; + } + } + + public function doFoo2(): void + { + // always false + switch (true) { + case $this->doBar2(): + break; + } + } + +} + +class Foo +{ + + use FooTrait; + + public function doBar(): false + { + + } + + public function doBar2(): false + { + + } + +} + +class FooAnother +{ + + use FooTrait; + + public function doBar(): bool + { + + } + + public function doBar2(): false + { + + } + +} From 08605978e2955b8cd00d38a437dea84701fce8bf Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Fri, 12 Jun 2026 10:12:54 +0000 Subject: [PATCH 03/11] Report always-true switch case comparisons MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/Analyser/NodeScopeResolver.php | 17 ++- src/Node/SwitchConditionArm.php | 52 +++++++++ src/Node/SwitchConditionNode.php | 24 ++-- src/Rules/Comparison/SwitchConditionRule.php | 107 ++++++++++++------ .../Analyser/AnalyserIntegrationTest.php | 3 +- .../Comparison/SwitchConditionRuleTest.php | 45 ++++++-- ...itch-condition-always-false-impossible.php | 1 - .../data/switch-condition-always-true.php | 65 +++++++++++ .../data/switch-condition-in-trait.php | 21 ++++ 9 files changed, 279 insertions(+), 56 deletions(-) create mode 100644 src/Node/SwitchConditionArm.php create mode 100644 tests/PHPStan/Rules/Comparison/data/switch-condition-always-true.php diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 43f712411ee..dc37eefc153 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -106,6 +106,7 @@ use PHPStan\Node\PropertyHookStatementNode; use PHPStan\Node\ReturnStatement; use PHPStan\Node\StaticMethodCallableNode; +use PHPStan\Node\SwitchConditionArm; use PHPStan\Node\SwitchConditionNode; use PHPStan\Node\UnreachableStatementNode; use PHPStan\Node\VariableAssignNode; @@ -169,6 +170,7 @@ use function array_fill_keys; use function array_filter; use function array_key_exists; +use function array_key_last; use function array_keys; use function array_last; use function array_map; @@ -2021,7 +2023,9 @@ public function processStmtNode( $throwPoints = $condResult->getThrowPoints(); $impurePoints = $condResult->getImpurePoints(); $fullCondExpr = null; - foreach ($stmt->cases as $caseNode) { + $switchConditionArms = []; + $lastCaseKey = array_key_last($stmt->cases); + foreach ($stmt->cases as $caseKey => $caseNode) { if ($caseNode->cond !== null) { $condExpr = new BinaryOp\Equal($stmt->cond, $caseNode->cond); $fullCondExpr = $fullCondExpr === null ? $condExpr : new BooleanOr($fullCondExpr, $condExpr); @@ -2030,7 +2034,12 @@ public function processStmtNode( $hasYield = $hasYield || $caseResult->hasYield(); $throwPoints = array_merge($throwPoints, $caseResult->getThrowPoints()); $impurePoints = array_merge($impurePoints, $caseResult->getImpurePoints()); - $this->callNodeCallback($nodeCallback, new SwitchConditionNode($stmt->cond, $caseNode->cond, $caseNode), $scopeForBranches, $storage); + $switchConditionArms[] = new SwitchConditionArm( + $caseNode->cond, + $scopeForBranches, + $caseNode->cond->getStartLine(), + $caseKey === $lastCaseKey, + ); $branchScope = $caseResult->getScope()->filterByTruthyValue($condExpr); } else { $hasDefaultCase = true; @@ -2068,6 +2077,10 @@ public function processStmtNode( } } + if ($switchConditionArms !== []) { + $this->callNodeCallback($nodeCallback, new SwitchConditionNode($stmt->cond, $switchConditionArms, $stmt), $scope, $storage); + } + $exhaustive = $scopeForBranches->getType($stmt->cond) instanceof NeverType; if (!$hasDefaultCase && !$exhaustive) { diff --git a/src/Node/SwitchConditionArm.php b/src/Node/SwitchConditionArm.php new file mode 100644 index 00000000000..d565cd4795a --- /dev/null +++ b/src/Node/SwitchConditionArm.php @@ -0,0 +1,52 @@ +caseCondition; + } + + public function getScope(): Scope + { + return $this->scope; + } + + public function getLine(): int + { + return $this->line; + } + + /** + * Whether this is the last `case` of the `switch` (no other `case` or + * `default` follows it), in which case an always-true comparison is fine + * because it does not make any subsequent case unreachable. + */ + public function isLast(): bool + { + return $this->isLast; + } + +} diff --git a/src/Node/SwitchConditionNode.php b/src/Node/SwitchConditionNode.php index 428e21743d3..f17c11ae1fb 100644 --- a/src/Node/SwitchConditionNode.php +++ b/src/Node/SwitchConditionNode.php @@ -3,25 +3,28 @@ namespace PHPStan\Node; use Override; -use PhpParser\Node; use PhpParser\Node\Expr; +use PhpParser\Node\Stmt\Switch_; use PhpParser\NodeAbstract; /** - * Virtual node emitted for every non-default `case` of a `switch`. It pairs the - * switch subject with the case condition so rules can inspect the loose `==` - * comparison the `switch` performs, using the scope captured at the case - * condition (which already excludes the values matched by earlier cases). + * Virtual node emitted once per `switch` statement. It pairs the switch subject + * with each non-default `case` condition so rules can inspect the loose `==` + * comparison the `switch` performs, using the scope captured at each case + * (which already excludes the values matched by earlier cases). * * @api */ final class SwitchConditionNode extends NodeAbstract implements VirtualNode { + /** + * @param SwitchConditionArm[] $arms + */ public function __construct( private Expr $subject, - private Expr $caseCondition, - Node $originalNode, + private array $arms, + Switch_ $originalNode, ) { parent::__construct($originalNode->getAttributes()); @@ -32,9 +35,12 @@ public function getSubject(): Expr return $this->subject; } - public function getCaseCondition(): Expr + /** + * @return SwitchConditionArm[] + */ + public function getArms(): array { - return $this->caseCondition; + return $this->arms; } #[Override] diff --git a/src/Rules/Comparison/SwitchConditionRule.php b/src/Rules/Comparison/SwitchConditionRule.php index 94fda45cff5..1de05dfe7a5 100644 --- a/src/Rules/Comparison/SwitchConditionRule.php +++ b/src/Rules/Comparison/SwitchConditionRule.php @@ -37,51 +37,88 @@ public function getNodeType(): string public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataEmitter $scope): array { $subject = $node->getSubject(); - $caseCondition = $node->getCaseCondition(); - $conditionExpr = new Equal($subject, $caseCondition); + $errors = []; + $nextCaseIsDeadForType = false; + $nextCaseIsDeadForNativeType = false; - $conditionType = $scope->getType($conditionExpr); - if (!$this->isConstantBoolean($conditionType)) { - $this->constantConditionInTraitHelper->emitNoError(self::class, $scope, $conditionExpr); - return []; - } + foreach ($node->getArms() as $arm) { + if ( + $nextCaseIsDeadForNativeType + || ($nextCaseIsDeadForType && $this->treatPhpDocTypesAsCertain) + ) { + continue; + } - if (!$this->treatPhpDocTypesAsCertain) { - $conditionNativeType = $scope->getNativeType($conditionExpr); - if (!$this->isConstantBoolean($conditionNativeType)) { + $armScope = $arm->getScope(); + $caseCondition = $arm->getCaseCondition(); + $conditionExpr = new Equal($subject, $caseCondition); + + $conditionType = $armScope->getType($conditionExpr); + if (!$this->isConstantBoolean($conditionType)) { $this->constantConditionInTraitHelper->emitNoError(self::class, $scope, $conditionExpr); - return []; + continue; + } + if ($conditionType->isTrue()->yes()) { + $nextCaseIsDeadForType = true; } - } - $subjectType = $scope->getType($subject); - if ($this->isConstantBoolean($subjectType)) { - $caseConditionStandaloneType = $this->constantConditionRuleHelper->getBooleanType($scope, $caseCondition); - if (!$this->isConstantBoolean($caseConditionStandaloneType)) { - $this->constantConditionInTraitHelper->emitNoError(self::class, $scope, $conditionExpr); - return []; + if (!$this->treatPhpDocTypesAsCertain) { + $conditionNativeType = $armScope->getNativeType($conditionExpr); + if (!$this->isConstantBoolean($conditionNativeType)) { + $this->constantConditionInTraitHelper->emitNoError(self::class, $scope, $conditionExpr); + continue; + } + if ($conditionNativeType->isTrue()->yes()) { + $nextCaseIsDeadForNativeType = true; + } } - } - if (!$conditionType->isFalse()->yes()) { - $this->constantConditionInTraitHelper->emitNoError(self::class, $scope, $conditionExpr); - return []; - } + $subjectType = $armScope->getType($subject); + if ($this->isConstantBoolean($subjectType)) { + $caseConditionStandaloneType = $this->constantConditionRuleHelper->getBooleanType($armScope, $caseCondition); + if (!$this->isConstantBoolean($caseConditionStandaloneType)) { + $this->constantConditionInTraitHelper->emitNoError(self::class, $scope, $conditionExpr); + continue; + } + } + + if ($conditionType->isFalse()->yes()) { + $errorBuilder = RuleErrorBuilder::message(sprintf( + 'Switch condition comparison between %s and %s is always false.', + $subjectType->describe(VerbosityLevel::value()), + $armScope->getType($caseCondition)->describe(VerbosityLevel::value()), + ))->line($arm->getLine())->identifier('switch.alwaysFalse'); + $this->possiblyImpureTipHelper->addTip($armScope, $conditionExpr, $errorBuilder); + $ruleError = $errorBuilder->build(); + if ($scope->isInTrait()) { + $this->constantConditionInTraitHelper->emitError(self::class, $scope, $conditionExpr, false, $ruleError); + } else { + $errors[] = $ruleError; + } + continue; + } - $errorBuilder = RuleErrorBuilder::message(sprintf( - 'Switch condition comparison between %s and %s is always false.', - $subjectType->describe(VerbosityLevel::value()), - $scope->getType($caseCondition)->describe(VerbosityLevel::value()), - ))->line($caseCondition->getStartLine())->identifier('switch.alwaysFalse'); - $this->possiblyImpureTipHelper->addTip($scope, $conditionExpr, $errorBuilder); - $ruleError = $errorBuilder->build(); - - if ($scope->isInTrait()) { - $this->constantConditionInTraitHelper->emitError(self::class, $scope, $conditionExpr, false, $ruleError); - return []; + if ($arm->isLast()) { + $this->constantConditionInTraitHelper->emitNoError(self::class, $scope, $conditionExpr); + continue; + } + + $errorBuilder = RuleErrorBuilder::message(sprintf( + 'Switch condition comparison between %s and %s is always true.', + $subjectType->describe(VerbosityLevel::value()), + $armScope->getType($caseCondition)->describe(VerbosityLevel::value()), + ))->line($arm->getLine())->identifier('switch.alwaysTrue') + ->tip('Remove remaining cases below this one and this error will disappear too.'); + $this->possiblyImpureTipHelper->addTip($armScope, $conditionExpr, $errorBuilder); + $ruleError = $errorBuilder->build(); + if ($scope->isInTrait()) { + $this->constantConditionInTraitHelper->emitError(self::class, $scope, $conditionExpr, true, $ruleError); + } else { + $errors[] = $ruleError; + } } - return [$ruleError]; + return $errors; } private function isConstantBoolean(Type $type): bool diff --git a/tests/PHPStan/Analyser/AnalyserIntegrationTest.php b/tests/PHPStan/Analyser/AnalyserIntegrationTest.php index 06788c89415..9a35be2b316 100644 --- a/tests/PHPStan/Analyser/AnalyserIntegrationTest.php +++ b/tests/PHPStan/Analyser/AnalyserIntegrationTest.php @@ -1116,9 +1116,10 @@ public function testBug7927(): void { // crash $errors = $this->runAnalyse(__DIR__ . '/data/bug-7927.php'); - $this->assertCount(2, $errors); + $this->assertCount(3, $errors); $this->assertSame('Enum case Bug7927\Test::One does not have a value but the enum is backed with the "int" type.', $errors[0]->getMessage()); $this->assertSame('Enum case Bug7927\Test::Two does not have a value but the enum is backed with the "int" type.', $errors[1]->getMessage()); + $this->assertSame('Switch condition comparison between Bug7927\Test::Two and Bug7927\Test::Two is always true.', $errors[2]->getMessage()); } public static function getAdditionalConfigFiles(): array diff --git a/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php index 0df59afe029..b1af9a76acc 100644 --- a/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php @@ -50,15 +50,15 @@ public function testRule(): void $this->analyse([__DIR__ . '/data/switch-condition-always-false.php'], [ [ - 'Switch condition comparison between int and 1 is always false.', + 'Switch condition comparison between int|int<3, max> and 1 is always false.', 46, ], [ - 'Switch condition comparison between mixed and true is always false.', + 'Switch condition comparison between \'0\' and true is always false.', 107, ], [ - 'Switch condition comparison between mixed and null is always false.', + 'Switch condition comparison between \'0\' and null is always false.', 109, ], ]); @@ -68,14 +68,37 @@ public function testRuleEnum(): void { $this->analyse([__DIR__ . '/data/switch-condition-always-false-enum.php'], [ [ - 'Switch condition comparison between SwitchConditionAlwaysFalseEnum\Status and SwitchConditionAlwaysFalseEnum\Status::Active is always false.', + 'Switch condition comparison between SwitchConditionAlwaysFalseEnum\Status::Pending and SwitchConditionAlwaysFalseEnum\Status::Active is always false.', 24, ], ]); } + public function testAlwaysTrue(): void + { + $tipText = 'Remove remaining cases below this one and this error will disappear too.'; + $this->analyse([__DIR__ . '/data/switch-condition-always-true.php'], [ + [ + 'Switch condition comparison between SwitchConditionAlwaysTrue\Suit::Diamonds and SwitchConditionAlwaysTrue\Suit::Diamonds is always true.', + 21, + $tipText, + ], + [ + 'Switch condition comparison between 2 and 2 is always true.', + 46, + $tipText, + ], + [ + 'Switch condition comparison between SwitchConditionAlwaysTrue\Suit::Diamonds and SwitchConditionAlwaysTrue\Suit::Diamonds is always true.', + 58, + $tipText, + ], + ]); + } + public function testImpossibleCases(): void { + $tipText = 'Remove remaining cases below this one and this error will disappear too.'; $this->analyse([__DIR__ . '/data/switch-condition-always-false-impossible.php'], [ [ 'Switch condition comparison between int and \'foo\' is always false.', @@ -86,16 +109,17 @@ public function testImpossibleCases(): void 22, ], [ - 'Switch condition comparison between \'a\'|\'b\' and \'c\' is always false.', - 39, + 'Switch condition comparison between \'b\' and \'b\' is always true.', + 37, + $tipText, ], [ 'Switch condition comparison between int<5, max> and 1 is always false.', 50, ], [ - 'Switch condition comparison between \'a\'|\'b\' and string is always false.', - 67, + 'Switch condition comparison between *NEVER* and string is always false.', + 66, ], ]); } @@ -119,6 +143,11 @@ public function testInTrait(): void 'Switch condition comparison between true and false is always false.', 21, ], + [ + 'Switch condition comparison between true and true is always true.', + 30, + 'Remove remaining cases below this one and this error will disappear too.', + ], ]); } diff --git a/tests/PHPStan/Rules/Comparison/data/switch-condition-always-false-impossible.php b/tests/PHPStan/Rules/Comparison/data/switch-condition-always-false-impossible.php index 0a3eecbca87..02e88b38d66 100644 --- a/tests/PHPStan/Rules/Comparison/data/switch-condition-always-false-impossible.php +++ b/tests/PHPStan/Rules/Comparison/data/switch-condition-always-false-impossible.php @@ -61,7 +61,6 @@ public function nonConstantCaseOnExhaustedSubject(string $s, string $other): voi { switch ($s) { case 'a': - break; case 'b': break; case $other: diff --git a/tests/PHPStan/Rules/Comparison/data/switch-condition-always-true.php b/tests/PHPStan/Rules/Comparison/data/switch-condition-always-true.php new file mode 100644 index 00000000000..1fd95a47f5d --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/switch-condition-always-true.php @@ -0,0 +1,65 @@ += 8.1 + +namespace SwitchConditionAlwaysTrue; + +enum Suit +{ + + case Hearts; + case Diamonds; + +} + +class Foo +{ + + public function redundantCaseAfterExhaustiveEnum(Suit $suit): void + { + switch ($suit) { + case Suit::Hearts: + break; + case Suit::Diamonds: + break; + case Suit::Hearts: + break; + } + } + + public function lastCaseAlwaysTrueIsAllowed(Suit $suit): void + { + switch ($suit) { + case Suit::Hearts: + break; + case Suit::Diamonds: + break; + } + } + + /** + * @param 1|2 $i + */ + public function intUnion(int $i): void + { + switch ($i) { + case 1: + break; + case 2: + break; + case 3: + break; + } + } + + public function alwaysTrueBeforeDefault(Suit $suit): void + { + switch ($suit) { + case Suit::Hearts: + break; + case Suit::Diamonds: + break; + default: + break; + } + } + +} diff --git a/tests/PHPStan/Rules/Comparison/data/switch-condition-in-trait.php b/tests/PHPStan/Rules/Comparison/data/switch-condition-in-trait.php index 05f363d3b04..fcf0b9f826a 100644 --- a/tests/PHPStan/Rules/Comparison/data/switch-condition-in-trait.php +++ b/tests/PHPStan/Rules/Comparison/data/switch-condition-in-trait.php @@ -23,6 +23,17 @@ public function doFoo2(): void } } + public function doFoo3(): void + { + // always true + switch (true) { + case $this->doBar3(): + break; + default: + break; + } + } + } class Foo @@ -40,6 +51,11 @@ public function doBar2(): false } + public function doBar3(): true + { + + } + } class FooAnother @@ -57,4 +73,9 @@ public function doBar2(): false } + public function doBar3(): true + { + + } + } From dc2099a7d419faa597b4bbe58fda3d1860eb5528 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Fri, 12 Jun 2026 11:56:41 +0000 Subject: [PATCH 04/11] Treat the last non-default switch case as last for always-true reporting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `case` followed only by `default` does not make any subsequent `case` unreachable, so an always-true comparison there is fine — just like the genuinely last `case`. This mirrors reportAlwaysTrueInLastCondition and keeps the idiomatic enum-switch + `default: throw` pattern from being flagged. Co-Authored-By: Claude Opus 4.8 --- src/Analyser/NodeScopeResolver.php | 10 +++++++--- src/Node/SwitchConditionArm.php | 7 ++++--- tests/PHPStan/Analyser/AnalyserIntegrationTest.php | 3 +-- .../Rules/Comparison/SwitchConditionRuleTest.php | 5 ----- .../Comparison/data/switch-condition-in-trait.php | 2 ++ 5 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index dc37eefc153..74d9ca2f360 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -170,7 +170,6 @@ use function array_fill_keys; use function array_filter; use function array_key_exists; -use function array_key_last; use function array_keys; use function array_last; use function array_map; @@ -2024,7 +2023,12 @@ public function processStmtNode( $impurePoints = $condResult->getImpurePoints(); $fullCondExpr = null; $switchConditionArms = []; - $lastCaseKey = array_key_last($stmt->cases); + $lastNonDefaultCaseKey = null; + foreach ($stmt->cases as $caseKey => $caseNode) { + if ($caseNode->cond !== null) { + $lastNonDefaultCaseKey = $caseKey; + } + } foreach ($stmt->cases as $caseKey => $caseNode) { if ($caseNode->cond !== null) { $condExpr = new BinaryOp\Equal($stmt->cond, $caseNode->cond); @@ -2038,7 +2042,7 @@ public function processStmtNode( $caseNode->cond, $scopeForBranches, $caseNode->cond->getStartLine(), - $caseKey === $lastCaseKey, + $caseKey === $lastNonDefaultCaseKey, ); $branchScope = $caseResult->getScope()->filterByTruthyValue($condExpr); } else { diff --git a/src/Node/SwitchConditionArm.php b/src/Node/SwitchConditionArm.php index d565cd4795a..1dcfed79927 100644 --- a/src/Node/SwitchConditionArm.php +++ b/src/Node/SwitchConditionArm.php @@ -40,9 +40,10 @@ public function getLine(): int } /** - * Whether this is the last `case` of the `switch` (no other `case` or - * `default` follows it), in which case an always-true comparison is fine - * because it does not make any subsequent case unreachable. + * Whether this is the last non-default `case` of the `switch` (only a + * `default` may follow it), in which case an always-true comparison is fine + * because it does not make any subsequent `case` unreachable. A trailing + * `default` is not considered a `case` it would make unreachable. */ public function isLast(): bool { diff --git a/tests/PHPStan/Analyser/AnalyserIntegrationTest.php b/tests/PHPStan/Analyser/AnalyserIntegrationTest.php index 9a35be2b316..06788c89415 100644 --- a/tests/PHPStan/Analyser/AnalyserIntegrationTest.php +++ b/tests/PHPStan/Analyser/AnalyserIntegrationTest.php @@ -1116,10 +1116,9 @@ public function testBug7927(): void { // crash $errors = $this->runAnalyse(__DIR__ . '/data/bug-7927.php'); - $this->assertCount(3, $errors); + $this->assertCount(2, $errors); $this->assertSame('Enum case Bug7927\Test::One does not have a value but the enum is backed with the "int" type.', $errors[0]->getMessage()); $this->assertSame('Enum case Bug7927\Test::Two does not have a value but the enum is backed with the "int" type.', $errors[1]->getMessage()); - $this->assertSame('Switch condition comparison between Bug7927\Test::Two and Bug7927\Test::Two is always true.', $errors[2]->getMessage()); } public static function getAdditionalConfigFiles(): array diff --git a/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php index b1af9a76acc..635261277b2 100644 --- a/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php @@ -88,11 +88,6 @@ public function testAlwaysTrue(): void 46, $tipText, ], - [ - 'Switch condition comparison between SwitchConditionAlwaysTrue\Suit::Diamonds and SwitchConditionAlwaysTrue\Suit::Diamonds is always true.', - 58, - $tipText, - ], ]); } diff --git a/tests/PHPStan/Rules/Comparison/data/switch-condition-in-trait.php b/tests/PHPStan/Rules/Comparison/data/switch-condition-in-trait.php index fcf0b9f826a..2139165ea42 100644 --- a/tests/PHPStan/Rules/Comparison/data/switch-condition-in-trait.php +++ b/tests/PHPStan/Rules/Comparison/data/switch-condition-in-trait.php @@ -27,6 +27,8 @@ public function doFoo3(): void { // always true switch (true) { + case $this->doBar3(): + break; case $this->doBar3(): break; default: From c5911a5dc37083d693f371632e7078fce0ed8e51 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Fri, 12 Jun 2026 12:15:44 +0000 Subject: [PATCH 05/11] Use early exit in last-non-default-case loop Co-Authored-By: Claude Opus 4.8 --- src/Analyser/NodeScopeResolver.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 74d9ca2f360..031cb6e455a 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -2025,9 +2025,11 @@ public function processStmtNode( $switchConditionArms = []; $lastNonDefaultCaseKey = null; foreach ($stmt->cases as $caseKey => $caseNode) { - if ($caseNode->cond !== null) { - $lastNonDefaultCaseKey = $caseKey; + if ($caseNode->cond === null) { + continue; } + + $lastNonDefaultCaseKey = $caseKey; } foreach ($stmt->cases as $caseKey => $caseNode) { if ($caseNode->cond !== null) { From 9a85c41424b16c4e04300c9a349db7d33e72a421 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Fri, 12 Jun 2026 12:28:09 +0000 Subject: [PATCH 06/11] Use @param mixed PHPDoc instead of native mixed type for PHP 7.4 The switch-condition-always-false.php test data file used the native `mixed` type hint, which is only available on PHP 8.0+. Use a `@param mixed` PHPDoc instead so the file lints on PHP 7.4. Co-Authored-By: Claude Opus 4.8 --- .../Rules/Comparison/SwitchConditionRuleTest.php | 4 ++-- .../Comparison/data/switch-condition-always-false.php | 10 ++++++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php index 635261277b2..b10f0d06155 100644 --- a/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php @@ -55,11 +55,11 @@ public function testRule(): void ], [ 'Switch condition comparison between \'0\' and true is always false.', - 107, + 110, ], [ 'Switch condition comparison between \'0\' and null is always false.', - 109, + 112, ], ]); } diff --git a/tests/PHPStan/Rules/Comparison/data/switch-condition-always-false.php b/tests/PHPStan/Rules/Comparison/data/switch-condition-always-false.php index e4fb7ade6e0..1a7e34cffa8 100644 --- a/tests/PHPStan/Rules/Comparison/data/switch-condition-always-false.php +++ b/tests/PHPStan/Rules/Comparison/data/switch-condition-always-false.php @@ -97,7 +97,10 @@ public function fallthroughGroups(string $s): void } } - public function boolAndNullCases(mixed $m): void + /** + * @param mixed $m + */ + public function boolAndNullCases($m): void { switch ($m) { case true: @@ -137,7 +140,10 @@ public function nonConstantConditions(string $s, string $foo): void } } - public function looseEquality(mixed $m): void + /** + * @param mixed $m + */ + public function looseEquality($m): void { switch ($m) { case 1: From 3a19c53a6aca4dc26e23ab89a80de76b71125db8 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sat, 13 Jun 2026 17:13:52 +0000 Subject: [PATCH 07/11] Use @return PHPDoc instead of native false/true types for PHP 7.4 Co-Authored-By: Claude Opus 4.8 --- .../data/switch-condition-in-trait.php | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/tests/PHPStan/Rules/Comparison/data/switch-condition-in-trait.php b/tests/PHPStan/Rules/Comparison/data/switch-condition-in-trait.php index 2139165ea42..2827aff3479 100644 --- a/tests/PHPStan/Rules/Comparison/data/switch-condition-in-trait.php +++ b/tests/PHPStan/Rules/Comparison/data/switch-condition-in-trait.php @@ -43,17 +43,26 @@ class Foo use FooTrait; - public function doBar(): false + /** + * @return false + */ + public function doBar(): bool { } - public function doBar2(): false + /** + * @return false + */ + public function doBar2(): bool { } - public function doBar3(): true + /** + * @return true + */ + public function doBar3(): bool { } @@ -70,12 +79,18 @@ public function doBar(): bool } - public function doBar2(): false + /** + * @return false + */ + public function doBar2(): bool { } - public function doBar3(): true + /** + * @return true + */ + public function doBar3(): bool { } From 1d978d9f6d47be9215d6ca75d5508fdf45ed7e9c Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sat, 13 Jun 2026 17:33:48 +0000 Subject: [PATCH 08/11] Skip version-dependent SwitchConditionRule tests on older PHP Enum-based tests (testRuleEnum, testAlwaysTrue, testImpossibleCases) require PHP 8.1, so guard them with #[RequiresPhp]. The int == 'foo' loose comparison is only always-false since PHP 8.0 (before that a non-numeric string loosely equals 0), so testDoNotTreatPhpDocTypesAsCertain expects no error on PHP < 8.0. Co-Authored-By: Claude Opus 4.8 --- .../Rules/Comparison/SwitchConditionRuleTest.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php index b10f0d06155..cfdd4e901e7 100644 --- a/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php @@ -5,8 +5,10 @@ use PHPStan\Rules\Rule; use PHPStan\Testing\CompositeRule; use PHPStan\Testing\RuleTestCase; +use PHPUnit\Framework\Attributes\RequiresPhp; use function define; use function defined; +use const PHP_VERSION_ID; /** * @extends RuleTestCase @@ -64,6 +66,7 @@ public function testRule(): void ]); } + #[RequiresPhp('>= 8.1.0')] public function testRuleEnum(): void { $this->analyse([__DIR__ . '/data/switch-condition-always-false-enum.php'], [ @@ -74,6 +77,7 @@ public function testRuleEnum(): void ]); } + #[RequiresPhp('>= 8.1.0')] public function testAlwaysTrue(): void { $tipText = 'Remove remaining cases below this one and this error will disappear too.'; @@ -91,6 +95,7 @@ public function testAlwaysTrue(): void ]); } + #[RequiresPhp('>= 8.1.0')] public function testImpossibleCases(): void { $tipText = 'Remove remaining cases below this one and this error will disappear too.'; @@ -122,6 +127,13 @@ public function testImpossibleCases(): void public function testDoNotTreatPhpDocTypesAsCertain(): void { $this->treatPhpDocTypesAsCertain = false; + + if (PHP_VERSION_ID < 80000) { + // Before PHP 8.0 a non-numeric string loosely equals 0, so int == 'foo' is not always false. + $this->analyse([__DIR__ . '/data/switch-condition-always-false-native.php'], []); + return; + } + $this->analyse([__DIR__ . '/data/switch-condition-always-false-native.php'], [ [ 'Switch condition comparison between int and \'foo\' is always false.', From ec6d215988110e9a373f94c83b72e79938da76cc Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sat, 27 Jun 2026 07:32:06 +0000 Subject: [PATCH 09/11] Keep loose comparison with a never operand undecided Reverts the resolveEqualType() short-circuit that made `never == X` / `never != X` evaluate to a constant `false` / `true`. A `never` operand denotes unreachable code that carries no comparable value, so the result should stay an undecided `bool` (via NeverType::looseCompare()), matching the direction taken for strict comparison in phpstan-src#5906. Co-Authored-By: Claude Opus 4.8 --- src/Reflection/InitializerExprTypeResolver.php | 4 ---- tests/PHPStan/Analyser/nsrt/bcmath-number.php | 4 ++-- tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php | 4 ---- 3 files changed, 2 insertions(+), 10 deletions(-) diff --git a/src/Reflection/InitializerExprTypeResolver.php b/src/Reflection/InitializerExprTypeResolver.php index ddc3751d1f9..5599286bb8a 100644 --- a/src/Reflection/InitializerExprTypeResolver.php +++ b/src/Reflection/InitializerExprTypeResolver.php @@ -1978,10 +1978,6 @@ public function resolveIdenticalType(Type $leftType, Type $rightType): TypeResul */ public function resolveEqualType(Type $leftType, Type $rightType): TypeResult { - if ($leftType instanceof NeverType || $rightType instanceof NeverType) { - return new TypeResult(new ConstantBooleanType(false), []); - } - if ( ($leftType->isEnum()->yes() && $rightType->isTrue()->no()) || ($rightType->isEnum()->yes() && $leftType->isTrue()->no()) diff --git a/tests/PHPStan/Analyser/nsrt/bcmath-number.php b/tests/PHPStan/Analyser/nsrt/bcmath-number.php index e16c6819209..d78887e4efb 100644 --- a/tests/PHPStan/Analyser/nsrt/bcmath-number.php +++ b/tests/PHPStan/Analyser/nsrt/bcmath-number.php @@ -393,8 +393,8 @@ public function bcVsNever(Number $a): void assertType('bool', $a > $b); assertType('bool', $a >= $b); assertType('*NEVER*', $a <=> $b); - assertType('false', $a == $b); - assertType('true', $a != $b); + assertType('bool', $a == $b); + assertType('bool', $a != $b); assertType('*NEVER*', $a & $b); assertType('*NEVER*', $a ^ $b); assertType('*NEVER*', $a | $b); diff --git a/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php index cfdd4e901e7..df44b9eabc7 100644 --- a/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php @@ -117,10 +117,6 @@ public function testImpossibleCases(): void 'Switch condition comparison between int<5, max> and 1 is always false.', 50, ], - [ - 'Switch condition comparison between *NEVER* and string is always false.', - 66, - ], ]); } From 7043ed6318807eb80bb9a03b84356ef3195a7d4e Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sat, 27 Jun 2026 07:32:22 +0000 Subject: [PATCH 10/11] Report duplicate switch cases The scope-based always-false detection cannot flag a duplicate `case` when the switch subject is a general, non-narrowable type such as `string` (subtracting a single literal from `string` is not representable), so duplicates like the ones in phpstan/phpstan#712 went unreported. Track the constant scalar / enum-case value of each `case` condition and report a later `case` carrying a value already seen earlier as a `switch.duplicateCase` error pointing at the original case. This takes precedence over the always-false/always-true reporting for that arm, so every exact-duplicate case is reported the same way regardless of whether the subject type happens to narrow. Closes https://github.com/phpstan/phpstan/issues/712 Co-Authored-By: Claude Opus 4.8 --- src/Rules/Comparison/SwitchConditionRule.php | 53 +++++++++++++++++++ .../Comparison/SwitchConditionRuleTest.php | 38 +++++++++++-- 2 files changed, 87 insertions(+), 4 deletions(-) diff --git a/src/Rules/Comparison/SwitchConditionRule.php b/src/Rules/Comparison/SwitchConditionRule.php index 1de05dfe7a5..b522e6e35ad 100644 --- a/src/Rules/Comparison/SwitchConditionRule.php +++ b/src/Rules/Comparison/SwitchConditionRule.php @@ -7,11 +7,13 @@ use PHPStan\Analyser\CollectedDataEmitter; use PHPStan\Analyser\NodeCallbackInvoker; use PHPStan\Analyser\Scope; +use PHPStan\Node\Printer\ExprPrinter; use PHPStan\Node\SwitchConditionNode; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; use PHPStan\Type\Type; use PHPStan\Type\VerbosityLevel; +use function count; use function sprintf; /** @@ -24,6 +26,7 @@ public function __construct( private ConstantConditionRuleHelper $constantConditionRuleHelper, private PossiblyImpureTipHelper $possiblyImpureTipHelper, private ConstantConditionInTraitHelper $constantConditionInTraitHelper, + private ExprPrinter $exprPrinter, private bool $treatPhpDocTypesAsCertain, ) { @@ -40,6 +43,7 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataE $errors = []; $nextCaseIsDeadForType = false; $nextCaseIsDeadForNativeType = false; + $seenCases = []; foreach ($node->getArms() as $arm) { if ( @@ -51,6 +55,34 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataE $armScope = $arm->getScope(); $caseCondition = $arm->getCaseCondition(); + + $caseKey = $this->getCaseKey($armScope->getType($caseCondition)); + if ($caseKey !== null) { + $firstSeen = null; + foreach ($seenCases as $seenCase) { + if ($seenCase['key'] === $caseKey) { + $firstSeen = $seenCase; + break; + } + } + + if ($firstSeen !== null) { + $errors[] = RuleErrorBuilder::message(sprintf( + 'Case %s in switch is a duplicate of case %s on line %d.', + $this->exprPrinter->printExpr($caseCondition), + $firstSeen['printed'], + $firstSeen['line'], + ))->line($arm->getLine())->identifier('switch.duplicateCase')->build(); + continue; + } + + $seenCases[] = [ + 'key' => $caseKey, + 'printed' => $this->exprPrinter->printExpr($caseCondition), + 'line' => $arm->getLine(), + ]; + } + $conditionExpr = new Equal($subject, $caseCondition); $conditionType = $armScope->getType($conditionExpr); @@ -126,4 +158,25 @@ private function isConstantBoolean(Type $type): bool return $type->isTrue()->yes() || $type->isFalse()->yes(); } + /** + * Builds a comparable key identifying a single constant case value (scalar or + * enum case), or null when the case condition does not have one definite value. + * + * @return array{'scalar', int|float|string|bool|null}|array{'enum', string, string}|null + */ + private function getCaseKey(Type $caseConditionType): ?array + { + $scalarValues = $caseConditionType->getConstantScalarValues(); + if (count($scalarValues) === 1) { + return ['scalar', $scalarValues[0]]; + } + + $enumCases = $caseConditionType->getEnumCases(); + if (count($enumCases) === 1) { + return ['enum', $enumCases[0]->getClassName(), $enumCases[0]->getEnumCaseName()]; + } + + return null; + } + } diff --git a/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php index df44b9eabc7..4fbbc841ced 100644 --- a/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php @@ -2,6 +2,7 @@ namespace PHPStan\Rules\Comparison; +use PHPStan\Node\Printer\ExprPrinter; use PHPStan\Rules\Rule; use PHPStan\Testing\CompositeRule; use PHPStan\Testing\RuleTestCase; @@ -33,6 +34,7 @@ protected function getRule(): Rule ), new PossiblyImpureTipHelper(true), self::getContainer()->getByType(ConstantConditionInTraitHelper::class), + self::getContainer()->getByType(ExprPrinter::class), $this->treatPhpDocTypesAsCertain, ), new ConstantConditionInTraitRule(), @@ -52,15 +54,43 @@ public function testRule(): void $this->analyse([__DIR__ . '/data/switch-condition-always-false.php'], [ [ - 'Switch condition comparison between int|int<3, max> and 1 is always false.', + 'Case \'lb\' in switch is a duplicate of case \'lb\' on line 24.', + 30, + ], + [ + 'Case \'oz\' in switch is a duplicate of case \'oz\' on line 27.', + 33, + ], + [ + 'Case 1 in switch is a duplicate of case 1 on line 42.', 46, ], [ - 'Switch condition comparison between \'0\' and true is always false.', + 'Case \'x\' in switch is a duplicate of case \'x\' on line 54.', + 58, + ], + [ + 'Case \'x\' in switch is a duplicate of case \'x\' on line 54.', + 60, + ], + [ + 'Case self::EQ in switch is a duplicate of case \'=\' on line 68.', + 72, + ], + [ + 'Case DUPLICATE_SWITCH_CASE_CONST in switch is a duplicate of case \'unknown\' on line 80.', + 82, + ], + [ + 'Case \'a\' in switch is a duplicate of case \'a\' on line 90.', + 94, + ], + [ + 'Case true in switch is a duplicate of case true on line 106.', 110, ], [ - 'Switch condition comparison between \'0\' and null is always false.', + 'Case null in switch is a duplicate of case null on line 108.', 112, ], ]); @@ -71,7 +101,7 @@ public function testRuleEnum(): void { $this->analyse([__DIR__ . '/data/switch-condition-always-false-enum.php'], [ [ - 'Switch condition comparison between SwitchConditionAlwaysFalseEnum\Status::Pending and SwitchConditionAlwaysFalseEnum\Status::Active is always false.', + 'Case \SwitchConditionAlwaysFalseEnum\Status::Active in switch is a duplicate of case \SwitchConditionAlwaysFalseEnum\Status::Active on line 20.', 24, ], ]); From a77edf94392a2a41bfea74db1eb4a04a43012b18 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sat, 27 Jun 2026 07:45:57 +0000 Subject: [PATCH 11/11] Identify duplicate switch cases by type instead of a hand-rolled key Replace the bespoke scalar/enum case-key construction with the Type API: a case carries a single definite value when getFiniteTypes() returns exactly one type, and two such values are duplicates when they equals() each other. This works uniformly for scalars, enum cases, bool and null without reconstructing identity by hand, and follows the codebase convention of comparing types via equals() rather than ad-hoc keys. Co-Authored-By: Claude Opus 4.8 --- src/Rules/Comparison/SwitchConditionRule.php | 33 +++++--------------- 1 file changed, 7 insertions(+), 26 deletions(-) diff --git a/src/Rules/Comparison/SwitchConditionRule.php b/src/Rules/Comparison/SwitchConditionRule.php index b522e6e35ad..464071ec94a 100644 --- a/src/Rules/Comparison/SwitchConditionRule.php +++ b/src/Rules/Comparison/SwitchConditionRule.php @@ -56,11 +56,13 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataE $armScope = $arm->getScope(); $caseCondition = $arm->getCaseCondition(); - $caseKey = $this->getCaseKey($armScope->getType($caseCondition)); - if ($caseKey !== null) { + $caseConditionType = $armScope->getType($caseCondition); + $finiteTypes = $caseConditionType->getFiniteTypes(); + if (count($finiteTypes) === 1) { + $caseValueType = $finiteTypes[0]; $firstSeen = null; foreach ($seenCases as $seenCase) { - if ($seenCase['key'] === $caseKey) { + if ($seenCase['type']->equals($caseValueType)) { $firstSeen = $seenCase; break; } @@ -77,7 +79,7 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataE } $seenCases[] = [ - 'key' => $caseKey, + 'type' => $caseValueType, 'printed' => $this->exprPrinter->printExpr($caseCondition), 'line' => $arm->getLine(), ]; @@ -118,7 +120,7 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataE $errorBuilder = RuleErrorBuilder::message(sprintf( 'Switch condition comparison between %s and %s is always false.', $subjectType->describe(VerbosityLevel::value()), - $armScope->getType($caseCondition)->describe(VerbosityLevel::value()), + $caseConditionType->describe(VerbosityLevel::value()), ))->line($arm->getLine())->identifier('switch.alwaysFalse'); $this->possiblyImpureTipHelper->addTip($armScope, $conditionExpr, $errorBuilder); $ruleError = $errorBuilder->build(); @@ -158,25 +160,4 @@ private function isConstantBoolean(Type $type): bool return $type->isTrue()->yes() || $type->isFalse()->yes(); } - /** - * Builds a comparable key identifying a single constant case value (scalar or - * enum case), or null when the case condition does not have one definite value. - * - * @return array{'scalar', int|float|string|bool|null}|array{'enum', string, string}|null - */ - private function getCaseKey(Type $caseConditionType): ?array - { - $scalarValues = $caseConditionType->getConstantScalarValues(); - if (count($scalarValues) === 1) { - return ['scalar', $scalarValues[0]]; - } - - $enumCases = $caseConditionType->getEnumCases(); - if (count($enumCases) === 1) { - return ['enum', $enumCases[0]->getClassName(), $enumCases[0]->getEnumCaseName()]; - } - - return null; - } - }