mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-11-22 14:52:40 +01:00
Be more strict with the type of .arclint
properties.
Summary: Although this provides less context in terms of the error message (for example, `Parameter has invalid type. Expected type 'optional regex|list<regex>', got type 'list<string>'.`), I think that it is the right approach. I think that `PhutilTypeSpec::checkMap` should be improved such that additional context is provided in the exception message. Test Plan: Ran `arc lint`. Modified `.arclint` to contain an invalid regex and ran `arc lint` again. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: epriestley, Korvin Differential Revision: https://secure.phabricator.com/D9089
This commit is contained in:
parent
79285208c4
commit
b744ed9a19
1 changed files with 28 additions and 43 deletions
|
@ -22,15 +22,21 @@ final class ArcanistConfigurationDrivenLintEngine extends ArcanistLintEngine {
|
||||||
|
|
||||||
$linters = $this->loadAvailableLinters();
|
$linters = $this->loadAvailableLinters();
|
||||||
|
|
||||||
PhutilTypeSpec::checkMap(
|
try {
|
||||||
$config,
|
PhutilTypeSpec::checkMap(
|
||||||
array(
|
$config,
|
||||||
'exclude' => 'optional string | list<string>',
|
array(
|
||||||
'linters' => 'map<string, map<string, wild>>',
|
'exclude' => 'optional regex | list<regex>',
|
||||||
));
|
'linters' => 'map<string, map<string, wild>>',
|
||||||
|
));
|
||||||
|
} catch (PhutilTypeCheckException $ex) {
|
||||||
|
$message = pht(
|
||||||
|
'Error in parsing ".arclint" file: %s',
|
||||||
|
$ex->getMessage());
|
||||||
|
throw new PhutilProxyException($message, $ex);
|
||||||
|
}
|
||||||
|
|
||||||
$global_exclude = (array)idx($config, 'exclude', array());
|
$global_exclude = (array)idx($config, 'exclude', array());
|
||||||
$this->validateRegexps($global_exclude);
|
|
||||||
|
|
||||||
$built_linters = array();
|
$built_linters = array();
|
||||||
$all_paths = $this->getPaths();
|
$all_paths = $this->getPaths();
|
||||||
|
@ -63,13 +69,21 @@ final class ArcanistConfigurationDrivenLintEngine extends ArcanistLintEngine {
|
||||||
$more = array();
|
$more = array();
|
||||||
}
|
}
|
||||||
|
|
||||||
PhutilTypeSpec::checkMap(
|
try {
|
||||||
$spec,
|
PhutilTypeSpec::checkMap(
|
||||||
array(
|
$spec,
|
||||||
'type' => 'string',
|
array(
|
||||||
'include' => 'optional string | list<string>',
|
'type' => 'string',
|
||||||
'exclude' => 'optional string | list<string>',
|
'include' => 'optional regex | list<regex>',
|
||||||
) + $more);
|
'exclude' => 'optional regex | list<regex>',
|
||||||
|
) + $more);
|
||||||
|
} catch (PhutilTypeCheckException $ex) {
|
||||||
|
$message = pht(
|
||||||
|
'Error in parsing ".arclint" file, for linter "%s": %s',
|
||||||
|
$name,
|
||||||
|
$ex->getMessage());
|
||||||
|
throw new PhutilProxyException($message, $ex);
|
||||||
|
}
|
||||||
|
|
||||||
foreach ($more as $key => $value) {
|
foreach ($more as $key => $value) {
|
||||||
if (array_key_exists($key, $spec)) {
|
if (array_key_exists($key, $spec)) {
|
||||||
|
@ -90,9 +104,6 @@ final class ArcanistConfigurationDrivenLintEngine extends ArcanistLintEngine {
|
||||||
$include = (array)idx($spec, 'include', array());
|
$include = (array)idx($spec, 'include', array());
|
||||||
$exclude = (array)idx($spec, 'exclude', array());
|
$exclude = (array)idx($spec, 'exclude', array());
|
||||||
|
|
||||||
$this->validateRegexps($include, $name, 'include');
|
|
||||||
$this->validateRegexps($exclude, $name, 'exclude');
|
|
||||||
|
|
||||||
$console = PhutilConsole::getConsole();
|
$console = PhutilConsole::getConsole();
|
||||||
$console->writeLog("Examining paths for linter \"%s\".\n", $name);
|
$console->writeLog("Examining paths for linter \"%s\".\n", $name);
|
||||||
$paths = $this->matchPaths(
|
$paths = $this->matchPaths(
|
||||||
|
@ -219,30 +230,4 @@ final class ArcanistConfigurationDrivenLintEngine extends ArcanistLintEngine {
|
||||||
return $match;
|
return $match;
|
||||||
}
|
}
|
||||||
|
|
||||||
private function validateRegexps(
|
|
||||||
array $regexps,
|
|
||||||
$linter = null,
|
|
||||||
$config = null) {
|
|
||||||
|
|
||||||
foreach ($regexps as $regexp) {
|
|
||||||
$ok = @preg_match($regexp, '');
|
|
||||||
if ($ok === false) {
|
|
||||||
if ($linter) {
|
|
||||||
throw new Exception(
|
|
||||||
pht(
|
|
||||||
'Regular expression "%s" (in "%s" configuration for linter '.
|
|
||||||
'"%s") is not a valid regular expression.',
|
|
||||||
$regexp,
|
|
||||||
$config,
|
|
||||||
$linter));
|
|
||||||
} else {
|
|
||||||
throw new Exception(
|
|
||||||
pht(
|
|
||||||
'Regular expression "%s" is not a valid regular expression.',
|
|
||||||
$regexp));
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue