mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-12-23 05:50:54 +01:00
Add a linter rule to detect duplicate case
statements
Summary: Fixes T6843. Adds a linter rule to `ArcanistXHPASTLinter` to detect duplicate `case` statements within a `switch` statement. Test Plan: Added test cases. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: Korvin, epriestley Maniphest Tasks: T6843 Differential Revision: https://secure.phabricator.com/D11171
This commit is contained in:
parent
4e3df80584
commit
b31e9c0cfe
2 changed files with 70 additions and 1 deletions
|
@ -49,6 +49,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
|||
const LINT_EMPTY_STATEMENT = 47;
|
||||
const LINT_ARRAY_SEPARATOR = 48;
|
||||
const LINT_CONSTRUCTOR_PARENTHESES = 49;
|
||||
const LINT_DUPLICATE_SWITCH_CASE = 50;
|
||||
|
||||
private $naminghook;
|
||||
private $switchhook;
|
||||
|
@ -109,6 +110,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
|||
self::LINT_EMPTY_STATEMENT => 'Empty Block Statement',
|
||||
self::LINT_ARRAY_SEPARATOR => 'Array Separator',
|
||||
self::LINT_CONSTRUCTOR_PARENTHESES => 'Constructor Parentheses',
|
||||
self::LINT_DUPLICATE_SWITCH_CASE => 'Duplicate Case Statements',
|
||||
);
|
||||
}
|
||||
|
||||
|
@ -199,7 +201,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
|||
|
||||
public function getVersion() {
|
||||
// The version number should be incremented whenever a new rule is added.
|
||||
return '10';
|
||||
return '11';
|
||||
}
|
||||
|
||||
protected function resolveFuture($path, Future $future) {
|
||||
|
@ -271,6 +273,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
|||
'lintEmptyBlockStatements' => self::LINT_EMPTY_STATEMENT,
|
||||
'lintArraySeparator' => self::LINT_ARRAY_SEPARATOR,
|
||||
'lintConstructorParentheses' => self::LINT_CONSTRUCTOR_PARENTHESES,
|
||||
'lintSwitchStatements' => self::LINT_DUPLICATE_SWITCH_CASE,
|
||||
);
|
||||
|
||||
foreach ($method_codes as $method => $codes) {
|
||||
|
@ -2868,6 +2871,44 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
|||
}
|
||||
}
|
||||
|
||||
private function lintSwitchStatements(XHPASTNode $root) {
|
||||
$switch_statements = $root->selectDescendantsOfType('n_SWITCH');
|
||||
|
||||
foreach ($switch_statements as $switch_statement) {
|
||||
$case_statements = $switch_statement
|
||||
->getChildOfType(1, 'n_STATEMENT_LIST')
|
||||
->getChildrenOfType('n_CASE');
|
||||
$nodes_by_case = array();
|
||||
|
||||
foreach ($case_statements as $case_statement) {
|
||||
$case = $case_statement
|
||||
->getChildByIndex(0)
|
||||
->getSemanticString();
|
||||
$nodes_by_case[$case][] = $case_statement;
|
||||
}
|
||||
|
||||
foreach ($nodes_by_case as $case => $nodes) {
|
||||
if (count($nodes) <= 1) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$node = array_pop($nodes_by_case[$case]);
|
||||
$message = $this->raiseLintAtNode(
|
||||
$node,
|
||||
self::LINT_DUPLICATE_SWITCH_CASE,
|
||||
pht(
|
||||
'Duplicate case in switch statement. PHP will ignore all '.
|
||||
'but the first case.'));
|
||||
|
||||
$locations = array();
|
||||
foreach ($nodes_by_case[$case] as $node) {
|
||||
$locations[] = $this->getOtherLocation($node->getOffset());
|
||||
}
|
||||
$message->setOtherLocations($locations);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
public function getSuperGlobalNames() {
|
||||
return array(
|
||||
'$GLOBALS',
|
||||
|
|
|
@ -0,0 +1,28 @@
|
|||
<?php
|
||||
$x = null;
|
||||
$y = null;
|
||||
switch ($x) {}
|
||||
switch ($x) {
|
||||
case 1: break;
|
||||
case '1': break;
|
||||
case true: break;
|
||||
case null: break;
|
||||
default: break;
|
||||
}
|
||||
switch ($x) {
|
||||
case 'foo': break;
|
||||
case 'bar': break;
|
||||
case 'foo': break;
|
||||
|
||||
default:
|
||||
switch ($y) {
|
||||
case 'foo': break;
|
||||
case 'bar': break;
|
||||
case 'baz': break;
|
||||
case 'baz': break;
|
||||
}
|
||||
break;
|
||||
}
|
||||
~~~~~~~~~~
|
||||
error:15:3
|
||||
error:22:7
|
Loading…
Reference in a new issue