Add XHAST linter standards

Summary: Ref T8742. As mentioned in D13512. This still needs some work, but looks roughly how I expect it to. Mainly, I want to move the standards stuff to the linter itself rather than the linter rule. I wanted to push this out for some initial feedback though.

Test Plan: This still needs work.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Maniphest Tasks: T8742

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

.arclint

@@ -45,15 +45,7 @@
     "xhpast": {
       "type": "xhpast",
       "include": "(\\.php$)",
-      "severity": {
+      "standard": "phutil.xhpast"
-        "16": "advice",
-        "34": "error"
-      },
-      "xhpast.blacklisted.function": {
-        "eval": "The eval() function should be avoided. It is potentially unsafe and makes debugging more difficult."
-      },
-      "xhpast.php-version": "5.2.3",
-      "xhpast.php-version.windows": "5.3.0"
     }
   }
 }

src/__phutil_library_map__.php

@@ -161,6 +161,8 @@ phutil_register_library_map(array(
     'ArcanistLintSeverity' => 'lint/ArcanistLintSeverity.php',
     'ArcanistLintWorkflow' => 'workflow/ArcanistLintWorkflow.php',
     'ArcanistLinter' => 'lint/linter/ArcanistLinter.php',
+    'ArcanistLinterStandard' => 'lint/linter/standards/ArcanistLinterStandard.php',
+    'ArcanistLinterStandardTestCase' => 'lint/linter/standards/__tests__/ArcanistLinterStandardTestCase.php',
     'ArcanistLinterTestCase' => 'lint/linter/__tests__/ArcanistLinterTestCase.php',
     'ArcanistLintersWorkflow' => 'workflow/ArcanistLintersWorkflow.php',
     'ArcanistListAssignmentXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistListAssignmentXHPASTLinterRule.php',
@@ -202,6 +204,7 @@ phutil_register_library_map(array(
     'ArcanistPhrequentWorkflow' => 'workflow/ArcanistPhrequentWorkflow.php',
     'ArcanistPhutilLibraryLinter' => 'lint/linter/ArcanistPhutilLibraryLinter.php',
     'ArcanistPhutilXHPASTLinter' => 'lint/linter/ArcanistPhutilXHPASTLinter.php',
+    'ArcanistPhutilXHPASTLinterStandard' => 'lint/linter/standards/phutil/ArcanistPhutilXHPASTLinterStandard.php',
     'ArcanistPhutilXHPASTLinterTestCase' => 'lint/linter/__tests__/ArcanistPhutilXHPASTLinterTestCase.php',
     'ArcanistPlusOperatorOnStringsXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistPlusOperatorOnStringsXHPASTLinterRule.php',
     'ArcanistPregQuoteMisuseXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistPregQuoteMisuseXHPASTLinterRule.php',
@@ -450,6 +453,8 @@ phutil_register_library_map(array(
     'ArcanistLintSeverity' => 'Phobject',
     'ArcanistLintWorkflow' => 'ArcanistWorkflow',
     'ArcanistLinter' => 'Phobject',
+    'ArcanistLinterStandard' => 'Phobject',
+    'ArcanistLinterStandardTestCase' => 'PhutilTestCase',
     'ArcanistLinterTestCase' => 'PhutilTestCase',
     'ArcanistLintersWorkflow' => 'ArcanistWorkflow',
     'ArcanistListAssignmentXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
@@ -491,6 +496,7 @@ phutil_register_library_map(array(
     'ArcanistPhrequentWorkflow' => 'ArcanistWorkflow',
     'ArcanistPhutilLibraryLinter' => 'ArcanistLinter',
     'ArcanistPhutilXHPASTLinter' => 'ArcanistBaseXHPASTLinter',
+    'ArcanistPhutilXHPASTLinterStandard' => 'ArcanistLinterStandard',
     'ArcanistPhutilXHPASTLinterTestCase' => 'ArcanistLinterTestCase',
     'ArcanistPlusOperatorOnStringsXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistPregQuoteMisuseXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',

src/lint/linter/ArcanistLinter.php

@@ -196,7 +196,6 @@ abstract class ArcanistLinter extends Phobject {
     return;
   }
 
-
   public function getLinterPriority() {
     return 1.0;
   }
@@ -209,6 +208,11 @@ abstract class ArcanistLinter extends Phobject {
     return $this;
   }
 
+  public function addCustomSeverityMap(array $map) {
+    $this->customSeverityMap = $this->customSeverityMap + $map;
+    return $this;
+  }
+
   final public function setCustomSeverityRules(array $rules) {
     $this->customSeverityRules = $rules;
     return $this;
@@ -490,6 +494,10 @@ abstract class ArcanistLinter extends Phobject {
           'Provide a map of regular expressions to severity levels. All '.
           'matching codes have their severity adjusted.'),
       ),
+      'standard' => array(
+        'type' => 'optional string | list<string>',
+        'help' => pht('The coding standard(s) to apply.'),
+      ),
     );
   }
 
@@ -548,6 +556,22 @@ abstract class ArcanistLinter extends Phobject {
         }
         $this->setCustomSeverityRules($value);
         return;
+
+      case 'standard':
+        $standards = (array)$value;
+
+        foreach ($standards as $standard) {
+          $standard = ArcanistLinterStandard::getStandard($value, $this);
+
+          foreach ($standard->getLinterConfiguration() as $k => $v) {
+            $this->setLinterConfigurationValue($k, $v);
+          }
+          $this->addCustomSeverityMap($standard->getLinterSeverityMap());
+        }
+
+        return;
+
+
     }
 
     throw new Exception(pht('Incomplete implementation: %s!', $key));

src/lint/linter/standards/ArcanistLinterStandard.php

@@ -0,0 +1,121 @@
+<?php
+
+/**
+ * A "linter standard" is a collection of linter rules with associated
+ * severities and configuration.
+ *
+ * Basically, a linter standard allows a set of linter rules and configuration
+ * to be easily reused across multiple repositories without duplicating the
+ * contents of the `.arclint` file (and the associated maintenance costs in
+ * keeping changes to this file synchronized).
+ */
+abstract class ArcanistLinterStandard extends Phobject {
+
+  /**
+   * Returns a unique identifier for the linter standard.
+   *
+   * @return string
+   */
+  abstract public function getKey();
+
+  /**
+   * Returns a human-readable name for the linter standard.
+   *
+   * @return string
+   */
+  abstract public function getName();
+
+  /**
+   * Returns a human-readable description for the linter standard.
+   *
+   * @return string
+   */
+  abstract public function getDescription();
+
+  /**
+   * Checks whether the linter standard supports a specified linter.
+   *
+   * @param  ArcanistLinter  The linter which is being configured.
+   * @return bool            True if the linter standard supports the specified
+   *                         linter, otherwise false.
+   */
+  abstract public function supportsLinter(ArcanistLinter $linter);
+
+  /**
+   * Get linter configuration.
+   *
+   * Returns linter configuration which is passed to
+   * @{method:ArcanistLinter::setLinterConfigurationValue}.
+   *
+   * @return map<string, wild>
+   */
+  public function getLinterConfiguration() {
+    return array();
+  }
+
+  /**
+   * Get linter severities.
+   *
+   * Returns linter severities which are passed to
+   * @{method:ArcanistLinter::addCustomSeverityMap}.
+   *
+   * @return map
+   */
+  public function getLinterSeverityMap() {
+    return array();
+  }
+
+  /**
+   * Load a linter standard by key.
+   *
+   * @param  string
+   * @param  ArcanistLinter
+   * @return ArcanistLinterStandard
+   */
+  final public static function getStandard($key, ArcanistLinter $linter) {
+    $standards = self::loadAllStandardsForLinter($linter);
+
+    if (empty($standards[$key])) {
+      throw new ArcanistUsageException(
+        pht(
+          'No such linter standard. Available standards are: %s.',
+          implode(', ', array_keys($standards))));
+    }
+
+    return $standards[$key];
+  }
+
+  /**
+   * Load all linter standards.
+   *
+   * @return list<ArcanistLinterStandard>
+   */
+  final public static function loadAllStandards() {
+    return id(new PhutilClassMapQuery())
+      ->setAncestorClass(__CLASS__)
+      ->setUniqueMethod('getKey')
+      ->execute();
+  }
+
+  /**
+   * Load all linter standards which support a specified linter.
+   *
+   * @param  ArcanistLinter
+   * @return list<ArcanistLinterStandard>
+   */
+  final public static function loadAllStandardsForLinter(
+    ArcanistLinter $linter) {
+
+    $all_standards = self::loadAllStandards();
+    $standards = array();
+
+    foreach ($all_standards as $standard) {
+      if ($standard->supportsLinter($linter)) {
+        $standards[$standard->getKey()] = $standard;
+      }
+    }
+
+    return $standards;
+  }
+
+}

src/lint/linter/standards/__tests__/ArcanistLinterStandardTestCase.php

@@ -0,0 +1,10 @@
+<?php
+
+final class ArcanistLinterStandardTestCase extends PhutilTestCase {
+
+  public function testLoadAllStandards() {
+    ArcanistLinterStandard::loadAllStandards();
+    $this->assertTrue(true);
+  }
+
+}

src/lint/linter/standards/phutil/ArcanistPhutilXHPASTLinterStandard.php

@@ -0,0 +1,45 @@
+<?php
+
+final class ArcanistPhutilXHPASTLinterStandard
+  extends ArcanistLinterStandard {
+
+  public function getKey() {
+    return 'phutil.xhpast';
+  }
+
+  public function getName() {
+    return pht('Phutil XHPAST');
+  }
+
+  public function getDescription() {
+    return pht('PHP Coding Standards for Phutil libraries.');
+  }
+
+  public function supportsLinter(ArcanistLinter $linter) {
+    return $linter instanceof ArcanistXHPASTLinter;
+  }
+
+  public function getLinterConfiguration() {
+    return array(
+      'xhpast.blacklisted.function' => array(
+        'eval' => pht(
+          'The `%s` function should be avoided. It is potentially unsafe '.
+          'and makes debugging more difficult.',
+          'eval'),
+      'xhpast.php-version' => '5.2.3',
+      'xhpast.php-version.windows' => '5.3.0',
+      ),
+    );
+  }
+
+  public function getLinterSeverityMap() {
+    $advice = ArcanistLintSeverity::SEVERITY_ADVICE;
+    $error  = ArcanistLintSeverity::SEVERITY_ERROR;
+
+    return array(
+      ArcanistTodoCommentXHPASTLinterRule::ID    => $advice,
+      ArcanistCommentSpacingXHPASTLinterRule::ID => $error,
+    );
+  }
+
+}