Warn when accessing deprecated lint config

Summary:
Ref T2039. Addresses two issues:

  - Issues a warning for use of config which is deprecated by `.arclint`.
  - We no longer require an engine to be present for these linters, so `arc linters` doesn't fatal if they aren't configured.

Test Plan:
  - Ran `arc linters` in a repo with no JSHint.
  - Ran `arc linters` in a repo with junk in .arcconfig and got a warning when it was read. Verified it still took effect.

Reviewers: chad, btrahan, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T2039

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

src/lint/engine/ComprehensiveLintEngine.php

@@ -33,7 +33,7 @@ final class ComprehensiveLintEngine extends ArcanistLintEngine {
     $py_paths = preg_grep('/\.py$/', $paths);
     $linters[] = id(new ArcanistPyFlakesLinter())->setPaths($py_paths);
     $linters[] = id(new ArcanistPEP8Linter())
-      ->setConfig(array('options' => $this->getPEP8WithTextOptions()))
+      ->setConfig(array('flags' => array($this->getPEP8WithTextOptions())))
       ->setPaths($py_paths);
 
     $linters[] = id(new ArcanistRubyLinter())

src/lint/linter/ArcanistCSSLintLinter.php

@@ -34,15 +34,11 @@ final class ArcanistCSSLintLinter extends ArcanistExternalLinter {
   }
 
   public function getDefaultFlags() {
-    // TODO: Deprecation warning.
+    return $this->getDeprecatedConfiguration('lint.csslint.options', array());
-    $config = $this->getEngine()->getConfigurationManager();
-    return $config->getConfigFromAnySource('lint.csslint.options', array());
   }
 
   public function getDefaultBinary() {
-    // TODO: Deprecation warning.
+    return $this->getDeprecatedConfiguration('lint.csslint.bin', 'csslint');
-    $config = $this->getEngine()->getConfigurationManager();
-    return $config->getConfigFromAnySource('lint.csslint.bin', 'csslint');
   }
 
   public function getVersion() {

src/lint/linter/ArcanistCpplintLinter.php

@@ -14,10 +14,8 @@ final class ArcanistCpplintLinter extends ArcanistExternalLinter {
   }
 
   public function getDefaultBinary() {
-    // TODO: Warn that all of this is deprecated.
+    $prefix = $this->getDeprecatedConfiguration('lint.cpplint.prefix');
-    $config = $this->getEngine()->getConfigurationManager();
+    $bin = $this->getDeprecatedConfiguration('lint.cpplint.bin', 'cpplint.py');
-    $prefix = $config->getConfigFromAnySource('lint.cpplint.prefix');
-    $bin = $config->getConfigFromAnySource('lint.cpplint.bin', 'cpplint.py');
 
     if ($prefix) {
       return $prefix.'/'.$bin;
@@ -44,8 +42,7 @@ final class ArcanistCpplintLinter extends ArcanistExternalLinter {
   }
 
   protected function getDefaultFlags() {
-    $config = $this->getEngine()->getConfigurationManager();
+    return $this->getDeprecatedConfiguration('lint.cpplint.options', array());
-    return $config->getConfigFromAnySource('lint.cpplint.options', array());
   }
 
   protected function parseLinterOutput($path, $err, $stdout, $stderr) {

src/lint/linter/ArcanistFlake8Linter.php

@@ -29,21 +29,18 @@ final class ArcanistFlake8Linter extends ArcanistExternalLinter {
   }
 
   public function getDefaultFlags() {
-    // TODO: Deprecated.
+    return $this->getDeprecatedConfiguration('lint.flake8.options', array());
-    $config = $this->getEngine()->getConfigurationManager();
-    return $config->getConfigFromAnySource('lint.flake8.options', array());
   }
 
   public function getDefaultBinary() {
-    $config = $this->getEngine()->getConfigurationManager();
+    $prefix = $this->getDeprecatedConfiguration('lint.flake8.prefix');
-    $prefix = $config->getConfigFromAnySource('lint.flake8.prefix');
+    $bin = $this->getDeprecatedConfiguration('lint.flake8.bin', 'flake8');
-    $bin = $config->getConfigFromAnySource('lint.flake8.bin', 'flake8');
 
-    if ($prefix || ($bin != 'flake8')) {
+    if ($prefix) {
       return $prefix.'/'.$bin;
+    } else {
+      return $bin;
     }
-
-    return 'flake8';
   }
 
   public function getVersion() {

src/lint/linter/ArcanistJSHintLinter.php

@@ -35,9 +35,8 @@ final class ArcanistJSHintLinter extends ArcanistExternalLinter {
   }
 
   public function getDefaultBinary() {
-    $config = $this->getEngine()->getConfigurationManager();
+    $prefix = $this->getDeprecatedConfiguration('lint.jshint.prefix');
-    $prefix = $config->getConfigFromAnySource('lint.jshint.prefix');
+    $bin = $this->getDeprecatedConfiguration('lint.jshint.bin', 'jshint');
-    $bin = $config->getConfigFromAnySource('lint.jshint.bin', 'jshint');
 
     if ($prefix) {
       return $prefix.'/'.$bin;
@@ -81,12 +80,11 @@ final class ArcanistJSHintLinter extends ArcanistExternalLinter {
   }
 
   protected function getDefaultFlags() {
-    $config_manager = $this->getEngine()->getConfigurationManager();
+    $options = $this->getDeprecatedConfiguration(
-    $options = $config_manager->getConfigFromAnySource(
       'lint.jshint.options',
       array());
 
-    $config = $config_manager->getConfigFromAnySource('lint.jshint.config');
+    $config = $this->getDeprecatedConfiguration('lint.jshint.config');
     if ($config) {
       $options[] = '--config='.$config;
     }

src/lint/linter/ArcanistLinter.php

@@ -438,4 +438,49 @@ abstract class ArcanistLinter {
     return $code;
   }
 
+
+  /**
+   * Retrieve an old lint configuration value from `.arcconfig` or a similar
+   * source.
+   *
+   * Modern linters should use @{method:getConfig} to read configuration from
+   * `.arclint`.
+   *
+   * @param   string  Configuration key to retrieve.
+   * @param   wild    Default value to return if key is not present in config.
+   * @return  wild    Configured value, or default if no configuration exists.
+   */
+  protected function getDeprecatedConfiguration($key, $default = null) {
+
+    // If we're being called in a context without an engine (probably from
+    // `arc linters`), just return the default value.
+    if (!$this->engine) {
+      return $default;
+    }
+
+    $config = $this->getEngine()->getConfigurationManager();
+
+    // Construct a sentinel object so we can tell if we're reading config
+    // or not.
+    $sentinel = (object)array();
+    $result = $config->getConfigFromAnySource($key, $sentinel);
+
+    // If we read config, warn the user that this mechanism is deprecated and
+    // discouraged.
+    if ($result !== $sentinel) {
+      $console = PhutilConsole::getConsole();
+      $console->writeErr(
+        "**%s**: %s\n",
+        pht('Deprecation Warning'),
+        pht(
+          'Configuration option "%s" is deprecated. Generally, linters should '.
+          'now be configured using an `.arclint` file. See "Arcanist User '.
+          'Guide: Lint" in the documentation for more information.',
+          $key));
+      return $result;
+    }
+
+    return $default;
+  }
+
 }

src/lint/linter/ArcanistPEP8Linter.php

@@ -28,11 +28,7 @@ final class ArcanistPEP8Linter extends ArcanistExternalLinter {
   }
 
   public function getDefaultFlags() {
-    // TODO: Warn that all of this is deprecated.
+    return $this->getDeprecatedConfiguration('lint.pep8.options', array());
-    $config = $this->getEngine()->getConfigurationManager();
-    return $config->getConfigFromAnySource(
-      'lint.pep8.options',
-      $this->getConfig('options', array()));
   }
 
   public function shouldUseInterpreter() {
@@ -48,12 +44,9 @@ final class ArcanistPEP8Linter extends ArcanistExternalLinter {
       return 'pep8';
     }
 
-    $config = $this->getEngine()->getConfigurationManager();
+    $old_prefix = $this->getDeprecatedConfiguration('lint.pep8.prefix');
-    $old_prefix = $config->getConfigFromAnySource('lint.pep8.prefix');
+    $old_bin = $this->getDeprecatedConfiguration('lint.pep8.bin');
-    $old_bin = $config->getConfigFromAnySource('lint.pep8.bin');
-
     if ($old_prefix || $old_bin) {
-      // TODO: Deprecation warning.
       $old_bin = nonempty($old_bin, 'pep8');
       return $old_prefix.'/'.$old_bin;
     }

src/lint/linter/ArcanistPhpcsLinter.php

@@ -48,13 +48,8 @@ final class ArcanistPhpcsLinter extends ArcanistExternalLinter {
   }
 
   public function getDefaultFlags() {
-    // TODO: Deprecation warnings.
+    $options = $this->getDeprecatedConfiguration('lint.phpcs.options', array());
-
+    $standard = $this->getDeprecatedConfiguration('lint.phpcs.standard');
-    $config = $this->getEngine()->getConfigurationManager();
-
-    $options = $config->getConfigFromAnySource('lint.phpcs.options', array());
-
-    $standard = $config->getConfigFromAnySource('lint.phpcs.standard');
     if (!empty($standard)) {
       if (is_array($options)) {
         $options[] = '--standard='.$standard;
@@ -67,9 +62,7 @@ final class ArcanistPhpcsLinter extends ArcanistExternalLinter {
   }
 
   public function getDefaultBinary() {
-    // TODO: Deprecation warnings.
+    return $this->getDeprecatedConfiguration('lint.phpcs.bin', 'phpcs');
-    $config = $this->getEngine()->getConfigurationManager();
-    return $config->getConfigFromAnySource('lint.phpcs.bin', 'phpcs');
   }
 
   public function getVersion() {

src/lint/linter/ArcanistPyFlakesLinter.php

@@ -28,9 +28,8 @@ final class ArcanistPyFlakesLinter extends ArcanistExternalLinter {
   }
 
   public function getDefaultBinary() {
-    $config = $this->getEngine()->getConfigurationManager();
+    $prefix = $this->getDeprecatedConfiguration('lint.pyflakes.prefix');
-    $prefix = $config->getConfigFromAnySource('lint.pyflakes.prefix');
+    $bin = $this->getDeprecatedConfiguration('lint.pyflakes.bin', 'pyflakes');
-    $bin = $config->getConfigFromAnySource('lint.pyflakes.bin', 'pyflakes');
 
     if ($prefix) {
       return $prefix.'/'.$bin;

src/lint/linter/ArcanistRubyLinter.php

@@ -26,9 +26,7 @@ final class ArcanistRubyLinter extends ArcanistExternalLinter {
   }
 
   public function getDefaultBinary() {
-    // TODO: Deprecation warning.
+    $prefix = $this->getDeprecatedConfiguration('lint.ruby.prefix');
-    $config = $this->getEngine()->getConfigurationManager();
-    $prefix = $config->getConfigFromAnySource('lint.ruby.prefix');
     if ($prefix !== null) {
       $ruby_bin = $prefix.'ruby';
     }

src/lint/linter/ArcanistScalaSBTLinter.php

@@ -2,8 +2,6 @@
 
 /**
  * Uses `sbt compile` to detect various warnings/errors in Scala code.
- *
- * @group linter
  */
 final class ArcanistScalaSBTLinter extends ArcanistLinter {
 
@@ -26,24 +24,11 @@ final class ArcanistScalaSBTLinter extends ArcanistLinter {
   private function getSBTPath() {
     $sbt_bin = "sbt";
 
-    // Use the SBT prefix specified in the config file
+    $prefix = $this->getDeprecatedConfiguration('lint.scala_sbt.prefix');
-    $config = $this->getEngine()->getConfigurationManager();
-    $prefix = $config->getConfigFromAnySource('lint.scala_sbt.prefix');
     if ($prefix !== null) {
       $sbt_bin = $prefix . $sbt_bin;
     }
 
-    if (!Filesystem::pathExists($sbt_bin)) {
-
-      list($err) = exec_manual('which %s', $sbt_bin);
-      if ($err) {
-        throw new ArcanistUsageException(
-          "SBT does not appear to be installed on this system. Install it or ".
-          "add 'lint.scala_sbt.prefix' in your .arcconfig to point to ".
-          "the directory where it resides.");
-      }
-    }
-
     return $sbt_bin;
   }