From cd7f86d380af759e63eb90b675909df2ab2fde44 Mon Sep 17 00:00:00 2001 From: vrana Date: Wed, 21 Nov 2012 14:52:50 -0800 Subject: [PATCH] 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 ++++ src/lint/engine/ArcanistLintEngine.php | 67 +++++++++++++++++++------- src/lint/linter/ArcanistLinter.php | 4 ++ src/workflow/ArcanistLintWorkflow.php | 43 ++++++++--------- 4 files changed, 82 insertions(+), 42 deletions(-) diff --git a/src/lint/ArcanistLintResult.php b/src/lint/ArcanistLintResult.php index ab3a5d1e..dcb0968a 100644 --- a/src/lint/ArcanistLintResult.php +++ b/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()) { diff --git a/src/lint/engine/ArcanistLintEngine.php b/src/lint/engine/ArcanistLintEngine.php index 255b0493..cbf7670a 100644 --- a/src/lint/engine/ArcanistLintEngine.php +++ b/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>> + * @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; } diff --git a/src/lint/linter/ArcanistLinter.php b/src/lint/linter/ArcanistLinter.php index 2b5dd53b..0c20fab9 100644 --- a/src/lint/linter/ArcanistLinter.php +++ b/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; } diff --git a/src/workflow/ArcanistLintWorkflow.php b/src/workflow/ArcanistLintWorkflow.php index 1bb1934b..10e8a37b 100644 --- a/src/workflow/ArcanistLintWorkflow.php +++ b/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,11 +227,7 @@ EOTEXT try { $engine->run(); } catch (Exception $ex) { - if ($ex instanceof ArcanistNoEffectException && $cached) { - // Swallow. - } else { - $failed = $ex; - } + $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);