Skip to content
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
ecd6b4b
Emit SwitchConditionNode and report always-false switch case comparisons
ondrejmirtes Jun 12, 2026
1df998c
Add in-trait test coverage for SwitchConditionRule
phpstan-bot Jun 12, 2026
342ddec
Report always-true switch case comparisons
phpstan-bot Jun 12, 2026
07988ec
Treat the last non-default switch case as last for always-true reporting
phpstan-bot Jun 12, 2026
efc693e
Use early exit in last-non-default-case loop
phpstan-bot Jun 12, 2026
7f93065
Use @param mixed PHPDoc instead of native mixed type for PHP 7.4
phpstan-bot Jun 12, 2026
3d5be75
Use @return PHPDoc instead of native false/true types for PHP 7.4
phpstan-bot Jun 13, 2026
80a6a1a
Skip version-dependent SwitchConditionRule tests on older PHP
phpstan-bot Jun 13, 2026
522731e
Keep loose comparison with a never operand undecided
phpstan-bot Jun 27, 2026
39fefc7
Report duplicate switch cases
phpstan-bot Jun 27, 2026
3cc85ce
Identify duplicate switch cases by type instead of a hand-rolled key
phpstan-bot Jun 27, 2026
7df67da
fix bad merge
staabm Jul 18, 2026
21ff471
Document that a never switch subject is intentionally not always-false
phpstan-bot Jul 18, 2026
d4b98fb
Report always-false switch case on a never subject exhausted by earli…
phpstan-bot Jul 18, 2026
1d57aaa
remove outdated comments
staabm Jul 18, 2026
85b48e7
Report loosely-equal numeric switch cases as duplicates
phpstan-bot Jul 18, 2026
ca306a2
I don't see why we should handle only numeric constant values
staabm Jul 18, 2026
95594cf
fix build
staabm Jul 30, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions conf/bleedingEdge.neon
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,4 @@ parameters:
newOnNonObject: true
unnecessaryNullCoalesce: true
finiteTypesInHaystack: true
switchConditionAlwaysFalse: true
7 changes: 7 additions & 0 deletions conf/config.level4.neon
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -42,3 +44,8 @@ services:
class: PHPStan\Rules\Comparison\ImpossibleInArrayHaystackFiniteTypesRule
arguments:
treatPhpDocTypesAsCertain: %treatPhpDocTypesAsCertain%

-
class: PHPStan\Rules\Comparison\SwitchConditionRule
arguments:
treatPhpDocTypesAsCertain: %treatPhpDocTypesAsCertain%
1 change: 1 addition & 0 deletions conf/config.neon
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ parameters:
newOnNonObject: false
unnecessaryNullCoalesce: false
finiteTypesInHaystack: false
switchConditionAlwaysFalse: false
Comment on lines 49 to +53
fileExtensions:
- php
checkAdvancedIsset: false
Expand Down
1 change: 1 addition & 0 deletions conf/parametersSchema.neon
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ parametersSchema:
newOnNonObject: bool()
unnecessaryNullCoalesce: bool()
finiteTypesInHaystack: bool()
switchConditionAlwaysFalse: bool()
])
fileExtensions: listOf(string())
checkAdvancedIsset: bool()
Expand Down
23 changes: 22 additions & 1 deletion src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@
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;
use PHPStan\Node\VarTagChangedExpressionTypeNode;
Expand Down Expand Up @@ -2075,7 +2077,16 @@ public function processStmtNode(
$throwPoints = $condResult->getThrowPoints();
$impurePoints = $condResult->getImpurePoints();
$fullCondExpr = null;
foreach ($stmt->cases as $caseNode) {
$switchConditionArms = [];
$lastNonDefaultCaseKey = null;
foreach ($stmt->cases as $caseKey => $caseNode) {
if ($caseNode->cond === null) {
continue;
}

$lastNonDefaultCaseKey = $caseKey;
}
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);
Expand All @@ -2084,6 +2095,12 @@ public function processStmtNode(
$hasYield = $hasYield || $caseResult->hasYield();
$throwPoints = array_merge($throwPoints, $caseResult->getThrowPoints());
$impurePoints = array_merge($impurePoints, $caseResult->getImpurePoints());
$switchConditionArms[] = new SwitchConditionArm(
$caseNode->cond,
$scopeForBranches,
$caseNode->cond->getStartLine(),
$caseKey === $lastNonDefaultCaseKey,
);
$branchScope = $caseResult->getScope()->filterByTruthyValue($condExpr);
} else {
$hasDefaultCase = true;
Expand Down Expand Up @@ -2121,6 +2138,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) {
Expand Down
53 changes: 53 additions & 0 deletions src/Node/SwitchConditionArm.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<?php declare(strict_types = 1);

namespace PHPStan\Node;

use PhpParser\Node\Expr;
use PHPStan\Analyser\Scope;

/**
* A single non-default `case` of a `switch`, paired with the scope captured
* right after the case condition was processed (which already excludes the
* values matched by earlier terminating cases).
*
* @api
*/
final class SwitchConditionArm
{

public function __construct(
private Expr $caseCondition,
private Scope $scope,
private int $line,
private bool $isLast,
)
{
}

public function getCaseCondition(): Expr
{
return $this->caseCondition;
}

public function getScope(): Scope
{
return $this->scope;
}

public function getLine(): int
{
return $this->line;
}

/**
* 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
{
return $this->isLast;
}

}
61 changes: 61 additions & 0 deletions src/Node/SwitchConditionNode.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
<?php declare(strict_types = 1);

namespace PHPStan\Node;

use Override;
use PhpParser\Node\Expr;
use PhpParser\Node\Stmt\Switch_;
use PhpParser\NodeAbstract;

/**
* 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).
Comment on lines +11 to +14
*
* @api
*/
final class SwitchConditionNode extends NodeAbstract implements VirtualNode
{

/**
* @param SwitchConditionArm[] $arms
*/
public function __construct(
private Expr $subject,
private array $arms,
Switch_ $originalNode,
)
{
parent::__construct($originalNode->getAttributes());
}

public function getSubject(): Expr
{
return $this->subject;
}

/**
* @return SwitchConditionArm[]
*/
public function getArms(): array
{
return $this->arms;
}

#[Override]
public function getType(): string
{
return 'PHPStan_Node_SwitchCondition';
}

/**
* @return string[]
*/
#[Override]
public function getSubNodeNames(): array
{
return [];
}

}
4 changes: 4 additions & 0 deletions src/Reflection/InitializerExprTypeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -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), []);
}
Comment thread
staabm marked this conversation as resolved.

if (
($leftType->isEnum()->yes() && $rightType->isTrue()->no())
|| ($rightType->isEnum()->yes() && $leftType->isTrue()->no())
Expand Down
180 changes: 180 additions & 0 deletions src/Rules/Comparison/SwitchConditionRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Comparison;

use PhpParser\Node;
use PhpParser\Node\Expr\BinaryOp\Equal;
use PHPStan\Analyser\CollectedDataEmitter;
use PHPStan\Analyser\NodeCallbackInvoker;
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;
use PHPStan\Type\VerbosityLevel;
use function count;
use function sprintf;

/**
* @implements Rule<SwitchConditionNode>
*/
final class SwitchConditionRule implements Rule
{

public function __construct(
private ConstantConditionRuleHelper $constantConditionRuleHelper,
private PossiblyImpureTipHelper $possiblyImpureTipHelper,
private ConstantConditionInTraitHelper $constantConditionInTraitHelper,
private ExprPrinter $exprPrinter,
private PhpVersion $phpVersion,
private bool $treatPhpDocTypesAsCertain,
)
{
}

public function getNodeType(): string
{
return SwitchConditionNode::class;
}

public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataEmitter $scope): array
{
$subject = $node->getSubject();
$errors = [];
$nextCaseIsDeadForType = false;
$nextCaseIsDeadForNativeType = false;
$seenCases = [];

foreach ($node->getArms() as $arm) {
if (
$nextCaseIsDeadForNativeType
|| ($nextCaseIsDeadForType && $this->treatPhpDocTypesAsCertain)
) {
continue;
}

$armScope = $arm->getScope();
$caseCondition = $arm->getCaseCondition();

$caseConditionType = $armScope->getType($caseCondition);
$finiteTypes = $caseConditionType->getFiniteTypes();
if (count($finiteTypes) === 1) {
$caseValueType = $finiteTypes[0];
$firstSeen = null;
foreach ($seenCases as $seenCase) {
if ($this->isDuplicateCase($seenCase['type'], $caseValueType)) {
$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[] = [
'type' => $caseValueType,
'printed' => $this->exprPrinter->printExpr($caseCondition),
'line' => $arm->getLine(),
];
}

$conditionExpr = new Equal($subject, $caseCondition);

$conditionType = $armScope->getType($conditionExpr);
if (!$this->isConstantBoolean($conditionType)) {
$this->constantConditionInTraitHelper->emitNoError(self::class, $scope, $conditionExpr);
continue;
Comment thread
staabm marked this conversation as resolved.
}
if ($conditionType->isTrue()->yes()) {
$nextCaseIsDeadForType = true;
}

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;
}
}

$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()),
$caseConditionType->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;
}

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 $errors;
}

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`.
*/
private function isDuplicateCase(Type $seenType, Type $caseValueType): bool
{
if ($seenType->equals($caseValueType)) {
return true;
}

return $seenType->looseCompare($caseValueType, $this->phpVersion)->isTrue()->yes();
}

}
Loading
Loading