Skip to content

Commit

Permalink
New: Implement cacheStrategy (refs eslint/rfcs#63) (#14119)
Browse files Browse the repository at this point in the history 
* 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
@chambo-e @billiegoose
@mdjermanovic @bmish
4 people committed Feb 26, 2021
1 parent 5e51fd2 commit 08ae31e
Show file tree
Hide file tree
Showing 12 changed files with 434 additions and 60 deletions.
1 change: 1 addition & 0 deletions conf/default-cli-options.js
View file
Expand Up @@ -24,6 +24,7 @@ module.exports = {
*/
cacheLocation: "",
cacheFile: ".eslintcache",
cacheStrategy: "metadata",
fix: false,
allowInlineConfig: true,
reportUnusedDisableDirectives: void 0,
Expand Down
2 changes: 2 additions & 0 deletions docs/developer-guide/nodejs-api.md
View file
Expand Up @@ -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)

Expand Down
11 changes: 11 additions & 0 deletions docs/user-guide/command-line-interface.md
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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`
Expand Down
2 changes: 1 addition & 1 deletion lib/cli-engine/cli-engine.js
View file
Expand Up @@ -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[]} */
Expand Down
64 changes: 57 additions & 7 deletions lib/cli-engine/lint-result-cache.js
View file
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}

/**
Expand All @@ -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");
}

Expand All @@ -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);
Expand All @@ -135,6 +184,7 @@ class LintResultCache {
* @returns {void}
*/
reconcile() {
debug(`Persisting cached results: ${this.cacheFileLocation}`);
this.fileEntryCache.reconcile();
}
}
Expand Down
2 changes: 2 additions & 0 deletions lib/cli.js
View file
Expand Up @@ -62,6 +62,7 @@ function translateOptions({
cache,
cacheFile,
cacheLocation,
cacheStrategy,
config,
env,
errorOnUnmatchedPattern,
Expand All @@ -88,6 +89,7 @@ function translateOptions({
allowInlineConfig: inlineConfig,
cache,
cacheLocation: cacheLocation || cacheFile,
cacheStrategy,
errorOnUnmatchedPattern,
extensions: ext,
fix: (fix || fixDryRun) && (quiet ? quietFixPredicate : true),
Expand Down
9 changes: 9 additions & 0 deletions lib/eslint/eslint.js
View file
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.");
}
Expand Down Expand Up @@ -284,6 +292,7 @@ function processOptions({
baseConfig,
cache,
cacheLocation,
cacheStrategy,
configFile: overrideConfigFile,
cwd,
errorOnUnmatchedPattern,
Expand Down
8 changes: 8 additions & 0 deletions lib/options.js
View file
Expand Up @@ -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"
},
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Expand Up @@ -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",
Expand Down
121 changes: 121 additions & 0 deletions tests/lib/cli-engine/cli-engine.js
View file
Expand Up @@ -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", () => {
Expand Down

0 comments on commit 08ae31e

Please sign in to comment.