diff --git a/lib/cli-engine/lint-result-cache.js b/lib/cli-engine/lint-result-cache.js index e36eb74bada..97d2ee40b39 100644 --- a/lib/cli-engine/lint-result-cache.js +++ b/lib/cli-engine/lint-result-cache.js @@ -128,16 +128,28 @@ class LintResultCache { return null; } + const cachedResults = fileDescriptor.meta.results; + + // Just in case, not sure if this can ever happen. + if (!cachedResults) { + return cachedResults; + } + + /* + * Shallow clone the object to ensure that any properties added or modified afterwards + * will not be accidentally stored in the cache file when `reconcile()` is called. + * https://github.com/eslint/eslint/issues/13507 + * All intentional changes to the cache file must be done through `setCachedLintResults()`. + */ + const results = { ...cachedResults }; + // If source is present but null, need to reread the file from the filesystem. - if ( - fileDescriptor.meta.results && - fileDescriptor.meta.results.source === null - ) { + if (results.source === null) { debug(`Rereading cached result source from filesystem: ${filePath}`); - fileDescriptor.meta.results.source = fs.readFileSync(filePath, "utf-8"); + results.source = fs.readFileSync(filePath, "utf-8"); } - return fileDescriptor.meta.results; + return results; } /** diff --git a/tests/lib/eslint/eslint.js b/tests/lib/eslint/eslint.js index b9db3543f7c..0753cf685c2 100644 --- a/tests/lib/eslint/eslint.js +++ b/tests/lib/eslint/eslint.js @@ -2993,6 +2993,105 @@ describe("ESLint", () => { assert.deepStrictEqual(result, cachedResult, "result should be the same with or without cache"); }); + // https://github.com/eslint/eslint/issues/13507 + it("should not store `usedDeprecatedRules` in the cache file", async () => { + cacheFilePath = getFixturePath(".eslintcache"); + doDelete(cacheFilePath); + assert(!shell.test("-f", cacheFilePath), "the cache file already exists and wasn't successfully deleted"); + + const deprecatedRuleId = "space-in-parens"; + + eslint = new ESLint({ + cwd: path.join(fixtureDir, ".."), + useEslintrc: false, + + // specifying cache true the cache will be created + cache: true, + cacheLocation: cacheFilePath, + overrideConfig: { + rules: { + [deprecatedRuleId]: 2 + } + } + }); + + const filePath = fs.realpathSync(getFixturePath("cache/src", "test-file.js")); + + /* + * Run linting on the same file 3 times to cover multiple cases: + * Run 1: Lint result wasn't already cached. + * Run 2: Lint result was already cached. The cached lint result is used but the cache is reconciled before the run ends. + * Run 3: Lint result was already cached. The cached lint result was being used throughout the previous run, so possible + * mutations in the previous run that occured after the cache was reconciled may have side effects for this run. + */ + for (let i = 0; i < 3; i++) { + const [result] = await eslint.lintFiles([filePath]); + + assert( + result.usedDeprecatedRules && result.usedDeprecatedRules.some(rule => rule.ruleId === deprecatedRuleId), + "the deprecated rule should have been in result.usedDeprecatedRules" + ); + + assert(shell.test("-f", cacheFilePath), "the cache for eslint should have been created"); + + const fileCache = fCache.create(cacheFilePath); + const descriptor = fileCache.getFileDescriptor(filePath); + + assert(typeof descriptor === "object", "an entry for the file should have been in the cache file"); + assert(typeof descriptor.meta.results === "object", "lint result for the file should have been in its cache entry in the cache file"); + assert(typeof descriptor.meta.results.usedDeprecatedRules === "undefined", "lint result in the cache file contains `usedDeprecatedRules`"); + } + + }); + + // https://github.com/eslint/eslint/issues/13507 + it("should store `source` as `null` in the cache file if the lint result has `source` property", async () => { + cacheFilePath = getFixturePath(".eslintcache"); + doDelete(cacheFilePath); + assert(!shell.test("-f", cacheFilePath), "the cache file already exists and wasn't successfully deleted"); + + eslint = new ESLint({ + cwd: path.join(fixtureDir, ".."), + useEslintrc: false, + + // specifying cache true the cache will be created + cache: true, + cacheLocation: cacheFilePath, + overrideConfig: { + rules: { + "no-unused-vars": 2 + } + } + }); + + const filePath = fs.realpathSync(getFixturePath("cache/src", "fail-file.js")); + + /* + * Run linting on the same file 3 times to cover multiple cases: + * Run 1: Lint result wasn't already cached. + * Run 2: Lint result was already cached. The cached lint result is used but the cache is reconciled before the run ends. + * Run 3: Lint result was already cached. The cached lint result was being used throughout the previous run, so possible + * mutations in the previous run that occured after the cache was reconciled may have side effects for this run. + */ + for (let i = 0; i < 3; i++) { + const [result] = await eslint.lintFiles([filePath]); + + assert(typeof result.source === "string", "the result should have contained the `source` property"); + + assert(shell.test("-f", cacheFilePath), "the cache for eslint should have been created"); + + const fileCache = fCache.create(cacheFilePath); + const descriptor = fileCache.getFileDescriptor(filePath); + + assert(typeof descriptor === "object", "an entry for the file should have been in the cache file"); + assert(typeof descriptor.meta.results === "object", "lint result for the file should have been in its cache entry in the cache file"); + + // if the lint result contains `source`, it should be stored as `null` in the cache file + assert.strictEqual(descriptor.meta.results.source, null, "lint result in the cache file contains non-null `source`"); + } + + }); + describe("cacheStrategy", () => { it("should detect changes using a file's modification time when set to 'metadata'", async () => { cacheFilePath = getFixturePath(".eslintcache"); diff --git a/tests/lib/eslint/flat-eslint.js b/tests/lib/eslint/flat-eslint.js index f9f36d909fd..443452bc096 100644 --- a/tests/lib/eslint/flat-eslint.js +++ b/tests/lib/eslint/flat-eslint.js @@ -2876,6 +2876,105 @@ describe("FlatESLint", () => { assert.deepStrictEqual(result, cachedResult, "result should be the same with or without cache"); }); + // https://github.com/eslint/eslint/issues/13507 + it("should not store `usedDeprecatedRules` in the cache file", async () => { + cacheFilePath = getFixturePath(".eslintcache"); + doDelete(cacheFilePath); + assert(!shell.test("-f", cacheFilePath), "the cache file already exists and wasn't successfully deleted"); + + const deprecatedRuleId = "space-in-parens"; + + eslint = new FlatESLint({ + cwd: path.join(fixtureDir, ".."), + overrideConfigFile: true, + + // specifying cache true the cache will be created + cache: true, + cacheLocation: cacheFilePath, + overrideConfig: { + rules: { + [deprecatedRuleId]: 2 + } + } + }); + + const filePath = fs.realpathSync(getFixturePath("cache/src", "test-file.js")); + + /* + * Run linting on the same file 3 times to cover multiple cases: + * Run 1: Lint result wasn't already cached. + * Run 2: Lint result was already cached. The cached lint result is used but the cache is reconciled before the run ends. + * Run 3: Lint result was already cached. The cached lint result was being used throughout the previous run, so possible + * mutations in the previous run that occured after the cache was reconciled may have side effects for this run. + */ + for (let i = 0; i < 3; i++) { + const [result] = await eslint.lintFiles([filePath]); + + assert( + result.usedDeprecatedRules && result.usedDeprecatedRules.some(rule => rule.ruleId === deprecatedRuleId), + "the deprecated rule should have been in result.usedDeprecatedRules" + ); + + assert(shell.test("-f", cacheFilePath), "the cache for eslint should have been created"); + + const fileCache = fCache.create(cacheFilePath); + const descriptor = fileCache.getFileDescriptor(filePath); + + assert(typeof descriptor === "object", "an entry for the file should have been in the cache file"); + assert(typeof descriptor.meta.results === "object", "lint result for the file should have been in its cache entry in the cache file"); + assert(typeof descriptor.meta.results.usedDeprecatedRules === "undefined", "lint result in the cache file contains `usedDeprecatedRules`"); + } + + }); + + // https://github.com/eslint/eslint/issues/13507 + it("should store `source` as `null` in the cache file if the lint result has `source` property", async () => { + cacheFilePath = getFixturePath(".eslintcache"); + doDelete(cacheFilePath); + assert(!shell.test("-f", cacheFilePath), "the cache file already exists and wasn't successfully deleted"); + + eslint = new FlatESLint({ + cwd: path.join(fixtureDir, ".."), + overrideConfigFile: true, + + // specifying cache true the cache will be created + cache: true, + cacheLocation: cacheFilePath, + overrideConfig: { + rules: { + "no-unused-vars": 2 + } + } + }); + + const filePath = fs.realpathSync(getFixturePath("cache/src", "fail-file.js")); + + /* + * Run linting on the same file 3 times to cover multiple cases: + * Run 1: Lint result wasn't already cached. + * Run 2: Lint result was already cached. The cached lint result is used but the cache is reconciled before the run ends. + * Run 3: Lint result was already cached. The cached lint result was being used throughout the previous run, so possible + * mutations in the previous run that occured after the cache was reconciled may have side effects for this run. + */ + for (let i = 0; i < 3; i++) { + const [result] = await eslint.lintFiles([filePath]); + + assert(typeof result.source === "string", "the result should have contained the `source` property"); + + assert(shell.test("-f", cacheFilePath), "the cache for eslint should have been created"); + + const fileCache = fCache.create(cacheFilePath); + const descriptor = fileCache.getFileDescriptor(filePath); + + assert(typeof descriptor === "object", "an entry for the file should have been in the cache file"); + assert(typeof descriptor.meta.results === "object", "lint result for the file should have been in its cache entry in the cache file"); + + // if the lint result contains `source`, it should be stored as `null` in the cache file + assert.strictEqual(descriptor.meta.results.source, null, "lint result in the cache file contains non-null `source`"); + } + + }); + describe("cacheStrategy", () => { it("should detect changes using a file's modification time when set to 'metadata'", async () => { cacheFilePath = getFixturePath(".eslintcache");