Add a linter rule for array elements

Summary: Add a linter rule to ensure that array elements occupy a single line each.

Test Plan: Added test cases.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D13842

src/__phutil_library_map__.php

@@ -15,6 +15,7 @@ phutil_register_library_map(array(
     'ArcanistAnoidWorkflow' => 'workflow/ArcanistAnoidWorkflow.php',
     'ArcanistArrayIndexSpacingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistArrayIndexSpacingXHPASTLinterRule.php',
     'ArcanistArraySeparatorXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistArraySeparatorXHPASTLinterRule.php',
+    'ArcanistArrayValueXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistArrayValueXHPASTLinterRule.php',
     'ArcanistBackoutWorkflow' => 'workflow/ArcanistBackoutWorkflow.php',
     'ArcanistBaseCommitParser' => 'parser/ArcanistBaseCommitParser.php',
     'ArcanistBaseCommitParserTestCase' => 'parser/__tests__/ArcanistBaseCommitParserTestCase.php',
@@ -292,6 +293,7 @@ phutil_register_library_map(array(
     'ArcanistAnoidWorkflow' => 'ArcanistWorkflow',
     'ArcanistArrayIndexSpacingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistArraySeparatorXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
+    'ArcanistArrayValueXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistBackoutWorkflow' => 'ArcanistWorkflow',
     'ArcanistBaseCommitParser' => 'Phobject',
     'ArcanistBaseCommitParserTestCase' => 'PhutilTestCase',

src/lint/linter/__tests__/xhpast/array-value.lint-test

@@ -0,0 +1,48 @@
+<?php
+
+array(
+  1, 2, 3,
+);
+
+array(
+  'foo' => 'bar', 'bar' => 'baz',
+);
+
+array(
+  1, /* quack */ 2, /* quack */3,
+);
+
+array(
+  /* OPEN   */ 1,
+  /* CLOSED */ 2,
+);
+~~~~~~~~~~
+warning:4:5
+warning:4:8
+warning:8:18
+warning:12:17
+warning:12:32
+~~~~~~~~~~
+<?php
+
+array(
+  1,
+  2,
+  3,
+);
+
+array(
+  'foo' => 'bar',
+  'bar' => 'baz',
+);
+
+array(
+  1, /* quack */
+  2, /* quack */
+  3,
+);
+
+array(
+  /* OPEN   */ 1,
+  /* CLOSED */ 2,
+);

src/lint/linter/xhpast/rules/ArcanistArrayValueXHPASTLinterRule.php

@@ -0,0 +1,56 @@
+<?php
+
+final class ArcanistArrayValueXHPASTLinterRule
+  extends ArcanistXHPASTLinterRule {
+
+  const ID = 76;
+
+  public function getLintName() {
+    return pht('Array Element');
+  }
+
+  public function getLintSeverity() {
+    return ArcanistLintSeverity::SEVERITY_WARNING;
+  }
+
+  public function process(XHPASTNode $root) {
+    $arrays = $root->selectDescendantsOfType('n_ARRAY_LITERAL');
+
+    foreach ($arrays as $array) {
+      $array_values = $array
+        ->getChildOfType(0, 'n_ARRAY_VALUE_LIST')
+        ->getChildrenOfType('n_ARRAY_VALUE');
+
+      if (!$array_values) {
+        // There is no need to check an empty array.
+        continue;
+      }
+
+      $multiline = $array->getLineNumber() != $array->getEndLineNumber();
+
+      if (!$multiline) {
+        continue;
+      }
+
+      foreach ($array_values as $value) {
+        list($before, $after) = $value->getSurroundingNonsemanticTokens();
+
+        if (strpos(implode('', mpull($before, 'getValue')), "\n") === false) {
+          if (last($before)->isAnyWhitespace()) {
+            $token = last($before);
+            $replacement = "\n".$value->getIndentation();
+          } else {
+            $token = head($value->getTokens());
+            $replacement = "\n".$value->getIndentation().$token->getValue();
+          }
+
+          $this->raiseLintAtToken(
+            $token,
+            pht('Array elements should each occupy a single line.'),
+            $replacement);
+        }
+      }
+    }
+  }
+
+}