From 619dd62f31340dad0e112c04a9eebb94bd45d259 Mon Sep 17 00:00:00 2001
From: vrana <jakubv@fb.com>
Date: Wed, 6 Feb 2013 15:27:31 -0800
Subject: [PATCH] Respect granularity in reading cached lint messages

Summary:
We save repository version to lint cache but ignore it when reading the cache.
Fix it.

Test Plan: Made an error for linter with repo granularity, deleted the error from the cache. Relinted, didn't see the error. Changed another file and relinted, saw the error.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4841
---
 src/lint/ArcanistLintMessage.php       | 12 ++++---
 src/lint/engine/ArcanistLintEngine.php | 45 ++++++++++++++------------
 src/workflow/ArcanistLintWorkflow.php  |  9 ++----
 3 files changed, 34 insertions(+), 32 deletions(-)

diff --git a/src/lint/ArcanistLintMessage.php b/src/lint/ArcanistLintMessage.php
index 92e2c6b4..cce2ca21 100644
--- a/src/lint/ArcanistLintMessage.php
+++ b/src/lint/ArcanistLintMessage.php
@@ -20,7 +20,7 @@ final class ArcanistLintMessage {
   protected $dependentMessages = array();
   protected $otherLocations = array();
   protected $obsolete;
-  protected $uncacheable;
+  protected $granularity;
 
   public static function newFromDictionary(array $dict) {
     $message = new ArcanistLintMessage();
@@ -38,6 +38,7 @@ final class ArcanistLintMessage {
     if (isset($dict['replacement'])) {
       $message->setReplacementText($dict['replacement']);
     }
+    $message->setGranularity(idx($dict, 'granularity'));
     $message->setOtherLocations(idx($dict, 'locations', array()));
     return $message;
   }
@@ -53,6 +54,7 @@ final class ArcanistLintMessage {
       'description' => $this->getDescription(),
       'original'    => $this->getOriginalText(),
       'replacement' => $this->getReplacementText(),
+      'granularity' => $this->getGranularity(),
       'locations'   => $this->getOtherLocations(),
     );
   }
@@ -196,13 +198,13 @@ final class ArcanistLintMessage {
     return $this->appliedToDisk;
   }
 
-  public function setUncacheable($bool) {
-    $this->uncacheable = $bool;
+  public function setGranularity($granularity) {
+    $this->granularity = $granularity;
     return $this;
   }
 
-  public function isUncacheable() {
-    return $this->uncacheable;
+  public function getGranularity() {
+    return $this->granularity;
   }
 
   public function setDependentMessages(array $messages) {
diff --git a/src/lint/engine/ArcanistLintEngine.php b/src/lint/engine/ArcanistLintEngine.php
index a273963a..39f995ea 100644
--- a/src/lint/engine/ArcanistLintEngine.php
+++ b/src/lint/engine/ArcanistLintEngine.php
@@ -203,8 +203,6 @@ abstract class ArcanistLintEngine {
         }
         $paths = $linter->getPaths();
 
-        $cache_granularity = $linter->getCacheGranularity();
-
         foreach ($paths as $key => $path) {
           // Make sure each path has a result generated, even if it is empty
           // (i.e., the file has no lint messages).
@@ -215,19 +213,9 @@ abstract class ArcanistLintEngine {
           if (isset($this->cachedResults[$path][$this->cacheVersion])) {
             $cached_result = $this->cachedResults[$path][$this->cacheVersion];
 
-            switch ($cache_granularity) {
-              case ArcanistLinter::GRANULARITY_FILE:
-                $use_cache = true;
-                break;
-              case ArcanistLinter::GRANULARITY_DIRECTORY:
-              case ArcanistLinter::GRANULARITY_REPOSITORY:
-                $repository_version = idx($cached_result, 'repository_version');
-                $use_cache = ($this->repositoryVersion == $repository_version);
-                break;
-              default:
-                $use_cache = false;
-                break;
-            }
+            $use_cache = $this->shouldUseCache(
+              $linter->getCacheGranularity(),
+              idx($cached_result, 'repository_version'));
 
             if ($use_cache) {
               unset($paths[$key]);
@@ -260,7 +248,6 @@ abstract class ArcanistLintEngine {
 
     foreach ($linters as $linter) {
       $minimum = $this->minimumSeverity;
-      $cache_granularity = $linter->getCacheGranularity();
       foreach ($linter->getLintMessages() as $message) {
         if (!ArcanistLintSeverity::isAtLeastAsSevere($message, $minimum)) {
           continue;
@@ -268,9 +255,7 @@ abstract class ArcanistLintEngine {
         if (!$this->isRelevantMessage($message)) {
           continue;
         }
-        if ($cache_granularity == ArcanistLinter::GRANULARITY_GLOBAL) {
-          $message->setUncacheable(true);
-        }
+        $message->setGranularity($linter->getCacheGranularity());
         $result = $this->getResultForPath($message->getPath());
         $result->addMessage($message);
       }
@@ -279,11 +264,17 @@ abstract class ArcanistLintEngine {
     if ($this->cachedResults) {
       foreach ($this->cachedResults as $path => $messages) {
         $messages = idx($messages, $this->cacheVersion, array());
+        $repository_version = idx($messages, 'repository_version');
         unset($messages['stopped']);
         unset($messages['repository_version']);
         foreach ($messages as $message) {
-          $this->getResultForPath($path)->addMessage(
-            ArcanistLintMessage::newFromDictionary($message));
+          $use_cache = $this->shouldUseCache(
+            idx($message, 'granularity'),
+            $repository_version);
+          if ($use_cache) {
+            $this->getResultForPath($path)->addMessage(
+              ArcanistLintMessage::newFromDictionary($message));
+          }
         }
       }
     }
@@ -316,6 +307,18 @@ abstract class ArcanistLintEngine {
     return $this->results;
   }
 
+  private function shouldUseCache($cache_granularity, $repository_version) {
+    switch ($cache_granularity) {
+      case ArcanistLinter::GRANULARITY_FILE:
+        return true;
+      case ArcanistLinter::GRANULARITY_DIRECTORY:
+      case ArcanistLinter::GRANULARITY_REPOSITORY:
+        return ($this->repositoryVersion == $repository_version);
+      default:
+        return false;
+    }
+  }
+
   /**
    * @param dict<string path, dict<string version, list<dict message>>>
    * @return this
diff --git a/src/workflow/ArcanistLintWorkflow.php b/src/workflow/ArcanistLintWorkflow.php
index 1f3cd213..6463c463 100644
--- a/src/workflow/ArcanistLintWorkflow.php
+++ b/src/workflow/ArcanistLintWorkflow.php
@@ -195,15 +195,11 @@ EOTEXT
     $engine = newv($engine, array());
     $this->engine = $engine;
     $engine->setWorkingCopy($working_copy);
-
-    if ($use_cache) {
-      $engine->setRepositoryVersion($this->getRepositoryVersion());
-    }
-
     $engine->setMinimumSeverity(
       $this->getArgument('severity', self::DEFAULT_SEVERITY));
 
     if ($use_cache) {
+      $engine->setRepositoryVersion($this->getRepositoryVersion());
       $cache = $this->readScratchJSONFile('lint-cache.json');
       $cache = idx($cache, $this->getCacheKey(), array());
       $cache = array_intersect_key($cache, array_flip($paths));
@@ -527,7 +523,8 @@ EOTEXT
         }
         $cached_path['repository_version'] = $this->getRepositoryVersion();
         foreach ($result->getMessages() as $message) {
-          if ($message->isUncacheable()) {
+          $granularity = $message->getGranularity();
+          if ($granularity == ArcanistLinter::GRANULARITY_GLOBAL) {
             continue;
           }
           if (!$message->isPatchApplied()) {