diff --git a/src/lint/ArcanistLintResult.php b/src/lint/ArcanistLintResult.php index dcb0968a..0e2dc18e 100644 --- a/src/lint/ArcanistLintResult.php +++ b/src/lint/ArcanistLintResult.php @@ -82,7 +82,7 @@ final class ArcanistLintResult { return true; } - private function sortAndFilterMessages() { + public function sortAndFilterMessages() { $messages = $this->messages; foreach ($messages as $key => $message) { diff --git a/src/parser/diff/ArcanistDiffChange.php b/src/parser/diff/ArcanistDiffChange.php index bd9799b9..48d380f1 100644 --- a/src/parser/diff/ArcanistDiffChange.php +++ b/src/parser/diff/ArcanistDiffChange.php @@ -190,6 +190,42 @@ final class ArcanistDiffChange { return $this->hunks; } + /** + * @return array $old => array($new, ) + */ + public function buildLineMap() { + $line_map = array(); + $old = 1; + $new = 1; + foreach ($this->getHunks() as $hunk) { + for ($n = $old; $n < $hunk->getOldOffset(); $n++) { + $line_map[$n] = array($n + $new - $old); + } + $old = $hunk->getOldOffset(); + $new = $hunk->getNewOffset(); + $olds = array(); + $news = array(); + $lines = explode("\n", $hunk->getCorpus()); + foreach ($lines as $line) { + $type = substr($line, 0, 1); + if ($type == '-' || $type == ' ') { + $olds[] = $old; + $old++; + } + if ($type == '+' || $type == ' ') { + $news[] = $new; + $new++; + } + if ($type == ' ' || $type == '') { + $line_map += array_fill_keys($olds, $news); + $olds = array(); + $news = array(); + } + } + } + return $line_map; + } + public function convertToBinaryChange() { $this->hunks = array(); $this->setFileType(ArcanistDiffChangeType::FILE_BINARY); diff --git a/src/workflow/ArcanistBaseWorkflow.php b/src/workflow/ArcanistBaseWorkflow.php index f53936c8..cab6f04d 100644 --- a/src/workflow/ArcanistBaseWorkflow.php +++ b/src/workflow/ArcanistBaseWorkflow.php @@ -985,7 +985,7 @@ abstract class ArcanistBaseWorkflow { return array_keys($lines); } - private function getChange($path) { + protected function getChange($path) { $repository_api = $this->getRepositoryAPI(); if ($repository_api instanceof ArcanistSubversionAPI) { diff --git a/src/workflow/ArcanistDiffWorkflow.php b/src/workflow/ArcanistDiffWorkflow.php index 10addb72..b42d27d0 100644 --- a/src/workflow/ArcanistDiffWorkflow.php +++ b/src/workflow/ArcanistDiffWorkflow.php @@ -240,6 +240,14 @@ EOTEXT "Require excuse for lint advice in addition to lint warnings and ". "errors.", ), + 'only-new' => array( + 'param' => 'bool', + 'help' => + 'Display only lint messages not present in the original code.', + 'passthru' => array( + 'lint' => true, + ), + ), 'apply-patches' => array( 'help' => 'Apply patches suggested by lint to the working copy without '. diff --git a/src/workflow/ArcanistLintWorkflow.php b/src/workflow/ArcanistLintWorkflow.php index fce3eccb..2ad678ab 100644 --- a/src/workflow/ArcanistLintWorkflow.php +++ b/src/workflow/ArcanistLintWorkflow.php @@ -83,6 +83,11 @@ EOTEXT "With 'json', show lint warnings in machine-readable JSON format. ". "With 'compiler', show lint warnings in suitable for your editor." ), + 'only-new' => array( + 'param' => 'bool', + 'supports' => array('git', 'hg'), // TODO: svn + 'help' => 'Display only messages not present in the original code.', + ), 'engine' => array( 'param' => 'classname', 'help' => @@ -129,6 +134,10 @@ EOTEXT ); } + public function requiresAuthentication() { + return (bool)$this->getArgument('only-new'); + } + public function requiresWorkingCopy() { return true; } @@ -228,6 +237,41 @@ EOTEXT $engine->setEnableAsyncLint(false); } + if ($this->getArgument('only-new')) { + $conduit = $this->getConduit(); + $api = $this->getRepositoryAPI(); + $relative_commit = ($rev ? $rev : $api->resolveBaseCommit()); + $api->setRelativeCommit($relative_commit); + $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( + 'arcanistProject' => $this->getWorkingCopy()->getProjectID(), + 'branch' => '', // TODO: Tracking branch. + 'commit' => $api->getRelativeCommit(), + 'files' => array_keys($all_paths), + )); + } + $failed = null; try { $engine->run(); @@ -237,6 +281,65 @@ 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 = $lint_future->resolve($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(); + } + } + } + // 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