Differential D10704 Diff 30539 src/lint/linter/ArcanistScriptAndRegexLinter.php

Changeset View

Standalone View

src/lint/linter/ArcanistScriptAndRegexLinter.php

<?php		<?php

/**		/**
* Simple glue linter which runs some script on each path, and then uses a		* Simple glue linter which runs some script on each path, and then uses a
* regex to parse lint messages from the script's output. (This linter uses a		* regex to parse lint messages from the script's output. (This linter uses a
* script and a regex to interpret the results of some real linter, it does		* script and a regex to interpret the results of some real linter, it does
* not itself lint both scripts and regexes).		* not itself lint both scripts and regexes).
*		*
* Configure this linter by setting these keys in your configuration:		* Configure this linter by setting these keys in your .arclint section:
*		*
* - `linter.scriptandregex.script` Script command to run. This can be		* - `script-and-regex.script` Script command to run. This can be
		joshuaspenceUnsubmitted Not Done Inline Actions We should update this. joshuaspence: We should update this.
* the path to a linter script, but may also include flags or use shell		* the path to a linter script, but may also include flags or use shell
* features (see below for examples).		* 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
		joshuaspenceUnsubmitted Not Done Inline Actions We should update this. joshuaspence: We should update this.
* regex uses named capturing groups (detailed below) to interpret output.		* regex uses named capturing groups (detailed below) to interpret output.
*		*
* The script will be invoked from the project root, so you can specify a		* 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		* relative path like `scripts/lint.sh` or an absolute path like
* `/opt/lint/lint.sh`.		* `/opt/lint/lint.sh`.
*		*
* This linter is necessarily more limited in its capabilities than a normal		* This linter is necessarily more limited in its capabilities than a normal
* linter which can perform custom processing, but may be somewhat simpler to		* linter which can perform custom processing, but may be somewhat simpler to
▲ Show 20 Lines • Show All 127 Lines • ▼ Show 20 Lines
*		*
* @task lint Linting		* @task lint Linting
* @task linterinfo Linter Information		* @task linterinfo Linter Information
* @task parse Parsing Output		* @task parse Parsing Output
* @task config Validating Configuration		* @task config Validating Configuration
*/		*/
final class ArcanistScriptAndRegexLinter extends ArcanistLinter {		final class ArcanistScriptAndRegexLinter extends ArcanistLinter {

