Remove deprecated linter configuration

Summary:
Removes the deprecated method of configuring linters (via the `.arcconfig` file). There is one main caveat here:

- There is currently no convenient method by which to change the path for an external linter (T5057). This means that there is no direct replacement for the deprecated `lint.ruby.prefix` configuration. The workaround is to symlink these binaries into `arcanist/externals/bin`.

Test Plan: Wait a sufficient amount of time before landing this.

Reviewers: chad, epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: aik099, epriestley, Korvin

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

src/lint/linter/ArcanistCSSLintLinter.php

@@ -33,12 +33,8 @@ final class ArcanistCSSLintLinter extends ArcanistExternalLinter {
     );
   }
 
-  protected function getDefaultFlags() {
-    return $this->getDeprecatedConfiguration('lint.csslint.options', array());
-  }
-
   public function getDefaultBinary() {
-    return $this->getDeprecatedConfiguration('lint.csslint.bin', 'csslint');
+    return 'csslint';
   }
 
   public function getVersion() {

src/lint/linter/ArcanistCppcheckLinter.php

@@ -28,14 +28,7 @@ final class ArcanistCppcheckLinter extends ArcanistExternalLinter {
   }
 
   public function getDefaultBinary() {
-    $prefix = $this->getDeprecatedConfiguration('lint.cppcheck.prefix');
-    $bin = $this->getDeprecatedConfiguration('lint.cppcheck.bin', 'cppcheck');
-
-    if ($prefix) {
-      return $prefix.'/'.$bin;
-    } else {
-      return $bin;
-    }
+    return 'cppcheck';
   }
 
   public function getVersion() {
@@ -56,6 +49,13 @@ final class ArcanistCppcheckLinter extends ArcanistExternalLinter {
       'apt-get install cppcheck');
   }
 
+  protected function getDefaultFlags() {
+    return array(
+      '-j2',
+      '--enable=performance,style,portability,information',
+    );
+  }
+
   protected function getMandatoryFlags() {
     return array(
       '--quiet',
@@ -65,12 +65,6 @@ final class ArcanistCppcheckLinter extends ArcanistExternalLinter {
     );
   }
 
-  protected function getDefaultFlags() {
-    return $this->getDeprecatedConfiguration(
-      'lint.cppcheck.options',
-      array('-j2', '--enable=performance,style,portability,information'));
-  }
-
   public function shouldExpectCommandErrors() {
     return false;
   }

src/lint/linter/ArcanistCpplintLinter.php

@@ -14,14 +14,7 @@ final class ArcanistCpplintLinter extends ArcanistExternalLinter {
   }
 
   public function getDefaultBinary() {
-    $prefix = $this->getDeprecatedConfiguration('lint.cpplint.prefix');
-    $bin = $this->getDeprecatedConfiguration('lint.cpplint.bin', 'cpplint.py');
-
-    if ($prefix) {
-      return $prefix.'/'.$bin;
-    } else {
-      return $bin;
-    }
+    return 'cpplint';
   }
 
   public function getInstallInstructions() {
@@ -31,10 +24,6 @@ final class ArcanistCpplintLinter extends ArcanistExternalLinter {
       '/svn/trunk/cpplint/cpplint.py');
   }
 
-  protected function getDefaultFlags() {
-    return $this->getDeprecatedConfiguration('lint.cpplint.options', array());
-  }
-
   protected function getDefaultMessageSeverity($code) {
     return ArcanistLintSeverity::SEVERITY_WARNING;
   }

src/lint/linter/ArcanistFlake8Linter.php

@@ -29,19 +29,8 @@ final class ArcanistFlake8Linter extends ArcanistExternalLinter {
     return 'flake8';
   }
 
-  protected function getDefaultFlags() {
-    return $this->getDeprecatedConfiguration('lint.flake8.options', array());
-  }
-
   public function getDefaultBinary() {
-    $prefix = $this->getDeprecatedConfiguration('lint.flake8.prefix');
-    $bin = $this->getDeprecatedConfiguration('lint.flake8.bin', 'flake8');
-
-    if ($prefix) {
-      return $prefix.'/'.$bin;
-    } else {
-      return $bin;
-    }
+    return 'flake8';
   }
 
   public function getVersion() {

src/lint/linter/ArcanistJSHintLinter.php

@@ -46,14 +46,7 @@ final class ArcanistJSHintLinter extends ArcanistExternalLinter {
   }
 
   public function getDefaultBinary() {
-    $prefix = $this->getDeprecatedConfiguration('lint.jshint.prefix');
-    $bin = $this->getDeprecatedConfiguration('lint.jshint.bin', 'jshint');
-
-    if ($prefix) {
-      return $prefix.'/'.$bin;
-    } else {
-      return $bin;
-    }
+    return 'jshint';
   }
 
   public function getVersion() {
@@ -120,19 +113,6 @@ final class ArcanistJSHintLinter extends ArcanistExternalLinter {
     return parent::setLinterConfigurationValue($key, $value);
   }
 
-  protected function getDefaultFlags() {
-    $options = $this->getDeprecatedConfiguration(
-      'lint.jshint.options',
-      array());
-
-    $config = $this->getDeprecatedConfiguration('lint.jshint.config');
-    if ($config) {
-      $options[] = '--config='.$config;
-    }
-
-    return $options;
-  }
-
   protected function parseLinterOutput($path, $err, $stdout, $stderr) {
     $errors = null;
     try {

src/lint/linter/ArcanistLinter.php

@@ -621,48 +621,4 @@ 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.
-   */
-  final 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 `%s` file. See "Arcanist User '.
-          'Guide: Lint" in the documentation for more information.',
-          $key,
-          '.arclint'));
-      return $result;
-    }
-
-    return $default;
-  }
-
 }

src/lint/linter/ArcanistPEP8Linter.php

@@ -27,19 +27,8 @@ final class ArcanistPEP8Linter extends ArcanistExternalLinter {
     return 'pep8';
   }
 
-  protected function getDefaultFlags() {
-    return $this->getDeprecatedConfiguration('lint.pep8.options', array());
-  }
-
   public function getDefaultBinary() {
-    $prefix = $this->getDeprecatedConfiguration('lint.pep8.prefix');
-    $bin = $this->getDeprecatedConfiguration('lint.pep8.bin', 'pep8');
-
-    if ($prefix) {
-      return $prefix.'/'.$bin;
-    } else {
-      return $bin;
-    }
+    return 'pep8';
   }
 
   public function getVersion() {

src/lint/linter/ArcanistPhpcsLinter.php

@@ -65,23 +65,8 @@ final class ArcanistPhpcsLinter extends ArcanistExternalLinter {
     return $options;
   }
 
-  protected function getDefaultFlags() {
-    $options = $this->getDeprecatedConfiguration('lint.phpcs.options', array());
-    $standard = $this->getDeprecatedConfiguration('lint.phpcs.standard');
-
-    if (!empty($standard)) {
-      if (is_array($options)) {
-        $options[] = '--standard='.$standard;
-      } else {
-        $options .= ' --standard='.$standard;
-      }
-    }
-
-    return $options;
-  }
-
   public function getDefaultBinary() {
-    return $this->getDeprecatedConfiguration('lint.phpcs.bin', 'phpcs');
+    return 'phpcs';
   }
 
   public function getVersion() {

src/lint/linter/ArcanistPyFlakesLinter.php

@@ -28,14 +28,7 @@ final class ArcanistPyFlakesLinter extends ArcanistExternalLinter {
   }
 
   public function getDefaultBinary() {
-    $prefix = $this->getDeprecatedConfiguration('lint.pyflakes.prefix');
-    $bin = $this->getDeprecatedConfiguration('lint.pyflakes.bin', 'pyflakes');
-
-    if ($prefix) {
-      return $prefix.'/'.$bin;
-    } else {
-      return $bin;
-    }
+    return 'pyflakes';
   }
 
   public function getVersion() {

src/lint/linter/ArcanistPyLintLinter.php

@@ -31,14 +31,7 @@ final class ArcanistPyLintLinter extends ArcanistExternalLinter {
   }
 
   public function getDefaultBinary() {
-    $prefix = $this->getDeprecatedConfiguration('lint.pylint.prefix');
-    $bin = $this->getDeprecatedConfiguration('lint.pylint.bin', 'pylint');
-
-    if ($prefix) {
-      return $prefix.'/bin/'.$bin;
-    } else {
-      return $bin;
-    }
+    return 'pylint';
   }
 
   public function getVersion() {
@@ -95,10 +88,6 @@ final class ArcanistPyLintLinter extends ArcanistExternalLinter {
     // Stupidly, the command line args above are overridden by rcfile, so be
     // careful.
     $config = $this->config;
-    if (!$config) {
-      $config = $this->getDeprecatedConfiguration('lint.pylint.rcfile');
-    }
-
     if ($config !== null) {
       $options[] = '--rcfile='.$config;
     }
@@ -109,12 +98,6 @@ final class ArcanistPyLintLinter extends ArcanistExternalLinter {
   protected function getDefaultFlags() {
     $options = array();
 
-    // Add any options defined in the config file for PyLint.
-    $config_options = $this->getDeprecatedConfiguration(
-      'lint.pylint.options',
-      array());
-    $options = array_merge($options, $config_options);
-
     $installed_version = $this->getVersion();
     $minimum_version = '1.0.0';
     if (version_compare($installed_version, $minimum_version, '<')) {

src/lint/linter/ArcanistRubyLinter.php

@@ -28,11 +28,6 @@ final class ArcanistRubyLinter extends ArcanistExternalLinter {
   }
 
   public function getDefaultBinary() {
-    $prefix = $this->getDeprecatedConfiguration('lint.ruby.prefix');
-    if ($prefix !== null) {
-      $ruby_bin = $prefix.'ruby';
-    }
-
     return 'ruby';
   }
 

src/lint/linter/ArcanistScriptAndRegexLinter.php

@@ -179,12 +179,11 @@ final class ArcanistScriptAndRegexLinter extends ArcanistLinter {
    * @task lint
    */
   public function willLintPaths(array $paths) {
-    $script = $this->getConfiguredScript();
-    $root   = $this->getProjectRoot();
+    $root = $this->getProjectRoot();
 
     $futures = array();
     foreach ($paths as $path) {
-      $future = new ExecFuture('%C %s', $script, $path);
+      $future = new ExecFuture('%C %s', $this->script, $path);
       $future->setCWD($root);
       $futures[$path] = $future;
     }
@@ -203,8 +202,6 @@ final class ArcanistScriptAndRegexLinter extends ArcanistLinter {
    * @task lint
    */
   public function lintPath($path) {
-    $regex = $this->getConfiguredRegex();
-
     $output = idx($this->output, $path);
     if (!strlen($output)) {
       // No output, but it exited 0, so just move on.
@@ -212,7 +209,7 @@ final class ArcanistScriptAndRegexLinter extends ArcanistLinter {
     }
 
     $matches = null;
-    if (!preg_match_all($regex, $output, $matches, PREG_SET_ORDER)) {
+    if (!preg_match_all($this->regex, $output, $matches, PREG_SET_ORDER)) {
       // Output with no matches. This might be a configuration error, but more
       // likely it's something like "No lint errors." and the user just hasn't
       // written a sufficiently powerful/ridiculous regexp to capture it into an
@@ -296,11 +293,11 @@ final class ArcanistScriptAndRegexLinter extends ArcanistLinter {
     // These fields are optional only to avoid breaking things.
     $options = array(
       'script-and-regex.script' => array(
-        'type' => 'optional string',
+        'type' => 'string',
         'help' => pht('Script to execute.'),
       ),
       'script-and-regex.regex' => array(
-        'type' => 'optional regex',
+        'type' => 'regex',
         'help' => pht('The regex to process output with.'),
       ),
     );
@@ -377,74 +374,4 @@ final class ArcanistScriptAndRegexLinter extends ArcanistLinter {
     return ArcanistLintSeverity::SEVERITY_ERROR;
   }
 
-
-/* -(  Validating Configuration  )------------------------------------------- */
-
-  /**
-   * Load, validate, and return the "script" configuration.
-   *
-   * @return string The shell command fragment to use to run the linter.
-   *
-   * @task config
-   */
-  private function getConfiguredScript() {
-    if (strlen($this->script)) {
-      return $this->script;
-    }
-
-    $config = $this->getDeprecatedConfiguration('linter.scriptandregex.script');
-
-    if (!$config) {
-      throw new ArcanistUsageException(
-        pht(
-          'No "script" configured for script-and-regex linter, which '.
-          'requires a script. Use "%s" to configure one.',
-          'script-and-regex.script'));
-    }
-
-    // NOTE: No additional validation since the "script" can be some random
-    // shell command and/or include flags, so it does not need to point to some
-    // file on disk.
-
-    return $config;
-  }
-
-  /**
-   * Load, validate, and return the "regex" configuration.
-   *
-   * @return string A valid PHP PCRE regular expression.
-   *
-   * @task config
-   */
-  private function getConfiguredRegex() {
-    if (strlen($this->regex)) {
-      return $this->regex;
-    }
-
-    $key = 'linter.scriptandregex.regex';
-    $config = $this->getDeprecatedConfiguration($key);
-
-    if (!$config) {
-      throw new ArcanistUsageException(
-        pht(
-          'No "regex" configured for script-and-regex linter, which '.
-          'requires a regex. Use "%s" to configure one.',
-          'script-and-regex.regex'));
-    }
-
-    // NOTE: preg_match() returns 0 for no matches and false for compile error;
-    // this won't match, but will validate the syntax of the regex.
-
-    $ok = @preg_match($config, 'syntax-check');
-    if ($ok === false) {
-      throw new ArcanistUsageException(
-        pht(
-          'Regular expression passed to script-and-regex linter ("%s") is '.
-          'not a valid regular expression.',
-          $config));
-    }
-
-    return $config;
-  }
-
 }

src/lint/linter/ArcanistXHPASTLinter.php

@@ -959,9 +959,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
     $hook_obj = null;
     $working_copy = $this->getEngine()->getWorkingCopy();
     if ($working_copy) {
-      $hook_class = $this->switchhook
-        ? $this->switchhook
-        : $this->getDeprecatedConfiguration('lint.xhpast.switchhook');
+      $hook_class = $this->switchhook;
       if ($hook_class) {
         $hook_obj = newv($hook_class, array());
         assert_instances_of(array($hook_obj), 'ArcanistXHPASTLintSwitchHook');
@@ -2261,9 +2259,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
     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 = $this->naminghook
-        ? $this->naminghook
-        : $working_copy->getProjectConfig('lint.xhpast.naminghook');
+      $hook_class = $this->naminghook;
       if ($hook_class) {
         $hook_obj = newv($hook_class, array());
         foreach ($names as $k => $name_attrs) {