1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-22 21:40:55 +01:00

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
This commit is contained in:
Andrew Gallagher 2012-06-14 17:09:24 -07:00
parent 47cb5d3cc3
commit 3223defe96
6 changed files with 197 additions and 5 deletions

View file

@ -138,6 +138,7 @@ phutil_register_library_map(array(
'ConduitAPI_differential_createrawdiff_Method' => 'applications/conduit/method/differential/ConduitAPI_differential_createrawdiff_Method.php', '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_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_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_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_getcommitmessage_Method' => 'applications/conduit/method/differential/ConduitAPI_differential_getcommitmessage_Method.php',
'ConduitAPI_differential_getcommitpaths_Method' => 'applications/conduit/method/differential/ConduitAPI_differential_getcommitpaths_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_createrawdiff_Method' => 'ConduitAPI_differential_Method',
'ConduitAPI_differential_createrevision_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_createrevision_Method' => 'ConduitAPIMethod',
'ConduitAPI_differential_find_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_find_Method' => 'ConduitAPIMethod',
'ConduitAPI_differential_finishpostponedlinters_Method' => 'ConduitAPIMethod',
'ConduitAPI_differential_getalldiffs_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_getalldiffs_Method' => 'ConduitAPIMethod',
'ConduitAPI_differential_getcommitmessage_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_getcommitmessage_Method' => 'ConduitAPIMethod',
'ConduitAPI_differential_getcommitpaths_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_getcommitpaths_Method' => 'ConduitAPIMethod',

View file

@ -0,0 +1,131 @@
<?php
/*
* Copyright 2012 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
/**
* @group conduit
*/
final class ConduitAPI_differential_finishpostponedlinters_Method
extends ConduitAPIMethod {
public function getMethodDescription() {
return "Update diff with new lint messages and mark postponed ".
"linters as finished.";
}
public function defineParamTypes() {
return array(
'diffID' => '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();
}
}

View file

@ -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); $diff = id(new DifferentialDiff())->load($diff_id);
if (!$diff) { if (!$diff) {
throw new ConduitException('ERR_NOT_FOUND'); 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_error = false;
$has_warning = false; $has_warning = false;
foreach ($results as $result) { foreach ($results as $result) {
@ -66,6 +91,8 @@ final class ConduitAPI_differential_setdiffproperty_Method
$diff->setLintStatus(DifferentialLintStatus::LINT_FAIL); $diff->setLintStatus(DifferentialLintStatus::LINT_FAIL);
} else if ($has_warning) { } else if ($has_warning) {
$diff->setLintStatus(DifferentialLintStatus::LINT_WARN); $diff->setLintStatus(DifferentialLintStatus::LINT_WARN);
} else if (!empty($postponed_linters)) {
$diff->setLintStatus(DifferentialLintStatus::LINT_POSTPONED);
} else if ($results && } else if ($results &&
$diff->getLintStatus() === DifferentialLintStatus::LINT_NONE) { $diff->getLintStatus() === DifferentialLintStatus::LINT_NONE) {
$diff->setLintStatus(DifferentialLintStatus::LINT_OKAY); $diff->setLintStatus(DifferentialLintStatus::LINT_OKAY);
@ -80,8 +107,8 @@ final class ConduitAPI_differential_setdiffproperty_Method
self::updateDiffProperty($diff_id, $name, $data); self::updateDiffProperty($diff_id, $name, $data);
if ($name === 'arc:lint') { if ($name === 'arc:lint' || $name == 'arc:lint-postponed') {
self::updateLintStatus($diff_id, $data); self::updateLintStatus($diff_id);
} }
return; return;

View file

@ -1,7 +1,7 @@
<?php <?php
/* /*
* Copyright 2011 Facebook, Inc. * Copyright 2012 Facebook, Inc.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -23,5 +23,6 @@ final class DifferentialLintStatus {
const LINT_WARN = 2; const LINT_WARN = 2;
const LINT_FAIL = 3; const LINT_FAIL = 3;
const LINT_SKIP = 4; const LINT_SKIP = 4;
const LINT_POSTPONED = 5;
} }

View file

@ -28,13 +28,17 @@ final class DifferentialLintFieldSpecification
} }
public function getRequiredDiffProperties() { public function getRequiredDiffProperties() {
return array('arc:lint', 'arc:lint-excuse'); return array('arc:lint', 'arc:lint-excuse', 'arc:lint-postponed');
} }
private function getLintExcuse() { private function getLintExcuse() {
return $this->getDiffProperty('arc:lint-excuse'); return $this->getDiffProperty('arc:lint-excuse');
} }
private function getPostponedLinters() {
return $this->getDiffProperty('arc:lint-postponed');
}
public function renderValueForRevisionView() { public function renderValueForRevisionView() {
$diff = $this->getDiff(); $diff = $this->getDiff();
$path_changesets = mpull($diff->loadChangesets(), 'getID', 'getFilename'); $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); $show_string = $this->renderShowString($hidden);
$view = new DifferentialResultsTableView(); $view = new DifferentialResultsTableView();
@ -151,6 +171,10 @@ final class DifferentialLintFieldSpecification
return idx($map, $severity); return idx($map, $severity);
} }
private function getPostponedStyle() {
return 'blue';
}
private function renderShowString(array $hidden) { private function renderShowString(array $hidden) {
if (!$hidden) { if (!$hidden) {
return null; return null;
@ -165,6 +189,7 @@ final class DifferentialLintFieldSpecification
ArcanistLintSeverity::SEVERITY_AUTOFIX, ArcanistLintSeverity::SEVERITY_AUTOFIX,
ArcanistLintSeverity::SEVERITY_ADVICE, ArcanistLintSeverity::SEVERITY_ADVICE,
'details', 'details',
'postponed',
)) + $hidden; )) + $hidden;
$show = array(); $show = array();
@ -185,6 +210,9 @@ final class DifferentialLintFieldSpecification
case 'details': case 'details':
$show[] = pht('%d Detail(s)', $value); $show[] = pht('%d Detail(s)', $value);
break; break;
case 'postponed':
$show[] = pht('%d Postponed', $value);
break;
default: default:
$show[] = $value; $show[] = $value;
break; break;

View file

@ -252,6 +252,7 @@ final class DifferentialRevisionUpdateHistoryView extends AphrontView {
DifferentialLintStatus::LINT_WARN => self::STAR_WARN, DifferentialLintStatus::LINT_WARN => self::STAR_WARN,
DifferentialLintStatus::LINT_FAIL => self::STAR_FAIL, DifferentialLintStatus::LINT_FAIL => self::STAR_FAIL,
DifferentialLintStatus::LINT_SKIP => self::STAR_SKIP, DifferentialLintStatus::LINT_SKIP => self::STAR_SKIP,
DifferentialLintStatus::LINT_POSTPONED => self::STAR_SKIP
); );
$star = idx($map, $diff->getLintStatus(), self::STAR_FAIL); $star = idx($map, $diff->getLintStatus(), self::STAR_FAIL);
@ -286,6 +287,8 @@ final class DifferentialRevisionUpdateHistoryView extends AphrontView {
return 'Lint Errors'; return 'Lint Errors';
case DifferentialLintStatus::LINT_SKIP: case DifferentialLintStatus::LINT_SKIP:
return 'Lint Skipped'; return 'Lint Skipped';
case DifferentialLintStatus::LINT_POSTPONED:
return 'Lint Postponed';
} }
return '???'; return '???';
} }