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 @@ -6,147 +6,9 @@ * script and a regex to interpret the results of some real linter, it does * not itself lint both scripts and regexes). * - * Configure this linter by setting these keys in your configuration: + * Please see the documentation in the Arcanist User Guide for details. * - * - `linter.scriptandregex.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 - * regex uses named capturing groups (detailed below) to interpret output. - * - * 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`. - * - * This linter is necessarily more limited in its capabilities than a normal - * linter which can perform custom processing, but may be somewhat simpler to - * configure. - * - * == Script... == - * - * The script will be invoked once for each file that is to be linted, with - * the file passed as the first argument. The file may begin with a "-"; ensure - * your script will not interpret such files as flags (perhaps by ending your - * script configuration with "--", if its argument parser supports that). - * - * Note that when run via `arc diff`, the list of files to be linted includes - * deleted files and files that were moved away by the change. The linter should - * not assume the path it is given exists, and it is not an error for the - * linter to be invoked with paths which are no longer there. (Every affected - * path is subject to lint because some linters may raise errors in other files - * when a file is removed, or raise an error about its removal.) - * - * The script should emit lint messages to stdout, which will be parsed with - * the provided regex. - * - * For example, you might use a configuration like this: - * - * /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' - * - * 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' - * - * 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. - * For instance, this configuration would not work properly, because several - * 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' - * - * 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 - * things that this linter can't handle, you can write a phutil linter and move - * the logic to handle those cases into PHP. PHP is a better general-purpose - * programming language than regular expressions are, if only by a small margin. - * - * == ...and Regex == - * - * 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 - * - * The regex should capture these named patterns with `(?P...)`: - * - * - `message` (required) Text describing the lint message. For example, - * "This is a syntax error.". - * - `name` (optional) Text summarizing the lint message. For example, - * "Syntax Error". - * - `severity` (optional) The word "error", "warning", "autofix", "advice", - * or "disabled", in any combination of upper and lower case. Instead, you - * may match groups called `error`, `warning`, `advice`, `autofix`, or - * `disabled`. These allow you to match output formats like "E123" and - * "W123" to indicate errors and warnings, even though the word "error" is - * not present in the output. If no severity capturing group is present, - * messages are raised with "error" severity. If multiple severity capturing - * groups are present, messages are raised with the highest captured - * serverity. Capturing groups like `error` supersede the `severity` - * capturing group. - * - `error` (optional) Match some nonempty substring to indicate that this - * message has "error" severity. - * - `warning` (optional) Match some nonempty substring to indicate that this - * message has "warning" severity. - * - `advice` (optional) Match some nonempty substring to indicate that this - * message has "advice" severity. - * - `autofix` (optional) Match some nonempty substring to indicate that this - * message has "autofix" severity. - * - `disabled` (optional) Match some nonempty substring to indicate that this - * message has "disabled" severity. - * - `file` (optional) The name of the file to raise the lint message in. If - * not specified, defaults to the linted file. It is generally not necessary - * to capture this unless the linter can raise messages in files other than - * the one it is linting. - * - `line` (optional) The line number of the message. - * - `char` (optional) The character offset of the message. - * - `offset` (optional) The byte offset of the message. If captured, this - * supersedes `line` and `char`. - * - `original` (optional) The text the message affects. - * - `replacement` (optional) The text that the range captured by `original` - * should be automatically replaced by to resolve the message. - * - `code` (optional) A short error type identifier which can be used - * elsewhere to configure handling of specific types of messages. For - * example, "EXAMPLE1", "EXAMPLE2", etc., where each code identifies a - * class of message like "syntax error", "missing whitespace", etc. This - * allows configuration to later change the severity of all whitespace - * messages, for example. - * - `ignore` (optional) Match some nonempty substring to ignore the match. - * You can use this if your linter sometimes emits text like "No lint - * errors". - * - `stop` (optional) Match some nonempty substring to stop processing input. - * Remaining matches for this file will be discarded, but linting will - * continue with other linters and other files. - * - `halt` (optional) Match some nonempty substring to halt all linting of - * this file by any linter. Linting will continue with other files. - * - `throw` (optional) Match some nonempty substring to throw an error, which - * will stop `arc` completely. You can use this to fail abruptly if you - * encounter unexpected output. All processing will abort. - * - * Numbered capturing groups are ignored. - * - * For example, if your lint script's output looks like this: - * - * error:13 Too many goats! - * warning:22 Not enough boats. - * - * ...you could use this regex to parse it: - * - * /^(?Pwarning|error):(?P\d+) (?P.*)$/m - * - * The simplest valid regex for line-oriented output is something like this: - * - * /^(?P.*)$/m + * https://secure.phabricator.com/book/phabricator/article/arcanist_lint_script_and_regex/ * * @task lint Linting * @task linterinfo Linter Information @@ -158,17 +20,43 @@ final class ArcanistScriptAndRegexLinter extends ArcanistLinter { private $output = array(); - + private $script = null; + private $regex = null; public function getInfoName() { return pht('Script and Regex'); } + public function getInfoURI() { + return 'https://secure.phabricator.com/'. + 'book/phabricator/article/arcanist_lint_script_and_regex/'; + } + public function getInfoDescription() { return pht( 'Run an external script, then parse its output with a regular '. 'expression. This is a generic binding that can be used to '. - 'run custom lint scripts.'); + 'run custom lint scripts. Please see the Arcanist User Guide article '. + 'for details on how to configure this linter.'); + } + + +/* -( 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 +68,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 +91,7 @@ * @task lint */ public function lintPath($path) { - $regex = $this->getConfiguredRegex(); + $this->validateConfiguredRegex(); $output = idx($this->output, $path); if (!strlen($output)) { @@ -212,7 +100,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 +160,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 +222,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 +282,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 +306,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; } }