Set cache version per linter

Summary: Also delete cache with `--cache 0`.

Test Plan:
  $ arc lint --lintall --cache 1
  $ cat .git/arc/lint-cache.json
  $ arc lint --lintall --cache 1 # with debug output

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2036

Differential Revision: https://secure.phabricator.com/D4013

src/lint/ArcanistLintResult.php

@@ -10,6 +10,7 @@ final class ArcanistLintResult {
   protected $path;
   protected $data;
   protected $filePathOnDisk;
+  protected $cacheVersion;
   protected $messages = array();
   protected $effectiveMessages = array();
   private $needsSort;
@@ -54,6 +55,15 @@ final class ArcanistLintResult {
     return $this->filePathOnDisk;
   }
 
+  public function setCacheVersion($version) {
+    $this->cacheVersion = $version;
+    return $this;
+  }
+
+  public function getCacheVersion() {
+    return $this->cacheVersion;
+  }
+
   public function isPatchable() {
     foreach ($this->messages as $message) {
       if ($message->isPatchable()) {

src/lint/engine/ArcanistLintEngine.php

@@ -50,6 +50,8 @@ abstract class ArcanistLintEngine {
 
   protected $charToLine = array();
   protected $lineToFirstChar = array();
+  private $cachedResults;
+  private $cacheVersion;
   private $results = array();
   private $minimumSeverity = ArcanistLintSeverity::SEVERITY_DISABLED;
 
@@ -165,6 +167,28 @@ abstract class ArcanistLintEngine {
     $stopped = array();
     $linters = $this->buildLinters();
 
+    if (!$linters) {
+      throw new ArcanistNoEffectException("No linters to run.");
+    }
+
+    $have_paths = false;
+    foreach ($linters as $linter) {
+      if ($linter->getPaths()) {
+        $have_paths = true;
+        break;
+      }
+    }
+
+    if (!$have_paths) {
+      throw new ArcanistNoEffectException("No paths are lintable.");
+    }
+
+    $versions = array($this->getCacheVersion());
+    foreach ($linters as $linter) {
+      $versions[] = get_class($linter).':'.$linter->getCacheVersion();
+    }
+    $this->cacheVersion = crc32(implode("\n", $versions));
+
     $exceptions = array();
     foreach ($linters as $linter_name => $linter) {
       try {
@@ -181,6 +205,10 @@ abstract class ArcanistLintEngine {
           if (isset($stopped[$path])) {
             unset($paths[$key]);
           }
+          // TODO: Some linters work with the whole directory.
+          if (isset($this->cachedResults[$path][$this->cacheVersion])) {
+            unset($paths[$key]);
+          }
         }
         $paths = array_values($paths);
 
@@ -214,6 +242,15 @@ abstract class ArcanistLintEngine {
       }
     }
 
+    if ($this->cachedResults) {
+      foreach ($this->cachedResults as $path => $messages) {
+        foreach (idx($messages, $this->cacheVersion, array()) as $message) {
+          $this->getResultForPath($path)->addMessage(
+            ArcanistLintMessage::newFromDictionary($message));
+        }
+      }
+    }
+
     foreach ($this->results as $path => $result) {
       $disk_path = $this->getFilePathOnDisk($path);
       $result->setFilePathOnDisk($disk_path);
@@ -235,22 +272,6 @@ abstract class ArcanistLintEngine {
       }
     }
 
-    if (!$linters) {
-      throw new ArcanistNoEffectException("No linters to run.");
-    }
-
-    $have_paths = false;
-    foreach ($linters as $linter) {
-      if ($linter->getPaths()) {
-        $have_paths = true;
-        break;
-      }
-    }
-
-    if (!$have_paths) {
-      throw new ArcanistNoEffectException("No paths are lintable.");
-    }
-
     if ($exceptions) {
       throw new PhutilAggregateException('Some linters failed:', $exceptions);
     }
@@ -258,6 +279,15 @@ abstract class ArcanistLintEngine {
     return $this->results;
   }
 
+  /**
+   * @param dict<string path, dict<int version, list<dict message>>>
+   * @return this
+   */
+  public function setCachedResults(array $results) {
+    $this->cachedResults = $results;
+    return $this;
+  }
+
   public function getResults() {
     return $this->results;
   }
@@ -291,10 +321,11 @@ abstract class ArcanistLintEngine {
     return false;
   }
 
-  public function getResultForPath($path) {
+  protected function getResultForPath($path) {
     if (empty($this->results[$path])) {
       $result = new ArcanistLintResult();
       $result->setPath($path);
+      $result->setCacheVersion($this->cacheVersion);
       $this->results[$path] = $result;
     }
     return $this->results[$path];
@@ -336,7 +367,7 @@ abstract class ArcanistLintEngine {
     return $this;
   }
 
-  public function getCacheVersion() {
+  protected function getCacheVersion() {
     return 0;
   }
 

src/lint/linter/ArcanistLinter.php

@@ -81,6 +81,10 @@ abstract class ArcanistLinter {
     return $this->engine;
   }
 
+  public function getCacheVersion() {
+    return 0;
+  }
+
   public function getLintMessageFullCode($short_code) {
     return $this->getLinterName().$short_code;
   }

src/workflow/ArcanistLintWorkflow.php

@@ -16,6 +16,7 @@ class ArcanistLintWorkflow extends ArcanistBaseWorkflow {
   const DEFAULT_SEVERITY = ArcanistLintSeverity::SEVERITY_ADVICE;
 
   private $unresolvedMessages;
+  private $shouldLintAll;
   private $shouldAmendChanges = false;
   private $shouldAmendWithoutPrompt = false;
   private $shouldAmendAutofixesWithoutPrompt = false;
@@ -140,8 +141,7 @@ EOTEXT
     return implode("\n", array(
       get_class($this->engine),
       $this->getArgument('severity', self::DEFAULT_SEVERITY),
-      $this->getArgument('lintall'),
-      $this->engine->getCacheVersion(),
+      $this->shouldLintAll,
     ));
   }
 
@@ -165,12 +165,12 @@ EOTEXT
       throw new ArcanistUsageException("Specify either --rev or paths.");
     }
 
-    $should_lint_all = $this->getArgument('lintall');
+    $this->shouldLintAll = $this->getArgument('lintall');
     if ($paths) {
       // 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.
-      $should_lint_all = true;
+      $this->shouldLintAll = true;
     }
 
     $paths = $this->selectPathsForWorkflow($paths, $rev);
@@ -189,30 +189,24 @@ EOTEXT
     $engine->setMinimumSeverity(
       $this->getArgument('severity', self::DEFAULT_SEVERITY));
 
-    $cached = false;
     if ($this->getArgument('cache')) {
-      $paths = array_combine($paths, $paths);
       $cache = $this->readScratchJSONFile('lint-cache.json');
       $cache = idx($cache, $this->getCacheKey(), array());
+      $cached = array();
       foreach ($cache as $path => $messages) {
         $messages = idx($messages, md5_file($engine->getFilePathOnDisk($path)));
-        // TODO: Some linters work with the whole directory.
         if ($messages !== null) {
-          foreach ($messages as $message) {
-            $engine->getResultForPath($path)->addMessage(
-              ArcanistLintMessage::newFromDictionary($message));
-          }
-          $cached = true;
-          unset($paths[$path]);
+          $cached[$path] = $messages;
         }
       }
+      $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 (!$should_lint_all) {
+    if (!$this->shouldLintAll) {
       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).
@@ -233,12 +227,8 @@ EOTEXT
     try {
       $engine->run();
     } catch (Exception $ex) {
-      if ($ex instanceof ArcanistNoEffectException && $cached) {
-        // Swallow.
-      } else {
       $failed = $ex;
     }
-    }
 
     $results = $engine->getResults();
 
@@ -401,19 +391,24 @@ EOTEXT
     }
     $this->unresolvedMessages = $unresolved;
 
-    if ($this->getArgument('cache')) {
-      $cached = array();
+    $cache = $this->readScratchJSONFile('lint-cache.json');
+    $cached = idx($cache, $this->getCacheKey(), array());
+    if ($cached || $this->getArgument('cache')) {
       foreach ($results as $result) {
         $path = $result->getPath();
+        if (!$this->getArgument('cache')) {
+          unset($cached[$path]);
+          continue;
+        }
         $hash = md5_file($engine->getFilePathOnDisk($path));
-        $cached[$path] = array($hash => array());
+        $version = $result->getCacheVersion();
+        $cached[$path] = array($hash => array($version => array()));
         foreach ($result->getMessages() as $message) {
           if (!$message->isPatchApplied()) {
-            $cached[$path][$hash][] = $message->toDictionary();
+            $cached[$path][$hash][$version][] = $message->toDictionary();
           }
         }
       }
-      $cache = $this->readScratchJSONFile('lint-cache.json');
       $cache[$this->getCacheKey()] = $cached;
       // TODO: Garbage collection.
       $this->writeScratchJSONFile('lint-cache.json', $cache);