mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-11-22 14:52:40 +01:00
Add a linter rule for self member references
Summary: Ref T7409. Adds a linter rule for the following: - `self::` is used instead of `Self::`. - `self::` is used for local static member reference. - `self::` is used instead of `self ::`. Based on [[https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/Standards/Squiz/Sniffs/Classes/SelfMemberReferenceSniff.php | Squiz_Sniffs_Classes_SelfMemberReferenceSniff]]. Test Plan: Added unit tests. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: Korvin, epriestley Maniphest Tasks: T7409 Differential Revision: https://secure.phabricator.com/D12370
This commit is contained in:
parent
6525a309c2
commit
fc70dfe268
2 changed files with 114 additions and 2 deletions
|
@ -56,6 +56,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
||||||
const LINT_FORMATTED_STRING = 54;
|
const LINT_FORMATTED_STRING = 54;
|
||||||
const LINT_UNNECESSARY_FINAL_MODIFIER = 55;
|
const LINT_UNNECESSARY_FINAL_MODIFIER = 55;
|
||||||
const LINT_UNNECESSARY_SEMICOLON = 56;
|
const LINT_UNNECESSARY_SEMICOLON = 56;
|
||||||
|
const LINT_SELF_MEMBER_REFERENCE = 57;
|
||||||
|
|
||||||
private $blacklistedFunctions = array();
|
private $blacklistedFunctions = array();
|
||||||
private $naminghook;
|
private $naminghook;
|
||||||
|
@ -125,6 +126,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
||||||
self::LINT_FORMATTED_STRING => 'Formatted String',
|
self::LINT_FORMATTED_STRING => 'Formatted String',
|
||||||
self::LINT_UNNECESSARY_FINAL_MODIFIER => 'Unnecessary Final Modifier',
|
self::LINT_UNNECESSARY_FINAL_MODIFIER => 'Unnecessary Final Modifier',
|
||||||
self::LINT_UNNECESSARY_SEMICOLON => 'Unnecessary Semicolon',
|
self::LINT_UNNECESSARY_SEMICOLON => 'Unnecessary Semicolon',
|
||||||
|
self::LINT_SELF_MEMBER_REFERENCE => 'Self Member Reference',
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -169,6 +171,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
||||||
self::LINT_IMPLICIT_VISIBILITY => $advice,
|
self::LINT_IMPLICIT_VISIBILITY => $advice,
|
||||||
self::LINT_UNNECESSARY_FINAL_MODIFIER => $advice,
|
self::LINT_UNNECESSARY_FINAL_MODIFIER => $advice,
|
||||||
self::LINT_UNNECESSARY_SEMICOLON => $advice,
|
self::LINT_UNNECESSARY_SEMICOLON => $advice,
|
||||||
|
self::LINT_SELF_MEMBER_REFERENCE => $advice,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -236,7 +239,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
||||||
|
|
||||||
public function getVersion() {
|
public function getVersion() {
|
||||||
// The version number should be incremented whenever a new rule is added.
|
// The version number should be incremented whenever a new rule is added.
|
||||||
return '19';
|
return '20';
|
||||||
}
|
}
|
||||||
|
|
||||||
protected function resolveFuture($path, Future $future) {
|
protected function resolveFuture($path, Future $future) {
|
||||||
|
@ -317,6 +320,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
||||||
'lintUnnecessaryFinalModifier' => self::LINT_UNNECESSARY_FINAL_MODIFIER,
|
'lintUnnecessaryFinalModifier' => self::LINT_UNNECESSARY_FINAL_MODIFIER,
|
||||||
'lintUnnecessarySemicolons' => self::LINT_UNNECESSARY_SEMICOLON,
|
'lintUnnecessarySemicolons' => self::LINT_UNNECESSARY_SEMICOLON,
|
||||||
'lintConstantDefinitions' => self::LINT_NAMING_CONVENTIONS,
|
'lintConstantDefinitions' => self::LINT_NAMING_CONVENTIONS,
|
||||||
|
'lintSelfMemberReference' => self::LINT_SELF_MEMBER_REFERENCE,
|
||||||
);
|
);
|
||||||
|
|
||||||
foreach ($method_codes as $method => $codes) {
|
foreach ($method_codes as $method => $codes) {
|
||||||
|
@ -1202,7 +1206,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
||||||
foreach ($foreach_loops as $foreach_loop) {
|
foreach ($foreach_loops as $foreach_loop) {
|
||||||
$var_map = array();
|
$var_map = array();
|
||||||
|
|
||||||
$foreach_expr = $foreach_loop->getChildOftype(0, 'n_FOREACH_EXPRESSION');
|
$foreach_expr = $foreach_loop->getChildOfType(0, 'n_FOREACH_EXPRESSION');
|
||||||
|
|
||||||
// We might use one or two vars, i.e. "foreach ($x as $y => $z)" or
|
// We might use one or two vars, i.e. "foreach ($x as $y => $z)" or
|
||||||
// "foreach ($x as $y)".
|
// "foreach ($x as $y)".
|
||||||
|
@ -3389,6 +3393,68 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private function lintSelfMemberReference(XHPASTNode $root) {
|
||||||
|
$class_declarations = $root->selectDescendantsOfType('n_CLASS_DECLARATION');
|
||||||
|
|
||||||
|
foreach ($class_declarations as $class_declaration) {
|
||||||
|
$class_name = $class_declaration
|
||||||
|
->getChildOfType(1, 'n_CLASS_NAME')
|
||||||
|
->getConcreteString();
|
||||||
|
|
||||||
|
$class_static_accesses = $class_declaration
|
||||||
|
->selectDescendantsOfType('n_CLASS_STATIC_ACCESS');
|
||||||
|
$self_member_references = array();
|
||||||
|
|
||||||
|
foreach ($class_static_accesses as $class_static_access) {
|
||||||
|
$double_colons = $class_static_access
|
||||||
|
->selectTokensOfType('T_PAAMAYIM_NEKUDOTAYIM');
|
||||||
|
$class_ref = $class_static_access
|
||||||
|
->getChildOfType(0, 'n_CLASS_NAME');
|
||||||
|
$class_ref_name = $class_ref->getConcreteString();
|
||||||
|
|
||||||
|
if (strtolower($class_name) == strtolower($class_ref_name)) {
|
||||||
|
$this->raiseLintAtNode(
|
||||||
|
$class_ref,
|
||||||
|
self::LINT_SELF_MEMBER_REFERENCE,
|
||||||
|
pht('Use `%s` for local static member references.', 'self::'),
|
||||||
|
'self');
|
||||||
|
}
|
||||||
|
|
||||||
|
static $self_refs = array(
|
||||||
|
'parent',
|
||||||
|
'self',
|
||||||
|
'static',
|
||||||
|
);
|
||||||
|
|
||||||
|
if (!in_array(strtolower($class_ref_name), $self_refs)) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
if ($class_ref_name != strtolower($class_ref_name)) {
|
||||||
|
$this->raiseLintAtNode(
|
||||||
|
$class_ref,
|
||||||
|
self::LINT_SELF_MEMBER_REFERENCE,
|
||||||
|
pht('PHP keywords should be lowercase.'),
|
||||||
|
strtolower($class_ref_name));
|
||||||
|
}
|
||||||
|
|
||||||
|
foreach ($double_colons as $double_colon) {
|
||||||
|
$tokens = $double_colon->getNonsemanticTokensBefore() +
|
||||||
|
$double_colon->getNonsemanticTokensAfter();
|
||||||
|
|
||||||
|
foreach ($tokens as $token) {
|
||||||
|
if ($token->isAnyWhitespace()) {
|
||||||
|
$this->raiseLintAtToken(
|
||||||
|
$token,
|
||||||
|
self::LINT_SELF_MEMBER_REFERENCE,
|
||||||
|
pht('Unnecessary whitespace around double colon operator.'),
|
||||||
|
'');
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Retrieve all calls to some specified function(s).
|
* Retrieve all calls to some specified function(s).
|
||||||
|
|
|
@ -0,0 +1,46 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
class Foo extends Bar {
|
||||||
|
const FOOBAR = 'FOOBAR';
|
||||||
|
|
||||||
|
public function __construct() {
|
||||||
|
PARENT::__construct(null);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function bar() {
|
||||||
|
echo self::FOOBAR;
|
||||||
|
echo Self :: FOOBAR;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function baz() {
|
||||||
|
echo static::FOOBAR;
|
||||||
|
echo Foo::FOOBAR;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
~~~~~~~~~~
|
||||||
|
error:3:7
|
||||||
|
advice:7:5
|
||||||
|
advice:12:10
|
||||||
|
advice:12:14
|
||||||
|
advice:12:17
|
||||||
|
advice:17:10
|
||||||
|
~~~~~~~~~~
|
||||||
|
<?php
|
||||||
|
|
||||||
|
class Foo extends Bar {
|
||||||
|
const FOOBAR = 'FOOBAR';
|
||||||
|
|
||||||
|
public function __construct() {
|
||||||
|
parent::__construct(null);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function bar() {
|
||||||
|
echo self::FOOBAR;
|
||||||
|
echo self::FOOBAR;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function baz() {
|
||||||
|
echo static::FOOBAR;
|
||||||
|
echo self::FOOBAR;
|
||||||
|
}
|
||||||
|
}
|
Loading…
Reference in a new issue