Allow XHPAST lint name rules to be overridden through configuration

Summary:
See T326. Allow lint rules to be selectively overridden, e.g. for Conduit
methods.

Since FB has a long history of suggesting crazy patches for this stuff I think
we're safer just adding a hook class than trying to do some kind of regexp
magic.

Test Plan: Wrote a hook for Phabricator and linted some Conduit files without
issues. Ran unit tests.

Reviewers: nh, jungejason, tuomaspelkonen, aran

Reviewed By: nh

CC: aran, nh

Differential Revision: 874

src/__phutil_library_map__.php

@@ -80,6 +80,7 @@ phutil_register_library_map(array(
     'ArcanistUsageException' => 'exception/usage',
     'ArcanistUserAbortException' => 'exception/usage/userabort',
     'ArcanistWorkingCopyIdentity' => 'workingcopyidentity',
+    'ArcanistXHPASTLintNamingHook' => 'lint/linter/xhpast/naminghook',
     'ArcanistXHPASTLinter' => 'lint/linter/xhpast',
     'ArcanistXHPASTLinterTestCase' => 'lint/linter/xhpast/__tests__',
     'BranchInfo' => 'branch',

src/lint/linter/xhpast/ArcanistXHPASTLinter.php

@@ -665,27 +665,37 @@ class ArcanistXHPASTLinter extends ArcanistLinter {
   }
 
   protected function lintNamingConventions($root) {
+
+    // We're going to build up a list of <type, name, token, error> tuples
+    // and then try to instantiate a hook class which has the opportunity to
+    // override us.
+    $names = array();
+
     $classes = $root->selectDescendantsOfType('n_CLASS_DECLARATION');
     foreach ($classes as $class) {
       $name_token = $class->getChildByIndex(1);
       $name_string = $name_token->getConcreteString();
       $is_xhp = ($name_string[0] == ':');
       if ($is_xhp) {
-        if (!$this->isLowerCaseWithXHP($name_string)) {
+        $names[] = array(
-          $this->raiseLintAtNode(
+          'xhp-class',
-            $name_token,
+          $name_string,
-            self::LINT_NAMING_CONVENTIONS,
+          $name_token,
-            'Follow naming conventions: xhp elements should be named using '.
+          $this->isLowerCaseWithXHP($name_string)
-            'lower case.');
+            ? null
-        }
+            : 'Follow naming conventions: XHP elements should be named using '.
+              'lower case.',
+        );
       } else {
-        if (!$this->isUpperCamelCase($name_string)) {
+        $names[] = array(
-          $this->raiseLintAtNode(
+          'class',
-            $name_token,
+          $name_string,
-            self::LINT_NAMING_CONVENTIONS,
+          $name_token,
-            'Follow naming conventions: classes should be named using '.
+          $this->isUpperCamelCase($name_string)
-            'UpperCamelCase.');
+            ? null
-        }
+            : 'Follow naming conventions: classes should be named using '.
+              'UpperCamelCase.',
+        );
       }
     }
 
@@ -693,13 +703,15 @@ class ArcanistXHPASTLinter extends ArcanistLinter {
     foreach ($ifaces as $iface) {
       $name_token = $iface->getChildByIndex(1);
       $name_string = $name_token->getConcreteString();
-      if (!$this->isUpperCamelCase($name_string)) {
+      $names[] = array(
-        $this->raiseLintAtNode(
+        'interface',
-          $name_token,
+        $name_string,
-          self::LINT_NAMING_CONVENTIONS,
+        $name_token,
-          'Follow naming conventions: interfaces should be named using '.
+        $this->isUpperCamelCase($name_string)
-          'UpperCamelCase.');
+          ? null
-      }
+          : 'Follow naming conventions: interfaces should be named using '.
+            'UpperCamelCase.',
+      );
     }
 
 
@@ -711,13 +723,15 @@ class ArcanistXHPASTLinter extends ArcanistLinter {
         continue;
       }
       $name_string = $name_token->getConcreteString();
-      if (!$this->isLowercaseWithUnderscores($name_string)) {
+      $names[] = array(
-        $this->raiseLintAtNode(
+        'function',
-          $name_token,
+        $name_string,
-          self::LINT_NAMING_CONVENTIONS,
+        $name_token,
-          'Follow naming conventions: functions should be named using '.
+        $this->isLowercaseWithUnderscores($name_string)
-          'lowercase_with_underscores.');
+          ? null
-      }
+          : 'Follow naming conventions: functions should be named using '.
+            'lowercase_with_underscores.',
+      );
     }
 
 
@@ -725,13 +739,15 @@ class ArcanistXHPASTLinter extends ArcanistLinter {
     foreach ($methods as $method) {
       $name_token = $method->getChildByIndex(2);
       $name_string = $name_token->getConcreteString();
-      if (!$this->isLowerCamelCase($name_string)) {
+      $names[] = array(
-        $this->raiseLintAtNode(
+        'method',
-          $name_token,
+        $name_string,
-          self::LINT_NAMING_CONVENTIONS,
+        $name_token,
-          'Follow naming conventions: methods should be named using '.
+        $this->isLowerCamelCase($name_string)
-          'lowerCamelCase.');
+          ? null
-      }
+          : 'Follow naming conventions: methods should be named using '.
+            'lowerCamelCase.',
+      );
     }
 
 
@@ -740,13 +756,15 @@ class ArcanistXHPASTLinter extends ArcanistLinter {
       foreach ($param_list->getChildren() as $param) {
         $name_token = $param->getChildByIndex(1);
         $name_string = $name_token->getConcreteString();
-        if (!$this->isLowercaseWithUnderscores($name_string)) {
+        $names[] = array(
-          $this->raiseLintAtNode(
+          'parameter',
-            $name_token,
+          $name_string,
-            self::LINT_NAMING_CONVENTIONS,
+          $name_token,
-            'Follow naming conventions: parameters should be named using '.
+          $this->isLowercaseWithUnderscores($name_string)
-            'lowercase_with_underscores.');
+            ? null
-        }
+            : 'Follow naming conventions: parameters should be named using '.
+              'lowercase_with_underscores.',
+        );
       }
     }
 
@@ -757,13 +775,15 @@ class ArcanistXHPASTLinter extends ArcanistLinter {
       foreach ($constant_list->getChildren() as $constant) {
         $name_token = $constant->getChildByIndex(0);
         $name_string = $name_token->getConcreteString();
-        if (!$this->isUppercaseWithUnderscores($name_string)) {
+        $names[] = array(
-          $this->raiseLintAtNode(
+          'constant',
-            $name_token,
+          $name_string,
-            self::LINT_NAMING_CONVENTIONS,
+          $name_token,
-            'Follow naming conventions: class constants should be named using '.
+          $this->isUppercaseWithUnderscores($name_string)
-            'UPPERCASE_WITH_UNDERSCORES.');
+            ? null
-        }
+            : 'Follow naming conventions: class constants should be named '.
+              'using UPPERCASE_WITH_UNDERSCORES.',
+        );
       }
     }
 
@@ -775,15 +795,45 @@ class ArcanistXHPASTLinter extends ArcanistLinter {
         }
         $name_token = $prop->getChildByIndex(0);
         $name_string = $name_token->getConcreteString();
-        if (!$this->isLowerCamelCase($name_string)) {
+        $names[] = array(
-          $this->raiseLintAtNode(
+          'member',
-            $name_token,
+          $name_string,
-            self::LINT_NAMING_CONVENTIONS,
+          $name_token,
-            'Follow naming conventions: class properties should be named '.
+          $this->isLowerCamelCase($name_string)
-            'using lowerCamelCase.');
+            ? null
+            : 'Follow naming conventions: class properties should be named '.
+              'using lowerCamelCase.',
+        );
+      }
+    }
+
+    $engine = $this->getEngine();
+    $working_copy = $engine->getWorkingCopy();
+
+    if ($working_copy) {
+      // If a naming hook is configured, give it a chance to override the
+      // default results for all the symbol names.
+      $hook_class = $working_copy->getConfig('lint.xhpast.naminghook');
+      if ($hook_class) {
+        $hook_obj = newv($hook_class, array());
+        foreach ($names as $k => $name_attrs) {
+          list($type, $name, $token, $default) = $name_attrs;
+          $result = $hook_obj->lintSymbolName($type, $name, $default);
+          $names[$k][3] = $result;
         }
       }
     }
+
+    // Raise anything we're left with.
+    foreach ($names as $k => $name_attrs) {
+      list($type, $name, $token, $result) = $name_attrs;
+      if ($result) {
+        $this->raiseLintAtNode(
+          $token,
+          self::LINT_NAMING_CONVENTIONS,
+          $result);
+      }
+    }
   }
 
   protected function isUpperCamelCase($str) {

src/lint/linter/xhpast/naminghook/ArcanistXHPASTLintNamingHook.php

@@ -0,0 +1,60 @@
+<?php
+
+/*
+ * Copyright 2011 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/**
+ * You can extend this class and set "lint.xhpast.naminghook" in your
+ * .arcconfig to have an opportunity to override lint results for symbol names.
+ *
+ * @task override Overriding Symbol Name Lint
+ * @group lint
+ */
+abstract class ArcanistXHPASTLintNamingHook {
+
+  final public function __construct() {
+    // <empty>
+  }
+
+  /**
+   * Callback invoked for each symbol, which can override the default
+   * determination of name validity or accept it by returning $default. The
+   * symbol types are: xhp-class, class, interface, function, method, parameter,
+   * constant, and member.
+   *
+   * For example, if you want to ban all symbols with "quack" in them and
+   * otherwise accept all the defaults, except allow any naming convention for
+   * methods with "duck" in them, you might implement the method like this:
+   *
+   *   if (preg_match('/quack/i', $name)) {
+   *     return 'Symbol names containing "quack" are forbidden.';
+   *   }
+   *   if ($type == 'method' && preg_match('/duck/i', $name)) {
+   *     return null; // Always accept.
+   *   }
+   *   return $default;
+   *
+   * @param   string      The symbol type.
+   * @param   string      The symbol name.
+   * @param   string|null The default result from the main rule engine.
+   * @return  string|null Null to accept the name, or a message to reject it
+   *                      with. You should return the default value if you don't
+   *                      want to specifically provide an override.
+   * @task override
+   */
+  abstract public function lintSymbolName($type, $name, $default);
+
+}

src/lint/linter/xhpast/naminghook/__init__.php

@@ -0,0 +1,10 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+
+phutil_require_source('ArcanistXHPASTLintNamingHook.php');