Add a linter rule for default parameters

Summary: Ref T7409. Adds `ArcanistXHPASTLinter::LINT_DEFAULT_PARAMETERS` for ensuring function/method parameters with a default value are declared after those without a default value. Based on [[https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/Standards/PEAR/Sniffs/Functions/ValidDefaultValueSniff.php | PEAR_Sniffs_Functions_ValidDefaultValueSniff]].

Test Plan: Added test cases.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7409

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

src/lint/linter/ArcanistXHPASTLinter.php

@@ -59,6 +59,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
   const LINT_SELF_MEMBER_REFERENCE      = 57;
   const LINT_LOGICAL_OPERATORS          = 58;
   const LINT_INNER_FUNCTION             = 59;
+  const LINT_DEFAULT_PARAMETERS         = 60;
 
   private $blacklistedFunctions = array();
   private $naminghook;
@@ -185,6 +186,8 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
         => pht('Logical Operators'),
       self::LINT_INNER_FUNCTION
         => pht('Inner Functions'),
+      self::LINT_DEFAULT_PARAMETERS
+        => pht('Default Parameters'),
     );
   }
 
@@ -232,6 +235,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
       self::LINT_SELF_MEMBER_REFERENCE      => $advice,
       self::LINT_LOGICAL_OPERATORS          => $advice,
       self::LINT_INNER_FUNCTION             => $warning,
+      self::LINT_DEFAULT_PARAMETERS         => $warning,
     );
   }
 
@@ -299,7 +303,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
 
   public function getVersion() {
     // The version number should be incremented whenever a new rule is added.
-    return '22';
+    return '23';
   }
 
   protected function resolveFuture($path, Future $future) {
@@ -385,6 +389,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
       'lintSelfMemberReference' => self::LINT_SELF_MEMBER_REFERENCE,
       'lintLogicalOperators' => self::LINT_LOGICAL_OPERATORS,
       'lintInnerFunctions' => self::LINT_INNER_FUNCTION,
+      'lintDefaultParameters' => self::LINT_DEFAULT_PARAMETERS,
     );
 
     foreach ($method_codes as $method => $codes) {
@@ -3650,6 +3655,32 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
     }
   }
 
+  private function lintDefaultParameters(XHPASTNode $root) {
+    $parameter_lists = $root->selectDescendantsOfType(
+      'n_DECLARATION_PARAMETER_LIST');
+
+    foreach ($parameter_lists as $parameter_list) {
+      $default_found = false;
+      $parameters    = $parameter_list->selectDescendantsOfType(
+        'n_DECLARATION_PARAMETER');
+
+      foreach ($parameters as $parameter) {
+        $default_value = $parameter->getChildByIndex(2);
+
+        if ($default_value->getTypeName() != 'n_EMPTY') {
+          $default_found = true;
+        } else if ($default_found) {
+          $this->raiseLintAtNode(
+            $parameter_list,
+            self::LINT_DEFAULT_PARAMETERS,
+            pht(
+              'Arguments with default values must be at the end '.
+              'of the argument list.'));
+        }
+      }
+    }
+  }
+
   /**
    * Retrieve all calls to some specified function(s).
    *

src/lint/linter/__tests__/xhpast/default-parameters.lint-test

@@ -0,0 +1,12 @@
+<?php
+function foo($x, $y, $z) {}
+function bar($x, $y = null, $z) {}
+function baz($x = null, $y = null, $z = null) {}
+
+class MyClass {
+  public function myMethod($x, $y = null, $z) {}
+}
+~~~~~~~~~~
+warning:3:13
+error:6:7
+warning:7:27

src/lint/linter/__tests__/xhpast/undeclared-variables.lint-test

@@ -71,9 +71,9 @@ function declparse(
   Q &$c,
   Q $d = null,
   Q &$e = null,
-  $f,
+  $f = null,
   $g = null,
-  &$h,
+  &$h = null,
   &$i = null) {
   $a++;
   $b++;