Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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 @@ -20,3 +20,4 @@ parameters:
reportMethodPurityOverride: true
checkDynamicConstantNameValues: true
unusedLabel: true
duplicateCasesInSwitch: true
5 changes: 5 additions & 0 deletions conf/config.level0.neon
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ conditionalTags:
phpstan.restrictedFunctionUsageExtension: %featureToggles.internalTag%
PHPStan\Rules\Classes\NewStaticInAbstractClassStaticMethodRule:
phpstan.rules.rule: %featureToggles.newStaticInAbstractClassStaticMethod%
PHPStan\Rules\Comparison\DuplicateCasesInSwitchRule:
phpstan.rules.rule: %featureToggles.duplicateCasesInSwitch%

services:
-
Expand All @@ -27,3 +29,6 @@ services:

-
class: PHPStan\Rules\InternalTag\RestrictedInternalFunctionUsageExtension

-
class: PHPStan\Rules\Comparison\DuplicateCasesInSwitchRule
1 change: 1 addition & 0 deletions conf/config.neon
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ parameters:
reportMethodPurityOverride: false
checkDynamicConstantNameValues: false
unusedLabel: false
duplicateCasesInSwitch: false
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 @@ -49,6 +49,7 @@ parametersSchema:
reportMethodPurityOverride: bool()
checkDynamicConstantNameValues: bool()
unusedLabel: bool()
duplicateCasesInSwitch: bool()
])
fileExtensions: listOf(string())
checkAdvancedIsset: bool()
Expand Down
83 changes: 83 additions & 0 deletions src/Rules/Comparison/DuplicateCasesInSwitchRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Comparison;

use PhpParser\Node;
use PhpParser\Node\Stmt\Switch_;
use PHPStan\Analyser\Scope;
use PHPStan\Node\Printer\ExprPrinter;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use function count;
use function sprintf;

/**
* @implements Rule<Switch_>
*/
final class DuplicateCasesInSwitchRule implements Rule
{

public function __construct(private ExprPrinter $exprPrinter)
{
}

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

public function processNode(Node $node, Scope $scope): array
{
$errors = [];
$seenCases = [];

foreach ($node->cases as $case) {
if ($case->cond === null) {
continue;
}

$condType = $scope->getType($case->cond);
$scalarValues = $condType->getConstantScalarValues();

if (count($scalarValues) === 1) {
$key = ['scalar', $scalarValues[0]];
} else {
$enumCases = $condType->getEnumCases();

if (count($enumCases) !== 1) {
continue;
}

$key = ['enum', $enumCases[0]->getClassName(), $enumCases[0]->getEnumCaseName()];
}

$firstSeen = null;

foreach ($seenCases as $seenCase) {
if ($seenCase['key'] === $key) {
$firstSeen = $seenCase;
break;
}
}

if ($firstSeen === null) {
$seenCases[] = [
'key' => $key,
'printed' => $this->exprPrinter->printExpr($case->cond),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expr printing can be expensive. maybe only remember the $case->cond and print the condition only when building the error?

'line' => $case->cond->getStartLine(),
];
continue;
}

$errors[] = RuleErrorBuilder::message(sprintf(
'Case %s in switch is a duplicate of case %s on line %d.',
$this->exprPrinter->printExpr($case->cond),
$firstSeen['printed'],
$firstSeen['line'],
))->identifier('switch.duplicateCase')->line($case->getStartLine())->build();
}

return $errors;
}

}
85 changes: 85 additions & 0 deletions tests/PHPStan/Rules/Comparison/DuplicateCasesInSwitchRuleTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Comparison;

use PHPStan\Node\Printer\ExprPrinter;
use PHPStan\Node\Printer\Printer;
use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use function define;
use function defined;

/**
* @extends RuleTestCase<DuplicateCasesInSwitchRule>
*/
class DuplicateCasesInSwitchRuleTest extends RuleTestCase
{

protected function getRule(): Rule
{
return new DuplicateCasesInSwitchRule(
new ExprPrinter(new Printer()),
);
}

public function testDuplicateCases(): void
{
if (!defined('DUPLICATE_SWITCH_CASE_CONST')) {
define('DUPLICATE_SWITCH_CASE_CONST', 'unknown');
}

$this->analyse([__DIR__ . '/data/duplicate-cases-in-switch.php'], [
[
'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,
],
[
'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 103.',
107,
],
[
'Case null in switch is a duplicate of case null on line 105.',
109,
],
]);
}

public function testDuplicateCasesEnum(): void
{
$this->analyse([__DIR__ . '/data/duplicate-cases-in-switch-enum.php'], [
[
'Case \DuplicateCasesInSwitchEnum\Status::Active in switch is a duplicate of case \DuplicateCasesInSwitchEnum\Status::Active on line 20.',
24,
],
]);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?php // lint >= 8.1

namespace DuplicateCasesInSwitchEnum;

enum Status: string
{

case Active = 'active';
case Inactive = 'inactive';
case Pending = 'pending';

}

class Foo
{

public function doFoo(Status $status): void
{
switch ($status) {
Comment thread
staabm marked this conversation as resolved.
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;
}
}

}
Loading
Loading