From e286ef66c8ce9334aad9820e4451b16c4f75a647 Mon Sep 17 00:00:00 2001
From: Joshua Spence <josh@joshuaspence.com>
Date: Thu, 23 Jul 2015 06:43:35 +1000
Subject: [PATCH] Improve the declaration parentheses linting

Summary: Improve `ArcanistClosingDeclarationParenthesesXHPASTLinterRule` (and rename it to `ArcanistDeclarationParenthesesXHPASTLinterRule`) to ensure that there is no whitespace before the opening parenthesis of a function/method declaration.

Test Plan: Added test cases.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D13675
---
 src/__phutil_library_map__.php                |  4 ++--
 .../xhpast/decl-parens-hug-closing.lint-test  | 21 +++++++++++--------
 ...eclarationParenthesesXHPASTLinterRule.php} | 19 +++++++++++++++--
 3 files changed, 31 insertions(+), 13 deletions(-)
 rename src/lint/linter/xhpast/rules/{ArcanistClosingDeclarationParenthesesXHPASTLinterRule.php => ArcanistDeclarationParenthesesXHPASTLinterRule.php} (64%)

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index e5e0583d..680febaf 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -43,7 +43,6 @@ phutil_register_library_map(array(
     'ArcanistCloseRevisionWorkflow' => 'workflow/ArcanistCloseRevisionWorkflow.php',
     'ArcanistCloseWorkflow' => 'workflow/ArcanistCloseWorkflow.php',
     'ArcanistClosingCallParenthesesXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistClosingCallParenthesesXHPASTLinterRule.php',
-    'ArcanistClosingDeclarationParenthesesXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistClosingDeclarationParenthesesXHPASTLinterRule.php',
     'ArcanistClosureLinter' => 'lint/linter/ArcanistClosureLinter.php',
     'ArcanistClosureLinterTestCase' => 'lint/linter/__tests__/ArcanistClosureLinterTestCase.php',
     'ArcanistCoffeeLintLinter' => 'lint/linter/ArcanistCoffeeLintLinter.php',
@@ -67,6 +66,7 @@ phutil_register_library_map(array(
     'ArcanistCppcheckLinterTestCase' => 'lint/linter/__tests__/ArcanistCppcheckLinterTestCase.php',
     'ArcanistCpplintLinter' => 'lint/linter/ArcanistCpplintLinter.php',
     'ArcanistCpplintLinterTestCase' => 'lint/linter/__tests__/ArcanistCpplintLinterTestCase.php',
+    'ArcanistDeclarationParenthesesXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistDeclarationParenthesesXHPASTLinterRule.php',
     'ArcanistDefaultParametersXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistDefaultParametersXHPASTLinterRule.php',
     'ArcanistDiffChange' => 'parser/diff/ArcanistDiffChange.php',
     'ArcanistDiffChangeType' => 'parser/diff/ArcanistDiffChangeType.php',
@@ -316,7 +316,6 @@ phutil_register_library_map(array(
     'ArcanistCloseRevisionWorkflow' => 'ArcanistWorkflow',
     'ArcanistCloseWorkflow' => 'ArcanistWorkflow',
     'ArcanistClosingCallParenthesesXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
-    'ArcanistClosingDeclarationParenthesesXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistClosureLinter' => 'ArcanistExternalLinter',
     'ArcanistClosureLinterTestCase' => 'ArcanistExternalLinterTestCase',
     'ArcanistCoffeeLintLinter' => 'ArcanistExternalLinter',
@@ -340,6 +339,7 @@ phutil_register_library_map(array(
     'ArcanistCppcheckLinterTestCase' => 'ArcanistExternalLinterTestCase',
     'ArcanistCpplintLinter' => 'ArcanistExternalLinter',
     'ArcanistCpplintLinterTestCase' => 'ArcanistExternalLinterTestCase',
+    'ArcanistDeclarationParenthesesXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistDefaultParametersXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistDiffChange' => 'Phobject',
     'ArcanistDiffChangeType' => 'Phobject',
diff --git a/src/lint/linter/__tests__/xhpast/decl-parens-hug-closing.lint-test b/src/lint/linter/__tests__/xhpast/decl-parens-hug-closing.lint-test
index fd04a9c2..a7ea5adf 100644
--- a/src/lint/linter/__tests__/xhpast/decl-parens-hug-closing.lint-test
+++ b/src/lint/linter/__tests__/xhpast/decl-parens-hug-closing.lint-test
@@ -2,9 +2,10 @@
 
 function f($x) {}
 function g($x ) {}
+function h ($x) {}
 
-function &h($x) {}
-function &i($x ) {}
+function &i($x) {}
+function &j($x ) {}
 
 final class X {
 
@@ -24,21 +25,23 @@ f(function($x ) {});
 f(function($x ) use ($z) {});
 ~~~~~~~~~~
 warning:4:14
-warning:7:15
-error:9:13
-warning:12:23
-warning:15:31
-warning:18:33
-warning:23:14
+warning:5:11
+warning:8:15
+error:10:13
+warning:11:23
+warning:16:31
+warning:19:33
 warning:24:14
+warning:25:14
 ~~~~~~~~~~
 <?php
 
 function f($x) {}
 function g($x) {}
+function h($x) {}
 
-function &h($x) {}
 function &i($x) {}
+function &j($x) {}
 
 final class X {
 
diff --git a/src/lint/linter/xhpast/rules/ArcanistClosingDeclarationParenthesesXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistDeclarationParenthesesXHPASTLinterRule.php
similarity index 64%
rename from src/lint/linter/xhpast/rules/ArcanistClosingDeclarationParenthesesXHPASTLinterRule.php
rename to src/lint/linter/xhpast/rules/ArcanistDeclarationParenthesesXHPASTLinterRule.php
index 7fb777c9..357ac77f 100644
--- a/src/lint/linter/xhpast/rules/ArcanistClosingDeclarationParenthesesXHPASTLinterRule.php
+++ b/src/lint/linter/xhpast/rules/ArcanistDeclarationParenthesesXHPASTLinterRule.php
@@ -1,6 +1,6 @@
 <?php
 
-final class ArcanistClosingDeclarationParenthesesXHPASTLinterRule
+final class ArcanistDeclarationParenthesesXHPASTLinterRule
   extends ArcanistXHPASTLinterRule {
 
   const ID = 38;
@@ -22,11 +22,26 @@ final class ArcanistClosingDeclarationParenthesesXHPASTLinterRule
     foreach ($decs as $dec) {
       $params = $dec->getChildOfType(3, 'n_DECLARATION_PARAMETER_LIST');
       $tokens = $params->getTokens();
-      $last = array_pop($tokens);
+
+      $first = head($tokens);
+      $last = last($tokens);
+
+      $leading = $first->getNonsemanticTokensBefore();
+      $leading_text = implode('', mpull($leading, 'getValue'));
 
       $trailing = $last->getNonsemanticTokensBefore();
       $trailing_text = implode('', mpull($trailing, 'getValue'));
 
+      if (preg_match('/^\s+$/', $leading_text)) {
+        $this->raiseLintAtOffset(
+          $first->getOffset() - strlen($leading_text),
+          pht(
+            'Convention: no spaces before opening parenthesis in '.
+            'function and method declarations.'),
+          $leading_text,
+          '');
+      }
+
       if (preg_match('/^\s+$/', $trailing_text)) {
         $this->raiseLintAtOffset(
           $last->getOffset() - strlen($trailing_text),