Skip to content

Commit 6e24090

Browse files
phpstan-botphpstan-bot
authored
Emit SwitchConditionNode and report always-false switch case comparisons
1 parent a97215a commit 6e24090

17 files changed

Lines changed: 1029 additions & 3 deletions

‎conf/bleedingEdge.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,3 +24,4 @@ parameters:
2424
newOnNonObject: true
2525
unnecessaryNullCoalesce: true
2626
finiteTypesInHaystack: true
27+
switchConditionAlwaysFalse: true

‎conf/config.level4.neon‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ conditionalTags:
1616
phpstan.rules.rule: %featureToggles.unusedLabel%
1717
PHPStan\Rules\Comparison\ImpossibleInArrayHaystackFiniteTypesRule:
1818
phpstan.rules.rule: %featureToggles.finiteTypesInHaystack%
19+
PHPStan\Rules\Comparison\SwitchConditionRule:
20+
phpstan.rules.rule: %featureToggles.switchConditionAlwaysFalse%
1921

2022
parameters:
2123
checkAdvancedIsset: true
@@ -42,3 +44,8 @@ services:
4244
class: PHPStan\Rules\Comparison\ImpossibleInArrayHaystackFiniteTypesRule
4345
arguments:
4446
treatPhpDocTypesAsCertain: %treatPhpDocTypesAsCertain%
47+
48+
-
49+
class: PHPStan\Rules\Comparison\SwitchConditionRule
50+
arguments:
51+
treatPhpDocTypesAsCertain: %treatPhpDocTypesAsCertain%

‎conf/config.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ parameters:
5050
newOnNonObject: false
5151
unnecessaryNullCoalesce: false
5252
finiteTypesInHaystack: false
53+
switchConditionAlwaysFalse: false
5354
fileExtensions:
5455
- php
5556
checkAdvancedIsset: false

‎conf/parametersSchema.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ parametersSchema:
5353
newOnNonObject: bool()
5454
unnecessaryNullCoalesce: bool()
5555
finiteTypesInHaystack: bool()
56+
switchConditionAlwaysFalse: bool()
5657
])
5758
fileExtensions: listOf(string())
5859
checkAdvancedIsset: bool()

