From 4b041b8f84311bcceb9ec3777bab62e10ddb9012 Mon Sep 17 00:00:00 2001
From: Lovro Puzar <lovro.puzar@gmail.com>
Date: Wed, 25 May 2011 12:24:25 -0700
Subject: [PATCH] Lint rule for duplicate keys in array literals

Summary:
Raise an error if an array is initialized with the same key present more than
once.

Test Plan:
bin/arc unit
Also added some duplicates to ArcanistXHPASTLinter.php and verified the output
of bin/arc lint.

Reviewed By: epriestley
Reviewers: jungejason, epriestley, aran
CC: aran, epriestley
Revert Plan:
Tags:

Differential Revision: 346
---
 .../linter/xhpast/ArcanistXHPASTLinter.php    | 57 +++++++++++++++++++
 .../data/duplicate-key-in-array.lint-test     | 49 ++++++++++++++++
 2 files changed, 106 insertions(+)
 create mode 100644 src/lint/linter/xhpast/__tests__/data/duplicate-key-in-array.lint-test

diff --git a/src/lint/linter/xhpast/ArcanistXHPASTLinter.php b/src/lint/linter/xhpast/ArcanistXHPASTLinter.php
index 38e21513..555ebe5e 100644
--- a/src/lint/linter/xhpast/ArcanistXHPASTLinter.php
+++ b/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,
diff --git a/src/lint/linter/xhpast/__tests__/data/duplicate-key-in-array.lint-test b/src/lint/linter/xhpast/__tests__/data/duplicate-key-in-array.lint-test
new file mode 100644
index 00000000..cba4132d
--- /dev/null
+++ b/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