Page MenuHomePhabricator
Files F14093214

D18645.diff
No OneTemporary
Actions

D18645.diff
View Options

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));
}
}
@@ -278,13 +297,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
@@ -171,7 +171,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
@@ -74,12 +74,7 @@
),
'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',
@@ -243,11 +238,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();
@@ -258,110 +250,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;
+ 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) {
@@ -391,20 +349,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;
@@ -433,11 +377,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;
}

File Metadata

Mime Type
text/plain
Expires
Tue, Nov 26, 10:03 AM (12 h, 19 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6788986
Default Alt Text
D18645.diff (16 KB)

Event Timeline

Phabricator · Built by Phacility