From 19e718a267fc5faaafe3a5b61bdb85d7f4d4caf2 Mon Sep 17 00:00:00 2001 From: Andrew Gallagher Date: Mon, 2 Jul 2012 15:53:22 -0700 Subject: [PATCH] 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 --- src/events/constant/ArcanistEventType.php | 1 + src/lint/engine/ArcanistLintEngine.php | 20 ++++++++ src/workflow/ArcanistDiffWorkflow.php | 62 ++++++++++++++++------- src/workflow/ArcanistLintWorkflow.php | 23 +++++++++ 4 files changed, 88 insertions(+), 18 deletions(-) diff --git a/src/events/constant/ArcanistEventType.php b/src/events/constant/ArcanistEventType.php index 411dbaaf..37879396 100644 --- a/src/events/constant/ArcanistEventType.php +++ b/src/events/constant/ArcanistEventType.php @@ -20,5 +20,6 @@ final class ArcanistEventType extends PhutilEventType { const TYPE_COMMIT_WILLCOMMITSVN = 'commit.willCommitSVN'; const TYPE_DIFF_WILLBUILDMESSAGE = 'diff.willBuildMessage'; + const TYPE_DIFF_WASCREATED = 'diff.wasCreated'; } diff --git a/src/lint/engine/ArcanistLintEngine.php b/src/lint/engine/ArcanistLintEngine.php index aed642ba..858285c0 100644 --- a/src/lint/engine/ArcanistLintEngine.php +++ b/src/lint/engine/ArcanistLintEngine.php @@ -72,6 +72,9 @@ abstract class ArcanistLintEngine { private $commitHookMode = false; private $hookAPI; + private $enableAsyncLint = false; + private $postponedLinters = array(); + public function __construct() { } @@ -126,6 +129,15 @@ abstract class ArcanistLintEngine { 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) { if (!isset($this->fileData[$path])) { if ($this->getCommitHookMode()) { @@ -295,5 +307,13 @@ abstract class ArcanistLintEngine { return array($line, $char); } + public function getPostponedLinters() { + return $this->postponedLinters; + } + + public function setPostponedLinters(array $linters) { + $this->postponedLinters = $linters; + return $this; + } } diff --git a/src/workflow/ArcanistDiffWorkflow.php b/src/workflow/ArcanistDiffWorkflow.php index b8aab48a..73c36f23 100644 --- a/src/workflow/ArcanistDiffWorkflow.php +++ b/src/workflow/ArcanistDiffWorkflow.php @@ -36,6 +36,8 @@ final class ArcanistDiffWorkflow extends ArcanistBaseWorkflow { private $diffID; private $revisionID; private $unitWorkflow; + private $lintWorkflow; + private $postponedLinters; public function getCommandSynopses() { return phutil_console_format(<<diffID = $diff_info['diffid']; + $this->dispatchDiffWasCreatedEvent($diff_info['diffid']); + if ($this->unitWorkflow) { $this->unitWorkflow->setDifferentialDiffID($diff_info['diffid']); } @@ -1093,6 +1097,7 @@ EOTEXT $argv[] = $repository_api->getRelativeCommit(); } $lint_workflow = $this->buildChildWorkflow('lint', $argv); + $this->lintWorkflow = $lint_workflow; if ($this->shouldAmend()) { // TODO: We should offer to create a checkpoint commit. @@ -1118,9 +1123,15 @@ EOTEXT "Lint issued unresolved errors!", 'lint-excuses'); break; + case ArcanistLintWorkflow::RESULT_POSTPONED: + echo phutil_console_format( + "** LINT POSTPONED ** ". + "Lint results are postponed.\n"); + break; } $this->unresolvedLint = $lint_workflow->getUnresolvedMessages(); + $this->postponedLinters = $lint_workflow->getPostponedLinters(); return $lint_result; } catch (ArcanistNoEngineException $ex) { @@ -1945,6 +1956,7 @@ EOTEXT ArcanistLintWorkflow::RESULT_ERRORS => 'fail', ArcanistLintWorkflow::RESULT_WARNINGS => 'warn', ArcanistLintWorkflow::RESULT_SKIP => 'skip', + ArcanistLintWorkflow::RESULT_POSTPONED => 'postponed', ); return idx($map, $lint_result, 'none'); } @@ -2052,28 +2064,33 @@ EOTEXT * @task diffprop */ 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(); - 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(), - ); + $postponed = $this->postponedLinters; + if ($postponed) { + $this->updateDiffProperty('arc:lint-postponed', json_encode($postponed)); } - $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'); } + private function dispatchDiffWasCreatedEvent($diff_id) { + $event = new PhutilEvent( + ArcanistEventType::TYPE_DIFF_WASCREATED, + array( + 'diffID' => $diff_id, + )); + + PhutilEventEngine::dispatchEvent($event); + } } diff --git a/src/workflow/ArcanistLintWorkflow.php b/src/workflow/ArcanistLintWorkflow.php index 2504e7ba..cd8cb8fc 100644 --- a/src/workflow/ArcanistLintWorkflow.php +++ b/src/workflow/ArcanistLintWorkflow.php @@ -27,11 +27,14 @@ class ArcanistLintWorkflow extends ArcanistBaseWorkflow { const RESULT_WARNINGS = 1; const RESULT_ERRORS = 2; const RESULT_SKIP = 3; + const RESULT_POSTPONED = 4; private $unresolvedMessages; private $shouldAmendChanges = false; private $shouldAmendWithoutPrompt = false; private $shouldAmendAutofixesWithoutPrompt = false; + private $engine; + private $postponedLinters; public function setShouldAmendChanges($should_amend) { $this->shouldAmendChanges = $should_amend; @@ -173,6 +176,7 @@ EOTEXT } $engine = newv($engine, array()); + $this->engine = $engine; $engine->setWorkingCopy($working_copy); 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(); + // 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')) { $apply_patches = false; } else { @@ -342,6 +359,8 @@ EOTEXT $result_code = self::RESULT_ERRORS; } else if ($has_warnings) { $result_code = self::RESULT_WARNINGS; + } else if (!empty($this->postponedLinters)) { + $result_code = self::RESULT_POSTPONED; } else { $result_code = self::RESULT_OKAY; } @@ -359,4 +378,8 @@ EOTEXT return $this->unresolvedMessages; } + public function getPostponedLinters() { + return $this->postponedLinters; + } + }