From cb2007825048f2b2ea7b9ec46a3c8a7b932bd821 Mon Sep 17 00:00:00 2001
From: Joshua Spence <josh@joshuaspence.com>
Date: Fri, 26 Sep 2014 07:06:19 +1000
Subject: [PATCH] Fix `LINT_ARRAY_SEPARATOR` for nested arrays

Summary: The `LINT_ARRAY_SEPARATOR` rule was added to `ArcanistXHPASTLinter` in D10535. Unfortunately, it does not correctly handle nested arrays correctly. This patch fixes this. Depends on D10554.

Test Plan: Added a test case.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10558
---
 src/lint/linter/ArcanistXHPASTLinter.php      | 31 +++++++------------
 .../__tests__/xhpast/array-comma.lint-test    |  9 ++++++
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/src/lint/linter/ArcanistXHPASTLinter.php b/src/lint/linter/ArcanistXHPASTLinter.php
index b059ddca..bb952d31 100644
--- a/src/lint/linter/ArcanistXHPASTLinter.php
+++ b/src/lint/linter/ArcanistXHPASTLinter.php
@@ -2722,37 +2722,28 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
     $arrays = $root->selectDescendantsOfType('n_ARRAY_LITERAL');
 
     foreach ($arrays as $array) {
-      $commas = $array->selectTokensOfType(',');
-      $values = $array->selectDescendantsOfType('n_ARRAY_VALUE');
+      $value_list = $array->getChildOfType(0, 'n_ARRAY_VALUE_LIST');
+      $values = $value_list->getChildrenOfType('n_ARRAY_VALUE');
 
-      if ($values->count() == 0) {
+      if (!$values) {
         // There is no need to check an empty array.
         continue;
       }
 
-      // This is a little messy... we want to do `last($values)` but
-      // `$values` is an `AASTNodeList`.
-      $last = null;
-      foreach ($values as $value) {
-        $last = $value;
-      }
-
       $multiline = $array->getLineNumber() != $array->getEndLineNumber();
 
-      if ($multiline && count($commas) != count($values)) {
+      $value = last($values);
+      $after = last($value->getTokens())->getNextToken();
+
+      if ($multiline && (!$after || $after->getValue() != ',')) {
         $this->raiseLintAtNode(
-          $last,
+          $value,
           self::LINT_ARRAY_SEPARATOR,
           pht('Multi-lined arrays should have trailing commas.'),
-          $last->getConcreteString().',');
-      } else if (!$multiline && count($commas) == count($values)) {
-        $last_comma = null;
-        foreach ($commas as $comma) {
-          $last_comma = $comma;
-        }
-
+          $value->getConcreteString().',');
+      } else if (!$multiline && $after && $after->getValue() == ',') {
         $this->raiseLintAtToken(
-          $last_comma,
+          $after,
           self::LINT_ARRAY_SEPARATOR,
           pht('Single lined arrays should not have a trailing comma.'),
           '');
diff --git a/src/lint/linter/__tests__/xhpast/array-comma.lint-test b/src/lint/linter/__tests__/xhpast/array-comma.lint-test
index 26096d29..74856a04 100644
--- a/src/lint/linter/__tests__/xhpast/array-comma.lint-test
+++ b/src/lint/linter/__tests__/xhpast/array-comma.lint-test
@@ -11,9 +11,14 @@ array(
   2,
   3
 );
+array(
+  'foo',
+  array('foo')
+);
 ~~~~~~~~~~
 advice:3:14
 advice:12:3
+advice:16:3
 ~~~~~~~~~~
 <?php
 array(1, 2, 3);
@@ -28,3 +33,7 @@ array(
   2,
   3,
 );
+array(
+  'foo',
+  array('foo'),
+);