From 3223defe96caafa3cb4c2c2e292a464f3191e230 Mon Sep 17 00:00:00 2001 From: Andrew Gallagher Date: Thu, 14 Jun 2012 17:09:24 -0700 Subject: [PATCH] Add support for postponed linters Summary: This adds: 1) A new "arc:lint-postponed" diff property which stores a list of lint names that are postponed and a finishpostponedlinters conduit method which removes linters from this list. Postponed linters are shown in the lint details. 2) A updatelintmessages conduit message, which adds additional lint messages to the "arc:lint" diff property. In combination, this provides very basic support for running asynchronous static analysis tools. When the diff is being created, a list of asynchronous static analysis runs can be added to the diff's postponed linters list. As these postponed linters finish up, then can report new lint messages back to the diff then mark themselves as complete. The client is currently responsible for filtering the lint messages by things like affected lines and files. Test Plan: Used conduit call API to add lint messages and remove postponed linters from a test diff. Reviewers: epriestley, vrana, nh, jungejason Reviewed By: epriestley CC: aran, Korvin Maniphest Tasks: T1332 Differential Revision: https://secure.phabricator.com/D2792 --- src/__phutil_library_map__.php | 2 + ...erential_finishpostponedlinters_Method.php | 131 ++++++++++++++++++ ...PI_differential_setdiffproperty_Method.php | 33 ++++- .../constants/DifferentialLintStatus.php | 3 +- .../DifferentialLintFieldSpecification.php | 30 +++- .../DifferentialRevisionUpdateHistoryView.php | 3 + 6 files changed, 197 insertions(+), 5 deletions(-) create mode 100644 src/applications/conduit/method/differential/ConduitAPI_differential_finishpostponedlinters_Method.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 38f7df862e..46279975fa 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -138,6 +138,7 @@ phutil_register_library_map(array( 'ConduitAPI_differential_createrawdiff_Method' => 'applications/conduit/method/differential/ConduitAPI_differential_createrawdiff_Method.php', 'ConduitAPI_differential_createrevision_Method' => 'applications/conduit/method/differential/ConduitAPI_differential_createrevision_Method.php', 'ConduitAPI_differential_find_Method' => 'applications/conduit/method/differential/ConduitAPI_differential_find_Method.php', + 'ConduitAPI_differential_finishpostponedlinters_Method' => 'applications/conduit/method/differential/ConduitAPI_differential_finishpostponedlinters_Method.php', 'ConduitAPI_differential_getalldiffs_Method' => 'applications/conduit/method/differential/ConduitAPI_differential_getalldiffs_Method.php', 'ConduitAPI_differential_getcommitmessage_Method' => 'applications/conduit/method/differential/ConduitAPI_differential_getcommitmessage_Method.php', 'ConduitAPI_differential_getcommitpaths_Method' => 'applications/conduit/method/differential/ConduitAPI_differential_getcommitpaths_Method.php', @@ -1214,6 +1215,7 @@ phutil_register_library_map(array( 'ConduitAPI_differential_createrawdiff_Method' => 'ConduitAPI_differential_Method', 'ConduitAPI_differential_createrevision_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_find_Method' => 'ConduitAPIMethod', + 'ConduitAPI_differential_finishpostponedlinters_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_getalldiffs_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_getcommitmessage_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_getcommitpaths_Method' => 'ConduitAPIMethod', diff --git a/src/applications/conduit/method/differential/ConduitAPI_differential_finishpostponedlinters_Method.php b/src/applications/conduit/method/differential/ConduitAPI_differential_finishpostponedlinters_Method.php new file mode 100644 index 0000000000..ec1b638071 --- /dev/null +++ b/src/applications/conduit/method/differential/ConduitAPI_differential_finishpostponedlinters_Method.php @@ -0,0 +1,131 @@ + 'required diffID', + 'linters' => 'required dict', + ); + } + + public function defineReturnType() { + return 'void'; + } + + public function defineErrorTypes() { + return array( + 'ERR-BAD-DIFF' => 'Bad diff ID.', + 'ERR-BAD-LINTER' => 'No postponed linter by the given name', + 'ERR-NO-LINT' => 'No postponed lint field available in diff', + ); + } + + protected function execute(ConduitAPIRequest $request) { + + $diff_id = $request->getValue('diffID'); + $linter_map = $request->getValue('linters'); + + $diff = id(new DifferentialDiff())->load($diff_id); + if (!$diff) { + throw new ConduitException('ERR-BAD-DIFF'); + } + + // Extract the finished linters and messages from the linter map. + $finished_linters = array_keys($linter_map); + $new_messages = array(); + foreach ($linter_map as $linter => $messages) { + $new_messages = array_merge($new_messages, $messages); + } + + // Load the postponed linters attached to this diff. + $postponed_linters_property = id( + new DifferentialDiffProperty())->loadOneWhere( + 'diffID = %d AND name = %s', + $diff_id, + 'arc:lint-postponed'); + if ($postponed_linters_property) { + $postponed_linters = $postponed_linters_property->getData(); + } else { + $postponed_linters = array(); + } + + foreach ($finished_linters as $linter) { + if (!in_array($linter, $postponed_linters)) { + throw new ConduitException('ERR-BAD-LINTER'); + } + } + + foreach ($postponed_linters as $idx => $linter) { + if (in_array($linter, $finished_linters)) { + unset($postponed_linters[$idx]); + } + } + + // Load the lint messages currenty attached to the diff. If this + // diff property doesn't exist, create it. + $messages_property = id(new DifferentialDiffProperty())->loadOneWhere( + 'diffID = %d AND name = %s', + $diff_id, + 'arc:lint' + ); + if ($messages_property) { + $messages = $messages_property->getData(); + } else { + $messages = array(); + } + + // Add new lint messages, removing duplicates. + foreach ($new_messages as $new_message) { + if (!in_array($new_message, $messages)) { + $messages[] = $new_message; + } + } + + // Use setdiffproperty to update the postponed linters and messages, + // as these will also update the lint status correctly. + $call = new ConduitCall( + 'differential.setdiffproperty', + array( + 'diff_id' => $diff_id, + 'name' => 'arc:lint', + 'data' => json_encode($messages))); + $call->setUser($request->getUser()); + $call->execute(); + $call = new ConduitCall( + 'differential.setdiffproperty', + array( + 'diff_id' => $diff_id, + 'name' => 'arc:lint-postponed', + 'data' => json_encode($postponed_linters))); + $call->setUser($request->getUser()); + $call->execute(); + + } + +} diff --git a/src/applications/conduit/method/differential/ConduitAPI_differential_setdiffproperty_Method.php b/src/applications/conduit/method/differential/ConduitAPI_differential_setdiffproperty_Method.php index 25e052ed64..703534ca45 100644 --- a/src/applications/conduit/method/differential/ConduitAPI_differential_setdiffproperty_Method.php +++ b/src/applications/conduit/method/differential/ConduitAPI_differential_setdiffproperty_Method.php @@ -44,12 +44,37 @@ final class ConduitAPI_differential_setdiffproperty_Method ); } - private static function updateLintStatus($diff_id, array $results) { + private static function updateLintStatus($diff_id) { + $diff = id(new DifferentialDiff())->load($diff_id); if (!$diff) { throw new ConduitException('ERR_NOT_FOUND'); } + // Load the postponed linters attached to this diff. + $postponed_linters_property = id( + new DifferentialDiffProperty())->loadOneWhere( + 'diffID = %d AND name = %s', + $diff_id, + 'arc:lint-postponed'); + if ($postponed_linters_property) { + $postponed_linters = $postponed_linters_property->getData(); + } else { + $postponed_linters = array(); + } + + // Load the lint messages currenty attached to the diff + $messages_property = id(new DifferentialDiffProperty())->loadOneWhere( + 'diffID = %d AND name = %s', + $diff_id, + 'arc:lint' + ); + if ($messages_property) { + $results = $messages_property->getData(); + } else { + $results = array(); + } + $has_error = false; $has_warning = false; foreach ($results as $result) { @@ -66,6 +91,8 @@ final class ConduitAPI_differential_setdiffproperty_Method $diff->setLintStatus(DifferentialLintStatus::LINT_FAIL); } else if ($has_warning) { $diff->setLintStatus(DifferentialLintStatus::LINT_WARN); + } else if (!empty($postponed_linters)) { + $diff->setLintStatus(DifferentialLintStatus::LINT_POSTPONED); } else if ($results && $diff->getLintStatus() === DifferentialLintStatus::LINT_NONE) { $diff->setLintStatus(DifferentialLintStatus::LINT_OKAY); @@ -80,8 +107,8 @@ final class ConduitAPI_differential_setdiffproperty_Method self::updateDiffProperty($diff_id, $name, $data); - if ($name === 'arc:lint') { - self::updateLintStatus($diff_id, $data); + if ($name === 'arc:lint' || $name == 'arc:lint-postponed') { + self::updateLintStatus($diff_id); } return; diff --git a/src/applications/differential/constants/DifferentialLintStatus.php b/src/applications/differential/constants/DifferentialLintStatus.php index c87d51fbd0..01ca2d03fd 100644 --- a/src/applications/differential/constants/DifferentialLintStatus.php +++ b/src/applications/differential/constants/DifferentialLintStatus.php @@ -1,7 +1,7 @@ getDiffProperty('arc:lint-excuse'); } + private function getPostponedLinters() { + return $this->getDiffProperty('arc:lint-postponed'); + } + public function renderValueForRevisionView() { $diff = $this->getDiff(); $path_changesets = mpull($diff->loadChangesets(), 'getID', 'getFilename'); @@ -132,6 +136,22 @@ final class DifferentialLintFieldSpecification } } + $postponed = $this->getPostponedLinters(); + if ($postponed) { + foreach ($postponed as $linter) { + $rows[] = array( + 'style' => $this->getPostponedStyle(), + 'name' => 'Postponed', + 'value' => phutil_escape_html($linter), + 'show' => false, + ); + if (empty($hidden['postponed'])) { + $hidden['postponed'] = 0; + } + $hidden['postponed']++; + } + } + $show_string = $this->renderShowString($hidden); $view = new DifferentialResultsTableView(); @@ -151,6 +171,10 @@ final class DifferentialLintFieldSpecification return idx($map, $severity); } + private function getPostponedStyle() { + return 'blue'; + } + private function renderShowString(array $hidden) { if (!$hidden) { return null; @@ -165,6 +189,7 @@ final class DifferentialLintFieldSpecification ArcanistLintSeverity::SEVERITY_AUTOFIX, ArcanistLintSeverity::SEVERITY_ADVICE, 'details', + 'postponed', )) + $hidden; $show = array(); @@ -185,6 +210,9 @@ final class DifferentialLintFieldSpecification case 'details': $show[] = pht('%d Detail(s)', $value); break; + case 'postponed': + $show[] = pht('%d Postponed', $value); + break; default: $show[] = $value; break; diff --git a/src/applications/differential/view/DifferentialRevisionUpdateHistoryView.php b/src/applications/differential/view/DifferentialRevisionUpdateHistoryView.php index d3c242b4d5..0c86d092c8 100644 --- a/src/applications/differential/view/DifferentialRevisionUpdateHistoryView.php +++ b/src/applications/differential/view/DifferentialRevisionUpdateHistoryView.php @@ -252,6 +252,7 @@ final class DifferentialRevisionUpdateHistoryView extends AphrontView { DifferentialLintStatus::LINT_WARN => self::STAR_WARN, DifferentialLintStatus::LINT_FAIL => self::STAR_FAIL, DifferentialLintStatus::LINT_SKIP => self::STAR_SKIP, + DifferentialLintStatus::LINT_POSTPONED => self::STAR_SKIP ); $star = idx($map, $diff->getLintStatus(), self::STAR_FAIL); @@ -286,6 +287,8 @@ final class DifferentialRevisionUpdateHistoryView extends AphrontView { return 'Lint Errors'; case DifferentialLintStatus::LINT_SKIP: return 'Lint Skipped'; + case DifferentialLintStatus::LINT_POSTPONED: + return 'Lint Postponed'; } return '???'; }