From 6c759ae34396a96b3ec94a269f0fe635a4a47879 Mon Sep 17 00:00:00 2001 From: Joshua Spence Date: Tue, 11 Aug 2015 06:49:20 +1000 Subject: [PATCH] Improve useless overriding method linter rule Summary: Improve `ArcanistUselessOverridingMethodXHPASTLinterRule` by allowing overriding methods which set default values. For example, the following scenario is perfectly valid: ```lang=php class SomeClass { public function __construct($x) {} } class SomeOtherClass extends Class { public function __construct($x = null) { parent::__construct($x); } } ``` Test Plan: Added test case. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D13840 --- .../__tests__/xhpast/useless-overriding-method.lint-test | 4 ++++ .../ArcanistUselessOverridingMethodXHPASTLinterRule.php | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/src/lint/linter/__tests__/xhpast/useless-overriding-method.lint-test b/src/lint/linter/__tests__/xhpast/useless-overriding-method.lint-test index 308bf76f..40771407 100644 --- a/src/lint/linter/__tests__/xhpast/useless-overriding-method.lint-test +++ b/src/lint/linter/__tests__/xhpast/useless-overriding-method.lint-test @@ -16,6 +16,10 @@ final class MyClass extends SomeOtherClass { public function anotherMethod($x, &$y) { return parent::anotherMethod($x, $y); } + + public function usefulMethodWithDefaultValue($x = null) { + return parent::usefulMethodWithDefaultValue($x); + } } ~~~~~~~~~~ error:3:13 diff --git a/src/lint/linter/xhpast/rules/ArcanistUselessOverridingMethodXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistUselessOverridingMethodXHPASTLinterRule.php index 8a4859e5..475ed25f 100644 --- a/src/lint/linter/xhpast/rules/ArcanistUselessOverridingMethodXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistUselessOverridingMethodXHPASTLinterRule.php @@ -26,12 +26,17 @@ final class ArcanistUselessOverridingMethodXHPASTLinterRule $parameters = array(); foreach ($parameter_list->getChildren() as $parameter) { + $default = $parameter->getChildByIndex(2); $parameter = $parameter->getChildByIndex(1); if ($parameter->getTypeName() == 'n_VARIABLE_REFERENCE') { $parameter = $parameter->getChildOfType(0, 'n_VARIABLE'); } + if ($default->getTypeName() != 'n_EMPTY') { + continue 2; + } + $parameters[] = $parameter->getConcreteString(); }