Lay groundwork for configuration-driven linters

Summary:
Ref T2039. That task has a bunch of discussion, but basically we do a poor job of serving the midrange of lint configuration right now.

If you have something simple, the default linters work.

If you have something complex, building your own engine lets you do whatever you want.

But many users want something in between, which isn't really well accommodated. The idea is to let you write a `.arclint` file, which looks something like this:

  {
   "linters" : {
      "css" : {
        "type" : "csslint",
        "include" : "(\.css$)",
        "exclude" : "(^externals/)",
        "bin" : "/usr/local/bin/csslint"
      },
      "js" : {
        "type" : "jshint",
        "include" : "(\.js$)",
        "exclude" : "(^externals/)",
        "bin" : "support/bin/jshint",
        "interpreter" : "/usr/local/bin/node"
      }
   }
  }

...which will provide a bunch of common options around lint severity, interpreter and binary locaitons, included and excluded files, etc.

This implements some basics, and very rough support in the Filename linter.

Test Plan:
Generated a `.arclint` file and saw it apply filename lint correctly. Used `debug` mode and tried invalid regexps.

  {
    "debug" : true,
    "linters" : {
      "filename" : {
        "type" : "filename",
        "exclude" : ["@^externals/@"]
      }
    }
  }

Next steps include:

  - Provide an external linter archetype (T3186) and expose a common set of configuration here ("bin", "interpreter", "flags", "severity").
  - Provide a `.arcunit` file which works similarly (it can probably be simpler).

Reviewers: btrahan, Firehed

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2039

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

src/__phutil_library_map__.php

