Skip to content

Commit 08ae31e

Browse files
chambo-ebilliegoosemdjermanovicbmish
authored
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>
1 parent 5e51fd2 commit 08ae31e

12 files changed

Lines changed: 434 additions & 60 deletions

File tree

‎conf/default-cli-options.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ module.exports = {
2424
*/
2525
cacheLocation: "",
2626
cacheFile: ".eslintcache",
27+
cacheStrategy: "metadata",
2728
fix: false,
2829
allowInlineConfig: true,
2930
reportUnusedDisableDirectives: void 0,

‎docs/developer-guide/nodejs-api.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,8 @@ The `ESLint` constructor takes an `options` object. If you omit the `options` ob
156156
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.
157157
* `options.cacheLocation` (`string`)<br>
158158
Default is `.eslintcache`. The [`eslint.lintFiles()`][eslint-lintfiles] method writes caches into this file.
159+
* `options.cacheStrategy` (`string`)<br>
160+
Default is `"metadata"`. Strategy for the cache to use for detecting changed files. Can be either `"metadata"` or `"content"`.
159161

160162
### ◆ eslint.lintFiles(patterns)
161163

‎docs/user-guide/command-line-interface.md‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ Caching:
7575
--cache Only check changed files - default: false
7676
--cache-file path::String Path to the cache file. Deprecated: use --cache-location - default: .eslintcache
7777
--cache-location path::String Path to the cache file or directory
78+
--cache-strategy String Strategy to use for detecting changed files - either: metadata or content - default: metadata
7879
7980
Miscellaneous:
8081
--init Run config initialization wizard - default: false
@@ -440,6 +441,16 @@ Example:
440441

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

444+
#### `--cache-strategy`
445+
446+
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.
447+
448+
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.
449+
450+
Example:
451+
452+
eslint "src/**/*.js" --cache --cache-strategy content
453+
443454
### Miscellaneous
444455

445456
#### `--init`

‎lib/cli-engine/cli-engine.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -589,7 +589,7 @@ class CLIEngine {
589589
ignore: options.ignore
590590
});
591591
const lintResultCache =
592-
options.cache ? new LintResultCache(cacheFilePath) : null;
592+
options.cache ? new LintResultCache(cacheFilePath, options.cacheStrategy) : null;
593593
const linter = new Linter({ cwd: options.cwd });
594594

595595
/** @type {ConfigArray[]} */

‎lib/cli-engine/lint-result-cache.js‎

Lines changed: 57 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,31 @@ const stringify = require("json-stable-stringify-without-jsonify");
1515
const pkg = require("../../package.json");
1616
const hash = require("./hash");
1717

18+
const debug = require("debug")("eslint:lint-result-cache");
19+
1820
//-----------------------------------------------------------------------------
1921
// Helpers
2022
//-----------------------------------------------------------------------------
2123

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

