mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-11-26 00:32:41 +01:00
arcanist: add postponed linter support
Summary: This uses a similiar approach as with postponed unittests, allowing the lint workflow/engine to report postponed linter names. After the lint engine is run, a separate method is used to collect any postponed linters and these are reposted to the diff via the "arc:lint-postponed" property. Also, a ##diff.wasCreated## was added allowing hooks to be called immediately after the call ##differential.creatediff## with the returned diff ID. Test Plan: Created diffs with a dummy lint engine which always reports a postponed linter. Reviewers: epriestley, vrana Reviewed By: epriestley CC: aran, Korvin Maniphest Tasks: T1332 Differential Revision: https://secure.phabricator.com/D2933
This commit is contained in:
parent
daa210022d
commit
19e718a267
4 changed files with 88 additions and 18 deletions
|
@ -20,5 +20,6 @@ final class ArcanistEventType extends PhutilEventType {
|
||||||
|
|
||||||
const TYPE_COMMIT_WILLCOMMITSVN = 'commit.willCommitSVN';
|
const TYPE_COMMIT_WILLCOMMITSVN = 'commit.willCommitSVN';
|
||||||
const TYPE_DIFF_WILLBUILDMESSAGE = 'diff.willBuildMessage';
|
const TYPE_DIFF_WILLBUILDMESSAGE = 'diff.willBuildMessage';
|
||||||
|
const TYPE_DIFF_WASCREATED = 'diff.wasCreated';
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -72,6 +72,9 @@ abstract class ArcanistLintEngine {
|
||||||
private $commitHookMode = false;
|
private $commitHookMode = false;
|
||||||
private $hookAPI;
|
private $hookAPI;
|
||||||
|
|
||||||
|
private $enableAsyncLint = false;
|
||||||
|
private $postponedLinters = array();
|
||||||
|
|
||||||
public function __construct() {
|
public function __construct() {
|
||||||
|
|
||||||
}
|
}
|
||||||
|
@ -126,6 +129,15 @@ abstract class ArcanistLintEngine {
|
||||||
return $this->hookAPI;
|
return $this->hookAPI;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function setEnableAsyncLint($enable_async_lint) {
|
||||||
|
$this->enableAsyncLint = $enable_async_lint;
|
||||||
|
return $this;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getEnableAsyncLint() {
|
||||||
|
return $this->enableAsyncLint;
|
||||||
|
}
|
||||||
|
|
||||||
public function loadData($path) {
|
public function loadData($path) {
|
||||||
if (!isset($this->fileData[$path])) {
|
if (!isset($this->fileData[$path])) {
|
||||||
if ($this->getCommitHookMode()) {
|
if ($this->getCommitHookMode()) {
|
||||||
|
@ -295,5 +307,13 @@ abstract class ArcanistLintEngine {
|
||||||
return array($line, $char);
|
return array($line, $char);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function getPostponedLinters() {
|
||||||
|
return $this->postponedLinters;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function setPostponedLinters(array $linters) {
|
||||||
|
$this->postponedLinters = $linters;
|
||||||
|
return $this;
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -36,6 +36,8 @@ final class ArcanistDiffWorkflow extends ArcanistBaseWorkflow {
|
||||||
private $diffID;
|
private $diffID;
|
||||||
private $revisionID;
|
private $revisionID;
|
||||||
private $unitWorkflow;
|
private $unitWorkflow;
|
||||||
|
private $lintWorkflow;
|
||||||
|
private $postponedLinters;
|
||||||
|
|
||||||
public function getCommandSynopses() {
|
public function getCommandSynopses() {
|
||||||
return phutil_console_format(<<<EOTEXT
|
return phutil_console_format(<<<EOTEXT
|
||||||
|
@ -387,6 +389,8 @@ EOTEXT
|
||||||
|
|
||||||
$this->diffID = $diff_info['diffid'];
|
$this->diffID = $diff_info['diffid'];
|
||||||
|
|
||||||
|
$this->dispatchDiffWasCreatedEvent($diff_info['diffid']);
|
||||||
|
|
||||||
if ($this->unitWorkflow) {
|
if ($this->unitWorkflow) {
|
||||||
$this->unitWorkflow->setDifferentialDiffID($diff_info['diffid']);
|
$this->unitWorkflow->setDifferentialDiffID($diff_info['diffid']);
|
||||||
}
|
}
|
||||||
|
@ -1093,6 +1097,7 @@ EOTEXT
|
||||||
$argv[] = $repository_api->getRelativeCommit();
|
$argv[] = $repository_api->getRelativeCommit();
|
||||||
}
|
}
|
||||||
$lint_workflow = $this->buildChildWorkflow('lint', $argv);
|
$lint_workflow = $this->buildChildWorkflow('lint', $argv);
|
||||||
|
$this->lintWorkflow = $lint_workflow;
|
||||||
|
|
||||||
if ($this->shouldAmend()) {
|
if ($this->shouldAmend()) {
|
||||||
// TODO: We should offer to create a checkpoint commit.
|
// TODO: We should offer to create a checkpoint commit.
|
||||||
|
@ -1118,9 +1123,15 @@ EOTEXT
|
||||||
"Lint issued unresolved errors!",
|
"Lint issued unresolved errors!",
|
||||||
'lint-excuses');
|
'lint-excuses');
|
||||||
break;
|
break;
|
||||||
|
case ArcanistLintWorkflow::RESULT_POSTPONED:
|
||||||
|
echo phutil_console_format(
|
||||||
|
"<bg:yellow>** LINT POSTPONED **</bg> ".
|
||||||
|
"Lint results are postponed.\n");
|
||||||
|
break;
|
||||||
}
|
}
|
||||||
|
|
||||||
$this->unresolvedLint = $lint_workflow->getUnresolvedMessages();
|
$this->unresolvedLint = $lint_workflow->getUnresolvedMessages();
|
||||||
|
$this->postponedLinters = $lint_workflow->getPostponedLinters();
|
||||||
|
|
||||||
return $lint_result;
|
return $lint_result;
|
||||||
} catch (ArcanistNoEngineException $ex) {
|
} catch (ArcanistNoEngineException $ex) {
|
||||||
|
@ -1945,6 +1956,7 @@ EOTEXT
|
||||||
ArcanistLintWorkflow::RESULT_ERRORS => 'fail',
|
ArcanistLintWorkflow::RESULT_ERRORS => 'fail',
|
||||||
ArcanistLintWorkflow::RESULT_WARNINGS => 'warn',
|
ArcanistLintWorkflow::RESULT_WARNINGS => 'warn',
|
||||||
ArcanistLintWorkflow::RESULT_SKIP => 'skip',
|
ArcanistLintWorkflow::RESULT_SKIP => 'skip',
|
||||||
|
ArcanistLintWorkflow::RESULT_POSTPONED => 'postponed',
|
||||||
);
|
);
|
||||||
return idx($map, $lint_result, 'none');
|
return idx($map, $lint_result, 'none');
|
||||||
}
|
}
|
||||||
|
@ -2052,28 +2064,33 @@ EOTEXT
|
||||||
* @task diffprop
|
* @task diffprop
|
||||||
*/
|
*/
|
||||||
private function updateLintDiffProperty() {
|
private function updateLintDiffProperty() {
|
||||||
if (!$this->unresolvedLint) {
|
|
||||||
return;
|
if ($this->unresolvedLint) {
|
||||||
|
$data = array();
|
||||||
|
foreach ($this->unresolvedLint as $message) {
|
||||||
|
$data[] = array(
|
||||||
|
'path' => $message->getPath(),
|
||||||
|
'line' => $message->getLine(),
|
||||||
|
'char' => $message->getChar(),
|
||||||
|
'code' => $message->getCode(),
|
||||||
|
'severity' => $message->getSeverity(),
|
||||||
|
'name' => $message->getName(),
|
||||||
|
'description' => $message->getDescription(),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
$this->updateDiffProperty('arc:lint', json_encode($data));
|
||||||
|
if (strlen($this->lintExcuse)) {
|
||||||
|
$this->updateDiffProperty('arc:lint-excuse',
|
||||||
|
json_encode($this->lintExcuse));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
$data = array();
|
$postponed = $this->postponedLinters;
|
||||||
foreach ($this->unresolvedLint as $message) {
|
if ($postponed) {
|
||||||
$data[] = array(
|
$this->updateDiffProperty('arc:lint-postponed', json_encode($postponed));
|
||||||
'path' => $message->getPath(),
|
|
||||||
'line' => $message->getLine(),
|
|
||||||
'char' => $message->getChar(),
|
|
||||||
'code' => $message->getCode(),
|
|
||||||
'severity' => $message->getSeverity(),
|
|
||||||
'name' => $message->getName(),
|
|
||||||
'description' => $message->getDescription(),
|
|
||||||
);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
$this->updateDiffProperty('arc:lint', json_encode($data));
|
|
||||||
if (strlen($this->lintExcuse)) {
|
|
||||||
$this->updateDiffProperty('arc:lint-excuse',
|
|
||||||
json_encode($this->lintExcuse));
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
@ -2157,4 +2174,13 @@ EOTEXT
|
||||||
return $event->getValue('fields');
|
return $event->getValue('fields');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private function dispatchDiffWasCreatedEvent($diff_id) {
|
||||||
|
$event = new PhutilEvent(
|
||||||
|
ArcanistEventType::TYPE_DIFF_WASCREATED,
|
||||||
|
array(
|
||||||
|
'diffID' => $diff_id,
|
||||||
|
));
|
||||||
|
|
||||||
|
PhutilEventEngine::dispatchEvent($event);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -27,11 +27,14 @@ class ArcanistLintWorkflow extends ArcanistBaseWorkflow {
|
||||||
const RESULT_WARNINGS = 1;
|
const RESULT_WARNINGS = 1;
|
||||||
const RESULT_ERRORS = 2;
|
const RESULT_ERRORS = 2;
|
||||||
const RESULT_SKIP = 3;
|
const RESULT_SKIP = 3;
|
||||||
|
const RESULT_POSTPONED = 4;
|
||||||
|
|
||||||
private $unresolvedMessages;
|
private $unresolvedMessages;
|
||||||
private $shouldAmendChanges = false;
|
private $shouldAmendChanges = false;
|
||||||
private $shouldAmendWithoutPrompt = false;
|
private $shouldAmendWithoutPrompt = false;
|
||||||
private $shouldAmendAutofixesWithoutPrompt = false;
|
private $shouldAmendAutofixesWithoutPrompt = false;
|
||||||
|
private $engine;
|
||||||
|
private $postponedLinters;
|
||||||
|
|
||||||
public function setShouldAmendChanges($should_amend) {
|
public function setShouldAmendChanges($should_amend) {
|
||||||
$this->shouldAmendChanges = $should_amend;
|
$this->shouldAmendChanges = $should_amend;
|
||||||
|
@ -173,6 +176,7 @@ EOTEXT
|
||||||
}
|
}
|
||||||
|
|
||||||
$engine = newv($engine, array());
|
$engine = newv($engine, array());
|
||||||
|
$this->engine = $engine;
|
||||||
$engine->setWorkingCopy($working_copy);
|
$engine->setWorkingCopy($working_copy);
|
||||||
|
|
||||||
if ($this->getArgument('advice')) {
|
if ($this->getArgument('advice')) {
|
||||||
|
@ -195,8 +199,21 @@ EOTEXT
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Enable possible async linting only for 'arc diff' not 'arc unit'
|
||||||
|
if ($this->getParentWorkflow()) {
|
||||||
|
$engine->setEnableAsyncLint(true);
|
||||||
|
} else {
|
||||||
|
$engine->setEnableAsyncLint(false);
|
||||||
|
}
|
||||||
|
|
||||||
$results = $engine->run();
|
$results = $engine->run();
|
||||||
|
|
||||||
|
// 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.
|
||||||
|
// However, to maintain compatibility with existing lint subclasses, use
|
||||||
|
// a separate method call to grab the postponed linters.
|
||||||
|
$this->postponedLinters = $engine->getPostponedLinters();
|
||||||
|
|
||||||
if ($this->getArgument('never-apply-patches')) {
|
if ($this->getArgument('never-apply-patches')) {
|
||||||
$apply_patches = false;
|
$apply_patches = false;
|
||||||
} else {
|
} else {
|
||||||
|
@ -342,6 +359,8 @@ EOTEXT
|
||||||
$result_code = self::RESULT_ERRORS;
|
$result_code = self::RESULT_ERRORS;
|
||||||
} else if ($has_warnings) {
|
} else if ($has_warnings) {
|
||||||
$result_code = self::RESULT_WARNINGS;
|
$result_code = self::RESULT_WARNINGS;
|
||||||
|
} else if (!empty($this->postponedLinters)) {
|
||||||
|
$result_code = self::RESULT_POSTPONED;
|
||||||
} else {
|
} else {
|
||||||
$result_code = self::RESULT_OKAY;
|
$result_code = self::RESULT_OKAY;
|
||||||
}
|
}
|
||||||
|
@ -359,4 +378,8 @@ EOTEXT
|
||||||
return $this->unresolvedMessages;
|
return $this->unresolvedMessages;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function getPostponedLinters() {
|
||||||
|
return $this->postponedLinters;
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue