Page MenuHomePhabricator

D11504.id28041.diff
No OneTemporary

D11504.id28041.diff

diff --git a/src/lint/linter/ArcanistXHPASTLinter.php b/src/lint/linter/ArcanistXHPASTLinter.php
--- a/src/lint/linter/ArcanistXHPASTLinter.php
+++ b/src/lint/linter/ArcanistXHPASTLinter.php
@@ -53,8 +53,10 @@
const LINT_BLACKLISTED_FUNCTION = 51;
const LINT_IMPLICIT_VISIBILITY = 52;
const LINT_CALL_TIME_PASS_BY_REF = 53;
+ const LINT_INDENTATION = 54;
private $blacklistedFunctions = array();
+ private $indentation = 2;
private $naminghook;
private $switchhook;
private $version;
@@ -118,6 +120,7 @@
self::LINT_BLACKLISTED_FUNCTION => 'Use of Blacklisted Function',
self::LINT_IMPLICIT_VISIBILITY => 'Implicit Method Visibility',
self::LINT_CALL_TIME_PASS_BY_REF => 'Call-Time Pass-By-Reference',
+ self::LINT_INDENTATION => 'Indentation',
);
}
@@ -160,6 +163,7 @@
self::LINT_ARRAY_SEPARATOR => $advice,
self::LINT_CONSTRUCTOR_PARENTHESES => $advice,
self::LINT_IMPLICIT_VISIBILITY => $advice,
+ self::LINT_INDENTATION => $advice,
);
}
@@ -169,6 +173,11 @@
'type' => 'optional map<string, string>',
'help' => pht('Blacklisted functions which should not be used.'),
),
+ 'xhpast.indentation' => array(
+ 'type' => 'optional int',
+ 'help' => pht(
+ 'Number of whitespace characters to be used for indentation.'),
+ ),
'xhpast.naminghook' => array(
'type' => 'optional string',
'help' => pht(
@@ -197,6 +206,9 @@
case 'xhpast.blacklisted.function':
$this->blacklistedFunctions = $value;
return;
+ case 'xhpast.indentation':
+ $this->indentation = $value;
+ return;
case 'xhpast.naminghook':
$this->naminghook = $value;
return;
@@ -216,7 +228,7 @@
public function getVersion() {
// The version number should be incremented whenever a new rule is added.
- return '15';
+ return '16';
}
protected function resolveFuture($path, Future $future) {
@@ -293,6 +305,7 @@
'lintMethodModifier' => self::LINT_IMPLICIT_VISIBILITY,
'lintPropertyModifier' => self::LINT_IMPLICIT_VISIBILITY,
'lintCallTimePassByReference' => self::LINT_CALL_TIME_PASS_BY_REF,
+ 'lintIndentation' => self::LINT_INDENTATION,
);
foreach ($method_codes as $method => $codes) {
@@ -3101,6 +3114,112 @@
}
}
+ private function lintIndentation(XHPASTNode $root) {
+ $statement_lists = $root->selectDescendantsOfType('n_STATEMENT_LIST');
+
+ foreach ($statement_lists as $statement_list) {
+ $statements = $statement_list->getChildrenOfType('n_STATEMENT');
+
+ foreach ($statements as $statement) {
+ list($before, $after) = $statement->getSurroundingNonsemanticTokens();
+
+ $indent = '';
+ foreach ($before as $token) {
+ if (!$token->isAnyWhitespace()) {
+ $indent = '';
+ continue;
+ }
+ $indent .= preg_replace("/^\s*\n/", '', $token->getValue());
+ }
+
+ if ($indent != $this->getLeadingIndentation($statement)) {
+ $this->raiseLintAtToken(
+ last($before),
+ self::LINT_INDENTATION,
+ pht('Incorrect indentation.'),
+ "\n".preg_replace(
+ "/(?<=\n)\s*/",
+ '',
+ $token->getValue()).$this->getLeadingIndentation($statement));
+ }
+
+ $indent = '';
+ foreach ($after as $token) {
+ if (!$token->isAnyWhitespace()) {
+ break;
+ }
+ $indent .= preg_replace("/^\s*\n/", '', $token->getValue());
+ }
+
+ $next = head($after);
+
+ if (!$next) {
+ continue;
+ }
+
+ if (!$next->getNextToken()) {
+ continue;
+ }
+
+ if ($next->getNextToken()->getTypeName() !== '}') {
+ continue;
+ }
+
+ if ($indent != $this->getTrailingIndentation($statement)) {
+ $this->raiseLintAtToken(
+ head($after),
+ self::LINT_INDENTATION,
+ pht('Incorrect indentation.'),
+ "\n".preg_replace(
+ "/(?<=\n)\s*/",
+ '',
+ $token->getValue()).$this->getTrailingIndentation($statement));
+ }
+ }
+ }
+ }
+
+ private function getLeadingIndentation(XHPASTNode $node) {
+ $level = -1;
+
+ for (; $node != null; $node = $node->getParentNode()) {
+ switch ($node->getTypeName()) {
+ case 'n_STATEMENT_LIST':
+ $level++;
+ break;
+
+ default:
+ break;
+ }
+ }
+
+ return str_repeat(' ', $this->indentation * $level);
+ }
+
+ private function getTrailingIndentation(XHPASTNode $node) {
+ $level = -2;
+
+ switch ($node->getParentNode()->getParentNode()->getTypeName()) {
+ case 'n_CASE':
+ case 'n_DEFAULT':
+ $level--;
+ break;
+ }
+
+ for (; $node != null; $node = $node->getParentNode()) {
+ switch ($node->getTypeName()) {
+ case 'n_STATEMENT_LIST':
+ $level++;
+ break;
+
+ default:
+ break;
+ }
+ }
+
+ return str_repeat(' ', $this->indentation * $level);
+ }
+
public function getSuperGlobalNames() {
return array(
'$GLOBALS',
diff --git a/src/lint/linter/__tests__/xhpast/call-time-pass-by-reference.lint-test b/src/lint/linter/__tests__/xhpast/call-time-pass-by-reference.lint-test
--- a/src/lint/linter/__tests__/xhpast/call-time-pass-by-reference.lint-test
+++ b/src/lint/linter/__tests__/xhpast/call-time-pass-by-reference.lint-test
@@ -1,8 +1,8 @@
<?php
class MyClass {
- public function myfunc($var) {
- echo $var;
- }
+ public function myfunc($var) {
+ echo $var;
+ }
}
$myvar = true;
diff --git a/src/lint/linter/__tests__/xhpast/duplicate-switch-case.lint-test b/src/lint/linter/__tests__/xhpast/duplicate-switch-case.lint-test
--- a/src/lint/linter/__tests__/xhpast/duplicate-switch-case.lint-test
+++ b/src/lint/linter/__tests__/xhpast/duplicate-switch-case.lint-test
@@ -3,26 +3,38 @@
$y = null;
switch ($x) {}
switch ($x) {
- case 1: break;
- case '1': break;
- case true: break;
- case null: break;
- default: break;
+ case 1:
+ break;
+ case '1':
+ break;
+ case true:
+ break;
+ case null:
+ break;
+ default:
+ break;
}
switch ($x) {
- case 'foo': break;
- case 'bar': break;
- case 'foo': break;
+ case 'foo':
+ break;
+ case 'bar':
+ break;
+ case 'foo':
+ break;
default:
switch ($y) {
- case 'foo': break;
- case 'bar': break;
- case 'baz': break;
- case 'baz': break;
+ case 'foo':
+ break;
+ case 'bar':
+ break;
+ case 'baz':
+ break;
+ case 'baz':
+ break;
}
break;
}
~~~~~~~~~~
-error:15:3
-error:22:7
+error:22:3
+error:33:7
diff --git a/src/lint/linter/__tests__/xhpast/indentation.lint-test b/src/lint/linter/__tests__/xhpast/indentation.lint-test
new file mode 100644
--- /dev/null
+++ b/src/lint/linter/__tests__/xhpast/indentation.lint-test
@@ -0,0 +1,50 @@
+<?php
+
+function foo() {
+ return;
+}
+
+switch ($x) {
+ case 'x':
+ break;
+
+ case 'y':
+ break;
+
+ default: break;
+}
+
+foo(); bar();
+
+if ($x) { // comment
+echo 'x';
+}
+~~~~~~~~~~
+advice:3:17
+advice:11:12
+advice:14:11
+advice:17:7
+advice:19:11
+~~~~~~~~~~
+<?php
+
+function foo() {
+ return;
+}
+
+switch ($x) {
+ case 'x':
+ break;
+
+ case 'y':
+ break;
+
+ default:
+ break;
+}
+
+foo(); bar();
+
+if ($x) { // comment
+ echo 'x';
+}
diff --git a/src/lint/linter/__tests__/xhpast/php54-features.lint-test b/src/lint/linter/__tests__/xhpast/php54-features.lint-test
--- a/src/lint/linter/__tests__/xhpast/php54-features.lint-test
+++ b/src/lint/linter/__tests__/xhpast/php54-features.lint-test
@@ -1,14 +1,12 @@
<?php
f()[0];
- m()[0];
// The check above should be this, see T4334.
// $o->m()[0];
~~~~~~~~~~
error:3:5
-error:4:9
~~~~~~~~~~
~~~~~~~~~~
{"config": {"xhpast.php-version": "5.3.0"}}
diff --git a/src/lint/linter/__tests__/xhpast/switches.lint-test b/src/lint/linter/__tests__/xhpast/switches.lint-test
--- a/src/lint/linter/__tests__/xhpast/switches.lint-test
+++ b/src/lint/linter/__tests__/xhpast/switches.lint-test
@@ -64,9 +64,17 @@
return;
}
case 4:
- function f() { throw new Exception(); }
- g(function() { return; });
- final class C { public function m() { return; } }
+ function f() {
+ throw new Exception();
+ }
+ g(function() {
+ return;
+ });
+ final class C {
+ public function m() {
+ return;
+ }
+ }
interface I {}
case 5:
do {
@@ -89,8 +97,8 @@
warning:53:3
warning:57:3
warning:66:3
-warning:71:3
-warning:75:3
+warning:79:3
+warning:83:3
~~~~~~~~~~
~~~~~~~~~~
{"config":{"xhpast.switchhook":"ArcanistXHPASTLintTestSwitchHook"}}
diff --git a/src/lint/linter/__tests__/xhpast/undeclared-variables.lint-test b/src/lint/linter/__tests__/xhpast/undeclared-variables.lint-test
--- a/src/lint/linter/__tests__/xhpast/undeclared-variables.lint-test
+++ b/src/lint/linter/__tests__/xhpast/undeclared-variables.lint-test
@@ -184,6 +184,22 @@
error:51:10 worst ever
warning:61:3
error:87:3 This stuff is basically testing the lexer/parser for function decls.
+advice:90:29
+advice:90:35
+advice:91:30
+advice:91:36
+advice:92:36
+advice:92:42
+advice:93:37
+advice:93:43
+advice:94:27
+advice:94:33
+advice:95:28
+advice:95:34
+advice:96:34
+advice:96:40
+advice:97:35
+advice:97:41
error:104:15 Variables in instance derefs should be checked, static should not.
error:118:3 isset() and empty() should not trigger errors.
error:122:3 Should only warn once in this function.

File Metadata

Mime Type
text/plain
Expires
Tue, Nov 5, 9:53 AM (1 w, 3 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6715033
Default Alt Text
D11504.id28041.diff (9 KB)

Event Timeline