From 14d49d25658f15e8e6ac94bbb7533565135175dc Mon Sep 17 00:00:00 2001 From: Ben Gertzfield Date: Fri, 6 Apr 2012 12:23:19 -0700 Subject: [PATCH] Add ArcanistLintSeverity::SEVERITY_AUTOFIX. Summary: Xcode (a popular code editor on Mac OS X) has no facility to trim trailing whitespace automatically. This adds a new lint severity "AUTOFIX" that's between WARNING and ERROR. When running the linter, any lint message whose severity is AUTOFIX will automatically be patched. Furthermore, if all lint messages returned from the engine are AUTOFIX, we'll automatically amend HEAD with the patch. Test Plan: arc lint on files with and without trailing whitespace, with and without UTF-8 contents to confirm those still error Reviewers: epriestley, jungejason Reviewed By: epriestley CC: aran Differential Revision: https://secure.phabricator.com/D2125 --- src/lint/linter/text/ArcanistTextLinter.php | 1 + src/lint/message/ArcanistLintMessage.php | 4 ++ src/lint/renderer/ArcanistLintRenderer.php | 23 +++++-- src/lint/result/ArcanistLintResult.php | 9 +++ src/lint/severity/ArcanistLintSeverity.php | 3 + src/workflow/diff/ArcanistDiffWorkflow.php | 16 +++++ src/workflow/lint/ArcanistLintWorkflow.php | 68 +++++++++++++++++++-- 7 files changed, 114 insertions(+), 10 deletions(-) diff --git a/src/lint/linter/text/ArcanistTextLinter.php b/src/lint/linter/text/ArcanistTextLinter.php index 8326af26..882a14be 100644 --- a/src/lint/linter/text/ArcanistTextLinter.php +++ b/src/lint/linter/text/ArcanistTextLinter.php @@ -49,6 +49,7 @@ final class ArcanistTextLinter extends ArcanistLinter { public function getLintSeverityMap() { return array( self::LINT_LINE_WRAP => ArcanistLintSeverity::SEVERITY_WARNING, + self::LINT_TRAILING_WHITESPACE => ArcanistLintSeverity::SEVERITY_AUTOFIX, ); } diff --git a/src/lint/message/ArcanistLintMessage.php b/src/lint/message/ArcanistLintMessage.php index bd25f101..4b997e09 100644 --- a/src/lint/message/ArcanistLintMessage.php +++ b/src/lint/message/ArcanistLintMessage.php @@ -144,6 +144,10 @@ final class ArcanistLintMessage { return $this->getSeverity() == ArcanistLintSeverity::SEVERITY_WARNING; } + public function isAutofix() { + return $this->getSeverity() == ArcanistLintSeverity::SEVERITY_AUTOFIX; + } + public function hasFileContext() { return ($this->getLine() !== null); } diff --git a/src/lint/renderer/ArcanistLintRenderer.php b/src/lint/renderer/ArcanistLintRenderer.php index baa1e116..c4a7d702 100644 --- a/src/lint/renderer/ArcanistLintRenderer.php +++ b/src/lint/renderer/ArcanistLintRenderer.php @@ -22,6 +22,13 @@ * @group lint */ final class ArcanistLintRenderer { + private $showAutofixPatches = false; + + public function setShowAutofixPatches($show_autofix_patches) { + $this->showAutofixPatches = $show_autofix_patches; + return $this; + } + public function renderLintResult(ArcanistLintResult $result) { $messages = $result->getMessages(); $path = $result->getPath(); @@ -29,9 +36,12 @@ final class ArcanistLintRenderer { $lines = explode("\n", $result->getData()); $text = array(); - $text[] = phutil_console_format('**>>>** Lint for __%s__:', $path); - $text[] = null; + foreach ($messages as $message) { + if (!$this->showAutofixPatches && $message->isAutofix()) { + continue; + } + if ($message->isError()) { $color = 'red'; } else { @@ -55,10 +65,13 @@ final class ArcanistLintRenderer { $text[] = $this->renderContext($message, $lines); } } - $text[] = null; - $text[] = null; - return implode("\n", $text); + if ($text) { + $prefix = phutil_console_format("**>>>** Lint for __%s__:\n\n\n", $path); + return $prefix . implode("\n", $text); + } else { + return null; + } } protected function renderContext( diff --git a/src/lint/result/ArcanistLintResult.php b/src/lint/result/ArcanistLintResult.php index 61e0e2b9..5f40ffa4 100644 --- a/src/lint/result/ArcanistLintResult.php +++ b/src/lint/result/ArcanistLintResult.php @@ -79,6 +79,15 @@ final class ArcanistLintResult { return false; } + public function isAllAutofix() { + foreach ($this->messages as $message) { + if (!$message->isAutofix()) { + return false; + } + } + return true; + } + private function sortAndFilterMessages() { $messages = $this->messages; diff --git a/src/lint/severity/ArcanistLintSeverity.php b/src/lint/severity/ArcanistLintSeverity.php index f4d3191b..83ba9c5b 100644 --- a/src/lint/severity/ArcanistLintSeverity.php +++ b/src/lint/severity/ArcanistLintSeverity.php @@ -24,6 +24,7 @@ final class ArcanistLintSeverity { const SEVERITY_ADVICE = 'advice'; + const SEVERITY_AUTOFIX = 'autofix'; const SEVERITY_WARNING = 'warning'; const SEVERITY_ERROR = 'error'; const SEVERITY_DISABLED = 'disabled'; @@ -31,6 +32,7 @@ final class ArcanistLintSeverity { public static function getStringForSeverity($severity_code) { static $map = array( self::SEVERITY_ADVICE => 'Advice', + self::SEVERITY_AUTOFIX => 'Auto-Fix', self::SEVERITY_WARNING => 'Warning', self::SEVERITY_ERROR => 'Error', self::SEVERITY_DISABLED => 'Disabled', @@ -50,6 +52,7 @@ final class ArcanistLintSeverity { static $map = array( self::SEVERITY_DISABLED => 10, self::SEVERITY_ADVICE => 20, + self::SEVERITY_AUTOFIX => 25, self::SEVERITY_WARNING => 30, self::SEVERITY_ERROR => 40, ); diff --git a/src/workflow/diff/ArcanistDiffWorkflow.php b/src/workflow/diff/ArcanistDiffWorkflow.php index 67dddc60..89de96fd 100644 --- a/src/workflow/diff/ArcanistDiffWorkflow.php +++ b/src/workflow/diff/ArcanistDiffWorkflow.php @@ -276,6 +276,22 @@ EOTEXT 'lint' => true, ), ), + 'amend-all' => array( + 'help' => + 'When linting git repositories, amend HEAD with all patches '. + 'suggested by lint without prompting.', + 'passthru' => array( + 'lint' => true, + ), + ), + 'amend-autofixes' => array( + 'help' => + 'When linting git repositories, amend HEAD with autofix '. + 'patches suggested by lint without prompting.', + 'passthru' => array( + 'lint' => true, + ), + ), 'json' => array( 'help' => 'Emit machine-readable JSON. EXPERIMENTAL! Probably does not work!', diff --git a/src/workflow/lint/ArcanistLintWorkflow.php b/src/workflow/lint/ArcanistLintWorkflow.php index b53c87ba..2ebc656d 100644 --- a/src/workflow/lint/ArcanistLintWorkflow.php +++ b/src/workflow/lint/ArcanistLintWorkflow.php @@ -30,12 +30,24 @@ class ArcanistLintWorkflow extends ArcanistBaseWorkflow { private $unresolvedMessages; private $shouldAmendChanges = false; + private $shouldAmendWithoutPrompt = false; + private $shouldAmendAutofixesWithoutPrompt = false; public function setShouldAmendChanges($should_amend) { $this->shouldAmendChanges = $should_amend; return $this; } + public function setShouldAmendWithoutPrompt($should_amend) { + $this->shouldAmendWithoutPrompt = $should_amend; + return $this; + } + + public function setShouldAmendAutofixesWithoutPrompt($should_amend) { + $this->shouldAmendAutofixesWithoutPrompt = $should_amend; + return $this; + } + public function getCommandSynopses() { return phutil_console_format(<< true, ), ), + 'amend-all' => array( + 'help' => + 'When linting git repositories, amend HEAD with all patches '. + 'suggested by lint without prompting.', + ), + 'amend-autofixes' => array( + 'help' => + 'When linting git repositories, amend HEAD with autofix '. + 'patches suggested by lint without prompting.', + ), '*' => 'paths', ); } @@ -155,7 +177,7 @@ EOTEXT if ($this->getArgument('advice')) { $engine->setMinimumSeverity(ArcanistLintSeverity::SEVERITY_ADVICE); } else { - $engine->setMinimumSeverity(ArcanistLintSeverity::SEVERITY_WARNING); + $engine->setMinimumSeverity(ArcanistLintSeverity::SEVERITY_AUTOFIX); } // Propagate information about which lines changed to the lint engine. @@ -186,6 +208,19 @@ EOTEXT $prompt_patches = true; } + if ($this->getArgument('amend-all')) { + $this->shouldAmendChanges = true; + $this->shouldAmendWithoutPrompt = true; + } + + if ($this->getArgument('amend-autofixes')) { + $prompt_autofix_patches = false; + $this->shouldAmendChanges = true; + $this->shouldAmendAutofixesWithoutPrompt = true; + } else { + $prompt_autofix_patches = true; + } + $wrote_to_disk = false; switch ($this->getArgument('output')) { @@ -204,22 +239,35 @@ EOTEXT break; default: $renderer = new ArcanistLintRenderer(); + $renderer->setShowAutofixPatches($prompt_autofix_patches); break; } + $all_autofix = true; + foreach ($results as $result) { - if (!$result->getMessages()) { + $result_all_autofix = $result->isAllAutofix(); + + if (!$result->getMessages() && !$result_all_autofix) { continue; } - echo $renderer->renderLintResult($result); + if (!$result_all_autofix) { + $all_autofix = false; + } + + $lint_result = $renderer->renderLintResult($result); + if ($lint_result) { + echo $lint_result; + } if ($apply_patches && $result->isPatchable()) { $patcher = ArcanistLintPatcher::newFromArcanistLintResult($result); $old = $patcher->getUnmodifiedFileContent(); $new = $patcher->getModifiedFileContent(); - if ($prompt_patches) { + if ($prompt_patches && + !($result_all_autofix && !$prompt_autofix_patches)) { $old_file = $result->getFilePathOnDisk(); if (!Filesystem::pathExists($old_file)) { $old_file = '/dev/null'; @@ -248,7 +296,17 @@ EOTEXT if ($wrote_to_disk && ($repository_api instanceof ArcanistGitAPI) && $this->shouldAmendChanges) { - $amend = phutil_console_confirm("Amend HEAD with lint patches?"); + + if ($this->shouldAmendWithoutPrompt || + ($this->shouldAmendAutofixesWithoutPrompt && $all_autofix)) { + echo phutil_console_format( + "** LINT NOTICE ** Automatically amending HEAD ". + "with lint patches.\n"); + $amend = true; + } else { + $amend = phutil_console_confirm("Amend HEAD with lint patches?"); + } + if ($amend) { execx( '(cd %s; git commit -a --amend -C HEAD)',