revi-archive/phorge-arcanist
1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-08 16:02:39 +01:00
Code Issues Releases Wiki Activity

Remove "arc lint --only-new"

Browse source 
Summary:
Ref T12996. Fixes T9749. This is a very old Facebook-specific thing which relies on other server-side Facebook-specific things and doesn't work properly anyway.

I don't actually remember what the use case for this flag was. It was either "the codebase has a million warnings, so showing warnings on files/lines you touched isn't good enough", or "weird warnings that raise lint on other lines", or some variation of those.

Regardless, I believe this feature benefits at most one install (Facebook circa 2011), and likely zero. It occasionally confuses normal users.

T9749 suggests a possible replacement workflow which is likely more practical, but I'd like to see a real problem description before considering this again.

Test Plan: Created this revision, grepped for `only-new`.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T12996, T9749

Differential Revision: https://secure.phabricator.com/D18642
This commit is contained in:
epriestley 2017-09-25 11:12:16 -07:00
parent 3453ef8358
commit 074dd8f3a6
1 changed files with 2 additions and 105 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

107
src/workflow/ArcanistLintWorkflow.php
View file

@ -68,9 +68,7 @@ EOTEXT
'only-changed' => array(
'help' => pht(
'Show lint warnings just on changed lines. When no paths are '.
'specified, this is the default. This differs from only-new '.
'in cases where line modifications introduce lint on other '.
'unmodified lines.'),
'specified, this is the default.'),
'conflicts' => array(
'lintall' => true,
),
@ -100,12 +98,6 @@ EOTEXT
'help' => pht(
'Output the linter results to a file. Defaults to stdout.'),
),
'only-new' => array(
'param' => 'bool',
'supports' => array('git', 'hg'), // TODO: svn
'help' => pht(
'Display only messages not present in the original code.'),
),
'engine' => array(
'param' => 'classname',
'help' => pht('Override configured lint engine for this project.'),
@ -167,7 +159,7 @@ EOTEXT
}
public function requiresAuthentication() {
return (bool)$this->getArgument('only-new');
return false;
}
public function requiresWorkingCopy() {
@ -286,42 +278,6 @@ EOTEXT
}
}
if ($this->getArgument('only-new')) {
$conduit = $this->getConduit();
$api = $this->getRepositoryAPI();
if ($rev) {
$api->setBaseCommit($rev);
}
$svn_root = id(new PhutilURI($api->getSourceControlPath()))->getPath();
$all_paths = array();
foreach ($paths as $path) {
$path = str_replace(DIRECTORY_SEPARATOR, '/', $path);
$full_paths = array($path);
$change = $this->getChange($path);
$type = $change->getType();
if (ArcanistDiffChangeType::isOldLocationChangeType($type)) {
$full_paths = $change->getAwayPaths();
} else if (ArcanistDiffChangeType::isNewLocationChangeType($type)) {
continue;
} else if (ArcanistDiffChangeType::isDeleteChangeType($type)) {
continue;
}
foreach ($full_paths as $full_path) {
$all_paths[$svn_root.'/'.$full_path] = $path;
}
}
$lint_future = $conduit->callMethod('diffusion.getlintmessages', array(
'repositoryPHID' => idx($this->loadProjectRepository(), 'phid'),
'branch' => '', // TODO: Tracking branch.
'commit' => $api->getBaseCommit(),
'files' => array_keys($all_paths),
));
}
$failed = null;
try {
$engine->run();
@ -331,65 +287,6 @@ EOTEXT
$results = $engine->getResults();
if ($this->getArgument('only-new')) {
$total = 0;
foreach ($results as $result) {
$total += count($result->getMessages());
}
// Don't wait for response with default value of --only-new.
$timeout = null;
if ($this->getArgument('only-new') === null || !$total) {
$timeout = 0;
}
$raw_messages = $this->resolveCall($lint_future, $timeout);
if ($raw_messages && $total) {
$old_messages = array();
$line_maps = array();
foreach ($raw_messages as $message) {
$path = $all_paths[$message['path']];
$line = $message['line'];
$code = $message['code'];
if (!isset($line_maps[$path])) {
$line_maps[$path] = $this->getChange($path)->buildLineMap();
}
$new_lines = idx($line_maps[$path], $line);
if (!$new_lines) { // Unmodified lines after last hunk.
$last_old = ($line_maps[$path] ? last_key($line_maps[$path]) : 0);
$news = array_filter($line_maps[$path]);
$last_new = ($news ? last(end($news)) : 0);
$new_lines = array($line + $last_new - $last_old);
}
$error = array($code => array(true));
foreach ($new_lines as $new) {
if (isset($old_messages[$path][$new])) {
$old_messages[$path][$new][$code][] = true;
break;
}
$old_messages[$path][$new] = &$error;
}
unset($error);
}
foreach ($results as $result) {
foreach ($result->getMessages() as $message) {
$path = str_replace(DIRECTORY_SEPARATOR, '/', $message->getPath());
$line = $message->getLine();
$code = $message->getCode();
if (!empty($old_messages[$path][$line][$code])) {
$message->setObsolete(true);
array_pop($old_messages[$path][$line][$code]);
}
}
$result->sortAndFilterMessages();
}
}
}
if ($this->getArgument('never-apply-patches')) {
$apply_patches = false;
} else {