Merge pull request #1 from lovrop/master

Lint rule for duplicate keys in array literals

src/lint/linter/xhpast/ArcanistXHPASTLinter.php

@@ -46,6 +46,7 @@ class ArcanistXHPASTLinter extends ArcanistLinter {
   const LINT_CLASS_FILENAME_MISMATCH  = 19;
   const LINT_TAUTOLOGICAL_EXPRESSION  = 20;
   const LINT_PLUS_OPERATOR_ON_STRINGS = 21;
+  const LINT_DUPLICATE_KEYS_IN_ARRAY  = 22;
 
 
   public function getLintNameMap() {
@@ -71,6 +72,7 @@ class ArcanistXHPASTLinter extends ArcanistLinter {
       self::LINT_CLASS_FILENAME_MISMATCH  => 'Class-Filename Mismatch',
       self::LINT_TAUTOLOGICAL_EXPRESSION  => 'Tautological Expression',
       self::LINT_PLUS_OPERATOR_ON_STRINGS => 'Not String Concatenation',
+      self::LINT_DUPLICATE_KEYS_IN_ARRAY  => 'Duplicate Keys in Array',
     );
   }
 
@@ -149,6 +151,7 @@ class ArcanistXHPASTLinter extends ArcanistLinter {
     $this->lintPrimaryDeclarationFilenameMatch($root);
     $this->lintTautologicalExpressions($root);
     $this->lintPlusOperatorOnStrings($root);
+    $this->lintDuplicateKeysInArray($root);
   }
 
   private function lintTautologicalExpressions($root) {
@@ -1008,6 +1011,60 @@ class ArcanistXHPASTLinter extends ArcanistLinter {
     }
   }
 
+  /**
+   * Finds duplicate keys in array initializers, as in
+   * array(1 => 'anything', 1 => 'foo').  Since the first entry is ignored,
+   * this is almost certainly an error.
+   */
+  private function lintDuplicateKeysInArray($root) {
+    $array_literals = $root->selectDescendantsOfType('n_ARRAY_LITERAL');
+    foreach ($array_literals as $array_literal) {
+      $nodes_by_key = array();
+      $keys_warn = array();
+      $list_node = $array_literal->getChildByIndex(0);
+      foreach ($list_node->getChildren() as $array_entry) {
+        $key_node = $array_entry->getChildByIndex(0);
+
+        switch ($key_node->getTypeName()) {
+          case 'n_STRING_SCALAR':
+          case 'n_NUMERIC_SCALAR':
+            // Scalars: array(1 => 'v1', '1' => 'v2');
+            $key = 'scalar:'.(string)$key_node->evalStatic();
+            break;
+
+          case 'n_SYMBOL_NAME':
+          case 'n_VARIABLE':
+          case 'n_CLASS_STATIC_ACCESS':
+            // Constants: array(CONST => 'v1', CONST => 'v2');
+            // Variables: array($a => 'v1', $a => 'v2');
+            // Class constants and vars: array(C::A => 'v1', C::A => 'v2');
+            $key = $key_node->getTypeName().':'.$key_node->getConcreteString();
+            break;
+
+          default:
+            $key = null;
+        }
+
+        if ($key !== null) {
+          if (isset($nodes_by_key[$key])) {
+            $keys_warn[$key] = true;
+          }
+          $nodes_by_key[$key][] = $key_node;
+        }
+      }
+
+      foreach ($keys_warn as $key => $_) {
+        foreach ($nodes_by_key[$key] as $node) {
+          $this->raiseLintAtNode(
+            $node,
+            self::LINT_DUPLICATE_KEYS_IN_ARRAY,
+            "Duplicate key in array initializer. PHP will ignore all ".
+            "but the last entry.");
+        }
+      }
+    }
+  }
+
   protected function raiseLintAtToken(
     XHPASTToken $token,
     $code,

src/lint/linter/xhpast/__tests__/data/duplicate-key-in-array.lint-test

@@ -0,0 +1,49 @@
+<?php
+$a = array(
+  'a' => 'val1',
+  $a => 'val2',
+  'a' => val3(),
+  1 => val4(),
+  '1' => val5(),
+  1 => $val6,
+  'b' => 'val7',
+  b() => 'val8',
+);
+static $b = array(
+  1 => 'one',
+  2 => 'two',
+  2 => 'TWO',
+);
+$c = array();
+$d = array(
+  A => 'ay',
+  A => 'aye',
+  'A' => 'AYE',
+);
+$e = array(
+  self::B => 'bee',
+  self::B => 'bea',
+  self::B() => 'ehh',
+  self::$B => 'weh',
+  B => 'b',
+  C::B => 'cb',
+);
+$f = array(
+  $a => 'var',
+  'a' => 'lit',
+  $a => 'var2',
+);
+~~~~~~~~~~
+error:3:3
+error:5:3
+error:6:3
+error:7:3
+error:8:3
+error:14:3
+error:15:3
+error:19:3
+error:20:3
+error:24:3
+error:25:3
+error:32:3
+error:34:3