From ecd6b4bfda36391ff88a8cc0d49a98edc82fed9e 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/18] 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 | 5 + 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, 597 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 140a90ae453..ecae15e4ac1 100644 --- a/conf/bleedingEdge.neon +++ b/conf/bleedingEdge.neon @@ -24,3 +24,4 @@ parameters: newOnNonObject: true unnecessaryNullCoalesce: true finiteTypesInHaystack: true + switchConditionAlwaysFalse: true diff --git a/conf/config.level4.neon b/conf/config.level4.neon index 91bed20886f..1d41fed289b 100644 --- a/conf/config.level4.neon +++ b/conf/config.level4.neon @@ -16,6 +16,8 @@ conditionalTags: phpstan.rules.rule: %featureToggles.unusedLabel% PHPStan\Rules\Comparison\ImpossibleInArrayHaystackFiniteTypesRule: phpstan.rules.rule: %featureToggles.finiteTypesInHaystack% + PHPStan\Rules\Comparison\SwitchConditionRule: + phpstan.rules.rule: %featureToggles.switchConditionAlwaysFalse% parameters: checkAdvancedIsset: true @@ -40,5 +42,8 @@ services: - class: PHPStan\Rules\Comparison\ImpossibleInArrayHaystackFiniteTypesRule + + - + class: PHPStan\Rules\Comparison\SwitchConditionRule arguments: treatPhpDocTypesAsCertain: %treatPhpDocTypesAsCertain% diff --git a/conf/config.neon b/conf/config.neon index abbce19f87e..1cd6efbf7f9 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -50,6 +50,7 @@ parameters: newOnNonObject: false unnecessaryNullCoalesce: false finiteTypesInHaystack: false + switchConditionAlwaysFalse: false fileExtensions: - php checkAdvancedIsset: false diff --git a/conf/parametersSchema.neon b/conf/parametersSchema.neon index cf5d3b4ef6b..953bab24371 100644 --- a/conf/parametersSchema.neon +++ b/conf/parametersSchema.neon @@ -53,6 +53,7 @@ parametersSchema: newOnNonObject: bool() unnecessaryNullCoalesce: bool() finiteTypesInHaystack: bool() + switchConditionAlwaysFalse: bool() ]) fileExtensions: listOf(string()) checkAdvancedIsset: bool() diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 9df08b89aeb..8e6438c6795 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -104,6 +104,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; @@ -2084,6 +2085,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 ac40ef08960..7332aab8f36 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 a8574c4d469..0b61cfe2c42 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 1df998c92b81dc91f9aaf3eab1e7e11e72b98812 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Fri, 12 Jun 2026 09:46:41 +0000 Subject: [PATCH 02/18] 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 342ddecc4fadfcbff88a7bc66f1887fbdc711a19 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Fri, 12 Jun 2026 10:12:54 +0000 Subject: [PATCH 03/18] 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 8e6438c6795..b93f90a7b9e 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -104,6 +104,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; @@ -176,6 +177,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; @@ -2076,7 +2078,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); @@ -2085,7 +2089,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; @@ -2123,6 +2132,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 18218040b39..2bc2416e0a1 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 07988ec6e191b24335bcdd879014fc2edb69fc12 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Fri, 12 Jun 2026 11:56:41 +0000 Subject: [PATCH 04/18] 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 b93f90a7b9e..67860a3c50d 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -177,7 +177,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; @@ -2079,7 +2078,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); @@ -2093,7 +2097,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 2bc2416e0a1..18218040b39 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 efc693e3f66bb0817096515d06e3e5226e8a693c Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Fri, 12 Jun 2026 12:15:44 +0000 Subject: [PATCH 05/18] 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 67860a3c50d..e0dd24233d9 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -2080,9 +2080,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 7f930656ee83d732bb3e3b3dcdddf38886782bf3 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Fri, 12 Jun 2026 12:28:09 +0000 Subject: [PATCH 06/18] 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 3d5be7558c8892013335de6cfb27e5a74e0c3dcb Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sat, 13 Jun 2026 17:13:52 +0000 Subject: [PATCH 07/18] 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 80a6a1a6c1f098c18c8e0f2cca59b6f3345a3570 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sat, 13 Jun 2026 17:33:48 +0000 Subject: [PATCH 08/18] 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 522731ed0483308750459f15a3c00cb05877eee2 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sat, 27 Jun 2026 07:32:06 +0000 Subject: [PATCH 09/18] 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 7332aab8f36..ac40ef08960 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 0b61cfe2c42..a8574c4d469 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 39fefc7e01cec13e4537edc5c62a5617926b433b Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sat, 27 Jun 2026 07:32:22 +0000 Subject: [PATCH 10/18] 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 3cc85ce4c3220bd967ee6d2dce1b8e8487329fcc Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sat, 27 Jun 2026 07:45:57 +0000 Subject: [PATCH 11/18] 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; - } - } From 7df67da3ced47ebf8533ba871016f8219d771c66 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 18 Jul 2026 13:42:35 +0200 Subject: [PATCH 12/18] fix bad merge --- conf/config.level4.neon | 2 ++ 1 file changed, 2 insertions(+) diff --git a/conf/config.level4.neon b/conf/config.level4.neon index 1d41fed289b..4206d36d3c1 100644 --- a/conf/config.level4.neon +++ b/conf/config.level4.neon @@ -42,6 +42,8 @@ services: - class: PHPStan\Rules\Comparison\ImpossibleInArrayHaystackFiniteTypesRule + arguments: + treatPhpDocTypesAsCertain: %treatPhpDocTypesAsCertain% - class: PHPStan\Rules\Comparison\SwitchConditionRule From 21ff47166376f17113effb25df40449cc586f31e Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sat, 18 Jul 2026 12:31:14 +0000 Subject: [PATCH 13/18] Document that a never switch subject is intentionally not always-false A case whose subject was already exhausted to never sits on unreachable code. A never operand keeps the loose comparison undecided (see #5906, which established never operands of ===/== as undecided precisely to stop piling always-true/false errors onto unreachable code), so the trailing case is deliberately not reported. Clarify the misleading fixture and add an explicit no-error note to the test. Co-Authored-By: Claude Opus 4.8 --- tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php | 4 ++++ .../data/switch-condition-always-false-impossible.php | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php index 4fbbc841ced..c0477a0cb7e 100644 --- a/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php @@ -147,6 +147,10 @@ public function testImpossibleCases(): void 'Switch condition comparison between int<5, max> and 1 is always false.', 50, ], + // No error for the non-constant `case $other` on line 66: the earlier + // cases exhaust the subject to `never`, and a `never` operand keeps the + // loose comparison undecided rather than always-false (see #5906), so we + // deliberately do not pile an always-false report onto unreachable code. ]); } 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 02e88b38d66..72cc7c00395 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 @@ -55,6 +55,12 @@ public function integerRange(int $i): void } /** + * Once the earlier cases have exhausted the subject, it narrows to `never`. + * A `never` operand leaves the loose comparison undecided (never has no + * value to compare), so the trailing case is intentionally not reported as + * always-false: it sits on already-unreachable code and flagging it would + * just pile onto that. No error is expected here. + * * @param 'a'|'b' $s */ public function nonConstantCaseOnExhaustedSubject(string $s, string $other): void From d4b98fb24f1f995c7a05c488ae451b185a5b5710 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sat, 18 Jul 2026 12:42:11 +0000 Subject: [PATCH 14/18] Report always-false switch case on a never subject exhausted by earlier cases resolveEqualType() now short-circuits a `never` operand to constant `false`, mirroring resolveIdenticalType(). A switch subject narrowed to `never` because earlier cases exhausted every possible value now produces an always-false `switch.alwaysFalse` report, consistently with how the equivalent `match` arm (via `===`) and `if`/`elseif` chain already behave on an exhausted subject. Co-Authored-By: Claude Opus 4.8 --- src/Reflection/InitializerExprTypeResolver.php | 4 ++++ tests/PHPStan/Analyser/nsrt/bcmath-number.php | 4 ++-- .../PHPStan/Rules/Comparison/SwitchConditionRuleTest.php | 8 ++++---- .../data/switch-condition-always-false-impossible.php | 8 ++++---- 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/Reflection/InitializerExprTypeResolver.php b/src/Reflection/InitializerExprTypeResolver.php index ac40ef08960..7332aab8f36 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/tests/PHPStan/Analyser/nsrt/bcmath-number.php b/tests/PHPStan/Analyser/nsrt/bcmath-number.php index a8574c4d469..0b61cfe2c42 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 index c0477a0cb7e..4f8ab166bdc 100644 --- a/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php @@ -147,10 +147,10 @@ public function testImpossibleCases(): void 'Switch condition comparison between int<5, max> and 1 is always false.', 50, ], - // No error for the non-constant `case $other` on line 66: the earlier - // cases exhaust the subject to `never`, and a `never` operand keeps the - // loose comparison undecided rather than always-false (see #5906), so we - // deliberately do not pile an always-false report onto unreachable code. + [ + 'Switch condition comparison between *NEVER* and string is always false.', + 72, + ], ]); } 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 72cc7c00395..f2487138a36 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 @@ -56,10 +56,10 @@ public function integerRange(int $i): void /** * Once the earlier cases have exhausted the subject, it narrows to `never`. - * A `never` operand leaves the loose comparison undecided (never has no - * value to compare), so the trailing case is intentionally not reported as - * always-false: it sits on already-unreachable code and flagging it would - * just pile onto that. No error is expected here. + * A `never` operand makes the loose comparison always-false (consistently + * with strict comparison), so the trailing case is reported as always-false + * just like the equivalent `match` arm / `if`-`elseif` chain on an exhausted + * subject. * * @param 'a'|'b' $s */ From 1d57aaa058d345207eb4fae2ee509356dea4947c Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 18 Jul 2026 15:11:32 +0200 Subject: [PATCH 15/18] remove outdated comments --- tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php | 2 +- .../data/switch-condition-always-false-impossible.php | 6 ------ 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php index 4f8ab166bdc..4d0fd9e9bde 100644 --- a/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php @@ -149,7 +149,7 @@ public function testImpossibleCases(): void ], [ 'Switch condition comparison between *NEVER* and string is always false.', - 72, + 66, ], ]); } 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 f2487138a36..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 @@ -55,12 +55,6 @@ public function integerRange(int $i): void } /** - * Once the earlier cases have exhausted the subject, it narrows to `never`. - * A `never` operand makes the loose comparison always-false (consistently - * with strict comparison), so the trailing case is reported as always-false - * just like the equivalent `match` arm / `if`-`elseif` chain on an exhausted - * subject. - * * @param 'a'|'b' $s */ public function nonConstantCaseOnExhaustedSubject(string $s, string $other): void From 85b48e73f3c2e1296396967aec7f99e19fc7a1a3 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sat, 18 Jul 2026 14:13:05 +0000 Subject: [PATCH 16/18] Report loosely-equal numeric switch cases as duplicates `switch` compares subjects with loose `==`, so numerically-equal constant cases like `case 1`, `case '1'` and `case 1.0` all match the exact same subjects and cannot be told apart. Detect a later such case as a duplicate of an earlier one, in addition to the strict-equality check. Booleans, null and non-numeric strings are left to the strict check because their loose match sets are broader than a single value. Co-Authored-By: Claude Opus 4.8 --- src/Rules/Comparison/SwitchConditionRule.php | 32 ++++++++++++++++++- .../Comparison/SwitchConditionRuleTest.php | 10 ++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/src/Rules/Comparison/SwitchConditionRule.php b/src/Rules/Comparison/SwitchConditionRule.php index 464071ec94a..7f2a49d76a6 100644 --- a/src/Rules/Comparison/SwitchConditionRule.php +++ b/src/Rules/Comparison/SwitchConditionRule.php @@ -9,6 +9,7 @@ use PHPStan\Analyser\Scope; use PHPStan\Node\Printer\ExprPrinter; use PHPStan\Node\SwitchConditionNode; +use PHPStan\Php\PhpVersion; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; use PHPStan\Type\Type; @@ -27,6 +28,7 @@ public function __construct( private PossiblyImpureTipHelper $possiblyImpureTipHelper, private ConstantConditionInTraitHelper $constantConditionInTraitHelper, private ExprPrinter $exprPrinter, + private PhpVersion $phpVersion, private bool $treatPhpDocTypesAsCertain, ) { @@ -62,7 +64,7 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataE $caseValueType = $finiteTypes[0]; $firstSeen = null; foreach ($seenCases as $seenCase) { - if ($seenCase['type']->equals($caseValueType)) { + if ($this->isDuplicateCase($seenCase['type'], $caseValueType)) { $firstSeen = $seenCase; break; } @@ -160,4 +162,32 @@ private function isConstantBoolean(Type $type): bool return $type->isTrue()->yes() || $type->isFalse()->yes(); } + /** + * A later `case` is a duplicate of an earlier one when both match the exact + * same set of subject values. Besides identical values, `switch` compares + * with loose `==`, so two numerically-equal constants (e.g. 1, '1' and 1.0) + * are duplicates too - they cannot be told apart by a `switch`. Booleans, + * null and non-numeric strings are intentionally left to the strict check + * because their loose-comparison match sets are broader than a single value. + */ + private function isDuplicateCase(Type $seenType, Type $caseValueType): bool + { + if ($seenType->equals($caseValueType)) { + return true; + } + + if (!$this->isNumericConstant($seenType) || !$this->isNumericConstant($caseValueType)) { + return false; + } + + return $seenType->looseCompare($caseValueType, $this->phpVersion)->isTrue()->yes(); + } + + private function isNumericConstant(Type $type): bool + { + return $type->isInteger()->yes() + || $type->isFloat()->yes() + || $type->isNumericString()->yes(); + } + } diff --git a/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php index 4d0fd9e9bde..54b812c5c7a 100644 --- a/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php @@ -3,6 +3,7 @@ namespace PHPStan\Rules\Comparison; use PHPStan\Node\Printer\ExprPrinter; +use PHPStan\Php\PhpVersion; use PHPStan\Rules\Rule; use PHPStan\Testing\CompositeRule; use PHPStan\Testing\RuleTestCase; @@ -35,6 +36,7 @@ protected function getRule(): Rule new PossiblyImpureTipHelper(true), self::getContainer()->getByType(ConstantConditionInTraitHelper::class), self::getContainer()->getByType(ExprPrinter::class), + self::getContainer()->getByType(PhpVersion::class), $this->treatPhpDocTypesAsCertain, ), new ConstantConditionInTraitRule(), @@ -93,6 +95,14 @@ public function testRule(): void 'Case null in switch is a duplicate of case null on line 108.', 112, ], + [ + 'Case \'1\' in switch is a duplicate of case 1 on line 149.', + 151, + ], + [ + 'Case 1.0 in switch is a duplicate of case 1 on line 149.', + 153, + ], ]); } From ca306a27aa4227385bf92389f9014878e673be64 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 18 Jul 2026 16:49:10 +0200 Subject: [PATCH 17/18] I don't see why we should handle only numeric constant values --- src/Rules/Comparison/SwitchConditionRule.php | 15 +-------------- .../Rules/Comparison/SwitchConditionRuleTest.php | 8 ++++++++ 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/src/Rules/Comparison/SwitchConditionRule.php b/src/Rules/Comparison/SwitchConditionRule.php index 7f2a49d76a6..cc030c0b9ad 100644 --- a/src/Rules/Comparison/SwitchConditionRule.php +++ b/src/Rules/Comparison/SwitchConditionRule.php @@ -166,9 +166,7 @@ private function isConstantBoolean(Type $type): bool * A later `case` is a duplicate of an earlier one when both match the exact * same set of subject values. Besides identical values, `switch` compares * with loose `==`, so two numerically-equal constants (e.g. 1, '1' and 1.0) - * are duplicates too - they cannot be told apart by a `switch`. Booleans, - * null and non-numeric strings are intentionally left to the strict check - * because their loose-comparison match sets are broader than a single value. + * are duplicates too - they cannot be told apart by a `switch`. */ private function isDuplicateCase(Type $seenType, Type $caseValueType): bool { @@ -176,18 +174,7 @@ private function isDuplicateCase(Type $seenType, Type $caseValueType): bool return true; } - if (!$this->isNumericConstant($seenType) || !$this->isNumericConstant($caseValueType)) { - return false; - } - return $seenType->looseCompare($caseValueType, $this->phpVersion)->isTrue()->yes(); } - private function isNumericConstant(Type $type): bool - { - return $type->isInteger()->yes() - || $type->isFloat()->yes() - || $type->isNumericString()->yes(); - } - } diff --git a/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php index 54b812c5c7a..6e8a1fbadb7 100644 --- a/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php @@ -103,6 +103,14 @@ public function testRule(): void 'Case 1.0 in switch is a duplicate of case 1 on line 149.', 153, ], + [ + 'Case true in switch is a duplicate of case 1 on line 149.', + 155, + ], + [ + 'Case false in switch is a duplicate of case 0 on line 157.', + 159, + ], ]); } From 95594cfddd8f546e0f02f75037241572327775d6 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 30 Jul 2026 09:22:00 +0200 Subject: [PATCH 18/18] fix build --- tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php index 6e8a1fbadb7..cb11c9ba967 100644 --- a/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/SwitchConditionRuleTest.php @@ -26,11 +26,6 @@ protected function getRule(): Rule return new CompositeRule([ new SwitchConditionRule( new ConstantConditionRuleHelper( - new ImpossibleCheckTypeHelper( - self::createReflectionProvider(), - $this->getTypeSpecifier(), - $this->treatPhpDocTypesAsCertain, - ), $this->treatPhpDocTypesAsCertain, ), new PossiblyImpureTipHelper(true),