Skip to content

Commit

Permalink
Update: Implement eslint/rfcs#63
Browse files Browse the repository at this point in the history
  • Loading branch information
billiegoose committed Oct 29, 2020
1 parent a1a9d14 commit 760eb5c
Show file tree
Hide file tree
Showing 9 changed files with 317 additions and 55 deletions.
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.

Examples:

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) {
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).
*/
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,
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}`);
return null;
}

if (fileDescriptor.notFound || changed) {
if (changed) {
debug(`Cached result changed: ${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 +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.
* @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
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", () => {
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 760eb5c

Please sign in to comment.