diff --git a/src/lint/engine/ArcanistLintEngine.php b/src/lint/engine/ArcanistLintEngine.php
--- a/src/lint/engine/ArcanistLintEngine.php
+++ b/src/lint/engine/ArcanistLintEngine.php
@@ -56,7 +56,6 @@
private $changedLines = array();
- private $enableAsyncLint = false;
private $configurationManager;
private $linterResources = array();
@@ -110,15 +109,6 @@
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
--- a/src/lint/renderer/ArcanistCheckstyleXMLLintRenderer.php
+++ b/src/lint/renderer/ArcanistCheckstyleXMLLintRenderer.php
@@ -1,10 +1,9 @@
<?php
-/**
- * Shows lint messages to the user.
- */
final class ArcanistCheckstyleXMLLintRenderer extends ArcanistLintRenderer {
+ const RENDERERKEY = 'xml';
+
private $writer;
public function __construct() {
@@ -14,11 +13,11 @@
$this->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 @@
}
$this->writer->endElement();
- return $this->writer->flush();
- }
-
- public function renderOkayResult() {
- return '';
+ $this->writeOut($this->writer->flush());
}
- 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
--- a/src/lint/renderer/ArcanistCompilerLintRenderer.php
+++ b/src/lint/renderer/ArcanistCompilerLintRenderer.php
@@ -1,10 +1,9 @@
<?php
-/**
- * Shows lint messages to the user.
- */
final class ArcanistCompilerLintRenderer extends ArcanistLintRenderer {
+ const RENDERERKEY = 'compiler';
+
public function renderLintResult(ArcanistLintResult $result) {
$lines = array();
$messages = $result->getMessages();
@@ -25,11 +24,7 @@
$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
--- a/src/lint/renderer/ArcanistConsoleLintRenderer.php
+++ b/src/lint/renderer/ArcanistConsoleLintRenderer.php
@@ -1,17 +1,10 @@
<?php
-/**
- * Shows lint messages to the user.
- */
final class ArcanistConsoleLintRenderer extends ArcanistLintRenderer {
- private $showAutofixPatches = false;
- private $testableMode;
+ const RENDERERKEY = 'console';
- public function setShowAutofixPatches($show_autofix_patches) {
- $this->showAutofixPatches = $show_autofix_patches;
- return $this;
- }
+ private $testableMode;
public function setTestableMode($testable_mode) {
$this->testableMode = $testable_mode;
@@ -22,6 +15,38 @@
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 @@
$text = array();
foreach ($messages as $message) {
- if (!$this->showAutofixPatches && $message->isAutofix()) {
- continue;
- }
-
if ($message->isError()) {
$color = 'red';
} else {
@@ -77,9 +98,7 @@
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 @@
$data);
}
- public function renderOkayResult() {
- return phutil_console_format(
- "<bg:green>** %s **</bg> %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
--- a/src/lint/renderer/ArcanistJSONLintRenderer.php
+++ b/src/lint/renderer/ArcanistJSONLintRenderer.php
@@ -1,10 +1,9 @@
<?php
-/**
- * Shows lint messages to the user.
- */
final class ArcanistJSONLintRenderer extends ArcanistLintRenderer {
+ const RENDERERKEY = 'json';
+
const LINES_OF_CONTEXT = 3;
public function renderLintResult(ArcanistLintResult $result) {
@@ -25,11 +24,7 @@
$output[$path][] = $dictionary;
}
- return json_encode($output)."\n";
- }
-
- public function renderOkayResult() {
- return '';
+ $this->writeOut(json_encode($output)."\n");
}
}
diff --git a/src/lint/renderer/ArcanistLintRenderer.php b/src/lint/renderer/ArcanistLintRenderer.php
--- a/src/lint/renderer/ArcanistLintRenderer.php
+++ b/src/lint/renderer/ArcanistLintRenderer.php
@@ -1,19 +1,61 @@
<?php
-/**
- * Shows lint messages to the user.
- */
abstract class ArcanistLintRenderer extends Phobject {
- public function renderPreamble() {
- return '';
+ private $output;
+
+ final public function getRendererKey() {
+ return $this->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
--- 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
--- a/src/lint/renderer/ArcanistSummaryLintRenderer.php
+++ b/src/lint/renderer/ArcanistSummaryLintRenderer.php
@@ -1,10 +1,9 @@
<?php
-/**
- * Shows lint messages to the user.
- */
final class ArcanistSummaryLintRenderer extends ArcanistLintRenderer {
+ const RENDERERKEY = 'summary';
+
public function renderLintResult(ArcanistLintResult $result) {
$messages = $result->getMessages();
$path = $result->getPath();
@@ -19,14 +18,16 @@
$text[] = "{$path}:{$line}:{$severity}: {$name}\n";
}
- return implode('', $text);
+ $this->writeOut(implode('', $text));
}
- public function renderOkayResult() {
- return phutil_console_format(
- "<bg:green>** %s **</bg> %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
--- a/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php
+++ b/src/lint/renderer/__tests__/ArcanistConsoleLintRendererTestCase.php
@@ -205,7 +205,13 @@
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
--- a/src/workflow/ArcanistLintWorkflow.php
+++ b/src/workflow/ArcanistLintWorkflow.php
@@ -13,7 +13,6 @@
const DEFAULT_SEVERITY = ArcanistLintSeverity::SEVERITY_ADVICE;
private $unresolvedMessages;
- private $shouldLintAll;
private $shouldAmendChanges = false;
private $shouldAmendWithoutPrompt = false;
private $shouldAmendAutofixesWithoutPrompt = false;
@@ -61,19 +60,6 @@
'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 @@
),
'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 @@
'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 @@
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 @@
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 @@
$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 @@
'--everything',
'--everything'));
}
- if ($use_cache === null) {
- $use_cache = (bool)$configuration_manager->getConfigFromAnySource(
- 'arc.lint.cache',
- false);
- }
- if ($rev && $paths) {
- throw new ArcanistUsageException(
- pht('Specify either %s or paths.', '--rev'));
+ if ($rev !== null) {
+ $this->parseBaseCommitArgument(array($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.
- // 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;
+ // In other modes, notably "arc lint <file>", 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.
+
+ // 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 @@
$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 @@
}
}
- // 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 @@
$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 @@
}
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 @@
$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);
+ $out_path = $this->getArgument('outfile');
+ if ($out_path !== null) {
+ $tmp = new TempFile();
+ $renderer->setOutputPath((string)$tmp);
+ } else {
+ $tmp = null;
}
- $preamble = $renderer->renderPreamble();
- if ($tmp) {
- Filesystem::appendFile($tmp, $preamble);
- } else {
- $console->writeOut('%s', $preamble);
+ if ($failed) {
+ $renderer->handleException($failed);
}
- foreach ($results as $result) {
- $result_all_autofix = $result->isAllAutofix();
+ $renderer->willRenderResults();
- if (!$result->getMessages() && !$result_all_autofix) {
+ $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 @@
}
}
- 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 @@
}
$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 @@
$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;
}