mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-11-29 10:12:41 +01:00
Add a linter rule for parent
references
Summary: Add a linter rule to detect static method calls which //should// reference the `parent` class instead of a hardcoded class reference. For example, consider the following: ```lang=php, counterexample class SomeClass extends AnotherClass { public function someMethod() { AnotherClass::someOtherMethod(); } } ``` This should instead be written as: ```lang=php class SomeClass extends AnotherClass { public function someMethod() { parent::someOtherMethod(); } } ``` Test Plan: Added unit tests. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: Korvin, epriestley Differential Revision: https://secure.phabricator.com/D14443
This commit is contained in:
parent
4d512c51d4
commit
7386afc953
5 changed files with 108 additions and 0 deletions
|
@ -248,6 +248,8 @@ phutil_register_library_map(array(
|
||||||
'ArcanistPHPOpenTagXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistPHPOpenTagXHPASTLinterRuleTestCase.php',
|
'ArcanistPHPOpenTagXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistPHPOpenTagXHPASTLinterRuleTestCase.php',
|
||||||
'ArcanistPHPShortTagXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistPHPShortTagXHPASTLinterRule.php',
|
'ArcanistPHPShortTagXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistPHPShortTagXHPASTLinterRule.php',
|
||||||
'ArcanistPHPShortTagXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistPHPShortTagXHPASTLinterRuleTestCase.php',
|
'ArcanistPHPShortTagXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistPHPShortTagXHPASTLinterRuleTestCase.php',
|
||||||
|
'ArcanistParentMemberReferenceXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistParentMemberReferenceXHPASTLinterRule.php',
|
||||||
|
'ArcanistParentMemberReferenceXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistParentMemberReferenceXHPASTLinterRuleTestCase.php',
|
||||||
'ArcanistParenthesesSpacingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistParenthesesSpacingXHPASTLinterRule.php',
|
'ArcanistParenthesesSpacingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistParenthesesSpacingXHPASTLinterRule.php',
|
||||||
'ArcanistParenthesesSpacingXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistParenthesesSpacingXHPASTLinterRuleTestCase.php',
|
'ArcanistParenthesesSpacingXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistParenthesesSpacingXHPASTLinterRuleTestCase.php',
|
||||||
'ArcanistParseStrUseXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistParseStrUseXHPASTLinterRule.php',
|
'ArcanistParseStrUseXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistParseStrUseXHPASTLinterRule.php',
|
||||||
|
@ -618,6 +620,8 @@ phutil_register_library_map(array(
|
||||||
'ArcanistPHPOpenTagXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
|
'ArcanistPHPOpenTagXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
|
||||||
'ArcanistPHPShortTagXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
|
'ArcanistPHPShortTagXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
|
||||||
'ArcanistPHPShortTagXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
|
'ArcanistPHPShortTagXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
|
||||||
|
'ArcanistParentMemberReferenceXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
|
||||||
|
'ArcanistParentMemberReferenceXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
|
||||||
'ArcanistParenthesesSpacingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
|
'ArcanistParenthesesSpacingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
|
||||||
'ArcanistParenthesesSpacingXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
|
'ArcanistParenthesesSpacingXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
|
||||||
'ArcanistParseStrUseXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
|
'ArcanistParseStrUseXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
|
||||||
|
|
|
@ -0,0 +1,71 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
final class ArcanistParentMemberReferenceXHPASTLinterRule
|
||||||
|
extends ArcanistXHPASTLinterRule {
|
||||||
|
|
||||||
|
const ID = 83;
|
||||||
|
|
||||||
|
public function getLintName() {
|
||||||
|
return pht('Parent Member Reference');
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getLintSeverity() {
|
||||||
|
return ArcanistLintSeverity::SEVERITY_WARNING;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function process(XHPASTNode $root) {
|
||||||
|
$class_declarations = $root->selectDescendantsOfType('n_CLASS_DECLARATION');
|
||||||
|
|
||||||
|
foreach ($class_declarations as $class_declaration) {
|
||||||
|
$extends_list = $class_declaration
|
||||||
|
->getChildByIndex(2);
|
||||||
|
$parent_class = null;
|
||||||
|
|
||||||
|
if ($extends_list->getTypeName() == 'n_EXTENDS_LIST') {
|
||||||
|
$parent_class = $extends_list
|
||||||
|
->getChildOfType(0, 'n_CLASS_NAME')
|
||||||
|
->getConcreteString();
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!$parent_class) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
$class_static_accesses = $class_declaration
|
||||||
|
->selectDescendantsOfType('n_CLASS_STATIC_ACCESS');
|
||||||
|
$closures = $this->getAnonymousClosures($class_declaration);
|
||||||
|
|
||||||
|
foreach ($class_static_accesses as $class_static_access) {
|
||||||
|
$double_colons = $class_static_access
|
||||||
|
->selectTokensOfType('T_PAAMAYIM_NEKUDOTAYIM');
|
||||||
|
$class_ref = $class_static_access->getChildByIndex(0);
|
||||||
|
|
||||||
|
if ($class_ref->getTypeName() != 'n_CLASS_NAME') {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
$class_ref_name = $class_ref->getConcreteString();
|
||||||
|
|
||||||
|
if (strtolower($parent_class) == strtolower($class_ref_name)) {
|
||||||
|
$in_closure = false;
|
||||||
|
|
||||||
|
foreach ($closures as $closure) {
|
||||||
|
if ($class_ref->isDescendantOf($closure)) {
|
||||||
|
$in_closure = true;
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (version_compare($this->version, '5.4.0', '>=') || !$in_closure) {
|
||||||
|
$this->raiseLintAtNode(
|
||||||
|
$class_ref,
|
||||||
|
pht(
|
||||||
|
'Use `%s` to call parent method.',
|
||||||
|
'parent::'),
|
||||||
|
'parent');
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
}
|
|
@ -0,0 +1,11 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
final class ArcanistParentMemberReferenceXHPASTLinterRuleTestCase
|
||||||
|
extends ArcanistXHPASTLinterRuleTestCase {
|
||||||
|
|
||||||
|
public function testLinter() {
|
||||||
|
$this->executeTestsInDirectory(
|
||||||
|
dirname(__FILE__).'/parent-member-references/');
|
||||||
|
}
|
||||||
|
|
||||||
|
}
|
|
@ -0,0 +1,7 @@
|
||||||
|
<?php
|
||||||
|
class Foobar {
|
||||||
|
public function someMethod() {
|
||||||
|
Bar::someOtherMethod();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
~~~~~~~~~~
|
|
@ -0,0 +1,15 @@
|
||||||
|
<?php
|
||||||
|
class Foo extends Bar {
|
||||||
|
public function someMethod() {
|
||||||
|
Bar::someOtherMethod();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
~~~~~~~~~~
|
||||||
|
warning:4:5
|
||||||
|
~~~~~~~~~~
|
||||||
|
<?php
|
||||||
|
class Foo extends Bar {
|
||||||
|
public function someMethod() {
|
||||||
|
parent::someOtherMethod();
|
||||||
|
}
|
||||||
|
}
|
Loading…
Reference in a new issue