From 72d9013d29dc6fd8e64351d3bb85b24e788803e7 Mon Sep 17 00:00:00 2001 From: Joshua Spence Date: Tue, 24 Nov 2015 08:19:35 +1100 Subject: [PATCH] Add a linter rule for `abstract` methods containing a body Summary: `abstract` methods cannot contain a body. ``` PHP Fatal error: Abstract function X::Y() cannot contain body in /home/josh/workspace/github.com/phacility/arcanist/test.php on line 4 ``` Test Plan: Added unit tests. Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D14560 --- src/__phutil_library_map__.php | 4 +++ .../xhpast/ArcanistXHPASTLinterRule.php | 28 +++++++++++++++++ ...nistAbstractMethodBodyXHPASTLinterRule.php | 30 +++++++++++++++++++ ...MustBeDeclaredAbstractXHPASTLinterRule.php | 28 ----------------- ...ractMethodBodyXHPASTLinterRuleTestCase.php | 10 +++++++ .../abstract-method-body/body.lint-test | 6 ++++ .../abstract-method-body/no-body.lint-test | 5 ++++ .../non-abstract.lint-test | 5 ++++ 8 files changed, 88 insertions(+), 28 deletions(-) create mode 100644 src/lint/linter/xhpast/rules/ArcanistAbstractMethodBodyXHPASTLinterRule.php create mode 100644 src/lint/linter/xhpast/rules/__tests__/ArcanistAbstractMethodBodyXHPASTLinterRuleTestCase.php create mode 100644 src/lint/linter/xhpast/rules/__tests__/abstract-method-body/body.lint-test create mode 100644 src/lint/linter/xhpast/rules/__tests__/abstract-method-body/no-body.lint-test create mode 100644 src/lint/linter/xhpast/rules/__tests__/abstract-method-body/non-abstract.lint-test diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index ae832c38..c85c8824 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -9,6 +9,8 @@ phutil_register_library_map(array( '__library_version__' => 2, 'class' => array( + 'ArcanistAbstractMethodBodyXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistAbstractMethodBodyXHPASTLinterRule.php', + 'ArcanistAbstractMethodBodyXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistAbstractMethodBodyXHPASTLinterRuleTestCase.php', 'ArcanistAbstractPrivateMethodXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistAbstractPrivateMethodXHPASTLinterRule.php', 'ArcanistAbstractPrivateMethodXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistAbstractPrivateMethodXHPASTLinterRuleTestCase.php', 'ArcanistAliasFunctionXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistAliasFunctionXHPASTLinterRule.php', @@ -395,6 +397,8 @@ phutil_register_library_map(array( ), 'function' => array(), 'xmap' => array( + 'ArcanistAbstractMethodBodyXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', + 'ArcanistAbstractMethodBodyXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistAbstractPrivateMethodXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistAbstractPrivateMethodXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistAliasFunctionXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', diff --git a/src/lint/linter/xhpast/ArcanistXHPASTLinterRule.php b/src/lint/linter/xhpast/ArcanistXHPASTLinterRule.php index 8c40e5c3..f780ec6c 100644 --- a/src/lint/linter/xhpast/ArcanistXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/ArcanistXHPASTLinterRule.php @@ -225,6 +225,34 @@ abstract class ArcanistXHPASTLinterRule extends Phobject { return AASTNodeList::newFromTreeAndNodes($root->getTree(), $nodes); } + /** + * Get class/method modifiers. + * + * @param XHPASTNode A node of type `n_CLASS_DECLARATION` or + * `n_METHOD_DECLARATION`. + * @return map Class/method modifiers. + */ + final protected 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; + } + /** * Get PHP superglobals. * diff --git a/src/lint/linter/xhpast/rules/ArcanistAbstractMethodBodyXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistAbstractMethodBodyXHPASTLinterRule.php new file mode 100644 index 00000000..277e15e0 --- /dev/null +++ b/src/lint/linter/xhpast/rules/ArcanistAbstractMethodBodyXHPASTLinterRule.php @@ -0,0 +1,30 @@ +selectDescendantsOfType('n_METHOD_DECLARATION'); + + foreach ($methods as $method) { + $modifiers = $this->getModifiers($method); + $body = $method->getChildByIndex(5); + + if (idx($modifiers, 'abstract') && $body->getTypeName() != 'n_EMPTY') { + $this->raiseLintAtNode( + $body, + pht( + '`%s` methods cannot contain a body. This construct will '. + 'cause a fatal error.', + 'abstract')); + } + } + } + +} diff --git a/src/lint/linter/xhpast/rules/ArcanistClassMustBeDeclaredAbstractXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistClassMustBeDeclaredAbstractXHPASTLinterRule.php index 1d3b4f6e..859dc319 100644 --- a/src/lint/linter/xhpast/rules/ArcanistClassMustBeDeclaredAbstractXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistClassMustBeDeclaredAbstractXHPASTLinterRule.php @@ -43,32 +43,4 @@ final class ArcanistClassMustBeDeclaredAbstractXHPASTLinterRule } } - /** - * Get class/method modifiers. - * - * @param XHPASTNode A node of type `n_CLASS_DECLARATION` or - * `n_METHOD_DECLARATION`. - * @return map 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; - } - } diff --git a/src/lint/linter/xhpast/rules/__tests__/ArcanistAbstractMethodBodyXHPASTLinterRuleTestCase.php b/src/lint/linter/xhpast/rules/__tests__/ArcanistAbstractMethodBodyXHPASTLinterRuleTestCase.php new file mode 100644 index 00000000..133e6b9a --- /dev/null +++ b/src/lint/linter/xhpast/rules/__tests__/ArcanistAbstractMethodBodyXHPASTLinterRuleTestCase.php @@ -0,0 +1,10 @@ +executeTestsInDirectory(dirname(__FILE__).'/abstract-method-body/'); + } + +} diff --git a/src/lint/linter/xhpast/rules/__tests__/abstract-method-body/body.lint-test b/src/lint/linter/xhpast/rules/__tests__/abstract-method-body/body.lint-test new file mode 100644 index 00000000..0ffc86e1 --- /dev/null +++ b/src/lint/linter/xhpast/rules/__tests__/abstract-method-body/body.lint-test @@ -0,0 +1,6 @@ +