diff --git a/conf/default-cli-options.js b/conf/default-cli-options.js index e09a829d17c..dad03d89e93 100644 --- a/conf/default-cli-options.js +++ b/conf/default-cli-options.js @@ -24,6 +24,7 @@ module.exports = { */ cacheLocation: "", cacheFile: ".eslintcache", + cacheStrategy: "metadata", fix: false, allowInlineConfig: true, reportUnusedDisableDirectives: void 0, diff --git a/docs/user-guide/command-line-interface.md b/docs/user-guide/command-line-interface.md index b8f10497a36..182905484be 100644 --- a/docs/user-guide/command-line-interface.md +++ b/docs/user-guide/command-line-interface.md @@ -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 @@ -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` diff --git a/lib/cli-engine/cli-engine.js b/lib/cli-engine/cli-engine.js index 9a414061501..8d4ded3017e 100644 --- a/lib/cli-engine/cli-engine.js +++ b/lib/cli-engine/cli-engine.js @@ -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[]} */ diff --git a/lib/cli-engine/lint-result-cache.js b/lib/cli-engine/lint-result-cache.js index 23a142097ba..c536d814698 100644 --- a/lib/cli-engine/lint-result-cache.js +++ b/lib/cli-engine/lint-result-cache.js @@ -15,6 +15,8 @@ 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 //----------------------------------------------------------------------------- @@ -22,6 +24,22 @@ const hash = require("./hash"); 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. @@ -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: {} }; } /** @@ -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"); } @@ -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); @@ -135,6 +186,7 @@ class LintResultCache { * @returns {void} */ reconcile() { + debug(`Persisting cached results: ${this.cacheFileLocation}`); this.fileEntryCache.reconcile(); } } diff --git a/lib/cli.js b/lib/cli.js index ce11878008f..4216126b6ce 100644 --- a/lib/cli.js +++ b/lib/cli.js @@ -62,6 +62,7 @@ function translateOptions({ cache, cacheFile, cacheLocation, + cacheStrategy, config, env, errorOnUnmatchedPattern, @@ -88,6 +89,7 @@ function translateOptions({ allowInlineConfig: inlineConfig, cache, cacheLocation: cacheLocation || cacheFile, + cacheStrategy, errorOnUnmatchedPattern, extensions: ext, fix: (fix || fixDryRun) && (quiet ? quietFixPredicate : true), diff --git a/lib/eslint/eslint.js b/lib/eslint/eslint.js index a51ffbfe41a..a40fad1c5c4 100644 --- a/lib/eslint/eslint.js +++ b/lib/eslint/eslint.js @@ -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. @@ -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. @@ -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."); } diff --git a/lib/options.js b/lib/options.js index 1681f1dbd1d..99a38a7700c 100644 --- a/lib/options.js +++ b/lib/options.js @@ -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" }, diff --git a/tests/lib/cli-engine/cli-engine.js b/tests/lib/cli-engine/cli-engine.js index 9e7c49c84d7..59243b0b7dc 100644 --- a/tests/lib/cli-engine/cli-engine.js +++ b/tests/lib/cli-engine/cli-engine.js @@ -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", () => { diff --git a/tests/lib/cli-engine/lint-result-cache.js b/tests/lib/cli-engine/lint-result-cache.js index a3b6ee3ab2c..6ea49501d6c 100644 --- a/tests/lib/cli-engine/lint-result-cache.js +++ b/tests/lib/cli-engine/lint-result-cache.js @@ -20,18 +20,32 @@ const assert = require("chai").assert, //----------------------------------------------------------------------------- describe("LintResultCache", () => { - const fixturePath = path.resolve(__dirname, "../../fixtures/lint-result-cache"); + const fixturePath = path.resolve( + __dirname, + "../../fixtures/lint-result-cache" + ); const cacheFileLocation = path.join(fixturePath, ".eslintcache"); const fileEntryCacheStubs = {}; let LintResultCache, hashStub, + sandbox, fakeConfig, fakeErrorResults, - fakeErrorResultsAutofix; + fakeErrorResultsAutofix, + fakeConfigHelper; before(() => { - hashStub = sinon.stub(); + sandbox = sinon.createSandbox(); + + fakeConfigHelper = { + getConfig: sandbox.stub(), + options: { + cacheStrategy: "metadata" + } + }; + + hashStub = sandbox.stub(); let shouldFix = false; @@ -44,17 +58,25 @@ describe("LintResultCache", () => { }); // Get results without autofixing... - fakeErrorResults = cliEngine.executeOnFiles([path.join(fixturePath, "test-with-errors.js")]).results[0]; + fakeErrorResults = cliEngine.executeOnFiles([ + path.join(fixturePath, "test-with-errors.js") + ]).results[0]; // ...and with autofixing shouldFix = true; - fakeErrorResultsAutofix = cliEngine.executeOnFiles([path.join(fixturePath, "test-with-errors.js")]).results[0]; + fakeErrorResultsAutofix = cliEngine.executeOnFiles([ + path.join(fixturePath, "test-with-errors.js") + ]).results[0]; // Set up LintResultCache with fake fileEntryCache module - LintResultCache = proxyquire("../../../lib/cli-engine/lint-result-cache.js", { - "file-entry-cache": fileEntryCacheStubs, - "./hash": hashStub - }); + LintResultCache = proxyquire( + "../../../lib/cli-engine/lint-result-cache.js", + { + "file-entry-cache": fileEntryCacheStubs, + "./hash": hashStub + }, + { options: {} } + ); }); afterEach(done => { @@ -71,11 +93,32 @@ describe("LintResultCache", () => { describe("constructor", () => { it("should throw an error if cache file path is not provided", () => { - assert.throws(() => new LintResultCache(), /Cache file location is required/u); + assert.throws( + () => new LintResultCache(), + /Cache file location is required/u + ); + }); + + it("should throw an error if config helper is not provided", () => { + 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 is provided", () => { - const instance = new LintResultCache(cacheFileLocation); + it("should successfully create an instance if cache file location and config helper are provided", () => { + const instance = new LintResultCache(cacheFileLocation, fakeConfigHelper); assert.ok(instance, "Instance should have been created successfully"); }); @@ -85,9 +128,7 @@ describe("LintResultCache", () => { const filePath = path.join(fixturePath, "test-with-errors.js"); const hashOfConfig = "hashOfConfig"; - let cacheEntry, - getFileDescriptorStub, - lintResultsCache; + let cacheEntry, getFileDescriptorStub, lintResultsCache; before(() => { getFileDescriptorStub = sinon.stub(); @@ -112,12 +153,11 @@ describe("LintResultCache", () => { } }; - getFileDescriptorStub.withArgs(filePath) - .returns(cacheEntry); + getFileDescriptorStub.withArgs(filePath).returns(cacheEntry); fakeConfig = {}; - lintResultsCache = new LintResultCache(cacheFileLocation); + lintResultsCache = new LintResultCache(cacheFileLocation, fakeConfigHelper); }); describe("when calculating the hashing", () => { @@ -127,7 +167,7 @@ describe("LintResultCache", () => { "../../package.json": { version }, "./hash": hashStub }); - const newLintResultCache = new NewLintResultCache(cacheFileLocation); + const newLintResultCache = new NewLintResultCache(cacheFileLocation, fakeConfigHelper); newLintResultCache.getCachedLintResults(filePath, fakeConfig); assert.ok(hashStub.calledOnce); @@ -141,7 +181,7 @@ describe("LintResultCache", () => { const NewLintResultCache = proxyquire("../../../lib/cli-engine/lint-result-cache.js", { "./hash": hashStub }); - const newLintResultCache = new NewLintResultCache(cacheFileLocation); + const newLintResultCache = new NewLintResultCache(cacheFileLocation, fakeConfigHelper); newLintResultCache.getCachedLintResults(filePath, fakeConfig); assert.ok(hashStub.calledOnce); @@ -156,7 +196,10 @@ describe("LintResultCache", () => { }); it("should return null", () => { - const result = lintResultsCache.getCachedLintResults(filePath, fakeConfig); + const result = lintResultsCache.getCachedLintResults( + filePath, + fakeConfig + ); assert.ok(getFileDescriptorStub.calledOnce); assert.isNull(result); @@ -169,7 +212,10 @@ describe("LintResultCache", () => { }); it("should return null", () => { - const result = lintResultsCache.getCachedLintResults(filePath, fakeConfig); + const result = lintResultsCache.getCachedLintResults( + filePath, + fakeConfig + ); assert.ok(getFileDescriptorStub.calledOnce); assert.isNull(result); @@ -183,7 +229,10 @@ describe("LintResultCache", () => { }); it("should return null", () => { - const result = lintResultsCache.getCachedLintResults(filePath, fakeConfig); + const result = lintResultsCache.getCachedLintResults( + filePath, + fakeConfig + ); assert.ok(getFileDescriptorStub.calledOnce); assert.isNull(result); @@ -196,10 +245,16 @@ describe("LintResultCache", () => { }); it("should return expected results", () => { - const result = lintResultsCache.getCachedLintResults(filePath, fakeConfig); + const result = lintResultsCache.getCachedLintResults( + filePath, + fakeConfig + ); assert.deepStrictEqual(result, fakeErrorResults); - assert.ok(result.source, "source property should be hydrated from filesystem"); + assert.ok( + result.source, + "source property should be hydrated from filesystem" + ); }); }); }); @@ -208,9 +263,7 @@ describe("LintResultCache", () => { const filePath = path.join(fixturePath, "test-with-errors.js"); const hashOfConfig = "hashOfConfig"; - let cacheEntry, - getFileDescriptorStub, - lintResultsCache; + let cacheEntry, getFileDescriptorStub, lintResultsCache; before(() => { getFileDescriptorStub = sinon.stub(); @@ -229,19 +282,22 @@ describe("LintResultCache", () => { meta: {} }; - getFileDescriptorStub.withArgs(filePath) - .returns(cacheEntry); + getFileDescriptorStub.withArgs(filePath).returns(cacheEntry); fakeConfig = {}; hashStub.returns(hashOfConfig); - lintResultsCache = new LintResultCache(cacheFileLocation); + lintResultsCache = new LintResultCache(cacheFileLocation, fakeConfigHelper); }); describe("When lint result has output property", () => { it("does not modify file entry", () => { - lintResultsCache.setCachedLintResults(filePath, fakeConfig, fakeErrorResultsAutofix); + lintResultsCache.setCachedLintResults( + filePath, + fakeConfig, + fakeErrorResultsAutofix + ); assert.notProperty(cacheEntry.meta, "results"); assert.notProperty(cacheEntry.meta, "hashOfConfig"); @@ -254,7 +310,11 @@ describe("LintResultCache", () => { }); it("does not modify file entry", () => { - lintResultsCache.setCachedLintResults(filePath, fakeConfig, fakeErrorResults); + lintResultsCache.setCachedLintResults( + filePath, + fakeConfig, + fakeErrorResults + ); assert.notProperty(cacheEntry.meta, "results"); assert.notProperty(cacheEntry.meta, "hashOfConfig"); @@ -263,7 +323,11 @@ describe("LintResultCache", () => { describe("When file is found on filesystem", () => { beforeEach(() => { - lintResultsCache.setCachedLintResults(filePath, fakeConfig, fakeErrorResults); + lintResultsCache.setCachedLintResults( + filePath, + fakeConfig, + fakeErrorResults + ); }); it("stores hash of config in file entry", () => { @@ -271,11 +335,9 @@ describe("LintResultCache", () => { }); it("stores results (except source) in file entry", () => { - const expectedCachedResults = Object.assign( - {}, - fakeErrorResults, - { source: null } - ); + const expectedCachedResults = Object.assign({}, fakeErrorResults, { + source: null + }); assert.deepStrictEqual(cacheEntry.meta.results, expectedCachedResults); }); @@ -295,11 +357,9 @@ describe("LintResultCache", () => { }); it("stores results (except source) in file entry", () => { - const expectedCachedResults = Object.assign( - {}, - fakeErrorResults, - { source: null } - ); + const expectedCachedResults = Object.assign({}, fakeErrorResults, { + source: null + }); assert.deepStrictEqual(cacheEntry.meta.results, expectedCachedResults); }); @@ -307,8 +367,7 @@ describe("LintResultCache", () => { }); describe("reconcile", () => { - let reconcileStub, - lintResultsCache; + let reconcileStub, lintResultsCache; before(() => { reconcileStub = sinon.stub(); @@ -323,7 +382,7 @@ describe("LintResultCache", () => { }); beforeEach(() => { - lintResultsCache = new LintResultCache(cacheFileLocation); + lintResultsCache = new LintResultCache(cacheFileLocation, fakeConfigHelper); }); it("calls reconcile on the underlying cache", () => {