Skip to content
Permalink
Browse files
New: Implement cacheStrategy (refs eslint/rfcs#63) (#14119)
* Update: Implement eslint/rfcs#63

* fix: correct tests

* fix: isValidCacheStrategy now directly takes the cache strategy as string instead of configHelper object

* fix: pass directly cacheStratgey as string instead of configHelper object

Updated file-entry-cache to ^6.0.1
Also add tests outside of the cli-engine

* style: define possible values for cacheStrategy

* test: remove only

* fix: remove now useless option object

* fix: correct jsdoc type

* Update docs/developer-guide/nodejs-api.md

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* Update lib/cli-engine/lint-result-cache.js

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* Update lib/cli-engine/lint-result-cache.js

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* Update lib/cli-engine/lint-result-cache.js

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* Update docs/user-guide/command-line-interface.md

Co-authored-by: Bryan Mishkin <698306+bmish@users.noreply.github.com>

Co-authored-by: William Hilton <wmhilton@gmail.com>
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
Co-authored-by: Bryan Mishkin <698306+bmish@users.noreply.github.com>
  • Loading branch information
4 people committed Feb 26, 2021
1 parent 5e51fd2 commit 08ae31e539e381cd0eabf6393fa5c20f1d59125f
Show file tree
Hide file tree
Showing 12 changed files with 434 additions and 60 deletions.
@@ -24,6 +24,7 @@ module.exports = {
*/
cacheLocation: "",
cacheFile: ".eslintcache",
cacheStrategy: "metadata",
fix: false,
allowInlineConfig: true,
reportUnusedDisableDirectives: void 0,
@@ -156,6 +156,8 @@ The `ESLint` constructor takes an `options` object. If you omit the `options` ob
Default is `false`. If `true` is present, the [`eslint.lintFiles()`][eslint-lintfiles] method caches lint results and uses it if each target file is not changed. Please mind that ESLint doesn't clear the cache when you upgrade ESLint plugins. In that case, you have to remove the cache file manually. The [`eslint.lintText()`][eslint-linttext] method doesn't use caches even if you pass the `options.filePath` to the method.
* `options.cacheLocation` (`string`)<br>
Default is `.eslintcache`. The [`eslint.lintFiles()`][eslint-lintfiles] method writes caches into this file.
* `options.cacheStrategy` (`string`)<br>
Default is `"metadata"`. Strategy for the cache to use for detecting changed files. Can be either `"metadata"` or `"content"`.

### ◆ eslint.lintFiles(patterns)

@@ -75,6 +75,7 @@ Caching:
--cache Only check changed files - default: false
--cache-file path::String Path to the cache file. Deprecated: use --cache-location - default: .eslintcache
--cache-location path::String Path to the cache file or directory
--cache-strategy String Strategy to use for detecting changed files - either: metadata or content - default: metadata
Miscellaneous:
--init Run config initialization wizard - default: false
@@ -440,6 +441,16 @@ Example:

eslint "src/**/*.js" --cache --cache-location "/Users/user/.eslintcache/"

#### `--cache-strategy`

Strategy for the cache to use for detecting changed files. Can be either `metadata` or `content`. If no strategy is specified, `metadata` will be used.

The `content` strategy can be useful in cases where the modification time of your files change even if their contents have not. For example, this can happen during git operations like git clone because git does not track file modification time.

Example:

eslint "src/**/*.js" --cache --cache-strategy content

### Miscellaneous

#### `--init`
@@ -589,7 +589,7 @@ class CLIEngine {
ignore: options.ignore
});
const lintResultCache =
options.cache ? new LintResultCache(cacheFilePath) : null;
options.cache ? new LintResultCache(cacheFilePath, options.cacheStrategy) : null;
const linter = new Linter({ cwd: options.cwd });

/** @type {ConfigArray[]} */
@@ -15,13 +15,31 @@ const stringify = require("json-stable-stringify-without-jsonify");
const pkg = require("../../package.json");
const hash = require("./hash");

const debug = require("debug")("eslint:lint-result-cache");

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

const configHashCache = new WeakMap();
const nodeVersion = process && process.version;

const validCacheStrategies = ["metadata", "content"];
const invalidCacheStrategyErrorMessage = `Cache strategy must be one of: ${validCacheStrategies
.map(strategy => `"${strategy}"`)
.join(", ")}`;

/**
* Tests whether a provided cacheStrategy is valid
* @param {string} cacheStrategy The cache strategy to use
* @returns {boolean} true if `cacheStrategy` is one of `validCacheStrategies`; false otherwise
*/
function isValidCacheStrategy(cacheStrategy) {
return (
validCacheStrategies.indexOf(cacheStrategy) !== -1
);
}

/**
* Calculates the hash of the config
* @param {ConfigArray} config The config.
@@ -50,11 +68,30 @@ class LintResultCache {
* Creates a new LintResultCache instance.
* @param {string} cacheFileLocation The cache file location.
* configuration lookup by file path).
* @param {"metadata" | "content"} cacheStrategy The cache strategy to use.
*/
constructor(cacheFileLocation) {
constructor(cacheFileLocation, cacheStrategy) {
assert(cacheFileLocation, "Cache file location is required");

this.fileEntryCache = fileEntryCache.create(cacheFileLocation);
assert(cacheStrategy, "Cache strategy is required");
assert(
isValidCacheStrategy(cacheStrategy),
invalidCacheStrategyErrorMessage
);

debug(`Caching results to ${cacheFileLocation}`);

const useChecksum = cacheStrategy === "content";

debug(
`Using "${cacheStrategy}" strategy to detect changes`
);

this.fileEntryCache = fileEntryCache.create(
cacheFileLocation,
void 0,
useChecksum
);
this.cacheFileLocation = cacheFileLocation;
}

/**
@@ -76,17 +113,28 @@ class LintResultCache {
* 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(config);
const changed = fileDescriptor.changed || fileDescriptor.meta.hashOfConfig !== hashOfConfig;
const changed =
fileDescriptor.changed ||
fileDescriptor.meta.hashOfConfig !== hashOfConfig;

if (fileDescriptor.notFound) {
debug(`File not found on the file system: ${filePath}`);
return null;
}

if (fileDescriptor.notFound || changed) {
if (changed) {
debug(`Cache entry not found or no longer valid: ${filePath}`);
return null;
}

// If source is present but null, need to reread the file from the filesystem.
if (fileDescriptor.meta.results && fileDescriptor.meta.results.source === null) {
if (
fileDescriptor.meta.results &&
fileDescriptor.meta.results.source === null
) {
debug(`Rereading cached result source from filesystem: ${filePath}`);
fileDescriptor.meta.results.source = fs.readFileSync(filePath, "utf-8");
}

@@ -112,6 +160,7 @@ class LintResultCache {
const fileDescriptor = this.fileEntryCache.getFileDescriptor(filePath);

if (fileDescriptor && !fileDescriptor.notFound) {
debug(`Updating cached result: ${filePath}`);

// Serialize the result, except that we want to remove the file source if present.
const resultToSerialize = Object.assign({}, result);
@@ -135,6 +184,7 @@ class LintResultCache {
* @returns {void}
*/
reconcile() {
debug(`Persisting cached results: ${this.cacheFileLocation}`);
this.fileEntryCache.reconcile();
}
}
@@ -62,6 +62,7 @@ function translateOptions({
cache,
cacheFile,
cacheLocation,
cacheStrategy,
config,
env,
errorOnUnmatchedPattern,
@@ -88,6 +89,7 @@ function translateOptions({
allowInlineConfig: inlineConfig,
cache,
cacheLocation: cacheLocation || cacheFile,
cacheStrategy,
errorOnUnmatchedPattern,
extensions: ext,
fix: (fix || fixDryRun) && (quiet ? quietFixPredicate : true),
@@ -43,6 +43,7 @@ const { version } = require("../../package.json");
* @property {ConfigData} [baseConfig] Base config object, extended by all configs used with this instance
* @property {boolean} [cache] Enable result caching.
* @property {string} [cacheLocation] The cache file to use instead of .eslintcache.
* @property {"metadata" | "content"} [cacheStrategy] The strategy used to detect changed files.
* @property {string} [cwd] The value to use for the current working directory.
* @property {boolean} [errorOnUnmatchedPattern] If `false` then `ESLint#lintFiles()` doesn't throw even if no target files found. Defaults to `true`.
* @property {string[]} [extensions] An array of file extensions to check.
@@ -157,6 +158,7 @@ function processOptions({
baseConfig = null,
cache = false,
cacheLocation = ".eslintcache",
cacheStrategy = "metadata",
cwd = process.cwd(),
errorOnUnmatchedPattern = true,
extensions = null, // ← should be null by default because if it's an array then it suppresses RFC20 feature.
@@ -216,6 +218,12 @@ function processOptions({
if (!isNonEmptyString(cacheLocation)) {
errors.push("'cacheLocation' must be a non-empty string.");
}
if (
cacheStrategy !== "metadata" &&
cacheStrategy !== "content"
) {
errors.push("'cacheStrategy' must be any of \"metadata\", \"content\".");
}
if (!isNonEmptyString(cwd) || !path.isAbsolute(cwd)) {
errors.push("'cwd' must be an absolute path.");
}
@@ -284,6 +292,7 @@ function processOptions({
baseConfig,
cache,
cacheLocation,
cacheStrategy,
configFile: overrideConfigFile,
cwd,
errorOnUnmatchedPattern,
@@ -214,6 +214,14 @@ module.exports = optionator({
type: "path::String",
description: "Path to the cache file or directory"
},
{
option: "cache-strategy",
dependsOn: ["cache"],
type: "String",
default: "metadata",
enum: ["metadata", "content"],
description: "Strategy to use for detecting changed files in the cache"
},
{
heading: "Miscellaneous"
},
@@ -60,7 +60,7 @@
"espree": "^7.3.1",
"esquery": "^1.4.0",
"esutils": "^2.0.2",
"file-entry-cache": "^6.0.0",
"file-entry-cache": "^6.0.1",
"functional-red-black-tree": "^1.0.1",
"glob-parent": "^5.0.0",
"globals": "^12.1.0",
@@ -2702,6 +2702,127 @@ describe("CLIEngine", () => {
assert.deepStrictEqual(result, cachedResult, "result is the same with or without cache");
});
});

describe("cacheStrategy", () => {
it("should detect changes using a file's modification time when set to 'metadata'", () => {
const cacheFile = getFixturePath(".eslintcache");
const badFile = getFixturePath("cache/src", "fail-file.js");
const goodFile = getFixturePath("cache/src", "test-file.js");

doDelete(cacheFile);

engine = new CLIEngine({
cwd: path.join(fixtureDir, ".."),
useEslintrc: false,

// specifying cache true the cache will be created
cache: true,
cacheFile,
cacheStrategy: "metadata",
rules: {
"no-console": 0,
"no-unused-vars": 2
},
extensions: ["js"]
});

engine.executeOnFiles([badFile, goodFile]);

let fileCache = fCache.createFromFile(cacheFile, false);
const entries = fileCache.normalizeEntries([badFile, goodFile]);

entries.forEach(entry => {
assert.isFalse(entry.changed, `the entry for ${entry.key} is initially unchanged`);
});

// this should result in a changed entry
shell.touch(goodFile);
fileCache = fCache.createFromFile(cacheFile, false);
assert.isFalse(fileCache.getFileDescriptor(badFile).changed, `the entry for ${badFile} is unchanged`);
assert.isTrue(fileCache.getFileDescriptor(goodFile).changed, `the entry for ${goodFile} is changed`);
});

it("should not detect changes using a file's modification time when set to 'content'", () => {
const cacheFile = getFixturePath(".eslintcache");
const badFile = getFixturePath("cache/src", "fail-file.js");
const goodFile = getFixturePath("cache/src", "test-file.js");

doDelete(cacheFile);

engine = new CLIEngine({
cwd: path.join(fixtureDir, ".."),
useEslintrc: false,

// specifying cache true the cache will be created
cache: true,
cacheFile,
cacheStrategy: "content",
rules: {
"no-console": 0,
"no-unused-vars": 2
},
extensions: ["js"]
});

engine.executeOnFiles([badFile, goodFile]);

let fileCache = fCache.createFromFile(cacheFile, true);
let entries = fileCache.normalizeEntries([badFile, goodFile]);

entries.forEach(entry => {
assert.isFalse(entry.changed, `the entry for ${entry.key} is initially unchanged`);
});

// this should NOT result in a changed entry
shell.touch(goodFile);
fileCache = fCache.createFromFile(cacheFile, true);
entries = fileCache.normalizeEntries([badFile, goodFile]);
entries.forEach(entry => {
assert.isFalse(entry.changed, `the entry for ${entry.key} remains unchanged`);
});
});

it("should detect changes using a file's contents when set to 'content'", () => {
const cacheFile = getFixturePath(".eslintcache");
const badFile = getFixturePath("cache/src", "fail-file.js");
const goodFile = getFixturePath("cache/src", "test-file.js");
const goodFileCopy = path.resolve(`${path.dirname(goodFile)}`, "test-file-copy.js");

shell.cp(goodFile, goodFileCopy);

doDelete(cacheFile);

engine = new CLIEngine({
cwd: path.join(fixtureDir, ".."),
useEslintrc: false,

// specifying cache true the cache will be created
cache: true,
cacheFile,
cacheStrategy: "content",
rules: {
"no-console": 0,
"no-unused-vars": 2
},
extensions: ["js"]
});

engine.executeOnFiles([badFile, goodFileCopy]);

let fileCache = fCache.createFromFile(cacheFile, true);
const entries = fileCache.normalizeEntries([badFile, goodFileCopy]);

entries.forEach(entry => {
assert.isFalse(entry.changed, `the entry for ${entry.key} is initially unchanged`);
});

// this should result in a changed entry
shell.sed("-i", "abc", "xzy", goodFileCopy);
fileCache = fCache.createFromFile(cacheFile, true);
assert.isFalse(fileCache.getFileDescriptor(badFile).changed, `the entry for ${badFile} is unchanged`);
assert.isTrue(fileCache.getFileDescriptor(goodFileCopy).changed, `the entry for ${goodFileCopy} is changed`);
});
});
});

describe("processors", () => {

0 comments on commit 08ae31e

Please sign in to comment.