Page MenuHomePhabricator
Files F14776245

D10687.id27043.diff
No OneTemporary
Actions

D10687.id27043.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
@@ -50,7 +50,8 @@
const LINT_ARRAY_SEPARATOR = 48;
const LINT_CONSTRUCTOR_PARENTHESES = 49;
const LINT_DUPLICATE_SWITCH_CASE = 50;
- const LINT_BLACKLISTED_FUNCTION = 50;
+ const LINT_BLACKLISTED_FUNCTION = 51;
+ const LINT_IMPLICIT_VISIBILITY = 52;
private $blacklistedFunctions = array();
private $naminghook;
@@ -114,6 +115,7 @@
self::LINT_CONSTRUCTOR_PARENTHESES => 'Constructor Parentheses',
self::LINT_DUPLICATE_SWITCH_CASE => 'Duplicate Case Statements',
self::LINT_BLACKLISTED_FUNCTION => 'Use of Blacklisted Function',
+ self::LINT_IMPLICIT_VISIBILITY => 'Implicit Method Visibility',
);
}
@@ -155,6 +157,7 @@
self::LINT_EMPTY_STATEMENT => $advice,
self::LINT_ARRAY_SEPARATOR => $advice,
self::LINT_CONSTRUCTOR_PARENTHESES => $advice,
+ self::LINT_IMPLICIT_VISIBILITY => $advice,
);
}
@@ -211,7 +214,7 @@
public function getVersion() {
// The version number should be incremented whenever a new rule is added.
- return '13';
+ return '14';
}
protected function resolveFuture($path, Future $future) {
@@ -285,6 +288,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,
);
foreach ($method_codes as $method => $codes) {
@@ -2998,6 +3003,63 @@
}
}
+ private function lintMethodModifier(XHPASTNode $root) {
+ $methods = $root->selectDescendantsOfType('n_METHOD_DECLARATION');
+
+ foreach ($methods as $method) {
+ $modifier_list = $method->getChildOfType(
+ 0,
+ 'n_METHOD_MODIFIER_LIST');
+
+ if (!$modifier_list->getChildren()) {
+ $this->raiseLintAtNode(
+ $method,
+ self::LINT_IMPLICIT_VISIBILITY,
+ pht('Methods should have their visibility declared explicitly.'),
+ 'public '.$method->getConcreteString());
+ }
+ }
+ }
+
+ private function lintPropertyModifier(XHPASTNode $root) {
+ static $visibilities = array(
+ 'public',
+ 'protected',
+ 'private',
+ );
+
+ $nodes = $root->selectDescendantsOfType('n_CLASS_MEMBER_MODIFIER_LIST');
+
+ foreach ($nodes as $node) {
+ $modifiers = $node->getChildren();
+
+ foreach ($modifiers as $modifier) {
+ if ($modifier->getConcreteString() == 'var') {
+ $this->raiseLintAtNode(
+ $modifier,
+ self::LINT_IMPLICIT_VISIBILITY,
+ pht(
+ 'Use `%s` instead of `%s` to indicate public visibility.',
+ 'public',
+ 'var'),
+ 'public');
+ continue 2;
+ }
+
+ if (in_array($modifier->getConcreteString(), $visibilities)) {
+ continue 2;
+ }
+ }
+
+ $this->raiseLintAtNode(
+ $node,
+ self::LINT_IMPLICIT_VISIBILITY,
+ pht('Properties should have their visibility declared explicitly.'),
+ 'public '.$node->getConcreteString());
+ }
+ }
+
+
public function getSuperGlobalNames() {
return array(
'$GLOBALS',
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
@@ -8,8 +8,8 @@
final class X {
- function a($x) {}
- function b($x ) {}
+ public function a($x) {}
+ public function b($x ) {}
final public static function &c($x) {}
final public static function &d($x ) {}
@@ -26,7 +26,7 @@
warning:4:14
warning:7:15
error:9:13
-warning:12:16
+warning:12:23
warning:15:37
warning:18:33
warning:23:14
@@ -42,8 +42,8 @@
final class X {
- function a($x) {}
- function b($x) {}
+ public function a($x) {}
+ public function b($x) {}
final public static function &c($x) {}
final public static function &d($x) {}
diff --git a/src/lint/linter/__tests__/xhpast/implicit-visibility.lint-test b/src/lint/linter/__tests__/xhpast/implicit-visibility.lint-test
new file mode 100644
--- /dev/null
+++ b/src/lint/linter/__tests__/xhpast/implicit-visibility.lint-test
@@ -0,0 +1,24 @@
+<?php
+final class Foo {
+ public function bar() {}
+ function baz() {}
+
+ var $x;
+ static $y;
+ private $z;
+}
+~~~~~~~~~~
+error:2:13 XHP19
+advice:4:3
+advice:6:3
+advice:7:3
+~~~~~~~~~~
+<?php
+final class Foo {
+ public function bar() {}
+ public function baz() {}
+
+ public $x;
+ public static $y;
+ private $z;
+}

File Metadata

Mime Type
text/plain
Expires
Sat, Jan 25, 3:55 AM (8 h, 58 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7042141
Default Alt Text
D10687.id27043.diff (4 KB)

Event Timeline

Phabricator · Built by Phacility