Skip to content

Commit

Permalink
Chore: Extract lint result cache logic (refs #9948) (#10562)
Browse files Browse the repository at this point in the history
* Chore: Extract current cache logic into lint-result-cache module

* Chore: Moved config hash validation to LintResultCache

* Chore: Added tests for lint-result-cache

* Chore: Small cleanup

* Chore: Removing unnecessary comments

* Chore: Remove unnecessary test fixture file

* Chore: options.cache to this.options.cache
  • Loading branch information
platinumazure committed Jul 11, 2018
1 parent 80b296e commit 9aaf195
Show file tree
Hide file tree
Showing 5 changed files with 449 additions and 50 deletions.
70 changes: 20 additions & 50 deletions lib/cli-engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,9 @@ const fs = require("fs"),
Linter = require("./linter"),
IgnoredPaths = require("./ignored-paths"),
Config = require("./config"),
fileEntryCache = require("file-entry-cache"),
LintResultCache = require("./util/lint-result-cache"),
globUtil = require("./util/glob-util"),
validator = require("./config/config-validator"),
stringify = require("json-stable-stringify-without-jsonify"),
hash = require("./util/hash"),
ModuleResolver = require("./util/module-resolver"),
naming = require("./util/naming"),
Expand Down Expand Up @@ -359,8 +358,6 @@ function getCacheFile(cacheFile, cwd) {
return resolvedCacheFile;
}

const configHashCache = new WeakMap();

//------------------------------------------------------------------------------
// Public Interface
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -403,18 +400,6 @@ class CLIEngine {
this.options = options;
this.linter = new Linter();

if (options.cache) {
const cacheFile = getCacheFile(this.options.cacheLocation || this.options.cacheFile, this.options.cwd);

/**
* Cache used to avoid operating on files that haven't changed since the
* last successful execution (e.g., file passed linting with no errors and
* no warnings).
* @type {Object}
*/
this._fileCache = fileEntryCache.create(cacheFile);
}

// load in additional rules
if (this.options.rulePaths) {
const cwd = this.options.cwd;
Expand All @@ -434,6 +419,17 @@ class CLIEngine {
}

this.config = new Config(this.options, this.linter);

if (this.options.cache) {
const cacheFile = getCacheFile(this.options.cacheLocation || this.options.cacheFile, this.options.cwd);

/**
* Cache used to avoid operating on files that haven't changed since the
* last successful execution.
* @type {Object}
*/
this._lintResultCache = new LintResultCache(cacheFile, this.config);
}
}

getRules() {
Expand Down Expand Up @@ -506,29 +502,14 @@ class CLIEngine {
*/
executeOnFiles(patterns) {
const options = this.options,
fileCache = this._fileCache,
lintResultCache = this._lintResultCache,
configHelper = this.config;
const cacheFile = getCacheFile(this.options.cacheLocation || this.options.cacheFile, this.options.cwd);

if (!options.cache && fs.existsSync(cacheFile)) {
fs.unlinkSync(cacheFile);
}

/**
* Calculates the hash of the config file used to validate a given file
* @param {string} filename The path of the file to retrieve a config object for to calculate the hash
* @returns {string} the hash of the config
*/
function hashOfConfigFor(filename) {
const config = configHelper.getConfig(filename);

if (!configHashCache.has(config)) {
configHashCache.set(config, hash(`${pkg.version}_${stringify(config)}`));
}

return configHashCache.get(config);
}

const startTime = Date.now();
const fileList = globUtil.listFilesToProcess(patterns, options);
const results = fileList.map(fileInfo => {
Expand All @@ -543,20 +524,12 @@ class CLIEngine {
* with the metadata and the flag that determines if
* the file has changed
*/
const descriptor = fileCache.getFileDescriptor(fileInfo.filename);
const hashOfConfig = hashOfConfigFor(fileInfo.filename);
const changed = descriptor.changed || descriptor.meta.hashOfConfig !== hashOfConfig;
const cachedLintResults = lintResultCache.getCachedLintResults(fileInfo.filename);

if (!changed) {
debug(`Skipping file since hasn't changed: ${fileInfo.filename}`);
if (cachedLintResults) {
debug(`Skipping file since it hasn't changed: ${fileInfo.filename}`);

/*
* Add the the cached results (always will be 0 error and
* 0 warnings). We should not cache results for files that
* failed, in order to guarantee that next execution will
* process those files as well.
*/
return descriptor.meta.results;
return cachedLintResults;
}
}

Expand All @@ -574,23 +547,20 @@ class CLIEngine {
* store the file in the cache so we can guarantee that
* next execution will also operate on this file
*/
fileCache.removeEntry(result.filePath);
lintResultCache.removeEntry(result.filePath);
} else {

/*
* since the file passed we store the result here
* TODO: it might not be necessary to store the results list in the cache,
* since it should always be 0 errors/warnings
*/
const descriptor = fileCache.getFileDescriptor(result.filePath);

descriptor.meta.hashOfConfig = hashOfConfigFor(result.filePath);
descriptor.meta.results = result;
lintResultCache.setCachedLintResults(result.filePath, result);
}
});

// persist the cache to disk
fileCache.reconcile();
lintResultCache.reconcile();
}

const stats = calculateStatsPerRun(results);
Expand Down
129 changes: 129 additions & 0 deletions lib/util/lint-result-cache.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
/**
* @fileoverview Utility for caching lint results.
* @author Kevin Partington
*/
"use strict";

//-----------------------------------------------------------------------------
// Requirements
//-----------------------------------------------------------------------------

const assert = require("assert"),
fileEntryCache = require("file-entry-cache"),
hash = require("./hash"),
pkg = require("../../package.json"),
stringify = require("json-stable-stringify-without-jsonify");

//-----------------------------------------------------------------------------
// Helpers
//-----------------------------------------------------------------------------

const configHashCache = new WeakMap();

/**
* Calculates the hash of the config file used to validate a given file
* @param {Object} configHelper The config helper for retrieving configuration information
* @param {string} filename The path of the file to retrieve a config object for to calculate the hash
* @returns {string} The hash of the config
*/
function hashOfConfigFor(configHelper, filename) {
const config = configHelper.getConfig(filename);

if (!configHashCache.has(config)) {
configHashCache.set(config, hash(`${pkg.version}_${stringify(config)}`));
}

return configHashCache.get(config);
}

//-----------------------------------------------------------------------------
// Public Interface
//-----------------------------------------------------------------------------

/**
* Lint result cache. This wraps around the file-entry-cache module,
* transparently removing properties that are difficult or expensive to
* serialize and adding them back in on retrieval.
*/
class LintResultCache {

/**
* Creates a new LintResultCache instance.
* @constructor
* @param {string} cacheFileLocation The cache file location.
* @param {Object} configHelper The configuration helper (used for
* configuration lookup by file path).
*/
constructor(cacheFileLocation, configHelper) {
assert(cacheFileLocation, "Cache file location is required");
assert(configHelper, "Config helper is required");

this.fileEntryCache = fileEntryCache.create(cacheFileLocation);
this.configHelper = configHelper;
}

/**
* Retrieve cached lint results for a given file path, if present in the
* cache. If the file is present and has not been changed, rebuild any
* missing result information.
* @param {string} filePath The file for which to retrieve lint results.
* @returns {Object|null} The rebuilt lint results, or null if the file is
* changed or not in the filesystem.
*/
getCachedLintResults(filePath) {

/*
* Cached lint results are valid if and only if:
* 1. The file is present in the filesystem
* 2. The file has not changed since the time it was previously linted
* 3. The ESLint configuration has not changed since the time the file
* was previously linted
* If any of these are not true, we will not reuse the lint results.
*/

const fileDescriptor = this.fileEntryCache.getFileDescriptor(filePath);
const hashOfConfig = hashOfConfigFor(this.configHelper, filePath);
const changed = fileDescriptor.changed || fileDescriptor.meta.hashOfConfig !== hashOfConfig;

if (fileDescriptor.notFound || changed) {
return null;
}

return fileDescriptor.meta.results;
}

/**
* Set the cached lint results for a given file path, after removing any
* information that will be both unnecessary and difficult to serialize.
* @param {string} filePath The file for which to set lint results.
* @param {Object} result The lint result to be set for the file.
* @returns {void}
*/
setCachedLintResults(filePath, result) {
const fileDescriptor = this.fileEntryCache.getFileDescriptor(filePath);

if (!fileDescriptor.notFound) {
fileDescriptor.meta.results = result;
fileDescriptor.meta.hashOfConfig = hashOfConfigFor(this.configHelper, result.filePath);
}
}

/**
* Persists the in-memory cache to disk.
* @returns {void}
*/
reconcile() {
this.fileEntryCache.reconcile();
}

/**
* Remove a file entry from this lint result cache.
* @param {string} filePath The file path to be removed from the cache.
* @returns {void}
*/
removeEntry(filePath) {
this.fileEntryCache.removeEntry(filePath);
}
}

module.exports = LintResultCache;
6 changes: 6 additions & 0 deletions tests/fixtures/lint-result-cache/.eslintrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"rules": {
"semi": ["error", "always"]
},
"root": true
}
1 change: 1 addition & 0 deletions tests/fixtures/lint-result-cache/test-with-errors.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log("Hello, world!")

0 comments on commit 9aaf195

Please sign in to comment.