revi-archive/phorge-arcanist
1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2025-01-10 23:01:04 +01:00
Code Issues Releases Wiki Activity

Add a linter rule to determine whether a class should be marked as abstract

Browse source 
Summary:
A class containing `abstract` methods must itself be marked as `abstract`.

```
PHP Fatal error:  Class X contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (X::Y) in /home/josh/workspace/github.com/phacility/arcanist/test.php on line 5
```

Test Plan: Added unit tests.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D14559
This commit is contained in:
Joshua Spence 2015-11-24 08:18:38 +11:00
parent 20e99acf0a
commit 37571d8839
6 changed files with 105 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
src/__phutil_library_map__.php
View file

@ -57,6 +57,8 @@ phutil_register_library_map(array(
'ArcanistClassExtendsObjectXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistClassExtendsObjectXHPASTLinterRule.php',
'ArcanistClassExtendsObjectXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistClassExtendsObjectXHPASTLinterRuleTestCase.php',
'ArcanistClassFilenameMismatchXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistClassFilenameMismatchXHPASTLinterRule.php',
'ArcanistClassMustBeDeclaredAbstractXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistClassMustBeDeclaredAbstractXHPASTLinterRule.php',
'ArcanistClassMustBeDeclaredAbstractXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistClassMustBeDeclaredAbstractXHPASTLinterRuleTestCase.php',
'ArcanistClassNameLiteralXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistClassNameLiteralXHPASTLinterRule.php',
'ArcanistClassNameLiteralXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistClassNameLiteralXHPASTLinterRuleTestCase.php',
'ArcanistCloseRevisionWorkflow' => 'workflow/ArcanistCloseRevisionWorkflow.php',
@ -441,6 +443,8 @@ phutil_register_library_map(array(
'ArcanistClassExtendsObjectXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
'ArcanistClassExtendsObjectXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
'ArcanistClassFilenameMismatchXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
'ArcanistClassMustBeDeclaredAbstractXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
'ArcanistClassMustBeDeclaredAbstractXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
'ArcanistClassNameLiteralXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
'ArcanistClassNameLiteralXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
'ArcanistCloseRevisionWorkflow' => 'ArcanistWorkflow',

74
src/lint/linter/xhpast/rules/ArcanistClassMustBeDeclaredAbstractXHPASTLinterRule.php Normal file
View file

@ -0,0 +1,74 @@
<?php
final class ArcanistClassMustBeDeclaredAbstractXHPASTLinterRule
extends ArcanistXHPASTLinterRule {
const ID = 113;
public function getLintName() {
return pht(
'`%s` Containing `%s` Methods Must Be Declared `%s`',
'class',
'abstract',
'abstract');
}
public function process(XHPASTNode $root) {
$classes = $root->selectDescendantsOfType('n_CLASS_DECLARATION');
foreach ($classes as $class) {
$class_modifiers = $this->getModifiers($class);
$abstract_methods = array();
$methods = $class->selectDescendantsOfType('n_METHOD_DECLARATION');
foreach ($methods as $method) {
$method_modifiers = $this->getModifiers($method);
if (idx($method_modifiers, 'abstract')) {
$abstract_methods[] = $method;
}
}
if (!idx($class_modifiers, 'abstract') && $abstract_methods) {
$this->raiseLintAtNode(
$class,
pht(
'Class contains %s %s method(s) and must therefore '.
'be declared `%s`.',
phutil_count($abstract_methods),
'abstract',
'abstract'));
}
}
}
/**
* Get class/method modifiers.
*
* @param XHPASTNode A node of type `n_CLASS_DECLARATION` or
* `n_METHOD_DECLARATION`.
* @return map<string, bool> Class/method modifiers.
*/
private function getModifiers(XHPASTNode $node) {
$modifier_list = $node->getChildByIndex(0);
switch ($modifier_list->getTypeName()) {
case 'n_CLASS_ATTRIBUTES':
case 'n_METHOD_MODIFIER_LIST':
break;
default:
return array();
}
$modifiers = array();
foreach ($modifier_list->selectDescendantsOfType('n_STRING') as $modifier) {
$modifiers[strtolower($modifier->getConcreteString())] = true;
}
return $modifiers;
}
}

11
src/lint/linter/xhpast/rules/__tests__/ArcanistClassMustBeDeclaredAbstractXHPASTLinterRuleTestCase.php Normal file
View file

@ -0,0 +1,11 @@
<?php
final class ArcanistClassMustBeDeclaredAbstractXHPASTLinterRuleTestCase
extends ArcanistXHPASTLinterRuleTestCase {
public function testLinter() {
$this->executeTestsInDirectory(
dirname(__FILE__).'/class-must-be-declared-abstract/');
}
}

5
src/lint/linter/xhpast/rules/__tests__/class-must-be-declared-abstract/is-abstract.lint-test Normal file
View file

@ -0,0 +1,5 @@
<?php
abstract class SomeClass {
abstract public function someMethod();
}
~~~~~~~~~~

5
src/lint/linter/xhpast/rules/__tests__/class-must-be-declared-abstract/is-not-abstract.lint-test Normal file
View file

@ -0,0 +1,5 @@
<?php
class SomeClass {
public function someMethod();
}
~~~~~~~~~~

6
src/lint/linter/xhpast/rules/__tests__/class-must-be-declared-abstract/should-be-abstract.lint-test Normal file
View file

@ -0,0 +1,6 @@
<?php
class SomeClass {
abstract public function someMethod();
}
~~~~~~~~~~
error:2:1