Page MenuHomePhabricator
Files F15530271

D12421.id29829.diff
No OneTemporary
Actions

D12421.id29829.diff
View Options

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
@@ -62,6 +62,7 @@
const LINT_DEFAULT_PARAMETERS = 60;
const LINT_USELESS_OVERRIDING_METHOD = 61;
const LINT_NO_PARENT_SCOPE = 62;
+ const LINT_MODIFIER_ORDERING = 63;
private $blacklistedFunctions = array();
private $naminghook;
@@ -194,6 +195,8 @@
=> pht('Useless Overriding Method'),
self::LINT_NO_PARENT_SCOPE
=> pht('No Parent Scope'),
+ self::LINT_MODIFIER_ORDERING
+ => pht('Modifier Ordering'),
);
}
@@ -243,6 +246,7 @@
self::LINT_INNER_FUNCTION => $warning,
self::LINT_DEFAULT_PARAMETERS => $warning,
self::LINT_USELESS_OVERRIDING_METHOD => $advice,
+ self::LINT_MODIFIER_ORDERING => $advice,
);
}
@@ -310,7 +314,7 @@
public function getVersion() {
// The version number should be incremented whenever a new rule is added.
- return '24';
+ return '25';
}
protected function resolveFuture($path, Future $future) {
@@ -386,8 +390,8 @@
'lintConstructorParentheses' => self::LINT_CONSTRUCTOR_PARENTHESES,
'lintSwitchStatements' => self::LINT_DUPLICATE_SWITCH_CASE,
'lintBlacklistedFunction' => self::LINT_BLACKLISTED_FUNCTION,
- 'lintMethodModifier' => self::LINT_IMPLICIT_VISIBILITY,
- 'lintPropertyModifier' => self::LINT_IMPLICIT_VISIBILITY,
+ 'lintMethodVisibility' => self::LINT_IMPLICIT_VISIBILITY,
+ 'lintPropertyVisibility' => self::LINT_IMPLICIT_VISIBILITY,
'lintCallTimePassByReference' => self::LINT_CALL_TIME_PASS_BY_REF,
'lintFormattedString' => self::LINT_FORMATTED_STRING,
'lintUnnecessaryFinalModifier' => self::LINT_UNNECESSARY_FINAL_MODIFIER,
@@ -399,6 +403,8 @@
'lintDefaultParameters' => self::LINT_DEFAULT_PARAMETERS,
'lintUselessOverridingMethods' => self::LINT_USELESS_OVERRIDING_METHOD,
'lintNoParentScope' => self::LINT_NO_PARENT_SCOPE,
+ 'lintMethodModifierOrdering' => self::LINT_MODIFIER_ORDERING,
+ 'lintPropertyModifierOrdering' => self::LINT_MODIFIER_ORDERING,
);
foreach ($method_codes as $method => $codes) {
@@ -3288,7 +3294,7 @@
}
}
- private function lintMethodModifier(XHPASTNode $root) {
+ private function lintMethodVisibility(XHPASTNode $root) {
static $visibilities = array(
'public',
'protected',
@@ -3322,7 +3328,7 @@
}
}
- private function lintPropertyModifier(XHPASTNode $root) {
+ private function lintPropertyVisibility(XHPASTNode $root) {
static $visibilities = array(
'public',
'protected',
@@ -3800,6 +3806,65 @@
}
}
+ private function lintMethodModifierOrdering(XHPASTNode $root) {
+ static $modifiers = array(
+ 'public',
+ 'protected',
+ 'private',
+ 'static',
+ 'abstract',
+ 'final',
+ );
+
+ $methods = $root->selectDescendantsOfType('n_METHOD_MODIFIER_LIST');
+
+ foreach ($methods as $method) {
+ $modifier_ordering = array_values(
+ mpull($method->getChildren(), 'getConcreteString'));
+ $expected_modifier_ordering = array_values(
+ array_intersect(
+ $modifiers,
+ $modifier_ordering));
+
+ if ($modifier_ordering != $expected_modifier_ordering) {
+ $this->raiseLintAtNode(
+ $method,
+ self::LINT_MODIFIER_ORDERING,
+ pht('Non-conventional modifier ordering.'),
+ implode(' ', $expected_modifier_ordering));
+ }
+ }
+ }
+
+ private function lintPropertyModifierOrdering(XHPASTNode $root) {
+ static $modifiers = array(
+ 'public',
+ 'protected',
+ 'private',
+ 'static',
+ );
+
+ $properties = $root->selectDescendantsOfType(
+ 'n_CLASS_MEMBER_MODIFIER_LIST');
+
+ foreach ($properties as $property) {
+ $modifier_ordering = array_values(
+ mpull($property->getChildren(), 'getConcreteString'));
+ $expected_modifier_ordering = array_values(
+ array_intersect(
+ $modifiers,
+ $modifier_ordering));
+
+ if ($modifier_ordering != $expected_modifier_ordering) {
+ $this->raiseLintAtNode(
+ $property,
+ self::LINT_MODIFIER_ORDERING,
+ pht('Non-conventional modifier ordering.'),
+ implode(' ', $expected_modifier_ordering));
+ }
+ }
+ }
+
/**
* Retrieve all calls to some specified function(s).
*
diff --git a/src/lint/linter/__tests__/xhpast/decl-parens-hug-closing.lint-test b/src/lint/linter/__tests__/xhpast/decl-parens-hug-closing.lint-test
--- a/src/lint/linter/__tests__/xhpast/decl-parens-hug-closing.lint-test
+++ b/src/lint/linter/__tests__/xhpast/decl-parens-hug-closing.lint-test
@@ -14,8 +14,8 @@
public static function &c($x) {}
public static function &d($x ) {}
- abstract private function e($x);
- abstract private function f($x );
+ private abstract function e($x);
+ private abstract function f($x );
}
@@ -48,8 +48,8 @@
public static function &c($x) {}
public static function &d($x) {}
- abstract private function e($x);
- abstract private function f($x);
+ private abstract function e($x);
+ private abstract function f($x);
}
diff --git a/src/lint/linter/__tests__/xhpast/modifier-ordering.lint-test b/src/lint/linter/__tests__/xhpast/modifier-ordering.lint-test
new file mode 100644
--- /dev/null
+++ b/src/lint/linter/__tests__/xhpast/modifier-ordering.lint-test
@@ -0,0 +1,24 @@
+<?php
+class Foo {
+ public $x;
+ static protected $y;
+
+ final public function bar() {}
+ private function baz() {}
+ static final public function foobar() {}
+}
+~~~~~~~~~~
+error:2:7 XHP19
+advice:4:3
+advice:6:3
+advice:8:3
+~~~~~~~~~~
+<?php
+class Foo {
+ public $x;
+ protected static $y;
+
+ public final function bar() {}
+ private function baz() {}
+ public static final function foobar() {}
+}
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
@@ -134,7 +134,7 @@
}
abstract class P {
- abstract public function q();
+ public abstract function q();
}
function x() {

File Metadata

Mime Type
text/plain
Expires
Thu, Apr 24, 3:16 AM (2 d, 8 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7533112
Default Alt Text
D12421.id29829.diff (6 KB)

Event Timeline

Phabricator · Built by Phacility