Provide better documentation for lint engines, linters, and the PyLint linter

Summary:
Document the relationship between lint engines and linters. Provide an example
linter. Improve the documentation of PyLintLinter, which has a bunch of
configuration stuff which you had to dig into the code to get.

Test Plan:
Ran "arc lint --engine ExampleLintEngine --lintall derp.py" on a file with a
Python syntax error in it. Read documentation.

Reviewed By: j3kuntz
Reviewers: j3kuntz, andrewjcg
CC: aran, j3kuntz
Differential Revision: 557

src/__phutil_library_map__.php

@@ -79,6 +79,7 @@ phutil_register_library_map(array(
     'ArcanistXHPASTLinter' => 'lint/linter/xhpast',
     'ArcanistXHPASTLinterTestCase' => 'lint/linter/xhpast/__tests__',
     'BranchInfo' => 'branch',
+    'ExampleLintEngine' => 'lint/engine/example',
     'PhutilLintEngine' => 'lint/engine/phutil',
     'PhutilModuleRequirements' => 'parser/phutilmodule',
     'PhutilUnitTestEngine' => 'unit/engine/phutil',
@@ -131,6 +132,7 @@ phutil_register_library_map(array(
     'ArcanistUserAbortException' => 'ArcanistUsageException',
     'ArcanistXHPASTLinter' => 'ArcanistLinter',
     'ArcanistXHPASTLinterTestCase' => 'ArcanistLinterTestCase',
+    'ExampleLintEngine' => 'ArcanistLintEngine',
     'PhutilLintEngine' => 'ArcanistLintEngine',
     'PhutilUnitTestEngine' => 'ArcanistBaseUnitTestEngine',
     'PhutilUnitTestEngineTestCase' => 'ArcanistPhutilTestCase',

src/lint/engine/base/ArcanistLintEngine.php

@@ -17,7 +17,43 @@
  */
 
 /**
- * Manages lint execution.
+ * Manages lint execution. When you run 'arc lint' or 'arc diff', Arcanist
+ * checks your .arcconfig to see if you have specified a lint engine in the
+ * key "lint_engine". The engine must extend this class. For example:
+ *
+ *  lang=js
+ *  {
+ *    // ...
+ *    "lint_engine" : "ExampleLintEngine",
+ *    // ...
+ *  }
+ *
+ * The lint engine is given a list of paths (generally, the paths that you
+ * modified in your change) and determines which linters to run on them. The
+ * linters themselves are responsible for actually analyzing file text and
+ * finding warnings and errors. For example, if the modified paths include some
+ * JS files and some Python files, you might want to run JSLint on the JS files
+ * and PyLint on the Python files.
+ *
+ * You can also run multiple linters on a single file. For instance, you might
+ * run one linter on all text files to make sure they don't have trailing
+ * whitespace, or enforce tab vs space rules, or make sure there are enough
+ * curse words in them.
+ *
+ * Because lint engines are pretty custom to the rules of a project, you will
+ * generally need to build your own. Fortunately, it's pretty easy (and you
+ * can use the prebuilt //linters//, you just need to write a little glue code
+ * to tell Arcanist which linters to run). For a simple example of how to build
+ * a lint engine, see @{class:ExampleLintEngine}.
+ *
+ * You can test an engine like this:
+ *
+ *   arc lint --engine ExampleLintEngine --lintall some_file.py
+ *
+ * ...which will show you all the lint issues raised in the file.
+ *
+ * See @{article@phabricator:Arcanist User Guide: Customizing Lint, Unit Tests
+ * and Workflows} for more information about configuring lint engines.
  *
  * @group lint
  */

src/lint/engine/example/ExampleLintEngine.php

@@ -0,0 +1,72 @@
+<?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.
+ */
+
+/**
+ * This a simple example lint engine which just applies the
+ * @{class:ArcanistPyLintLinter} to any Python files. For a more complex
+ * example, see @{class:PhutilLintEngine}.
+ *
+ * @group linter
+ */
+class ExampleLintEngine extends ArcanistLintEngine {
+
+  public function buildLinters() {
+
+    // This is a list of paths which the user wants to lint. Either they
+    // provided them explicitly, or arc figured them out from a commit or set
+    // of changes. The engine needs to return a list of ArcanistLinter objects,
+    // representing the linters which should be run on these files.
+    $paths = $this->getPaths();
+
+    // The ArcanistPyLintLinter runs "PyLint" (an open source python linter) on
+    // files you give it. There are several linters available by default like
+    // this one which you can use out of the box, or you can write your own.
+    // Linters are responsible for actually analyzing the contents of a file
+    // and raising warnings and errors.
+    $pylint_linter = new ArcanistPyLintLinter();
+
+    foreach ($paths as $path) {
+      if (!preg_match('/\.py$/', $path)) {
+        // This isn't a python file, so don't try to apply the PyLint linter
+        // to it.
+        continue;
+      }
+
+      if (preg_match('@^externals/@', $path)) {
+        // This is just an example of how to exclude a path so it doesn't get
+        // linted. If you put third-party code in an externals/ directory, you
+        // can just have your lint engine ignore it.
+        continue;
+      }
+
+      // Add the path, to tell the linter it should examine the source code
+      // to try to find problems.
+      $pylint_linter->addPath($path);
+    }
+
+    // We only built one linter, but you can build more than one (e.g., a
+    // Javascript linter for JS), and return a list of linters to execute. You
+    // can also add a path to more than one linter (for example, if you want
+    // to run a Python linter and a more general text linter on every .py file).
+
+    return array(
+      $pylint_linter,
+    );
+  }
+
+}

src/lint/engine/example/__init__.php

@@ -0,0 +1,13 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('arcanist', 'lint/engine/base');
+phutil_require_module('arcanist', 'lint/linter/pylint');
+
+
+phutil_require_source('ExampleLintEngine.php');

src/lint/linter/pylint/ArcanistPyLintLinter.php

@@ -17,34 +17,82 @@
  */
 
 /**
- * Uses "PyLint" to detect various errors in Python code.
+ * Uses "PyLint" to detect various errors in Python code. To use this linter,
+ * you must install pylint and configure which codes you want to be reported as
+ * errors, warnings and advice.
+ *
+ * You should be able to install pylint with ##sudo easy_install pylint##. If
+ * your system is unusual, you can manually specify the location of pylint and
+ * its dependencies by configuring these keys in your .arcrc:
+ *
+ *   lint.pylint.prefix
+ *   lint.pylint.logilab_astng.prefix
+ *   lint.pylint.logilab_common.prefix
+ *
+ * You can specify additional command-line options to pass to PyLint by setting
+ * this key:
+ *
+ *   lint.pylint.options
+ *
+ * Now, configure which warnings and errors you want PyLint to raise by setting
+ * these keys:
+ *
+ *   lint.pylint.codes.error
+ *   lint.pylint.codes.warning
+ *   lint.pylint.codes.advice
+ *
+ * Set these to regular expressions -- for instance, if you want to raise all
+ * PyLint errors as Arcanist errors, set this for ##lint.pylint.codes.error##:
+ *
+ *    ^E.*
+ *
+ * You can also match more granular errors:
+ *
+ *    ^E(0001|0002)$
+ *
+ * You can also provide a list of regular expressions.
  *
  * @group linter
  */
 class ArcanistPyLintLinter extends ArcanistLinter {
 
   private function getMessageCodeSeverity($code) {
+
+    $working_copy = $this->getEngine()->getWorkingCopy();
+
     // The config file defines how PyLint message codes translate to
     // arcanist severities.  The config options provide regex's to
     // match against the message codes generated by PyLint.  Severity's
     // are matched in the order of errors, warnings, then advice.
     // The first severity that matches, in that order, is returned.
-    $working_copy = $this->getEngine()->getWorkingCopy();
+    $error_regexp   = $working_copy->getConfig('lint.pylint.codes.error');
+    $warning_regexp = $working_copy->getConfig('lint.pylint.codes.warning');
+    $advice_regexp  = $working_copy->getConfig('lint.pylint.codes.advice');
+
+    if (!$error_regexp && !$warning_regexp && !$advice_regexp) {
+      throw new ArcanistUsageException(
+        "You are invoking the PyLint linter but have not configured any of ".
+        "'lint.pylint.codes.error', 'lint.pylint.codes.warning', or ".
+        "'lint.pylint.codes.advice'. Consult the documentation for ".
+        "ArcanistPyLintLinter.");
+    }
+
     $code_map = array(
-      ArcanistLintSeverity::SEVERITY_ERROR =>
+      ArcanistLintSeverity::SEVERITY_ERROR    => $error_regexp,
-          $working_copy->getConfig('lint.pylint.codes.error'),
+      ArcanistLintSeverity::SEVERITY_WARNING  => $warning_regexp,
-      ArcanistLintSeverity::SEVERITY_WARNING =>
+      ArcanistLintSeverity::SEVERITY_ADVICE   => $advice_regexp,
-          $working_copy->getConfig('lint.pylint.codes.warning'),
-      ArcanistLintSeverity::SEVERITY_ADVICE =>
-          $working_copy->getConfig('lint.pylint.codes.advice'),
     );
 
     foreach ($code_map as $sev => $codes) {
-      if ($codes !== null) {
+      if ($codes === null) {
-        foreach ($codes as $code_re) {
+        continue;
-          if (preg_match("/{$code_re}/", $code)) {
+      }
-            return $sev;
+      if (!is_array($codes)) {
-          }
+        $codes = array($codes);
+      }
+      foreach ($codes as $code_re) {
+        if (preg_match("/{$code_re}/", $code)) {
+          return $sev;
         }
       }
     }
@@ -64,6 +112,18 @@ class ArcanistPyLintLinter extends ArcanistLinter {
       $pylint_bin = $prefix."/bin/".$pylint_bin;
     }
 
+    if (!Filesystem::pathExists($pylint_bin)) {
+
+      list($err) = exec_manual('which %s', $pylint_bin);
+      if ($err) {
+        throw new ArcanistUsageException(
+          "PyLint does not appear to be installed on this system. Install it ".
+          "(e.g., with 'sudo easy_install pylint') or configure ".
+          "'lint.pylint.prefix' in your .arcconfig to point to the directory ".
+          "where it resides.");
+      }
+    }
+
     return $pylint_bin;
   }
 
@@ -154,6 +214,7 @@ class ArcanistPyLintLinter extends ArcanistLinter {
       foreach ($matches as $key => $match) {
         $matches[$key] = trim($match);
       }
+
       $message = new ArcanistLintMessage();
       $message->setPath($path);
       $message->setLine($matches[2]);

src/lint/linter/pylint/__init__.php

@@ -6,10 +6,12 @@
 
 
 
+phutil_require_module('arcanist', 'exception/usage');
 phutil_require_module('arcanist', 'lint/linter/base');
 phutil_require_module('arcanist', 'lint/message');
 phutil_require_module('arcanist', 'lint/severity');
 
+phutil_require_module('phutil', 'filesystem');
 phutil_require_module('phutil', 'future/exec');