From 68fa147033a38723df15d9dd358e8792749e3e49 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 1/8] 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 528084e3532..be21fdb25de 100644 --- a/conf/bleedingEdge.neon +++ b/conf/bleedingEdge.neon @@ -20,3 +20,4 @@ parameters: reportMethodPurityOverride: true checkDynamicConstantNameValues: true unusedLabel: 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 df8f46567a1..96e5846ba06 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -47,6 +47,7 @@ parameters: reportMethodPurityOverride: false checkDynamicConstantNameValues: false unusedLabel: false + switchConditionAlwaysFalse: false fileExtensions: - php checkAdvancedIsset: false diff --git a/conf/parametersSchema.neon b/conf/parametersSchema.neon index dc07b020e09..e9c585d339e 100644 --- a/conf/parametersSchema.neon +++ b/conf/parametersSchema.neon @@ -49,6 +49,7 @@ parametersSchema: reportMethodPurityOverride: bool() checkDynamicConstantNameValues: bool() unusedLabel: bool() + switchConditionAlwaysFalse: bool() ]) fileExtensions: listOf(string()) checkAdvancedIsset: bool() diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 9faedfce2f9..de482c4eedb 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; @@ -2014,6 +2015,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 99b28cd567b68fa8b4cd964e753d1cef2c522505 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Fri, 12 Jun 2026 09:46:41 +0000 Subject: [PATCH 2/8] 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 e2b5070aa6304b8f3dccce90761de15d8e35bca7 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Fri, 12 Jun 2026 10:12:54 +0000 Subject: [PATCH 3/8] 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 de482c4eedb..79a97901c8f 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; @@ -2006,7 +2008,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); @@ -2015,7 +2019,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; @@ -2053,6 +2062,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 418b57601d2..ab5854b3566 100644 --- a/tests/PHPStan/Analyser/AnalyserIntegrationTest.php +++ b/tests/PHPStan/Analyser/AnalyserIntegrationTest.php @@ -1115,9 +1115,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 03f2e93f14228e6a0d596a689a4f6b3a8d1ef3cd Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Fri, 12 Jun 2026 11:56:41 +0000 Subject: [PATCH 4/8] 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 79a97901c8f..1c420ad8fe1 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; @@ -2009,7 +2008,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); @@ -2023,7 +2027,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 ab5854b3566..418b57601d2 100644 --- a/tests/PHPStan/Analyser/AnalyserIntegrationTest.php +++ b/tests/PHPStan/Analyser/AnalyserIntegrationTest.php @@ -1115,10 +1115,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 2ff93e57a95b4735715d0a20975bb52f95827544 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Fri, 12 Jun 2026 12:15:44 +0000 Subject: [PATCH 5/8] 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 1c420ad8fe1..2aab6f5feac 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -2010,9 +2010,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 4eb19972561c63ad49d60475fb605beb89bc0188 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Fri, 12 Jun 2026 12:28:09 +0000 Subject: [PATCH 6/8] 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 36ba5284dd95b522a475b688967d2d6f6a68a0f5 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sat, 13 Jun 2026 17:13:52 +0000 Subject: [PATCH 7/8] 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 36c25f82bb931fa0d1fdea02973129c61716dea1 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sat, 13 Jun 2026 17:33:48 +0000 Subject: [PATCH 8/8] 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.',