Skip to content

Commit

Permalink
New: Add --cache-strategy CLI option (refs eslint#11319)
Browse files Browse the repository at this point in the history
  • Loading branch information
montmanu committed Mar 7, 2019
1 parent 62fee4a commit 968333c
Show file tree
Hide file tree
Showing 8 changed files with 193 additions and 5 deletions.
1 change: 1 addition & 0 deletions conf/default-cli-options.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ module.exports = {
*/
cacheLocation: "",
cacheFile: ".eslintcache",
cacheStrategy: "stat",
fix: false,
allowInlineConfig: true,
reportUnusedDisableDirectives: false,
Expand Down
11 changes: 11 additions & 0 deletions docs/user-guide/command-line-interface.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,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: stat or md5 - default: stat
Miscellaneous:
--init Run config initialization wizard - default: false
Expand Down Expand Up @@ -438,6 +439,16 @@ Example:

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

#### `--cache-strategy`

Strategy to use for detecting changed files. Can be either `stat` or `md5`. If no strategy is specified, `stat` will be used.

The `md5` 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 md5

### Miscellaneous

#### `--init`
Expand Down
9 changes: 8 additions & 1 deletion lib/cli-engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ const validFixTypes = new Set(["problem", "suggestion", "layout"]);
* @property {boolean} cache Enable result caching.
* @property {string} cacheLocation The cache file to use instead of .eslintcache.
* @property {string} configFile The configuration file to use.
* @property {string} cacheStrategy Strategy to use for detecting changes.
* @property {string} cwd The value to use for the current working directory.
* @property {string[]} envs An array of environments to load.
* @property {string[]} extensions An array of file extensions to check.
Expand Down Expand Up @@ -600,7 +601,7 @@ class CLIEngine {
debug(`Reprocessing cached file to allow autofix: ${fileInfo.filename}`);
} else {
debug(`Skipping file since it hasn't changed: ${fileInfo.filename}`);

cachedLintResults.skipped = true;
return cachedLintResults;
}
}
Expand All @@ -624,6 +625,12 @@ class CLIEngine {

if (options.cache) {
results.forEach(result => {
if (result.skipped) {

// avoid executing lintResultCache.setCachedLintResults for results that were not processed
delete result.skipped;
return;
}

/*
* Store the lint result in the LintResultCache.
Expand Down
1 change: 1 addition & 0 deletions lib/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ function translateOptions(cliOptions) {
cache: cliOptions.cache,
cacheFile: cliOptions.cacheFile,
cacheLocation: cliOptions.cacheLocation,
cacheStrategy: cliOptions.cacheStrategy,
fix: (cliOptions.fix || cliOptions.fixDryRun) && (cliOptions.quiet ? quietFixPredicate : true),
fixTypes: cliOptions.fixType,
allowInlineConfig: cliOptions.inlineConfig,
Expand Down
8 changes: 8 additions & 0 deletions lib/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,14 @@ module.exports = optionator({
type: "path::String",
description: "Path to the cache file or directory"
},
{
option: "cache-strategy",
dependsOn: ["cache"],
type: "String",
default: "stat",
enum: ["stat", "md5"],
description: "Strategy to use for detecting changed files"
},
{
heading: "Miscellaneous"
},
Expand Down
36 changes: 33 additions & 3 deletions lib/util/lint-result-cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,26 @@ const assert = require("assert"),
pkg = require("../../package.json"),
stringify = require("json-stable-stringify-without-jsonify");

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

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

const configHashCache = new WeakMap();

const validCacheStrategies = ["stat", "md5"];
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 file used to validate a given file
* @param {Object} configHelper The config helper for retrieving configuration information
Expand Down Expand Up @@ -58,8 +72,16 @@ class LintResultCache {
constructor(cacheFileLocation, configHelper) {
assert(cacheFileLocation, "Cache file location is required");
assert(configHelper, "Config helper is required");
assert(isValidCacheStrategy(configHelper), invalidCacheStrategyErrorMessage);

this.fileEntryCache = fileEntryCache.create(cacheFileLocation);
debug(`Caching results to ${cacheFileLocation}`);

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

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

this.fileEntryCache = fileEntryCache.create(cacheFileLocation, null, useChecksum);
this.cacheFileLocation = cacheFileLocation;
this.configHelper = configHelper;
}

Expand All @@ -81,17 +103,23 @@ 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(this.configHelper, filePath);
const changed = fileDescriptor.changed || fileDescriptor.meta.hashOfConfig !== hashOfConfig;

if (fileDescriptor.notFound || changed) {
if (fileDescriptor.notFound) {
debug(`Cached result not found: ${filePath}`);
return null;
}

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) {
debug(`Rereading cached result source from filesystem: ${filePath}`);
fileDescriptor.meta.results.source = fs.readFileSync(filePath, "utf-8");
}

Expand All @@ -116,6 +144,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 @@ -139,6 +168,7 @@ class LintResultCache {
* @returns {void}
*/
reconcile() {
debug(`Persisting cached results: ${this.cacheFileLocation}`);
this.fileEntryCache.reconcile();
}
}
Expand Down
121 changes: 121 additions & 0 deletions tests/lib/cli-engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -2608,6 +2608,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 'stat'", () => {
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: "stat",
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 'md5'", () => {
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: "md5",
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 'md5'", () => {
const cacheFile = getFixturePath(".eslintcache");
const badFile = getFixturePath("cache/src", "fail-file.js");
const goodFile = getFixturePath("cache/src", "test-file.js");
const goodFileCopy = `${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: "md5",
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
11 changes: 10 additions & 1 deletion tests/lib/util/lint-result-cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@ describe("LintResultCache", () => {
sandbox = sinon.sandbox.create();

fakeConfigHelper = {
getConfig: sandbox.stub()
getConfig: sandbox.stub(),
options: {
cacheStrategy: "stat"
}
};

hashStub = sandbox.stub();
Expand Down Expand Up @@ -85,6 +88,12 @@ describe("LintResultCache", () => {
assert.throws(() => new LintResultCache(cacheFileLocation), /Config helper is required/u);
});

it("should throw an error if cacheStrategy is an invalid value", () => {
const invalidConfigHelper = Object.assign({}, fakeConfigHelper, { options: { cacheStrategy: "foo" } });

assert.throws(() => new LintResultCache(cacheFileLocation, invalidConfigHelper), /Cache strategy must be one of/u);
});

it("should successfully create an instance if cache file location and config helper are provided", () => {
const instance = new LintResultCache(cacheFileLocation, fakeConfigHelper);

Expand Down

0 comments on commit 968333c

Please sign in to comment.