D11773.id28438.diff
No OneTemporary
Actions

Size

17 KB

Referenced Files

None

Subscribers

None

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: Tue, Mar 18, 2:34 PM (2 w, 5 d ago)
Storage Engine: blob
Storage Format: Encrypted (AES-256-CBC)
Storage Handle: 7709999
Default Alt Text: D11773.id28438.diff (17 KB)

D11773.id28438.diffNo OneTemporaryActions

D11773.id28438.diffView Options

File Metadata

Event Timeline

D11773.id28438.diff
No OneTemporary
Actions

D11773.id28438.diff
View Options