27+
const validCacheStrategies = ["metadata", "content"];
28+
const invalidCacheStrategyErrorMessage = `Cache strategy must be one of: ${validCacheStrategies
29+
.map(strategy => `"${strategy}"`)
30+
.join(", ")}`;
31+
32+
/**
33+
* Tests whether a provided cacheStrategy is valid
34+
* @param {string} cacheStrategy The cache strategy to use
35+
* @returns {boolean} true if `cacheStrategy` is one of `validCacheStrategies`; false otherwise
36+
*/
37+
function isValidCacheStrategy(cacheStrategy) {
38+
return (
39+
validCacheStrategies.indexOf(cacheStrategy) !== -1
40+
);
41+
}
42+
2543
/**
2644
* Calculates the hash of the config
2745
* @param {ConfigArray} config The config.
@@ -50,11 +68,30 @@ class LintResultCache {
5068
* Creates a new LintResultCache instance.
5169
* @param {string} cacheFileLocation The cache file location.
5270
* configuration lookup by file path).
71+
* @param {"metadata" | "content"} cacheStrategy The cache strategy to use.
5372
*/
54-
constructor(cacheFileLocation) {
73+
constructor(cacheFileLocation, cacheStrategy) {
5574
assert(cacheFileLocation, "Cache file location is required");
56-
57-
this.fileEntryCache = fileEntryCache.create(cacheFileLocation);
75+
assert(cacheStrategy, "Cache strategy is required");
76+
assert(
77+
isValidCacheStrategy(cacheStrategy),
78+
invalidCacheStrategyErrorMessage
79+
);
80+
81+
debug(`Caching results to ${cacheFileLocation}`);
82+
83+
const useChecksum = cacheStrategy === "content";
84+
85+
debug(
86+
`Using "${cacheStrategy}" strategy to detect changes`
87+
);
88+
89+
this.fileEntryCache = fileEntryCache.create(
90+
cacheFileLocation,
91+
void 0,
92+
useChecksum
93+
);
94+
this.cacheFileLocation = cacheFileLocation;
5895
}
5996

6097
/**
@@ -76,17 +113,28 @@ class LintResultCache {
76113
* was previously linted
77114
* If any of these are not true, we will not reuse the lint results.
78115
*/
79-
80116
const fileDescriptor = this.fileEntryCache.getFileDescriptor(filePath);
81117
const hashOfConfig = hashOfConfigFor(config);
82-
const changed = fileDescriptor.changed || fileDescriptor.meta.hashOfConfig !== hashOfConfig;
118+
const changed =
119+
fileDescriptor.changed ||
120+
fileDescriptor.meta.hashOfConfig !== hashOfConfig;
121+
122+
if (fileDescriptor.notFound) {
123+
debug(`File not found on the file system: ${filePath}`);
124+
return null;
125+
}
83126

84-
if (fileDescriptor.notFound || changed) {
127+
if (changed) {
128+
debug(`Cache entry not found or no longer valid: ${filePath}`);
85129
return null;
86130
}
87131

88132
// If source is present but null, need to reread the file from the filesystem.
89-
if (fileDescriptor.meta.results && fileDescriptor.meta.results.source === null) {
133+
if (
134+
fileDescriptor.meta.results &&
135+
fileDescriptor.meta.results.source === null
136+
) {
137+
debug(`Rereading cached result source from filesystem: ${filePath}`);
90138
fileDescriptor.meta.results.source = fs.readFileSync(filePath, "utf-8");
91139
}
92140

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

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

116165
// Serialize the result, except that we want to remove the file source if present.
117166
const resultToSerialize = Object.assign({}, result);
@@ -135,6 +184,7 @@ class LintResultCache {
135184
* @returns {void}
136185
*/
137186
reconcile() {
187+
debug(`Persisting cached results: ${this.cacheFileLocation}`);
138188
this.fileEntryCache.reconcile();
139189
}
140190
}

‎lib/cli.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ function translateOptions({
6262
cache,
6363
cacheFile,
6464
cacheLocation,
65+
cacheStrategy,
6566
config,
6667
env,
6768
errorOnUnmatchedPattern,
@@ -88,6 +89,7 @@ function translateOptions({
8889
allowInlineConfig: inlineConfig,
8990
cache,
9091
cacheLocation: cacheLocation || cacheFile,
92+
cacheStrategy,
9193
errorOnUnmatchedPattern,
9294
extensions: ext,
9395
fix: (fix || fixDryRun) && (quiet ? quietFixPredicate : true),

‎lib/eslint/eslint.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ const { version } = require("../../package.json");
4343
* @property {ConfigData} [baseConfig] Base config object, extended by all configs used with this instance
4444
* @property {boolean} [cache] Enable result caching.
4545
* @property {string} [cacheLocation] The cache file to use instead of .eslintcache.
46+
* @property {"metadata" | "content"} [cacheStrategy] The strategy used to detect changed files.
4647
* @property {string} [cwd] The value to use for the current working directory.
4748
* @property {boolean} [errorOnUnmatchedPattern] If `false` then `ESLint#lintFiles()` doesn't throw even if no target files found. Defaults to `true`.
4849
* @property {string[]} [extensions] An array of file extensions to check.
@@ -157,6 +158,7 @@ function processOptions({
157158
baseConfig = null,
158159
cache = false,
159160
cacheLocation = ".eslintcache",
161+
cacheStrategy = "metadata",
160162
cwd = process.cwd(),
161163
errorOnUnmatchedPattern = true,
162164
extensions = null, // ← should be null by default because if it's an array then it suppresses RFC20 feature.
@@ -216,6 +218,12 @@ function processOptions({
216218
if (!isNonEmptyString(cacheLocation)) {
217219
errors.push("'cacheLocation' must be a non-empty string.");
218220
}
221+
if (
222+
cacheStrategy !== "metadata" &&
223+
cacheStrategy !== "content"
224+
) {
225+
errors.push("'cacheStrategy' must be any of \"metadata\", \"content\".");
226+
}
219227
if (!isNonEmptyString(cwd) || !path.isAbsolute(cwd)) {
220228
errors.push("'cwd' must be an absolute path.");
221229
}
@@ -284,6 +292,7 @@ function processOptions({
284292
baseConfig,
285293
cache,
286294
cacheLocation,
295+
cacheStrategy,
287296
configFile: overrideConfigFile,
288297
cwd,
289298
errorOnUnmatchedPattern,

‎lib/options.js‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,14 @@ module.exports = optionator({
214214
type: "path::String",
215215
description: "Path to the cache file or directory"
216216
},
217+
{
218+
option: "cache-strategy",
219+
dependsOn: ["cache"],
220+
type: "String",
221+
default: "metadata",
222+
enum: ["metadata", "content"],
223+
description: "Strategy to use for detecting changed files in the cache"
224+
},
217225
{
218226
heading: "Miscellaneous"
219227
},

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@
6060
"espree": "^7.3.1",
6161
"esquery": "^1.4.0",
6262
"esutils": "^2.0.2",
63-
"file-entry-cache": "^6.0.0",
63+
"file-entry-cache": "^6.0.1",
6464
"functional-red-black-tree": "^1.0.1",
6565
"glob-parent": "^5.0.0",
6666
"globals": "^12.1.0",

‎tests/lib/cli-engine/cli-engine.js‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2702,6 +2702,127 @@ describe("CLIEngine", () => {
27022702
assert.deepStrictEqual(result, cachedResult, "result is the same with or without cache");
27032703
});
27042704
});
2705+
2706+
describe("cacheStrategy", () => {
2707+
it("should detect changes using a file's modification time when set to 'metadata'", () => {
2708+
const cacheFile = getFixturePath(".eslintcache");
2709+
const badFile = getFixturePath("cache/src", "fail-file.js");
2710+
const goodFile = getFixturePath("cache/src", "test-file.js");
2711+
2712+
doDelete(cacheFile);
2713+
2714+
engine = new CLIEngine({
2715+
cwd: path.join(fixtureDir, ".."),
2716+
useEslintrc: false,
2717+
2718+
// specifying cache true the cache will be created
2719+
cache: true,
2720+
cacheFile,
2721+
cacheStrategy: "metadata",
2722+
rules: {
2723+
"no-console": 0,
2724+
"no-unused-vars": 2
2725+
},
2726+
extensions: ["js"]
2727+
});
2728+
2729+
engine.executeOnFiles([badFile, goodFile]);
2730+
2731+
let fileCache = fCache.createFromFile(cacheFile, false);
2732+
const entries = fileCache.normalizeEntries([badFile, goodFile]);
2733+
2734+
entries.forEach(entry => {
2735+
assert.isFalse(entry.changed, `the entry for ${entry.key} is initially unchanged`);
2736+
});
2737+
2738+
// this should result in a changed entry
2739+
shell.touch(goodFile);
2740+
fileCache = fCache.createFromFile(cacheFile, false);
2741+
assert.isFalse(fileCache.getFileDescriptor(badFile).changed, `the entry for ${badFile} is unchanged`);
2742+
assert.isTrue(fileCache.getFileDescriptor(goodFile).changed, `the entry for ${goodFile} is changed`);
2743+
});
2744+
2745+
it("should not detect changes using a file's modification time when set to 'content'", () => {
2746+
const cacheFile = getFixturePath(".eslintcache");
2747+
const badFile = getFixturePath("cache/src", "fail-file.js");
2748+
const goodFile = getFixturePath("cache/src", "test-file.js");
2749+
2750+
doDelete(cacheFile);
2751+
2752+
engine = new CLIEngine({
2753+
cwd: path.join(fixtureDir, ".."),
2754+
useEslintrc: false,
2755+
2756+
// specifying cache true the cache will be created
2757+
cache: true,
2758+
cacheFile,
2759+
cacheStrategy: "content",
2760+
rules: {
2761+
"no-console": 0,
2762+
"no-unused-vars": 2
2763+
},
2764+
extensions: ["js"]
2765+
});
2766+
2767+
engine.executeOnFiles([badFile, goodFile]);
2768+
2769+
let fileCache = fCache.createFromFile(cacheFile, true);
2770+
let entries = fileCache.normalizeEntries([badFile, goodFile]);
2771+
2772+
entries.forEach(entry => {
2773+
assert.isFalse(entry.changed, `the entry for ${entry.key} is initially unchanged`);
2774+
});
2775+
2776+
// this should NOT result in a changed entry
2777+
shell.touch(goodFile);
2778+
fileCache = fCache.createFromFile(cacheFile, true);
2779+
entries = fileCache.normalizeEntries([badFile, goodFile]);
2780+
entries.forEach(entry => {
2781+
assert.isFalse(entry.changed, `the entry for ${entry.key} remains unchanged`);
2782+
});
2783+
});
2784+
2785+
it("should detect changes using a file's contents when set to 'content'", () => {
2786+
const cacheFile = getFixturePath(".eslintcache");
2787+
const badFile = getFixturePath("cache/src", "fail-file.js");
2788+
const goodFile = getFixturePath("cache/src", "test-file.js");
2789+
const goodFileCopy = path.resolve(`${path.dirname(goodFile)}`, "test-file-copy.js");
2790+
2791+
shell.cp(goodFile, goodFileCopy);
2792+
2793+
doDelete(cacheFile);
2794+
2795+
engine = new CLIEngine({
2796+
cwd: path.join(fixtureDir, ".."),
2797+
useEslintrc: false,
2798+
2799+
// specifying cache true the cache will be created
2800+
cache: true,
2801+
cacheFile,
2802+
cacheStrategy: "content",
2803+
rules: {
2804+
"no-console": 0,
2805+
"no-unused-vars": 2
2806+
},
2807+
extensions: ["js"]
2808+
});
2809+
2810+
engine.executeOnFiles([badFile, goodFileCopy]);
2811+
2812+
let fileCache = fCache.createFromFile(cacheFile, true);
2813+
const entries = fileCache.normalizeEntries([badFile, goodFileCopy]);
2814+
2815+
entries.forEach(entry => {
2816+
assert.isFalse(entry.changed, `the entry for ${entry.key} is initially unchanged`);
2817+
});
2818+
2819+
// this should result in a changed entry
2820+
shell.sed("-i", "abc", "xzy", goodFileCopy);
2821+
fileCache = fCache.createFromFile(cacheFile, true);
2822+
assert.isFalse(fileCache.getFileDescriptor(badFile).changed, `the entry for ${badFile} is unchanged`);
2823+
assert.isTrue(fileCache.getFileDescriptor(goodFileCopy).changed, `the entry for ${goodFileCopy} is changed`);
2824+
});
2825+
});
27052826
});
27062827

27072828
describe("processors", () => {

0 commit comments

Comments
 (0)