diff --git a/src/lint/linter/ArcanistXHPASTLinter.php b/src/lint/linter/ArcanistXHPASTLinter.php index 4a26767e..f076ac96 100644 --- a/src/lint/linter/ArcanistXHPASTLinter.php +++ b/src/lint/linter/ArcanistXHPASTLinter.php @@ -62,6 +62,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter { const LINT_DEFAULT_PARAMETERS = 60; const LINT_LOWERCASE_FUNCTIONS = 61; const LINT_CLASS_NAME_LITERAL = 62; + const LINT_USELESS_OVERRIDING_METHOD = 63; private $blacklistedFunctions = array(); private $naminghook; @@ -194,6 +195,8 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter { => pht('Lowercase Functions'), self::LINT_CLASS_NAME_LITERAL => pht('Class Name Literal'), + self::LINT_USELESS_OVERRIDING_METHOD + => pht('Useless Overriding Method'), ); } @@ -244,6 +247,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter { self::LINT_DEFAULT_PARAMETERS => $warning, self::LINT_LOWERCASE_FUNCTIONS => $advice, self::LINT_CLASS_NAME_LITERAL => $advice, + self::LINT_USELESS_OVERRIDING_METHOD => $advice, ); } @@ -311,7 +315,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter { public function getVersion() { // The version number should be incremented whenever a new rule is added. - return '25'; + return '26'; } protected function resolveFuture($path, Future $future) { @@ -400,6 +404,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter { 'lintDefaultParameters' => self::LINT_DEFAULT_PARAMETERS, 'lintLowercaseFunctions' => self::LINT_LOWERCASE_FUNCTIONS, 'lintClassNameLiteral' => self::LINT_CLASS_NAME_LITERAL, + 'lintUselessOverridingMethods' => self::LINT_USELESS_OVERRIDING_METHOD, ); foreach ($method_codes as $method => $codes) { @@ -3766,6 +3771,85 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter { } } + private function lintUselessOverridingMethods(XHPASTNode $root) { + $methods = $root->selectDescendantsOfType('n_METHOD_DECLARATION'); + + foreach ($methods as $method) { + $method_name = $method + ->getChildOfType(2, 'n_STRING') + ->getConcreteString(); + + $parameter_list = $method + ->getChildOfType(3, 'n_DECLARATION_PARAMETER_LIST'); + $parameters = array(); + + foreach ($parameter_list->getChildren() as $parameter) { + $parameters[] = $parameter + ->getChildOfType(1, 'n_VARIABLE') + ->getConcreteString(); + } + + $statements = $method->getChildByIndex(5); + + if ($statements->getTypeName() != 'n_STATEMENT_LIST') { + continue; + } + + if (count($statements->getChildren()) != 1) { + continue; + } + + $statement = $statements + ->getChildOfType(0, 'n_STATEMENT') + ->getChildByIndex(0); + + if ($statement->getTypeName() == 'n_RETURN') { + $statement = $statement->getChildByIndex(0); + } + + if ($statement->getTypeName() != 'n_FUNCTION_CALL') { + continue; + } + + $function = $statement->getChildByIndex(0); + + if ($function->getTypeName() != 'n_CLASS_STATIC_ACCESS') { + continue; + } + + $called_class = $function->getChildOfType(0, 'n_CLASS_NAME'); + $called_method = $function->getChildOfType(1, 'n_STRING'); + + if ($called_class->getConcreteString() != 'parent') { + continue; + } else if ($called_method->getConcreteString() != $method_name) { + continue; + } + + $params = $statement + ->getChildOfType(1, 'n_CALL_PARAMETER_LIST') + ->getChildren(); + + foreach ($params as $param) { + if ($param->getTypeName() != 'n_VARIABLE') { + continue 2; + } + + $expected = array_shift($parameters); + + if ($param->getConcreteString() != $expected) { + continue 2; + } + } + + $this->raiseLintAtNode( + $method, + self::LINT_USELESS_OVERRIDING_METHOD, + pht('Useless overriding method.')); + } + } + + /** * Retrieve all calls to some specified function(s). * diff --git a/src/lint/linter/__tests__/xhpast/useless-overriding-method.lint-test b/src/lint/linter/__tests__/xhpast/useless-overriding-method.lint-test new file mode 100644 index 00000000..a6e2780f --- /dev/null +++ b/src/lint/linter/__tests__/xhpast/useless-overriding-method.lint-test @@ -0,0 +1,19 @@ +