From b31e9c0cfee0fdc5c5f68a758322f3b7e89488a7 Mon Sep 17 00:00:00 2001
From: Joshua Spence <josh@joshuaspence.com>
Date: Sun, 4 Jan 2015 16:49:57 +1100
Subject: [PATCH] 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
---
 src/lint/linter/ArcanistXHPASTLinter.php      | 43 ++++++++++++++++++-
 .../xhpast/duplicate-switch-case.lint-test    | 28 ++++++++++++
 2 files changed, 70 insertions(+), 1 deletion(-)
 create mode 100644 src/lint/linter/__tests__/xhpast/duplicate-switch-case.lint-test

diff --git a/src/lint/linter/ArcanistXHPASTLinter.php b/src/lint/linter/ArcanistXHPASTLinter.php
index b43edad6..f75082cb 100644
--- a/src/lint/linter/ArcanistXHPASTLinter.php
+++ b/src/lint/linter/ArcanistXHPASTLinter.php
@@ -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',
diff --git a/src/lint/linter/__tests__/xhpast/duplicate-switch-case.lint-test b/src/lint/linter/__tests__/xhpast/duplicate-switch-case.lint-test
new file mode 100644
index 00000000..309e04d6
--- /dev/null
+++ b/src/lint/linter/__tests__/xhpast/duplicate-switch-case.lint-test
@@ -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