Add a linter rule for useless overriding methods

Summary: Ref T7409. Adds `ArcanistXHPASTLinter::LINT_USELESS_OVERRIDING_METHOD` for detecting method which only call their parent method. Based on [[https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/Standards/Generic/Sniffs/CodeAnalysis/UselessOverridingMethodSniff.php | Generic_Sniffs_CodeAnalysis_UselessOverridingMethodSniff]].

Test Plan: Added unit tests.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7409

Differential Revision: https://secure.phabricator.com/D12419

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).
    *

src/lint/linter/__tests__/xhpast/useless-overriding-method.lint-test

@@ -0,0 +1,19 @@
+<?php
+
+final class MyClass extends SomeOtherClass {
+  public function __construct() {
+    parent::__construct();
+  }
+
+  public function uselessMethod($x, array $y) {
+    return parent::uselessMethod($x, $y);
+  }
+
+  public function usefulMethod($x, array $y) {
+    return parent::usefulMethod($x, null);
+  }
+}
+~~~~~~~~~~
+error:3:13
+advice:4:3
+advice:8:3