Page MenuHomePhabricator
Files F13207384

D11773.id28438.diff
No OneTemporary
Actions

D11773.id28438.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -164,6 +164,7 @@
'ArcanistRubyLinter' => 'lint/linter/ArcanistRubyLinter.php',
'ArcanistRubyLinterTestCase' => 'lint/linter/__tests__/ArcanistRubyLinterTestCase.php',
'ArcanistScriptAndRegexLinter' => 'lint/linter/ArcanistScriptAndRegexLinter.php',
+ 'ArcanistScriptAndRegexLinterTestCase' => 'lint/linter/__tests__/ArcanistScriptAndRegexLinterTestCase.php',
'ArcanistSetConfigWorkflow' => 'workflow/ArcanistSetConfigWorkflow.php',
'ArcanistSettings' => 'configuration/ArcanistSettings.php',
'ArcanistShellCompleteWorkflow' => 'workflow/ArcanistShellCompleteWorkflow.php',
@@ -349,7 +350,8 @@
'ArcanistRevertWorkflow' => 'ArcanistWorkflow',
'ArcanistRubyLinter' => 'ArcanistExternalLinter',
'ArcanistRubyLinterTestCase' => 'ArcanistExternalLinterTestCase',
- 'ArcanistScriptAndRegexLinter' => 'ArcanistLinter',
+ 'ArcanistScriptAndRegexLinter' => 'ArcanistExternalLinter',
+ 'ArcanistScriptAndRegexLinterTestCase' => 'ArcanistExternalLinterTestCase',
'ArcanistSetConfigWorkflow' => 'ArcanistWorkflow',
'ArcanistShellCompleteWorkflow' => 'ArcanistWorkflow',
'ArcanistSingleLintEngine' => 'ArcanistLintEngine',
diff --git a/src/lint/ArcanistLintSeverity.php b/src/lint/ArcanistLintSeverity.php
--- a/src/lint/ArcanistLintSeverity.php
+++ b/src/lint/ArcanistLintSeverity.php
@@ -47,4 +47,19 @@
return $map[$message_sev] >= idx($map, $level, 0);
}
+ /**
+ * Returns a mapping from human-readable string to linter severity.
+ *
+ * @return map<string, string>
+ */
+ public static function getSeverityMap() {
+ return array(
+ 'error' => self::SEVERITY_ERROR,
+ 'warning' => self::SEVERITY_WARNING,
+ 'autofix' => self::SEVERITY_AUTOFIX,
+ 'advice' => self::SEVERITY_ADVICE,
+ 'disabled' => self::SEVERITY_DISABLED,
+ );
+ }
+
}
diff --git a/src/lint/linter/ArcanistExternalLinter.php b/src/lint/linter/ArcanistExternalLinter.php
--- a/src/lint/linter/ArcanistExternalLinter.php
+++ b/src/lint/linter/ArcanistExternalLinter.php
@@ -259,6 +259,10 @@
$binary = $this->getBinary();
+ if ($binary === null) {
+ return;
+ }
+
// NOTE: If we have an interpreter, we don't require the script to be
// executable (so we just check that the path exists). Otherwise, the
// binary must be executable.
diff --git a/src/lint/linter/ArcanistLinter.php b/src/lint/linter/ArcanistLinter.php
--- a/src/lint/linter/ArcanistLinter.php
+++ b/src/lint/linter/ArcanistLinter.php
@@ -366,13 +366,7 @@
}
public function setLinterConfigurationValue($key, $value) {
- $sev_map = array(
- 'error' => ArcanistLintSeverity::SEVERITY_ERROR,
- 'warning' => ArcanistLintSeverity::SEVERITY_WARNING,
- 'autofix' => ArcanistLintSeverity::SEVERITY_AUTOFIX,
- 'advice' => ArcanistLintSeverity::SEVERITY_ADVICE,
- 'disabled' => ArcanistLintSeverity::SEVERITY_DISABLED,
- );
+ $sev_map = ArcanistLintSeverity::getSeverityMap();
switch ($key) {
case 'severity':
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,11 +8,11 @@
*
* Configure this linter by setting these keys in your configuration:
*
- * - `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.
+ * - `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).
+ * - `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
* relative path like `scripts/lint.sh` or an absolute path like
@@ -22,35 +22,35 @@
* linter which can perform custom processing, but may be somewhat simpler to
* configure.
*
- * == Script... ==
+ * == 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
+ * 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).
+ * 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.)
+ * 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 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:
+ * `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
+ * 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
+ * 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'
@@ -63,13 +63,13 @@
* 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
+ * 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 ==
+ * == Regex ==
*
* The regex must be a valid PHP PCRE regex, including delimiters and flags.
*
@@ -90,10 +90,10 @@
* `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
- * severity. Capturing groups like `error` supersede the `severity`
- * capturing group.
+ * messages are raised with "error" severity. If multiple severity
+ * capturing groups are present, messages are raised with the highest
+ * captured severity. 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
@@ -153,11 +153,10 @@
* @task parse Parsing Output
* @task config Validating Configuration
*/
-final class ArcanistScriptAndRegexLinter extends ArcanistLinter {
+final class ArcanistScriptAndRegexLinter extends ArcanistExternalLinter {
private $script = null;
private $regex = null;
- private $output = array();
public function getInfoName() {
return pht('Script and Regex');
@@ -170,49 +169,90 @@
'run custom lint scripts.');
}
-/* -( Linting )------------------------------------------------------------ */
+ public function getLinterName() {
+ return 'S&RX';
+ }
+ public function getLinterConfigurationName() {
+ return 'script-and-regex';
+ }
- /**
- * Run the script on each file to be linted.
- *
- * @task lint
- */
- public function willLintPaths(array $paths) {
- $script = $this->getConfiguredScript();
- $root = $this->getEngine()->getWorkingCopy()->getProjectRoot();
-
- $futures = array();
- foreach ($paths as $path) {
- $future = new ExecFuture('%C %s', $script, $path);
- $future->setCWD($root);
- $futures[$path] = $future;
- }
+ public function getLinterConfigurationOptions() {
+ // These fields are optional only to avoid breaking things.
+ $options = array(
+ 'script-and-regex.script' => array(
+ 'type' => 'optional string',
+ 'help' => pht('Script to execute.'),
+ ),
+ 'script-and-regex.regex' => array(
+ 'type' => 'optional regex',
+ 'help' => pht('The regex to process output with.'),
+ ),
+ );
- $futures = id(new FutureIterator($futures))
- ->limit(4);
- foreach ($futures as $path => $future) {
- list($stdout) = $future->resolvex();
- $this->output[$path] = $stdout;
+ 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;
+ return;
+
+ default:
+ return parent::setLinterConfigurationValue($key, $value);
}
}
+ public function getDefaultBinary() {
+ return $this->script;
+ }
+
+ public function getInstallInstructions() {
+ return null;
+ }
+
+ public function shouldExpectCommandErrors() {
+ return false;
+ }
+
+ public function supportsReadDataFromStdin() {
+ return false;
+ }
+
+ public function getReadDataFromStdinFilename() {
+ return null;
+ }
+
+ protected function getMandatoryFlags() {
+ return array();
+ }
+
+ protected function getDefaultFlags() {
+ return array();
+ }
+
+
+/* -( Linting )------------------------------------------------------------ */
+
/**
- * Run the regex on the output of the script.
+ * Run the script on each file to be linted.
*
* @task lint
*/
- public function lintPath($path) {
+ protected function parseLinterOutput($path, $err, $stdout, $stderr) {
$regex = $this->getConfiguredRegex();
- $output = idx($this->output, $path);
- if (!strlen($output)) {
+ if (!strlen($stdout)) {
// No output, but it exited 0, so just move on.
return;
}
$matches = null;
- if (!preg_match_all($regex, $output, $matches, PREG_SET_ORDER)) {
+ if (!preg_match_all($regex, $stdout, $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
@@ -228,7 +268,7 @@
"ArcanistScriptAndRegexLinter: ".
"configuration captured a 'throw' named capturing group, ".
"'{$throw}'. Script output:\n".
- $output);
+ $stdout);
}
if (!empty($match['halt'])) {
@@ -272,59 +312,14 @@
}
-/* -( 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';
- }
-
- public function getLinterConfigurationOptions() {
- // These fields are optional only to avoid breaking things.
- $options = array(
- 'script' => array(
- 'type' => 'optional string',
- 'help' => pht('Script to execute.'),
- ),
- 'regex' => array(
- 'type' => 'optional regex',
- 'help' => pht('The regex to process output with.'),
- ),
- );
-
- return $options + parent::getLinterConfigurationOptions();
- }
-
- public function setLinterConfigurationValue($key, $value) {
- switch ($key) {
- case 'script':
- $this->script = $value;
- return;
- case 'regex':
- $this->regex = $value;
- return;
- }
-
- return parent::setLinterConfigurationValue($key, $value);
- }
-
/* -( Parsing Output )----------------------------------------------------- */
/**
* Get the line and character of the message from the regex match.
*
- * @param dict Captured groups from regex.
- * @return pair<int,int> Line and character of the message.
+ * @param dict Captured groups from regex.
+ * @param string The path of the current file being linted.
+ * @return pair<int, int> Line and character of the message.
*
* @task parse
*/
@@ -344,22 +339,16 @@
/**
* Map the regex matching groups to a message severity. We look for either
- * a nonempty severity name group like 'error', or a group called 'severity'
+ * a nonempty severity name group like `error`, or a group called `severity`
* with a valid name.
*
- * @param dict Captured groups from regex.
+ * @param dict Captured groups from regex.
* @return const @{class:ArcanistLintSeverity} constant.
*
* @task parse
*/
private function getMatchSeverity(array $match) {
- $map = array(
- 'error' => ArcanistLintSeverity::SEVERITY_ERROR,
- 'warning' => ArcanistLintSeverity::SEVERITY_WARNING,
- 'autofix' => ArcanistLintSeverity::SEVERITY_AUTOFIX,
- 'advice' => ArcanistLintSeverity::SEVERITY_ADVICE,
- 'disabled' => ArcanistLintSeverity::SEVERITY_DISABLED,
- );
+ $map = ArcanistLintSeverity::getSeverityMap();
$severity_name = strtolower(idx($match, 'severity'));
@@ -378,14 +367,14 @@
/* -( Validating Configuration )------------------------------------------- */
/**
- * Load, validate, and return the "script" configuration.
+ * Load, validate, and return the configured script.
*
- * @return string The shell command fragment to use to run the linter.
+ * @return string The shell command fragment to use to run the linter.
*
* @task config
*/
private function getConfiguredScript() {
- if (strlen($this->script)) {
+ if ($this->script) {
return $this->script;
}
@@ -403,14 +392,14 @@
}
/**
- * Load, validate, and return the "regex" configuration.
+ * Load, validate, and return the configured regex.
*
- * @return string A valid PHP PCRE regular expression.
+ * @return string A valid PHP PCRE regular expression.
*
* @task config
*/
private function getConfiguredRegex() {
- if (strlen($this->regex)) {
+ if ($this->regex) {
return $this->regex;
}
@@ -418,7 +407,10 @@
$config = $this->getDeprecatedConfiguration($key);
if (!$config) {
- throw new ArcanistUsageException('Parameter missing: regex');
+ throw new ArcanistUsageException(
+ pht(
+ 'Linter is missing a required parameter: %s.',
+ 'regex'));
}
// NOTE: preg_match() returns 0 for no matches and false for compile error;
@@ -427,9 +419,7 @@
$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.");
+ pht("Regex '%s' does not compile.", $config));
}
return $config;
diff --git a/src/lint/linter/__tests__/ArcanistScriptAndRegexLinterTestCase.php b/src/lint/linter/__tests__/ArcanistScriptAndRegexLinterTestCase.php
new file mode 100644
--- /dev/null
+++ b/src/lint/linter/__tests__/ArcanistScriptAndRegexLinterTestCase.php
@@ -0,0 +1,10 @@
+<?php
+
+final class ArcanistScriptAndRegexLinterTestCase
+ extends ArcanistExternalLinterTestCase {
+
+ public function testLinter() {
+ $this->executeTestsInDirectory(dirname(__FILE__).'/script-and-regex/');
+ }
+
+}
diff --git a/src/lint/linter/__tests__/script-and-regex/ack-grep.lint-test b/src/lint/linter/__tests__/script-and-regex/ack-grep.lint-test
new file mode 100644
--- /dev/null
+++ b/src/lint/linter/__tests__/script-and-regex/ack-grep.lint-test
@@ -0,0 +1,10 @@
+test
+~~~~~~~~~~
+error:1:1
+~~~~~~~~~~
+{
+ "config": {
+ "script-and-regex.script": "ack-grep --noenv --with-filename --column --nocolor 'test'"
+ "script-and-regex.regex": "(^(?P<line>\d+):(?P<char>\d+):(?P<message>.*)$)"
+ }
+}

File Metadata

Mime Type
text/plain
Expires
May 16 2024, 10:50 PM (4 w, 2 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6297953
Default Alt Text
D11773.id28438.diff (17 KB)

Event Timeline

Phabricator · Built by Phacility