1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-26 08:42:40 +01:00

Print lint results before throwing

Summary:
When some linter throws then we don't print any result.
This is bad in case when the linter threw e.g. because of syntax error in some file which some other linter will tell us about.

Test Plan: Threw from a linter, made lint error in a file, saw error then exception.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3756
This commit is contained in:
vrana 2012-10-20 04:42:53 -07:00
parent 134e9e5b4d
commit d09eb881a3
2 changed files with 55 additions and 31 deletions

View file

@ -196,44 +196,49 @@ abstract class ArcanistLintEngine {
throw new ArcanistNoEffectException("No paths are lintable."); throw new ArcanistNoEffectException("No paths are lintable.");
} }
$exceptions = array();
foreach ($linters as $linter) { foreach ($linters as $linter) {
$linter->setEngine($this); try {
if (!$linter->canRun()) { $linter->setEngine($this);
continue; if (!$linter->canRun()) {
} continue;
$paths = $linter->getPaths();
foreach ($paths as $key => $path) {
// Make sure each path has a result generated, even if it is empty
// (i.e., the file has no lint messages).
$result = $this->getResultForPath($path);
if (isset($stopped[$path])) {
unset($paths[$key]);
} }
} $paths = $linter->getPaths();
$paths = array_values($paths);
if ($paths) { foreach ($paths as $key => $path) {
$linter->willLintPaths($paths); // Make sure each path has a result generated, even if it is empty
foreach ($paths as $path) { // (i.e., the file has no lint messages).
$linter->willLintPath($path); $result = $this->getResultForPath($path);
$linter->lintPath($path); if (isset($stopped[$path])) {
if ($linter->didStopAllLinters()) { unset($paths[$key]);
$stopped[$path] = true;
} }
} }
} $paths = array_values($paths);
$minimum = $this->minimumSeverity; if ($paths) {
foreach ($linter->getLintMessages() as $message) { $linter->willLintPaths($paths);
if (!ArcanistLintSeverity::isAtLeastAsSevere($message, $minimum)) { foreach ($paths as $path) {
continue; $linter->willLintPath($path);
$linter->lintPath($path);
if ($linter->didStopAllLinters()) {
$stopped[$path] = true;
}
}
} }
if (!$this->isRelevantMessage($message)) {
continue; $minimum = $this->minimumSeverity;
foreach ($linter->getLintMessages() as $message) {
if (!ArcanistLintSeverity::isAtLeastAsSevere($message, $minimum)) {
continue;
}
if (!$this->isRelevantMessage($message)) {
continue;
}
$result = $this->getResultForPath($message->getPath());
$result->addMessage($message);
} }
$result = $this->getResultForPath($message->getPath()); } catch (Exception $ex) {
$result->addMessage($message); $exceptions[] = $ex;
} }
} }
@ -258,6 +263,14 @@ abstract class ArcanistLintEngine {
} }
} }
if ($exceptions) {
throw new PhutilAggregateException('Some linters failed:', $exceptions);
}
return $this->results;
}
public function getResults() {
return $this->results; return $this->results;
} }

View file

@ -201,7 +201,14 @@ EOTEXT
$engine->setEnableAsyncLint(false); $engine->setEnableAsyncLint(false);
} }
$results = $engine->run(); $failed = null;
try {
$engine->run();
} catch (Exception $ex) {
$failed = $ex;
}
$results = $engine->getResults();
// It'd be nice to just return a single result from the run method above // It'd be nice to just return a single result from the run method above
// which contains both the lint messages and the postponed linters. // which contains both the lint messages and the postponed linters.
@ -309,6 +316,10 @@ EOTEXT
} }
} }
if ($failed) {
throw $failed;
}
$repository_api = $this->getRepositoryAPI(); $repository_api = $this->getRepositoryAPI();
if ($wrote_to_disk && if ($wrote_to_disk &&
($repository_api instanceof ArcanistGitAPI) && ($repository_api instanceof ArcanistGitAPI) &&