‎src/Analyser/NodeScopeResolver.php‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,8 @@
104104
use PHPStan\Node\PropertyHookStatementNode;
105105
use PHPStan\Node\ReturnStatement;
106106
use PHPStan\Node\StaticMethodCallableNode;
107+
use PHPStan\Node\SwitchConditionArm;
108+
use PHPStan\Node\SwitchConditionNode;
107109
use PHPStan\Node\UnreachableStatementNode;
108110
use PHPStan\Node\VariableAssignNode;
109111
use PHPStan\Node\VarTagChangedExpressionTypeNode;
@@ -2075,7 +2077,16 @@ public function processStmtNode(
20752077
$throwPoints = $condResult->getThrowPoints();
20762078
$impurePoints = $condResult->getImpurePoints();
20772079
$fullCondExpr = null;
2078-
foreach ($stmt->cases as $caseNode) {
2080+
$switchConditionArms = [];
2081+
$lastNonDefaultCaseKey = null;
2082+
foreach ($stmt->cases as $caseKey => $caseNode) {
2083+
if ($caseNode->cond === null) {
2084+
continue;
2085+
}
2086+
2087+
$lastNonDefaultCaseKey = $caseKey;
2088+
}
2089+
foreach ($stmt->cases as $caseKey => $caseNode) {
20792090
if ($caseNode->cond !== null) {
20802091
$condExpr = new BinaryOp\Equal($stmt->cond, $caseNode->cond);
20812092
$fullCondExpr = $fullCondExpr === null ? $condExpr : new BooleanOr($fullCondExpr, $condExpr);
@@ -2084,6 +2095,12 @@ public function processStmtNode(
20842095
$hasYield = $hasYield || $caseResult->hasYield();
20852096
$throwPoints = array_merge($throwPoints, $caseResult->getThrowPoints());
20862097
$impurePoints = array_merge($impurePoints, $caseResult->getImpurePoints());
2098+
$switchConditionArms[] = new SwitchConditionArm(
2099+
$caseNode->cond,
2100+
$scopeForBranches,
2101+
$caseNode->cond->getStartLine(),
2102+
$caseKey === $lastNonDefaultCaseKey,
2103+
);
20872104
$branchScope = $caseResult->getScope()->filterByTruthyValue($condExpr);
20882105
} else {
20892106
$hasDefaultCase = true;
@@ -2121,6 +2138,10 @@ public function processStmtNode(
21212138
}
21222139
}
21232140

2141+
if ($switchConditionArms !== []) {
2142+
$this->callNodeCallback($nodeCallback, new SwitchConditionNode($stmt->cond, $switchConditionArms, $stmt), $scope, $storage);
2143+
}
2144+
21242145
$exhaustive = $scopeForBranches->getType($stmt->cond) instanceof NeverType;
21252146

21262147
if (!$hasDefaultCase && !$exhaustive) {

‎src/Node/SwitchConditionArm.php‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Node;
4+
5+
use PhpParser\Node\Expr;
6+
use PHPStan\Analyser\Scope;
7+
8+
/**
9+
* A single non-default `case` of a `switch`, paired with the scope captured
10+
* right after the case condition was processed (which already excludes the
11+
* values matched by earlier terminating cases).
12+
*
13+
* @api
14+
*/
15+
final class SwitchConditionArm
16+
{
17+
18+
public function __construct(
19+
private Expr $caseCondition,
20+
private Scope $scope,
21+
private int $line,
22+
private bool $isLast,
23+
)
24+
{
25+
}
26+
27+
public function getCaseCondition(): Expr
28+
{
29+
return $this->caseCondition;
30+
}
31+
32+
public function getScope(): Scope
33+
{
34+
return $this->scope;
35+
}
36+
37+
public function getLine(): int
38+
{
39+
return $this->line;
40+
}
41+
42+
/**
43+
* Whether this is the last non-default `case` of the `switch` (only a
44+
* `default` may follow it), in which case an always-true comparison is fine
45+
* because it does not make any subsequent `case` unreachable. A trailing
46+
* `default` is not considered a `case` it would make unreachable.
47+
*/
48+
public function isLast(): bool
49+
{
50+
return $this->isLast;
51+
}
52+
53+
}

‎src/Node/SwitchConditionNode.php‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Node;
4+
5+
use Override;
6+
use PhpParser\Node\Expr;
7+
use PhpParser\Node\Stmt\Switch_;
8+
use PhpParser\NodeAbstract;
9+
10+
/**
11+
* Virtual node emitted once per `switch` statement. It pairs the switch subject
12+
* with each non-default `case` condition so rules can inspect the loose `==`
13+
* comparison the `switch` performs, using the scope captured at each case
14+
* (which already excludes the values matched by earlier cases).
15+
*
16+
* @api
17+
*/
18+
final class SwitchConditionNode extends NodeAbstract implements VirtualNode
19+
{
20+
21+
/**
22+
* @param SwitchConditionArm[] $arms
23+
*/
24+
public function __construct(
25+
private Expr $subject,
26+
private array $arms,
27+
Switch_ $originalNode,
28+
)
29+
{
30+
parent::__construct($originalNode->getAttributes());
31+
}
32+
33+
public function getSubject(): Expr
34+
{
35+
return $this->subject;
36+
}
37+
38+
/**
39+
* @return SwitchConditionArm[]
40+
*/
41+
public function getArms(): array
42+
{
43+
return $this->arms;
44+
}
45+
46+
#[Override]
47+
public function getType(): string
48+
{
49+
return 'PHPStan_Node_SwitchCondition';
50+
}
51+
52+
/**
53+
* @return string[]
54+
*/
55+
#[Override]
56+
public function getSubNodeNames(): array
57+
{
58+
return [];
59+
}
60+
61+
}

‎src/Reflection/InitializerExprTypeResolver.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1978,6 +1978,10 @@ public function resolveIdenticalType(Type $leftType, Type $rightType): TypeResul
19781978
*/
19791979
public function resolveEqualType(Type $leftType, Type $rightType): TypeResult
19801980
{
1981+
if ($leftType instanceof NeverType || $rightType instanceof NeverType) {
1982+
return new TypeResult(new ConstantBooleanType(false), []);
1983+
}
1984+
19811985
if (
19821986
($leftType->isEnum()->yes() && $rightType->isTrue()->no())
19831987
|| ($rightType->isEnum()->yes() && $leftType->isTrue()->no())

‎src/Rules/Comparison/SwitchConditionRule.php‎

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Comparison;
4+
5+
use PhpParser\Node;
6+
use PhpParser\Node\Expr\BinaryOp\Equal;
7+
use PHPStan\Analyser\CollectedDataEmitter;
8+
use PHPStan\Analyser\NodeCallbackInvoker;
9+
use PHPStan\Analyser\Scope;
10+
use PHPStan\Node\Printer\ExprPrinter;
11+
use PHPStan\Node\SwitchConditionNode;
12+
use PHPStan\Php\PhpVersion;
13+
use PHPStan\Rules\Rule;
14+
use PHPStan\Rules\RuleErrorBuilder;
15+
use PHPStan\Type\Type;
16+
use PHPStan\Type\VerbosityLevel;
17+
use function count;
18+
use function sprintf;
19+
20+
/**
21+
* @implements Rule<SwitchConditionNode>
22+
*/
23+
final class SwitchConditionRule implements Rule
24+
{
25+
26+
public function __construct(
27+
private ConstantConditionRuleHelper $constantConditionRuleHelper,
28+
private PossiblyImpureTipHelper $possiblyImpureTipHelper,
29+
private ConstantConditionInTraitHelper $constantConditionInTraitHelper,
30+
private ExprPrinter $exprPrinter,
31+
private PhpVersion $phpVersion,
32+
private bool $treatPhpDocTypesAsCertain,
33+
)
34+
{
35+
}
36+
37+
public function getNodeType(): string
38+
{
39+
return SwitchConditionNode::class;
40+
}
41+
42+
public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataEmitter $scope): array
43+
{
44+
$subject = $node->getSubject();
45+
$errors = [];
46+
$nextCaseIsDeadForType = false;
47+
$nextCaseIsDeadForNativeType = false;
48+
$seenCases = [];
49+
50+
foreach ($node->getArms() as $arm) {
51+
if (
52+
$nextCaseIsDeadForNativeType
53+
|| ($nextCaseIsDeadForType && $this->treatPhpDocTypesAsCertain)
54+
) {
55+
continue;
56+
}
57+
58+
$armScope = $arm->getScope();
59+
$caseCondition = $arm->getCaseCondition();
60+
61+
$caseConditionType = $armScope->getType($caseCondition);
62+
$finiteTypes = $caseConditionType->getFiniteTypes();
63+
if (count($finiteTypes) === 1) {
64+
$caseValueType = $finiteTypes[0];
65+
$firstSeen = null;
66+
foreach ($seenCases as $seenCase) {
67+
if ($this->isDuplicateCase($seenCase['type'], $caseValueType)) {
68+
$firstSeen = $seenCase;
69+
break;
70+
}
71+
}
72+
73+
if ($firstSeen !== null) {
74+
$errors[] = RuleErrorBuilder::message(sprintf(
75+
'Case %s in switch is a duplicate of case %s on line %d.',
76+
$this->exprPrinter->printExpr($caseCondition),
77+
$firstSeen['printed'],
78+
$firstSeen['line'],
79+
))->line($arm->getLine())->identifier('switch.duplicateCase')->build();
80+
continue;
81+
}
82+
83+
$seenCases[] = [
84+
'type' => $caseValueType,
85+
'printed' => $this->exprPrinter->printExpr($caseCondition),
86+
'line' => $arm->getLine(),
87+
];
88+
}
89+
90+
$conditionExpr = new Equal($subject, $caseCondition);
91+
92+
$conditionType = $armScope->getType($conditionExpr);
93+
if (!$this->isConstantBoolean($conditionType)) {
94+
$this->constantConditionInTraitHelper->emitNoError(self::class, $scope, $conditionExpr);
95+
continue;
96+
}
97+
if ($conditionType->isTrue()->yes()) {
98+
$nextCaseIsDeadForType = true;
99+
}
100+
101+
if (!$this->treatPhpDocTypesAsCertain) {
102+
$conditionNativeType = $armScope->getNativeType($conditionExpr);
103+
if (!$this->isConstantBoolean($conditionNativeType)) {
104+
$this->constantConditionInTraitHelper->emitNoError(self::class, $scope, $conditionExpr);
105+
continue;
106+
}
107+
if ($conditionNativeType->isTrue()->yes()) {
108+
$nextCaseIsDeadForNativeType = true;
109+
}
110+
}
111+
112+
$subjectType = $armScope->getType($subject);
113+
if ($this->isConstantBoolean($subjectType)) {
114+
$caseConditionStandaloneType = $this->constantConditionRuleHelper->getBooleanType($armScope, $caseCondition);
115+
if (!$this->isConstantBoolean($caseConditionStandaloneType)) {
116+
$this->constantConditionInTraitHelper->emitNoError(self::class, $scope, $conditionExpr);
117+
continue;
118+
}
119+
}
120+
121+
if ($conditionType->isFalse()->yes()) {
122+
$errorBuilder = RuleErrorBuilder::message(sprintf(
123+
'Switch condition comparison between %s and %s is always false.',
124+
$subjectType->describe(VerbosityLevel::value()),
125+
$caseConditionType->describe(VerbosityLevel::value()),
126+
))->line($arm->getLine())->identifier('switch.alwaysFalse');
127+
$this->possiblyImpureTipHelper->addTip($armScope, $conditionExpr, $errorBuilder);
128+
$ruleError = $errorBuilder->build();
129+
if ($scope->isInTrait()) {
130+
$this->constantConditionInTraitHelper->emitError(self::class, $scope, $conditionExpr, false, $ruleError);
131+
} else {
132+
$errors[] = $ruleError;
133+
}
134+
continue;
135+
}
136+
137+
if ($arm->isLast()) {
138+
$this->constantConditionInTraitHelper->emitNoError(self::class, $scope, $conditionExpr);
139+
continue;
140+
}
141+
142+
$errorBuilder = RuleErrorBuilder::message(sprintf(
143+
'Switch condition comparison between %s and %s is always true.',
144+
$subjectType->describe(VerbosityLevel::value()),
145+
$armScope->getType($caseCondition)->describe(VerbosityLevel::value()),
146+
))->line($arm->getLine())->identifier('switch.alwaysTrue')
147+
->tip('Remove remaining cases below this one and this error will disappear too.');
148+
$this->possiblyImpureTipHelper->addTip($armScope, $conditionExpr, $errorBuilder);
149+
$ruleError = $errorBuilder->build();
150+
if ($scope->isInTrait()) {
151+
$this->constantConditionInTraitHelper->emitError(self::class, $scope, $conditionExpr, true, $ruleError);
152+
} else {
153+
$errors[] = $ruleError;
154+
}
155+
}
156+
157+
return $errors;
158+
}
159+
160+
private function isConstantBoolean(Type $type): bool
161+
{
162+
return $type->isTrue()->yes() || $type->isFalse()->yes();
163+
}
164+
165+
/**
166+
* A later `case` is a duplicate of an earlier one when both match the exact
167+
* same set of subject values. Besides identical values, `switch` compares
168+
* with loose `==`, so two numerically-equal constants (e.g. 1, '1' and 1.0)
169+
* are duplicates too - they cannot be told apart by a `switch`.
170+
*/
171+
private function isDuplicateCase(Type $seenType, Type $caseValueType): bool
172+
{
173+
if ($seenType->equals($caseValueType)) {
174+
return true;
175+
}
176+
177+
return $seenType->looseCompare($caseValueType, $this->phpVersion)->isTrue()->yes();
178+
}
179+
180+
}

0 commit comments

Comments
 (0)