Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New: Implement cacheStrategy (refs eslint/rfcs#63) #14119

Merged
merged 13 commits into from Feb 26, 2021
Merged
1 change: 1 addition & 0 deletions conf/default-cli-options.js
Expand Up @@ -24,6 +24,7 @@ module.exports = {
*/
cacheLocation: "",
cacheFile: ".eslintcache",
cacheStrategy: "metadata",
fix: false,
allowInlineConfig: true,
reportUnusedDisableDirectives: void 0,
Expand Down
11 changes: 11 additions & 0 deletions docs/user-guide/command-line-interface.md
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.
chambo-e marked this conversation as resolved.
Show resolved Hide resolved

Examples:
chambo-e marked this conversation as resolved.
Show resolved Hide resolved

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

### Miscellaneous

#### `--init`
Expand Down
2 changes: 1 addition & 1 deletion lib/cli-engine/cli-engine.js
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 }) : null;
const linter = new Linter({ cwd: options.cwd });

/** @type {ConfigArray[]} */
Expand Down
66 changes: 59 additions & 7 deletions lib/cli-engine/lint-result-cache.js
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 {Object} configHelper The config helper for retrieving configuration information
* @returns {boolean} true if `configHelper.options.cacheStrategy` is one of `validCacheStrategies`; false otherwise
*/
function isValidCacheStrategy(configHelper) {
chambo-e marked this conversation as resolved.
Show resolved Hide resolved
return (
validCacheStrategies.indexOf(configHelper.options.cacheStrategy) !== -1
);
}

/**
* Calculates the hash of the config
* @param {ConfigArray} config The config.
Expand Down Expand Up @@ -50,11 +68,32 @@ class LintResultCache {
* Creates a new LintResultCache instance.
* @param {string} cacheFileLocation The cache file location.
* configuration lookup by file path).
* @param {Object} configHelper The configuration helper (used for
* configuration lookup by file path).
chambo-e marked this conversation as resolved.
Show resolved Hide resolved
*/
constructor(cacheFileLocation) {
constructor(cacheFileLocation, configHelper) {
assert(cacheFileLocation, "Cache file location is required");

this.fileEntryCache = fileEntryCache.create(cacheFileLocation);
assert(configHelper, "Config helper is required");
assert(
isValidCacheStrategy(configHelper),
invalidCacheStrategyErrorMessage
);

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

const useChecksum = configHelper.options.cacheStrategy === "content";

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

this.fileEntryCache = fileEntryCache.create(
cacheFileLocation,
null,
chambo-e marked this conversation as resolved.
Show resolved Hide resolved
useChecksum
);
this.cacheFileLocation = cacheFileLocation;
this.configHelper = configHelper || { options: {} };
}

/**
Expand All @@ -76,17 +115,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(`Cached result not found: ${filePath}`);
chambo-e marked this conversation as resolved.
Show resolved Hide resolved
return null;
}

if (fileDescriptor.notFound || changed) {
if (changed) {
debug(`Cached result changed: ${filePath}`);
chambo-e marked this conversation as resolved.
Show resolved Hide resolved
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 +162,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 +186,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
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
8 changes: 8 additions & 0 deletions lib/eslint/eslint.js
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.
mdjermanovic marked this conversation as resolved.
Show resolved Hide resolved
* @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",
chambo-e marked this conversation as resolved.
Show resolved Hide resolved
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
8 changes: 8 additions & 0 deletions lib/options.js
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
121 changes: 121 additions & 0 deletions tests/lib/cli-engine/cli-engine.js
Expand Up @@ -2702,6 +2702,127 @@ describe("CLIEngine", () => {
assert.deepStrictEqual(result, cachedResult, "result is the same with or without cache");
});
});

describe("cacheStrategy", () => {
chambo-e marked this conversation as resolved.
Show resolved Hide resolved
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