		private $script = null;
		private $regex = null;
private $output = array();		private $output = array();

public function getInfoName() {		public function getInfoName() {
return pht('Script and Regex');		return pht('Script and Regex');
}		}

public function getInfoDescription() {		public function getInfoDescription() {
return pht(		return pht(
▲ Show 20 Lines • Show All 52 Lines • ▼ Show 20 Lines	if (!preg_match_all($regex, $output, $matches, PREG_SET_ORDER)) {
// capture 'throw' to handle this case.		// capture 'throw' to handle this case.
return;		return;
}		}

foreach ($matches as $match) {		foreach ($matches as $match) {
if (!empty($match['throw'])) {		if (!empty($match['throw'])) {
$throw = $match['throw'];		$throw = $match['throw'];
throw new ArcanistUsageException(		throw new ArcanistUsageException(
"ArcanistScriptAndRegexLinter: ".		pht(
"configuration captured a 'throw' named capturing group, ".		"%s: configuration captured a '%s' named capturing group, ".
"'{$throw}'. Script output:\n".		"'%s'. Script output:\n%s",
$output);		__CLASS__,
		'throw',
		$throw,
		$output));
}		}

if (!empty($match['halt'])) {		if (!empty($match['halt'])) {
$this->stopAllLinters();		$this->stopAllLinters();
break;		break;
}		}

if (!empty($match['stop'])) {		if (!empty($match['stop'])) {
▲ Show 20 Lines • Show All 44 Lines • ▼ Show 20 Lines	/* -( Linter Information )------------------------------------------------- */
public function getLinterName() {		public function getLinterName() {
return 'S&RX';		return 'S&RX';
}		}

public function getLinterConfigurationName() {		public function getLinterConfigurationName() {
return 'script-and-regex';		return 'script-and-regex';
}		}

		public function getLinterConfigurationOptions() {
		// These fields are optional only to avoid breaking things.
		$options = array(
		'script-and-regex.script' => array(
		joshuaspenceUnsubmitted Not Done Inline Actions Change this to `script-and-regex.script`. joshuaspence: Change this to `script-and-regex.script`.
		'type' => 'optional string',
		joshuaspenceUnsubmitted Not Done Inline Actions We should eventually make this non-optional I guess. joshuaspence: We should eventually make this non-optional I guess.
		aviveyAuthorUnsubmitted Not Done Inline Actions Yeah, that's the plan, but I'd like to avoid breaking this so abruptly; Give it some time as "deprecated". avivey: Yeah, that's the plan, but I'd like to avoid breaking this so abruptly; Give it some time as…
		'help' => pht('Script to execute.'),
		),
		'script-and-regex.regex' => array(
		joshuaspenceUnsubmitted Not Done Inline Actions Change this to `script-and-regex.reged`. joshuaspence: Change this to `script-and-regex.reged`.
		'type' => 'optional regex',
		joshuaspenceUnsubmitted Not Done Inline Actions As above. joshuaspence: As above.
		'help' => pht('The regex to process output with.'),
		),
		);

		return $options + parent::getLinterConfigurationOptions();
		}

		public function setLinterConfigurationValue($key, $value) {
		switch ($key) {
		case 'script-and-regex.script':
		joshuaspenceUnsubmitted Not Done Inline Actions As above. joshuaspence: As above.
		$this->script = $value;
		joshuaspenceUnsubmitted Not Done Inline Actions Maybe you should do `Filesystem::assertExists` to ensure that the script actually exists... or maybe this already happens somewhere else? joshuaspence: Maybe you should do `Filesystem::assertExists` to ensure that the script actually exists... or…
		epriestleyUnsubmitted Not Done Inline Actions This might be `grep` or some shell alias or something so I think it's OK to not check. epriestley: This might be `grep` or some shell alias or something so I think it's OK to not check.
		joshuaspenceUnsubmitted Not Done Inline Actions Ah, good point. joshuaspence: Ah, good point.
		return;
		case 'script-and-regex.regex':
		joshuaspenceUnsubmitted Not Done Inline Actions As above. joshuaspence: As above.
		$this->regex = $value;
		return;
		}

		return parent::setLinterConfigurationValue($key, $value);
		}

/* -( Parsing Output )----------------------------------------------------- */		/* -( Parsing Output )----------------------------------------------------- */

/**		/**
* Get the line and character of the message from the regex match.		* Get the line and character of the message from the regex match.
*		*
* @param dict Captured groups from regex.		* @param dict Captured groups from regex.
* @return pair<int,int> Line and character of the message.		* @return pair<int,int> Line and character of the message.
▲ Show 20 Lines • Show All 51 Lines • ▼ Show 20 Lines	/* -( Validating Configuration )------------------------------------------- */

/**		/**
* Load, validate, and return the "script" configuration.		* Load, validate, and return the "script" configuration.
*		*
* @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		* @task config
*/		*/
private function getConfiguredScript() {		private function getConfiguredScript() {
$key = 'linter.scriptandregex.script';		if (strlen($this->script)) {
		joshuaspenceUnsubmitted Not Done Inline Actions Probably just use `if ($this->script) { ... }` joshuaspence: Probably just use `if ($this->script) { ... }`
		aviveyAuthorUnsubmitted Not Done Inline Actions `if (strlen($x))` is the convention used, to assure $x is actually a string (@epriestley said so). Or at least it was. avivey: `if (strlen($x))` is the convention used, to assure $x is actually a string (@epriestley said…
$config = $this->getEngine()		return $this->script;
->getConfigurationManager()		}
->getConfigFromAnySource($key);
		$config = $this->getDeprecatedConfiguration('linter.scriptandregex.script');
		joshuaspenceUnsubmitted Not Done Inline Actions Change this to `$config->getDeprecatedConfiguration('linter.scriptandregex.script')` joshuaspence: Change this to `$config->getDeprecatedConfiguration('linter.scriptandregex.script')`
		aviveyAuthorUnsubmitted Not Done Inline Actions ok. avivey: ok.
		joshuaspenceUnsubmitted Not Done Inline Actions See line 413. joshuaspence: See line 413.
		joshuaspenceUnsubmitted Not Done Inline Actions For consistency, maybe this `if ($this->script) {` joshuaspence: For consistency, maybe this `if ($this->script) {`

if (!$config) {		if (!$config) {
throw new ArcanistUsageException(		throw new ArcanistUsageException(
"ArcanistScriptAndRegexLinter: ".		pht(
		joshuaspenceUnsubmitted Not Done Inline Actions See line 420. joshuaspence: See line 420.
"You must configure '{$key}' to point to a script to execute.");		'No "script" configured for script-and-regex linter, which '.
		'requires a script. Use "%s" to configure one.',
		'script-and-regex.script'));
		joshuaspenceUnsubmitted Not Done Inline Actions Maybe maybe this translatable with `pht()` joshuaspence: Maybe maybe this translatable with `pht()`
}		}

// NOTE: No additional validation since the "script" can be some random		// 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		// shell command and/or include flags, so it does not need to point to some
// file on disk.		// file on disk.

return $config;		return $config;
}		}

/**		/**
* Load, validate, and return the "regex" configuration.		* Load, validate, and return the "regex" configuration.
*		*
* @return string A valid PHP PCRE regular expression.		* @return string A valid PHP PCRE regular expression.
*		*
* @task config		* @task config
*/		*/
private function getConfiguredRegex() {		private function getConfiguredRegex() {
		if (strlen($this->regex)) {
		joshuaspenceUnsubmitted Not Done Inline Actions As above. joshuaspence: As above.
		joshuaspenceUnsubmitted Not Done Inline Actions I still think that we should just use `if ($this->regex) {` here... joshuaspence: I still think that we should just use `if ($this->regex) {` here...
		return $this->regex;
		}

$key = 'linter.scriptandregex.regex';		$key = 'linter.scriptandregex.regex';
$config = $this->getEngine()		$config = $this->getDeprecatedConfiguration($key);
		joshuaspenceUnsubmitted Not Done Inline Actions As above. joshuaspence: As above.
		aviveyAuthorUnsubmitted Not Done Inline Actions `$key` is used in the error message below. avivey: `$key` is used in the error message below.
->getConfigurationManager()
->getConfigFromAnySource($key);

if (!$config) {		if (!$config) {
throw new ArcanistUsageException(		throw new ArcanistUsageException(
"ArcanistScriptAndRegexLinter: ".		pht(
		joshuaspenceUnsubmitted Not Done Inline Actions ...or change this to be `if (!strlen($config)) {` joshuaspence: ...or change this to be `if (!strlen($config)) {`
"You must configure '{$key}' with a valid PHP PCRE regex.");		'No "regex" configured for script-and-regex linter, which '.
		'requires a regex. Use "%s" to configure one.',
		'script-and-regex.regex'));
		joshuaspenceUnsubmitted Not Done Inline Actions Make this translatable with this `pht()` joshuaspence: Make this translatable with this `pht()`
}		}

// NOTE: preg_match() returns 0 for no matches and false for compile error;		// 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.		// this won't match, but will validate the syntax of the regex.

$ok = preg_match($config, 'syntax-check');		$ok = @preg_match($config, 'syntax-check');
if ($ok === false) {		if ($ok === false) {
throw new ArcanistUsageException(		throw new ArcanistUsageException(
"ArcanistScriptAndRegexLinter: ".		pht(
"Regex '{$config}' does not compile. You must configure '{$key}' with ".		'Regular expression passed to script-and-regex linter ("%s") is '.
"a valid PHP PCRE regex, including delimiters.");		'not a valid regular expression.',
		$config));
		joshuaspenceUnsubmitted Not Done Inline Actions I'm thinking that we should just remove the second part of this message, since it encourages the use of deprecated configuration. joshuaspence: I'm thinking that we should just remove the second part of this message, since it encourages…
		joshuaspenceUnsubmitted Not Done Inline Actions Prefer to write this as `pht("%s: Regex '%s' does not compile.", __CLASS__, $regex)` joshuaspence: Prefer to write this as `pht("%s: Regex '%s' does not compile.", __CLASS__, $regex)`
}		}

return $config;		return $config;
}		}

}		}