diff --git a/src/lint/linter/ArcanistScriptAndRegexLinter.php b/src/lint/linter/ArcanistScriptAndRegexLinter.php --- a/src/lint/linter/ArcanistScriptAndRegexLinter.php +++ b/src/lint/linter/ArcanistScriptAndRegexLinter.php @@ -8,10 +8,10 @@ * * Configure this linter by setting these keys in your configuration: * - * - `linter.scriptandregex.script` Script command to run. This can be + * - `script-and-regex.script` Script command to run. This can be * the path to a linter script, but may also include flags or use shell * features (see below for examples). - * - `linter.scriptandregex.regex` The regex to process output with. This + * - `script-and-regex.regex` The regex to process output with. This * regex uses named capturing groups (detailed below) to interpret output. * * The script will be invoked from the project root, so you can specify a @@ -41,19 +41,19 @@ * * For example, you might use a configuration like this: * - * /opt/lint/lint.sh --flag value --other-flag -- + * "script-and-regex.script": "/opt/lint/lint.sh --flag value --other-flag -- " * * stderr is ignored. If you have a script which writes messages to stderr, * you can redirect stderr to stdout by using a configuration like this: * - * sh -c '/opt/lint/lint.sh "$0" 2>&1' + * "script-and-regex.script": "sh -c '/opt/lint/lint.sh \"$0\" 2>&1'" * * The return code of the script must be 0, or an exception will be raised * reporting that the linter failed. If you have a script which exits nonzero * under normal circumstances, you can force it to always exit 0 by using a * configuration like this: * - * sh -c '/opt/lint/lint.sh "$0" || true' + * "script-and-regex.script": "sh -c '/opt/lint/lint.sh \"$0\" || true'" * * Multiple instances of the script will be run in parallel if there are * multiple files to be linted, so they should not use any unique resources. @@ -61,7 +61,7 @@ * processes may attempt to write to the file at the same time: * * COUNTEREXAMPLE - * sh -c '/opt/lint/lint.sh --output /tmp/lint.out "$0" && cat /tmp/lint.out' + * "script-and-regex.script": "sh -c '/opt/lint/lint.sh --output /tmp/lint.out "$0" && cat /tmp/lint.out'" * * There are necessary limits to how gracefully this linter can deal with * edge cases, because it is just a script and a regex. If you need to do @@ -76,7 +76,7 @@ * The regex will be matched against the entire output of the script, so it * should generally be in this form if messages are one-per-line: * - * /^...$/m + * "script-and-regex.regex": "/^...$/m" * * The regex should capture these named patterns with `(?P...)`: * @@ -142,11 +142,11 @@ * * ...you could use this regex to parse it: * - * /^(?Pwarning|error):(?P\d+) (?P.*)$/m + * "script-and-regex.regex": "/^(?Pwarning|error):(?P\d+) (?P.*)$/m" * * The simplest valid regex for line-oriented output is something like this: * - * /^(?P.*)$/m + * "script-and-regex.regex": "/^(?P.*)$/m" * * @task lint Linting * @task linterinfo Linter Information @@ -158,7 +158,8 @@ final class ArcanistScriptAndRegexLinter extends ArcanistLinter { private $output = array(); - + private $script = null; + private $regex = null; public function getInfoName() { return pht('Script and Regex'); @@ -171,6 +172,25 @@ 'run custom lint scripts.'); } + +/* -( Linter Information )------------------------------------------------- */ + + + /** + * Return the short name of the linter. + * + * @return string Short linter identifier. + * + * @task linterinfo + */ + public function getLinterName() { + return 'S&RX'; + } + + public function getLinterConfigurationName() { + return 'script-and-regex'; + } + /* -( Linting )------------------------------------------------------------ */ @@ -180,12 +200,12 @@ * @task lint */ public function willLintPaths(array $paths) { - $script = $this->getConfiguredScript(); + $this->validateConfiguredScript(); $root = $this->getEngine()->getWorkingCopy()->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,7 +223,7 @@ * @task lint */ public function lintPath($path) { - $regex = $this->getConfiguredRegex(); + $this->validateConfiguredRegex(); $output = idx($this->output, $path); if (!strlen($output)) { @@ -212,7 +232,7 @@ } $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 @@ -272,24 +292,6 @@ } -/* -( Linter Information )------------------------------------------------- */ - - - /** - * Return the short name of the linter. - * - * @return string Short linter identifier. - * - * @task linterinfo - */ - public function getLinterName() { - return 'S&RX'; - } - - public function getLinterConfigurationName() { - return 'script-and-regex'; - } - /* -( Parsing Output )----------------------------------------------------- */ @@ -352,6 +354,58 @@ /* -( Validating Configuration )------------------------------------------- */ + public function getLinterConfigurationOptions() { + $options = array( + 'script-and-regex.script' => array( + 'type' => 'string', + 'help' => pht( + 'The script that this linter should run. '. + 'This can be the path to a linter script, but may also include '. + 'flags or use shell features. The script will be invoked from '. + 'the project root, so you can specify a relative path like '. + '`scripts/lint.sh` or an absolute path like `/opt/lint/lint.sh`.'), + ), + 'script-and-regex.regex' => array( + 'type' => 'string', + 'help' => pht( + 'The regex that should be used to evaluate the script output. '. + 'The regex must be a valid PHP PCRE regex, including delimiters '. + 'and flags. The regex will be matched against the entire output '. + 'of the script, so it should generally be in this form if '. + 'messages are one-per-line: /^...$/m'), + ), + ); + + return $options + parent::getLinterConfigurationOptions(); + } + + public function setLinterConfigurationValue($key, $value) { + switch ($key) { + case 'script-and-regex.script': + $this->script = $value; + return; + case 'script-and-regex.regex': + $this->regex = $value; + + try { + PhutilTypeSpec::checkMap(array ('regex' => $this->regex,), + array ('regex' => 'regex')); + } + catch (Exception $e) { + throw new ArcanistUsageException( + "ArcanistScriptAndRegexLinter: ". + "Regex '{$this->regex}' does not compile. ". + $e->getMessage (). + ". You must configure '{$key}' with ". + "a valid PHP PCRE regex, including delimiters: "); + } + return; + } + + return parent::setLinterConfigurationValue($key, $value); + } + + /** * Load, validate, and return the "script" configuration. @@ -360,26 +414,23 @@ * * @task config */ - private function getConfiguredScript() { - $key = 'linter.scriptandregex.script'; - $config = $this->getEngine() - ->getConfigurationManager() - ->getConfigFromAnySource($key); - - if (!$config) { - throw new ArcanistUsageException( - "ArcanistScriptAndRegexLinter: ". - "You must configure '{$key}' to point to a script to execute."); + private function validateConfiguredScript() { + if (!$this->script) { + $key = 'linter.scriptandregex.script'; + $this->script = $this->getDeprecatedConfiguration ($key); + } + + if (!$this->script) { + throw new ArcanistUsageException( + "ArcanistScriptAndRegexLinter: ". + "You must configure '{$key}' to point to a script to execute."); } // 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. * @@ -387,30 +438,33 @@ * * @task config */ - private function getConfiguredRegex() { - $key = 'linter.scriptandregex.regex'; - $config = $this->getEngine() - ->getConfigurationManager() - ->getConfigFromAnySource($key); - - if (!$config) { - throw new ArcanistUsageException( - "ArcanistScriptAndRegexLinter: ". - "You must configure '{$key}' with a valid PHP PCRE regex."); + private function validateConfiguredRegex() { + if (!$this->regex) { + $key = "linter.scriptandregex.regex"; + $this->regex = $this->getDeprecatedConfiguration ($key); + + // 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. + try { + PhutilTypeSpec::checkMap(array ('regex' => $this->regex,), + array ('regex' => 'regex')); + } + catch (Exception $e) { + throw new ArcanistUsageException( + "ArcanistScriptAndRegexLinter: ". + "Regex '{$this->regex}' does not compile. ". + $e->getMessage (). + ". You must configure '{$key}' with ". + "a valid PHP PCRE regex, including delimiters: "); + } } - // 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( - "ArcanistScriptAndRegexLinter: ". - "Regex '{$config}' does not compile. You must configure '{$key}' with ". - "a valid PHP PCRE regex, including delimiters."); + if (!$this->regex) { + throw new ArcanistUsageException( + "ArcanistScriptAndRegexLinter: ". + "You must configure '{$key}' with a valid PHP PCRE regex."); } - - return $config; } }