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

@@ -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);

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 @@ final class ArcanistCheckstyleXMLLintRenderer extends ArcanistLintRenderer {
     $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 @@ 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) {

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 @@ final class ArcanistCompilerLintRenderer extends ArcanistLintRenderer {
         $description);
     }
 
-    return implode('', $lines);
-  }
-
-  public function renderOkayResult() {
-    return '';
+    $this->writeOut(implode('', $lines));
   }
 
 }

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 @@ 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(
-      "<bg:green>** %s **</bg> %s\n",
-      pht('OKAY'),
-      pht('No lint warnings.'));
-  }
-
   private function newOffsetMap($data) {
     $lines = phutil_split_lines($data);
 

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 @@ final class ArcanistJSONLintRenderer extends ArcanistLintRenderer {
       $output[$path][] = $dictionary;
     }
 
-    return json_encode($output)."\n";
-  }
-
-  public function renderOkayResult() {
-    return '';
+    $this->writeOut(json_encode($output)."\n");
   }
 
 }

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;
   }
 
 }

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;
   }
 
 }

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 @@ 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(
-      "<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());
+    }
   }
 
 }

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);

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 <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.
 
-    // 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;
   }