@@ -40,6 +40,7 @@ phutil_register_library_map(array(
     'ArcanistCommitWorkflow' => 'workflow/ArcanistCommitWorkflow.php',
     'ArcanistConduitLinter' => 'lint/linter/ArcanistConduitLinter.php',
     'ArcanistConfiguration' => 'configuration/ArcanistConfiguration.php',
+    'ArcanistConfigurationDrivenLintEngine' => 'lint/engine/ArcanistConfigurationDrivenLintEngine.php',
     'ArcanistCoverWorkflow' => 'workflow/ArcanistCoverWorkflow.php',
     'ArcanistCppcheckLinter' => 'lint/linter/ArcanistCppcheckLinter.php',
     'ArcanistCpplintLinter' => 'lint/linter/ArcanistCpplintLinter.php',
@@ -81,6 +82,7 @@ phutil_register_library_map(array(
     'ArcanistLandWorkflow' => 'workflow/ArcanistLandWorkflow.php',
     'ArcanistLiberateWorkflow' => 'workflow/ArcanistLiberateWorkflow.php',
     'ArcanistLicenseLinter' => 'lint/linter/ArcanistLicenseLinter.php',
+    'ArcanistLintConfiguration' => 'lint/ArcanistLintConfiguration.php',
     'ArcanistLintConsoleRenderer' => 'lint/renderer/ArcanistLintConsoleRenderer.php',
     'ArcanistLintEngine' => 'lint/engine/ArcanistLintEngine.php',
     'ArcanistLintJSONRenderer' => 'lint/renderer/ArcanistLintJSONRenderer.php',
@@ -200,6 +202,7 @@ phutil_register_library_map(array(
     'ArcanistCommentRemoverTestCase' => 'ArcanistTestCase',
     'ArcanistCommitWorkflow' => 'ArcanistBaseWorkflow',
     'ArcanistConduitLinter' => 'ArcanistLinter',
+    'ArcanistConfigurationDrivenLintEngine' => 'ArcanistLintEngine',
     'ArcanistCoverWorkflow' => 'ArcanistBaseWorkflow',
     'ArcanistCppcheckLinter' => 'ArcanistLinter',
     'ArcanistCpplintLinter' => 'ArcanistLinter',
@@ -230,6 +233,7 @@ phutil_register_library_map(array(
     'ArcanistLandWorkflow' => 'ArcanistBaseWorkflow',
     'ArcanistLiberateWorkflow' => 'ArcanistBaseWorkflow',
     'ArcanistLicenseLinter' => 'ArcanistLinter',
+    'ArcanistLintConfiguration' => 'Phobject',
     'ArcanistLintConsoleRenderer' => 'ArcanistLintRenderer',
     'ArcanistLintJSONRenderer' => 'ArcanistLintRenderer',
     'ArcanistLintLikeCompilerRenderer' => 'ArcanistLintRenderer',

src/lint/engine/ArcanistConfigurationDrivenLintEngine.php

@@ -0,0 +1,183 @@
+<?php
+
+final class ArcanistConfigurationDrivenLintEngine extends ArcanistLintEngine {
+
+  private $debugMode;
+
+  public function buildLinters() {
+    $working_copy = $this->getWorkingCopy();
+    $config_path = $working_copy->getProjectPath('.arclint');
+
+    if (!Filesystem::pathExists($config_path)) {
+      throw new Exception(
+        "Unable to find '.arclint' file to configure linters. Create a ".
+        "'.arclint' file in the root directory of the working copy.");
+    }
+
+    $data = Filesystem::readFile($config_path);
+    $config = json_decode($data, true);
+    if (!is_array($config)) {
+      throw new Exception(
+        "Expected '.arclint' file to be a valid JSON file, but failed to ".
+        "decode it: {$config_path}");
+    }
+
+    $linters = $this->loadAvailableLinters();
+
+    PhutilTypeSpec::checkMap(
+      $config,
+      array(
+        'linters' => 'map<string, map<string, wild>>',
+        'debug'   => 'optional bool',
+      ));
+
+    $this->debugMode = idx($config, 'debug', false);
+
+    $built_linters = array();
+    $all_paths = $this->getPaths();
+    foreach ($config['linters'] as $name => $spec) {
+      PhutilTypeSpec::checkMap(
+        $spec,
+        array(
+          'type' => 'string',
+          'include' => 'optional string | list<string>',
+          'exclude' => 'optional string | list<string>',
+        ));
+
+      $type = $spec['type'];
+      if (empty($linters[$type])) {
+        $list = implode(', ', array_keys($linters));
+        throw new Exception(
+          "Linter '{$name}' specifies invalid type '{$type}'. Available ".
+          "linters are: {$list}.");
+      }
+
+      $linter = clone $linters[$type];
+
+      $include = (array)idx($spec, 'include', array());
+      $exclude = (array)idx($spec, 'exclude', array());
+
+      $this->validateRegexps($include, $name, 'include');
+      $this->validateRegexps($exclude, $name, 'exclude');
+
+      $this->debugLog('Examining paths for linter "%s".', $name);
+      $paths = $this->matchPaths($all_paths, $include, $exclude);
+      $this->debugLog(
+        'Found %d matching paths for linter "%s".',
+        count($paths),
+        $name);
+
+      $linter->setPaths($paths);
+
+      $built_linters[] = $linter;
+    }
+
+    return $built_linters;
+  }
+
+  private function loadAvailableLinters() {
+    $linters = id(new PhutilSymbolLoader())
+      ->setAncestorClass('ArcanistLinter')
+      ->loadObjects();
+
+    $map = array();
+    foreach ($linters as $linter) {
+      $name = $linter->getLinterConfigurationName();
+
+      // This linter isn't selectable through configuration.
+      if ($name === null) {
+        continue;
+      }
+
+      if (empty($map[$name])) {
+        $map[$name] = $linter;
+        continue;
+      }
+
+      $orig_class = get_class($map[$name]);
+      $this_class = get_class($linter);
+      throw new Exception(
+        "Two linters ({$orig_class}, {$this_class}) both have the same ".
+        "configuration name ({$name}). Linters must have unique configuration ".
+        "names.");
+    }
+
+    return $map;
+  }
+
+  private function matchPaths(array $paths, array $include, array $exclude) {
+    $debug = $this->debugMode;
+
+    $match = array();
+    foreach ($paths as $path) {
+      $this->debugLog("Examining path '%s'...", $path);
+
+      $keep = false;
+      if (!$include) {
+        $keep = true;
+        $this->debugLog(
+          "  Including path by default because there is no 'include' rule.");
+      } else {
+        $this->debugLog('  Testing "include" rules.');
+        foreach ($include as $rule) {
+          if (preg_match($rule, $path)) {
+            $keep = true;
+            $this->debugLog('  Path matches include rule: %s', $rule);
+            break;
+          } else {
+            $this->debugLog('  Path does not match include rule: %s', $rule);
+          }
+        }
+      }
+
+      if (!$keep) {
+        $this->debugLog('  Path does not match any include rules, discarding.');
+        continue;
+      }
+
+      if ($exclude) {
+        $this->debugLog('  Testing "exclude" rules.');
+        foreach ($exclude as $rule) {
+          if (preg_match($rule, $path)) {
+            $this->debugLog('  Path matches "exclude" rule: %s', $rule);
+            continue 2;
+          } else {
+            $this->debugLog('  Path does not match "exclude" rule: %s', $rule);
+          }
+        }
+      }
+
+      $this->debugLog('  Path matches.');
+      $match[] = $path;
+    }
+
+    return $match;
+  }
+
+  private function validateRegexps(array $regexps, $linter, $config) {
+    foreach ($regexps as $regexp) {
+      $ok = @preg_match($regexp, '');
+      if ($ok === false) {
+        throw new Exception(
+          pht(
+            'Regular expression "%s" (in "%s" configuration for linter "%s") '.
+            'is not a valid regular expression.',
+            $regexp,
+            $config,
+            $linter));
+      }
+    }
+  }
+
+  private function debugLog($pattern /* , $arg, ... */) {
+    if (!$this->debugMode) {
+      return;
+    }
+
+    $console = PhutilConsole::getConsole();
+    $argv = func_get_args();
+    $argv[0] .= "\n";
+    call_user_func_array(array($console, 'writeErr'), $argv);
+  }
+
+}

src/lint/linter/ArcanistConduitLinter.php

@@ -41,7 +41,11 @@ final class ArcanistConduitLinter extends ArcanistLinter {
   private $linterName;
   private $lintByPath; // array(/pa/th/ => <lint>), valid after willLintPaths().
 
-  public function __construct($conduit_uri, $linter_name) {
+  public function __construct($conduit_uri = null, $linter_name = null) {
+
+    // TODO: Facebook uses this (probably?) but we need to be able to
+    // construct it without arguments for ".arclint".
+
     $this->conduitURI = $conduit_uri;
     $this->linterName = $linter_name;
   }

src/lint/linter/ArcanistFilenameLinter.php

@@ -21,6 +21,10 @@ final class ArcanistFilenameLinter extends ArcanistLinter {
     return array();
   }
 
+  public function getLinterConfigurationName() {
+    return 'filename';
+  }
+
   public function getLintNameMap() {
     return array(
       self::LINT_BAD_FILENAME   => 'Bad Filename',

src/lint/linter/ArcanistLinter.php

@@ -244,4 +244,16 @@ abstract class ArcanistLinter {
     return ArcanistDiffUtils::isHeuristicBinaryFile($this->getData($path));
   }
 
+  /**
+   * If this linter is selectable via `.arclint` configuration files, return
+   * a short, human-readable name to identify it. For example, `"jshint"` or
+   * `"pep8"`.
+   *
+   * If you do not implement this method, the linter will not be selectable
+   * through `.arclint` files.
+   */
+  public function getLinterConfigurationName() {
+    return null;
+  }
+
 }

src/workflow/ArcanistLintWorkflow.php

@@ -172,13 +172,20 @@ EOTEXT
     $engine = $this->getArgument('engine');
     if (!$engine) {
       $engine = $working_copy->getConfigFromAnySource('lint.engine');
-      if (!$engine) {
+    }
-        throw new ArcanistNoEngineException(
+
-          "No lint engine configured for this project. Edit .arcconfig to ".
+    if (!$engine) {
-          "specify a lint engine.");
+      if (Filesystem::pathExists($working_copy->getProjectPath('.arclint'))) {
+        $engine = 'ArcanistConfigurationDrivenLintEngine';
       }
     }
 
+    if (!$engine) {
+      throw new ArcanistNoEngineException(
+        "No lint engine configured for this project. Edit '.arcconfig' to ".
+        "specify a lint engine, or create an '.arclint' file.");
+    }
+
     $rev = $this->getArgument('rev');
     $paths = $this->getArgument('paths');
     $use_cache = $this->getArgument('cache', null);

src/workingcopyidentity/ArcanistWorkingCopyIdentity.php

@@ -140,6 +140,10 @@ final class ArcanistWorkingCopyIdentity {
     return $this->projectRoot;
   }
 
+  public function getProjectPath($to_file) {
+    return $this->projectRoot.'/'.$to_file;
+  }
+
   public function getConduitURI() {
     return $this->getConfig('conduit_uri');
   }