mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-12-23 05:50:54 +01:00
Add a linter rule for array separators
Summary: Adds a rule to `ArcanistXHPASTLinter` which ensures that: # Single-lined arrays //do not// contain an unnecessary trailing comma separator after the final array value. # Multi-lined arrays //do// contain a trailing comma seperator after the final value value. Depends on D10534. Test Plan: Wrote and executed unit tests. Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: epriestley, Korvin Differential Revision: https://secure.phabricator.com/D10535
This commit is contained in:
parent
13492c8043
commit
be428e3207
2 changed files with 77 additions and 1 deletions
|
@ -47,6 +47,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
|||
const LINT_PHP_COMPATIBILITY = 45;
|
||||
const LINT_LANGUAGE_CONSTRUCT_PAREN = 46;
|
||||
const LINT_EMPTY_STATEMENT = 47;
|
||||
const LINT_ARRAY_SEPARATOR = 48;
|
||||
|
||||
private $naminghook;
|
||||
private $switchhook;
|
||||
|
@ -105,6 +106,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
|||
self::LINT_PHP_COMPATIBILITY => 'PHP Compatibility',
|
||||
self::LINT_LANGUAGE_CONSTRUCT_PAREN => 'Language Construct Parentheses',
|
||||
self::LINT_EMPTY_STATEMENT => 'Empty Block Statement',
|
||||
self::LINT_ARRAY_SEPARATOR => 'Array Separator',
|
||||
);
|
||||
}
|
||||
|
||||
|
@ -144,6 +146,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
|||
self::LINT_CONCATENATION_OPERATOR => $warning,
|
||||
self::LINT_LANGUAGE_CONSTRUCT_PAREN => $warning,
|
||||
self::LINT_EMPTY_STATEMENT => $advice,
|
||||
self::LINT_ARRAY_SEPARATOR => $advice,
|
||||
);
|
||||
}
|
||||
|
||||
|
@ -193,7 +196,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
|||
|
||||
public function getVersion() {
|
||||
// The version number should be incremented whenever a new rule is added.
|
||||
return '8';
|
||||
return '9';
|
||||
}
|
||||
|
||||
protected function resolveFuture($path, Future $future) {
|
||||
|
@ -263,6 +266,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
|||
'lintPHPCompatibility' => self::LINT_PHP_COMPATIBILITY,
|
||||
'lintLanguageConstructParentheses' => self::LINT_LANGUAGE_CONSTRUCT_PAREN,
|
||||
'lintEmptyBlockStatements' => self::LINT_EMPTY_STATEMENT,
|
||||
'lintArraySeparator' => self::LINT_ARRAY_SEPARATOR,
|
||||
);
|
||||
|
||||
foreach ($method_codes as $method => $codes) {
|
||||
|
@ -2714,6 +2718,48 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
|||
}
|
||||
}
|
||||
|
||||
protected function lintArraySeparator(XHPASTNode $root) {
|
||||
$arrays = $root->selectDescendantsOfType('n_ARRAY_LITERAL');
|
||||
|
||||
foreach ($arrays as $array) {
|
||||
$commas = $array->selectTokensOfType(',');
|
||||
$values = $array->selectDescendantsOfType('n_ARRAY_VALUE');
|
||||
|
||||
if ($values->count() == 0) {
|
||||
// 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)) {
|
||||
$this->raiseLintAtNode(
|
||||
$last,
|
||||
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;
|
||||
}
|
||||
|
||||
$this->raiseLintAtToken(
|
||||
$last_comma,
|
||||
self::LINT_ARRAY_SEPARATOR,
|
||||
pht('Single lined arrays should not have a trailing comma.'),
|
||||
'');
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
public function getSuperGlobalNames() {
|
||||
return array(
|
||||
'$GLOBALS',
|
||||
|
|
30
src/lint/linter/__tests__/xhpast/array-comma.lint-test
Normal file
30
src/lint/linter/__tests__/xhpast/array-comma.lint-test
Normal file
|
@ -0,0 +1,30 @@
|
|||
<?php
|
||||
array(1, 2, 3);
|
||||
array(1, 2, 3,);
|
||||
array(
|
||||
1,
|
||||
2,
|
||||
3,
|
||||
);
|
||||
array(
|
||||
1,
|
||||
2,
|
||||
3
|
||||
);
|
||||
~~~~~~~~~~
|
||||
advice:3:14
|
||||
advice:12:3
|
||||
~~~~~~~~~~
|
||||
<?php
|
||||
array(1, 2, 3);
|
||||
array(1, 2, 3);
|
||||
array(
|
||||
1,
|
||||
2,
|
||||
3,
|
||||
);
|
||||
array(
|
||||
1,
|
||||
2,
|
||||
3,
|
||||
);
|
Loading…
Reference in a new issue