mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-11-25 16:22:42 +01:00
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
This commit is contained in:
parent
e6e317db27
commit
cb20078250
2 changed files with 20 additions and 20 deletions
|
@ -2722,37 +2722,28 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
||||||
$arrays = $root->selectDescendantsOfType('n_ARRAY_LITERAL');
|
$arrays = $root->selectDescendantsOfType('n_ARRAY_LITERAL');
|
||||||
|
|
||||||
foreach ($arrays as $array) {
|
foreach ($arrays as $array) {
|
||||||
$commas = $array->selectTokensOfType(',');
|
$value_list = $array->getChildOfType(0, 'n_ARRAY_VALUE_LIST');
|
||||||
$values = $array->selectDescendantsOfType('n_ARRAY_VALUE');
|
$values = $value_list->getChildrenOfType('n_ARRAY_VALUE');
|
||||||
|
|
||||||
if ($values->count() == 0) {
|
if (!$values) {
|
||||||
// There is no need to check an empty array.
|
// There is no need to check an empty array.
|
||||||
continue;
|
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();
|
$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(
|
$this->raiseLintAtNode(
|
||||||
$last,
|
$value,
|
||||||
self::LINT_ARRAY_SEPARATOR,
|
self::LINT_ARRAY_SEPARATOR,
|
||||||
pht('Multi-lined arrays should have trailing commas.'),
|
pht('Multi-lined arrays should have trailing commas.'),
|
||||||
$last->getConcreteString().',');
|
$value->getConcreteString().',');
|
||||||
} else if (!$multiline && count($commas) == count($values)) {
|
} else if (!$multiline && $after && $after->getValue() == ',') {
|
||||||
$last_comma = null;
|
|
||||||
foreach ($commas as $comma) {
|
|
||||||
$last_comma = $comma;
|
|
||||||
}
|
|
||||||
|
|
||||||
$this->raiseLintAtToken(
|
$this->raiseLintAtToken(
|
||||||
$last_comma,
|
$after,
|
||||||
self::LINT_ARRAY_SEPARATOR,
|
self::LINT_ARRAY_SEPARATOR,
|
||||||
pht('Single lined arrays should not have a trailing comma.'),
|
pht('Single lined arrays should not have a trailing comma.'),
|
||||||
'');
|
'');
|
||||||
|
|
|
@ -11,9 +11,14 @@ array(
|
||||||
2,
|
2,
|
||||||
3
|
3
|
||||||
);
|
);
|
||||||
|
array(
|
||||||
|
'foo',
|
||||||
|
array('foo')
|
||||||
|
);
|
||||||
~~~~~~~~~~
|
~~~~~~~~~~
|
||||||
advice:3:14
|
advice:3:14
|
||||||
advice:12:3
|
advice:12:3
|
||||||
|
advice:16:3
|
||||||
~~~~~~~~~~
|
~~~~~~~~~~
|
||||||
<?php
|
<?php
|
||||||
array(1, 2, 3);
|
array(1, 2, 3);
|
||||||
|
@ -28,3 +33,7 @@ array(
|
||||||
2,
|
2,
|
||||||
3,
|
3,
|
||||||
);
|
);
|
||||||
|
array(
|
||||||
|
'foo',
|
||||||
|
array('foo'),
|
||||||
|
);
|
||||||
|
|
Loading…
Reference in a new issue