From 4c36860c4303c45aeb96b8f8f2b1852890adc083 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 13 Feb 2020 05:34:08 -0800 Subject: [PATCH] Merge Arcanist lint changes from "experimental" branch Summary: Ref T13395. Collapse changes to the linter workflow from "experimental" into "master". Test Plan: Ran `arc lint`. Noted flag changes in changelog. Maniphest Tasks: T13395 Differential Revision: https://secure.phabricator.com/D20986 --- src/lint/engine/ArcanistLintEngine.php | 10 - .../ArcanistCheckstyleXMLLintRenderer.php | 19 +- .../renderer/ArcanistCompilerLintRenderer.php | 11 +- .../renderer/ArcanistConsoleLintRenderer.php | 58 ++- .../renderer/ArcanistJSONLintRenderer.php | 11 +- src/lint/renderer/ArcanistLintRenderer.php | 58 ++- .../renderer/ArcanistNoneLintRenderer.php | 8 +- .../renderer/ArcanistSummaryLintRenderer.php | 19 +- .../ArcanistConsoleLintRendererTestCase.php | 8 +- src/workflow/ArcanistLintWorkflow.php | 401 +++--------------- 10 files changed, 182 insertions(+), 421 deletions(-) diff --git a/src/lint/engine/ArcanistLintEngine.php b/src/lint/engine/ArcanistLintEngine.php index ee80e6c2..f737fcd4 100644 --- a/src/lint/engine/ArcanistLintEngine.php +++ b/src/lint/engine/ArcanistLintEngine.php @@ -56,7 +56,6 @@ abstract class ArcanistLintEngine extends Phobject { private $changedLines = array(); - private $enableAsyncLint = false; private $configurationManager; private $linterResources = array(); @@ -110,15 +109,6 @@ abstract class ArcanistLintEngine extends Phobject { return $this; } - final public function setEnableAsyncLint($enable_async_lint) { - $this->enableAsyncLint = $enable_async_lint; - return $this; - } - - final public function getEnableAsyncLint() { - return $this->enableAsyncLint; - } - final public function loadData($path) { if (!isset($this->fileData[$path])) { $disk_path = $this->getFilePathOnDisk($path); diff --git a/src/lint/renderer/ArcanistCheckstyleXMLLintRenderer.php b/src/lint/renderer/ArcanistCheckstyleXMLLintRenderer.php index 32091bf5..7e43ec10 100644 --- a/src/lint/renderer/ArcanistCheckstyleXMLLintRenderer.php +++ b/src/lint/renderer/ArcanistCheckstyleXMLLintRenderer.php @@ -1,10 +1,9 @@ writer->setIndentString(' '); } - public function renderPreamble() { + public function willRenderResults() { $this->writer->startDocument('1.0', 'UTF-8'); $this->writer->startElement('checkstyle'); $this->writer->writeAttribute('version', '4.3'); - return $this->writer->flush(); + $this->writeOut($this->writer->flush()); } public function renderLintResult(ArcanistLintResult $result) { @@ -39,17 +38,13 @@ final class ArcanistCheckstyleXMLLintRenderer extends ArcanistLintRenderer { } $this->writer->endElement(); - return $this->writer->flush(); + $this->writeOut($this->writer->flush()); } - public function renderOkayResult() { - return ''; - } - - public function renderPostamble() { + public function didRenderResults() { $this->writer->endElement(); $this->writer->endDocument(); - return $this->writer->flush(); + $this->writeOut($this->writer->flush()); } private function getStringForSeverity($severity) { diff --git a/src/lint/renderer/ArcanistCompilerLintRenderer.php b/src/lint/renderer/ArcanistCompilerLintRenderer.php index 264750b1..947274d4 100644 --- a/src/lint/renderer/ArcanistCompilerLintRenderer.php +++ b/src/lint/renderer/ArcanistCompilerLintRenderer.php @@ -1,10 +1,9 @@ getMessages(); @@ -25,11 +24,7 @@ final class ArcanistCompilerLintRenderer extends ArcanistLintRenderer { $description); } - return implode('', $lines); - } - - public function renderOkayResult() { - return ''; + $this->writeOut(implode('', $lines)); } } diff --git a/src/lint/renderer/ArcanistConsoleLintRenderer.php b/src/lint/renderer/ArcanistConsoleLintRenderer.php index 4862f47f..5b99e7c5 100644 --- a/src/lint/renderer/ArcanistConsoleLintRenderer.php +++ b/src/lint/renderer/ArcanistConsoleLintRenderer.php @@ -1,17 +1,10 @@ showAutofixPatches = $show_autofix_patches; - return $this; - } + private $testableMode; public function setTestableMode($testable_mode) { $this->testableMode = $testable_mode; @@ -22,6 +15,38 @@ final class ArcanistConsoleLintRenderer extends ArcanistLintRenderer { return $this->testableMode; } + public function supportsPatching() { + return true; + } + + public function renderResultCode($result_code) { + if ($result_code == ArcanistLintWorkflow::RESULT_OKAY) { + $view = new PhutilConsoleInfo( + pht('OKAY'), + pht('No lint messages.')); + $this->writeOut($view->drawConsoleString()); + } + } + + public function promptForPatch( + ArcanistLintResult $result, + $old_path, + $new_path) { + + if ($old_path === null) { + $old_path = '/dev/null'; + } + + list($err, $stdout) = exec_manual('diff -u %s %s', $old_path, $new_path); + $this->writeOut($stdout); + + $prompt = pht( + 'Apply this patch to %s?', + tsprintf('__%s__', $result->getPath())); + + return phutil_console_confirm($prompt, $default_no = false); + } + public function renderLintResult(ArcanistLintResult $result) { $messages = $result->getMessages(); $path = $result->getPath(); @@ -31,10 +56,6 @@ final class ArcanistConsoleLintRenderer extends ArcanistLintRenderer { $text = array(); foreach ($messages as $message) { - if (!$this->showAutofixPatches && $message->isAutofix()) { - continue; - } - if ($message->isError()) { $color = 'red'; } else { @@ -77,9 +98,7 @@ final class ArcanistConsoleLintRenderer extends ArcanistLintRenderer { pht( 'Lint for %s:', phutil_console_format('__%s__', $path))); - return $prefix.implode("\n", $text); - } else { - return null; + $this->writeOut($prefix.implode("\n", $text)); } } @@ -288,13 +307,6 @@ final class ArcanistConsoleLintRenderer extends ArcanistLintRenderer { $data); } - public function renderOkayResult() { - return phutil_console_format( - "** %s ** %s\n", - pht('OKAY'), - pht('No lint warnings.')); - } - private function newOffsetMap($data) { $lines = phutil_split_lines($data); diff --git a/src/lint/renderer/ArcanistJSONLintRenderer.php b/src/lint/renderer/ArcanistJSONLintRenderer.php index 04432344..034ade8d 100644 --- a/src/lint/renderer/ArcanistJSONLintRenderer.php +++ b/src/lint/renderer/ArcanistJSONLintRenderer.php @@ -1,10 +1,9 @@ writeOut(json_encode($output)."\n"); } } diff --git a/src/lint/renderer/ArcanistLintRenderer.php b/src/lint/renderer/ArcanistLintRenderer.php index f153967d..1999fa6f 100644 --- a/src/lint/renderer/ArcanistLintRenderer.php +++ b/src/lint/renderer/ArcanistLintRenderer.php @@ -1,19 +1,61 @@ getPhobjectClassConstant('RENDERERKEY'); + } + + final public static function getAllRenderers() { + return id(new PhutilClassMapQuery()) + ->setAncestorClass(__CLASS__) + ->setUniqueMethod('getRendererKey') + ->execute(); + } + + final public function setOutputPath($path) { + $this->output = $path; + return $this; + } + + + /** + * Does this renderer support applying lint patches? + * + * @return bool True if patches should be applied when using this renderer. + */ + public function supportsPatching() { + return false; + } + + public function willRenderResults() { + return null; + } + + public function didRenderResults() { + return null; + } + + public function renderResultCode($result_code) { + return null; + } + + public function handleException(Exception $ex) { + throw $ex; } abstract public function renderLintResult(ArcanistLintResult $result); - abstract public function renderOkayResult(); - public function renderPostamble() { - return ''; + protected function writeOut($message) { + if ($this->output) { + Filesystem::appendFile($this->output, $message); + } else { + echo $message; + } + + return $this; } } diff --git a/src/lint/renderer/ArcanistNoneLintRenderer.php b/src/lint/renderer/ArcanistNoneLintRenderer.php index ce74e395..c28e2cb0 100644 --- a/src/lint/renderer/ArcanistNoneLintRenderer.php +++ b/src/lint/renderer/ArcanistNoneLintRenderer.php @@ -2,12 +2,10 @@ final class ArcanistNoneLintRenderer extends ArcanistLintRenderer { - public function renderLintResult(ArcanistLintResult $result) { - return ''; - } + const RENDERERKEY = 'none'; - public function renderOkayResult() { - return ''; + public function renderLintResult(ArcanistLintResult $result) { + return null; } } diff --git a/src/lint/renderer/ArcanistSummaryLintRenderer.php b/src/lint/renderer/ArcanistSummaryLintRenderer.php index 92eed3a4..22c6aba0 100644 --- a/src/lint/renderer/ArcanistSummaryLintRenderer.php +++ b/src/lint/renderer/ArcanistSummaryLintRenderer.php @@ -1,10 +1,9 @@ getMessages(); $path = $result->getPath(); @@ -19,14 +18,16 @@ final class ArcanistSummaryLintRenderer extends ArcanistLintRenderer { $text[] = "{$path}:{$line}:{$severity}: {$name}\n"; } - return implode('', $text); + $this->writeOut(implode('', $text)); } - public function renderOkayResult() { - return phutil_console_format( - "** %s ** %s\n", - pht('OKAY'), - pht('No lint warnings.')); + public function renderResultCode($result_code) { + if ($result_code == ArcanistLintWorkflow::RESULT_OKAY) { + $view = new PhutilConsoleInfo( + pht('OKAY'), + pht('No lint messages.')); + $this->writeOut($view->drawConsoleString()); + } } } diff --git a/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php b/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php index f65e40c9..98149a58 100644 --- a/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php +++ b/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php @@ -205,7 +205,13 @@ EOTEXT; try { PhutilConsoleFormatter::disableANSI(true); - $actual = $renderer->renderLintResult($result); + + $tmp = new TempFile(); + $renderer->setOutputPath($tmp); + $renderer->renderLintResult($result); + $actual = Filesystem::readFile($tmp); + unset($tmp); + PhutilConsoleFormatter::disableANSI(false); } catch (Exception $ex) { PhutilConsoleFormatter::disableANSI(false); diff --git a/src/workflow/ArcanistLintWorkflow.php b/src/workflow/ArcanistLintWorkflow.php index d61091a8..9ab1aca2 100644 --- a/src/workflow/ArcanistLintWorkflow.php +++ b/src/workflow/ArcanistLintWorkflow.php @@ -13,7 +13,6 @@ final class ArcanistLintWorkflow extends ArcanistWorkflow { const DEFAULT_SEVERITY = ArcanistLintSeverity::SEVERITY_ADVICE; private $unresolvedMessages; - private $shouldLintAll; private $shouldAmendChanges = false; private $shouldAmendWithoutPrompt = false; private $shouldAmendAutofixesWithoutPrompt = false; @@ -61,19 +60,6 @@ EOTEXT 'help' => pht( 'Show all lint warnings, not just those on changed lines. When '. 'paths are specified, this is the default behavior.'), - 'conflicts' => array( - 'only-changed' => true, - ), - ), - '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.'), - 'conflicts' => array( - 'lintall' => true, - ), ), 'rev' => array( 'param' => 'revision', @@ -88,24 +74,13 @@ EOTEXT ), 'output' => array( 'param' => 'format', - 'help' => pht( - "With 'summary', show lint warnings in a more compact format. ". - "With 'json', show lint warnings in machine-readable JSON format. ". - "With 'none', show no lint warnings. ". - "With 'compiler', show lint warnings in suitable for your editor. ". - "With 'xml', show lint warnings in the Checkstyle XML format."), + 'help' => pht('Select an output format.'), ), 'outfile' => array( 'param' => 'path', '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.'), @@ -139,7 +114,6 @@ EOTEXT 'Lint all tracked files in the working copy. Ignored files and '. 'untracked files will not be linted.'), 'conflicts' => array( - 'cache' => pht('%s lints all files', '--everything'), 'rev' => pht('%s lints all files', '--everything'), ), ), @@ -154,22 +128,12 @@ EOTEXT array_keys(ArcanistLintSeverity::getLintSeverities()))), self::DEFAULT_SEVERITY), ), - 'cache' => array( - 'param' => 'bool', - 'help' => pht( - "%d to disable cache, %d to enable. The default value is determined ". - "by '%s' in configuration, which defaults to off. See notes in '%s'.", - 0, - 1, - 'arc.lint.cache', - 'arc.lint.cache'), - ), '*' => 'paths', ); } public function requiresAuthentication() { - return (bool)$this->getArgument('only-new'); + return false; } public function requiresWorkingCopy() { @@ -180,14 +144,6 @@ EOTEXT return true; } - private function getCacheKey() { - return implode("\n", array( - get_class($this->engine), - $this->getArgument('severity', self::DEFAULT_SEVERITY), - $this->shouldLintAll, - )); - } - public function run() { $console = PhutilConsole::getConsole(); $working_copy = $this->getWorkingCopy(); @@ -197,7 +153,6 @@ EOTEXT $rev = $this->getArgument('rev'); $paths = $this->getArgument('paths'); - $use_cache = $this->getArgument('cache', null); $everything = $this->getArgument('everything'); if ($everything && $paths) { throw new ArcanistUsageException( @@ -207,31 +162,35 @@ EOTEXT '--everything', '--everything')); } - if ($use_cache === null) { - $use_cache = (bool)$configuration_manager->getConfigFromAnySource( - 'arc.lint.cache', - false); + + if ($rev !== null) { + $this->parseBaseCommitArgument(array($rev)); } - if ($rev && $paths) { - throw new ArcanistUsageException( - pht('Specify either %s or paths.', '--rev')); - } + // Sometimes, we hide low-severity messages which occur on lines which + // were not changed. This is the default behavior when you run "arc lint" + // with no arguments: if you touched a file, but there was already some + // minor warning about whitespace or spelling elsewhere in the file, you + // don't need to correct it. + // In other modes, notably "arc lint ", this is not the defualt + // behavior. If you ask us to lint a specific file, we show you all the + // lint messages in the file. - // NOTE: When the user specifies paths, we imply --lintall and show all - // warnings for the paths in question. This is easier to deal with for - // us and less confusing for users. - $this->shouldLintAll = $paths ? true : false; + // You can change this behavior with various flags, including "--lintall", + // "--rev", and "--everything". if ($this->getArgument('lintall')) { - $this->shouldLintAll = true; - } else if ($this->getArgument('only-changed')) { - $this->shouldLintAll = false; + $lint_all = true; + } else if ($rev !== null) { + $lint_all = false; + } else if ($paths || $everything) { + $lint_all = true; + } else { + $lint_all = false; } if ($everything) { $paths = iterator_to_array($this->getRepositoryAPI()->getAllFiles()); - $this->shouldLintAll = true; } else { $paths = $this->selectPathsForWorkflow($paths, $rev); } @@ -241,45 +200,11 @@ EOTEXT $engine->setMinimumSeverity( $this->getArgument('severity', self::DEFAULT_SEVERITY)); - $file_hashes = array(); - if ($use_cache) { - $engine->setRepositoryVersion($this->getRepositoryVersion()); - $cache = $this->readScratchJSONFile('lint-cache.json'); - $cache = idx($cache, $this->getCacheKey(), array()); - $cached = array(); - - foreach ($paths as $path) { - $abs_path = $engine->getFilePathOnDisk($path); - if (!Filesystem::pathExists($abs_path)) { - continue; - } - $file_hashes[$abs_path] = md5_file($abs_path); - - if (!isset($cache[$path])) { - continue; - } - $messages = idx($cache[$path], $file_hashes[$abs_path]); - if ($messages !== null) { - $cached[$path] = $messages; - } - } - - if ($cached) { - $console->writeErr( - "%s\n", - pht( - "Using lint cache, use '%s' to disable it.", - '--cache 0')); - } - - $engine->setCachedResults($cached); - } - // Propagate information about which lines changed to the lint engine. // This is used so that the lint engine can drop warning messages // concerning lines that weren't in the change. $engine->setPaths($paths); - if (!$this->shouldLintAll) { + if (!$lint_all) { foreach ($paths as $path) { // Note that getChangedLines() returns null to indicate that a file // is binary or a directory (i.e., changed lines are not relevant). @@ -289,49 +214,6 @@ EOTEXT } } - // Enable possible async linting only for 'arc diff' not 'arc lint' - if ($this->getParentWorkflow()) { - $engine->setEnableAsyncLint(true); - } else { - $engine->setEnableAsyncLint(false); - } - - 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(); @@ -341,65 +223,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 { @@ -418,11 +241,8 @@ EOTEXT } if ($this->getArgument('amend-autofixes')) { - $prompt_autofix_patches = false; $this->shouldAmendChanges = true; $this->shouldAmendAutofixesWithoutPrompt = true; - } else { - $prompt_autofix_patches = true; } $repository_api = $this->getRepositoryAPI(); @@ -433,111 +253,76 @@ EOTEXT $wrote_to_disk = false; - switch ($this->getArgument('output')) { - case 'json': - $renderer = new ArcanistJSONLintRenderer(); - $prompt_patches = false; - $apply_patches = $this->getArgument('apply-patches'); - break; - case 'summary': - $renderer = new ArcanistSummaryLintRenderer(); - break; - case 'none': - $prompt_patches = false; - $apply_patches = $this->getArgument('apply-patches'); - $renderer = new ArcanistNoneLintRenderer(); - break; - case 'compiler': - $renderer = new ArcanistCompilerLintRenderer(); - $prompt_patches = false; - $apply_patches = $this->getArgument('apply-patches'); - break; - case 'xml': - $renderer = new ArcanistCheckstyleXMLLintRenderer(); - $prompt_patches = false; - $apply_patches = $this->getArgument('apply-patches'); - break; - default: - $renderer = new ArcanistConsoleLintRenderer(); - $renderer->setShowAutofixPatches($prompt_autofix_patches); - break; + $default_renderer = ArcanistConsoleLintRenderer::RENDERERKEY; + $renderer_key = $this->getArgument('output', $default_renderer); + + $renderers = ArcanistLintRenderer::getAllRenderers(); + if (!isset($renderers[$renderer_key])) { + throw new Exception( + pht( + 'Lint renderer "%s" is unknown. Supported renderers are: %s.', + $renderer_key, + implode(', ', array_keys($renderers)))); } + $renderer = $renderers[$renderer_key]; $all_autofix = true; - $tmp = null; - if ($this->getArgument('outfile') !== null) { - $tmp = id(new TempFile()) - ->setPreserveFile(true); - } - - $preamble = $renderer->renderPreamble(); - if ($tmp) { - Filesystem::appendFile($tmp, $preamble); + $out_path = $this->getArgument('outfile'); + if ($out_path !== null) { + $tmp = new TempFile(); + $renderer->setOutputPath((string)$tmp); } else { - $console->writeOut('%s', $preamble); + $tmp = null; } - foreach ($results as $result) { - $result_all_autofix = $result->isAllAutofix(); + if ($failed) { + $renderer->handleException($failed); + } - if (!$result->getMessages() && !$result_all_autofix) { + $renderer->willRenderResults(); + + $should_patch = ($apply_patches && $renderer->supportsPatching()); + foreach ($results as $result) { + if (!$result->getMessages()) { continue; } + $result_all_autofix = $result->isAllAutofix(); if (!$result_all_autofix) { $all_autofix = false; } - $lint_result = $renderer->renderLintResult($result); - if ($lint_result) { - if ($tmp) { - Filesystem::appendFile($tmp, $lint_result); - } else { - $console->writeOut('%s', $lint_result); - } - } + $renderer->renderLintResult($result); - if ($apply_patches && $result->isPatchable()) { + if ($should_patch && $result->isPatchable()) { $patcher = ArcanistLintPatcher::newFromArcanistLintResult($result); - $old_file = $result->getFilePathOnDisk(); - if ($prompt_patches && - !($result_all_autofix && !$prompt_autofix_patches)) { + $apply = true; + if ($prompt_patches && !$result_all_autofix) { + $old_file = $result->getFilePathOnDisk(); if (!Filesystem::pathExists($old_file)) { - $old_file = '/dev/null'; + $old_file = null; } + $new_file = new TempFile(); $new = $patcher->getModifiedFileContent(); Filesystem::writeFile($new_file, $new); - // TODO: Improve the behavior here, make it more like - // difference_render(). - list(, $stdout, $stderr) = - exec_manual('diff -u %s %s', $old_file, $new_file); - $console->writeOut('%s', $stdout); - $console->writeErr('%s', $stderr); - - $prompt = pht( - 'Apply this patch to %s?', - phutil_console_format('__%s__', $result->getPath())); - if (!phutil_console_confirm($prompt, $default_no = false)) { - continue; - } + $apply = $renderer->promptForPatch($result, $old_file, $new_file); } - $patcher->writePatchToDisk(); - $wrote_to_disk = true; - $file_hashes[$old_file] = md5_file($old_file); + if ($apply) { + $patcher->writePatchToDisk(); + $wrote_to_disk = true; + } } } - $postamble = $renderer->renderPostamble(); + $renderer->didRenderResults(); + if ($tmp) { - Filesystem::appendFile($tmp, $postamble); - Filesystem::rename($tmp, $this->getArgument('outfile')); - } else { - $console->writeOut('%s', $postamble); + Filesystem::rename($tmp, $out_path); } if ($wrote_to_disk && $this->shouldAmendChanges) { @@ -568,20 +353,6 @@ EOTEXT } } - if ($this->getArgument('output') == 'json') { - // NOTE: Required by save_lint.php in Phabricator. - return 0; - } - - if ($failed) { - if ($failed instanceof ArcanistNoEffectException) { - if ($renderer instanceof ArcanistNoneLintRenderer) { - return 0; - } - } - throw $failed; - } - $unresolved = array(); $has_warnings = false; $has_errors = false; @@ -600,46 +371,6 @@ EOTEXT } $this->unresolvedMessages = $unresolved; - $cache = $this->readScratchJSONFile('lint-cache.json'); - $cached = idx($cache, $this->getCacheKey(), array()); - if ($cached || $use_cache) { - $stopped = $engine->getStoppedPaths(); - foreach ($results as $result) { - $path = $result->getPath(); - if (!$use_cache) { - unset($cached[$path]); - continue; - } - $abs_path = $engine->getFilePathOnDisk($path); - if (!Filesystem::pathExists($abs_path)) { - continue; - } - $version = $result->getCacheVersion(); - $cached_path = array(); - if (isset($stopped[$path])) { - $cached_path['stopped'] = $stopped[$path]; - } - $cached_path['repository_version'] = $this->getRepositoryVersion(); - foreach ($result->getMessages() as $message) { - $granularity = $message->getGranularity(); - if ($granularity == ArcanistLinter::GRANULARITY_GLOBAL) { - continue; - } - if (!$message->isPatchApplied()) { - $cached_path[] = $message->toDictionary(); - } - } - $hash = idx($file_hashes, $abs_path); - if (!$hash) { - $hash = md5_file($abs_path); - } - $cached[$path] = array($hash => array($version => $cached_path)); - } - $cache[$this->getCacheKey()] = $cached; - // TODO: Garbage collection. - $this->writeScratchJSONFile('lint-cache.json', $cache); - } - // Take the most severe lint message severity and use that // as the result code. if ($has_errors) { @@ -650,11 +381,7 @@ EOTEXT $result_code = self::RESULT_OKAY; } - if (!$this->getParentWorkflow()) { - if ($result_code == self::RESULT_OKAY) { - $console->writeOut('%s', $renderer->renderOkayResult()); - } - } + $renderer->renderResultCode($result_code); return $result_code; }