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.

Sign up for GitHub

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

Jump to bottom

feat: Implement caching for FlatESLint #16190

Merged
merged 1 commit into from Aug 14, 2022
Merged

feat: Implement caching for FlatESLint #16190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
65 changes: 53 additions & 12 deletions lib/config/flat-config-array.js
View file
Open in desktop
Expand Up @@ -139,31 +139,72 @@ class FlatConfigArray extends ConfigArray {
[ConfigArraySymbol.finalizeConfig](config) {

const { plugins, languageOptions, processor } = config;
let parserName, processorName;
let invalidParser = false,
invalidProcessor = false;

// Check parser value
if (languageOptions && languageOptions.parser && typeof languageOptions.parser === "string") {
const { pluginName, objectName: parserName } = splitPluginIdentifier(languageOptions.parser);
if (languageOptions && languageOptions.parser) {
if (typeof languageOptions.parser === "string") {
const { pluginName, objectName: localParserName } = splitPluginIdentifier(languageOptions.parser);

if (!plugins || !plugins[pluginName] || !plugins[pluginName].parsers || !plugins[pluginName].parsers[parserName]) {
throw new TypeError(`Key "parser": Could not find "${parserName}" in plugin "${pluginName}".`);
}
parserName = languageOptions.parser;

if (!plugins || !plugins[pluginName] || !plugins[pluginName].parsers || !plugins[pluginName].parsers[localParserName]) {
throw new TypeError(`Key "parser": Could not find "${localParserName}" in plugin "${pluginName}".`);
}

languageOptions.parser = plugins[pluginName].parsers[parserName];
languageOptions.parser = plugins[pluginName].parsers[localParserName];
} else {
invalidParser = true;
}
}

// Check processor value
if (processor && typeof processor === "string") {
const { pluginName, objectName: processorName } = splitPluginIdentifier(processor);
if (processor) {
if (typeof processor === "string") {
const { pluginName, objectName: localProcessorName } = splitPluginIdentifier(processor);

if (!plugins || !plugins[pluginName] || !plugins[pluginName].processors || !plugins[pluginName].processors[processorName]) {
throw new TypeError(`Key "processor": Could not find "${processorName}" in plugin "${pluginName}".`);
}
processorName = processor;

if (!plugins || !plugins[pluginName] || !plugins[pluginName].processors || !plugins[pluginName].processors[localProcessorName]) {
throw new TypeError(`Key "processor": Could not find "${localProcessorName}" in plugin "${pluginName}".`);
}

config.processor = plugins[pluginName].processors[processorName];
config.processor = plugins[pluginName].processors[localProcessorName];
} else {
invalidProcessor = true;
}
}

ruleValidator.validate(config);

// apply special logic for serialization into JSON
/* eslint-disable object-shorthand -- shorthand would change "this" value */
Object.defineProperty(config, "toJSON", {
value: function() {

if (invalidParser) {
throw new Error("Caching is not supported when parser is an object.");
}

if (invalidProcessor) {
throw new Error("Caching is not supported when processor is an object.");
}

return {
...this,
plugins: Object.keys(plugins),
languageOptions: {
...languageOptions,
parser: parserName
},
processor: processorName
};
}
});
/* eslint-enable object-shorthand -- ok to enable now */

return config;
}
/* eslint-enable class-methods-use-this -- Desired as instance method */
Expand Down
3 changes: 0 additions & 3 deletions lib/eslint/eslint-helpers.js
View file
Open in desktop
Expand Up @@ -436,9 +436,6 @@ function processOptions({
if (typeof cache !== "boolean") {
errors.push("'cache' must be a boolean.");
}
if (cache) {
errors.push("'cache' option is not yet supported.");
}
if (!isNonEmptyString(cacheLocation)) {
errors.push("'cacheLocation' must be a non-empty string.");
}
Expand Down
15 changes: 15 additions & 0 deletions lib/eslint/flat-eslint.js
View file
Open in desktop
Expand Up @@ -30,6 +30,7 @@ const {
const {
fileExists,
findFiles,
getCacheFile,

isNonEmptyString,
isArrayOfNonEmptyString,
Expand All @@ -41,6 +42,7 @@ const {
} = require("./eslint-helpers");
const { pathToFileURL } = require("url");
const { FlatConfigArray } = require("../config/flat-config-array");
const LintResultCache = require("../cli-engine/lint-result-cache");

/*
* This is necessary to allow overwriting writeFile for testing purposes.
Expand Down Expand Up @@ -606,9 +608,20 @@ class FlatESLint {
configType: "flat"
});

const cacheFilePath = getCacheFile(
processedOptions.cacheLocation,
processedOptions.cwd
);

const lintResultCache = processedOptions.cache
? new LintResultCache(cacheFilePath, processedOptions.cacheStrategy)
: null;

privateMembers.set(this, {
options: processedOptions,
linter,
cacheFilePath,
lintResultCache,
defaultConfigs,
defaultIgnores: () => false,
configs: null
Expand Down Expand Up @@ -782,6 +795,8 @@ class FlatESLint {

// Delete cache file; should this be done here?
if (!cache && cacheFilePath) {
debug(`Deleting cache file at ${cacheFilePath}`);

try {
await fs.unlink(cacheFilePath);
} catch (error) {
Expand Down
73 changes: 73 additions & 0 deletions tests/lib/config/flat-config-array.js
View file
Open in desktop
Expand Up @@ -13,6 +13,7 @@ const { FlatConfigArray } = require("../../../lib/config/flat-config-array");
const assert = require("chai").assert;
const allConfig = require("../../../conf/eslint-all");
const recommendedConfig = require("../../../conf/eslint-recommended");
const stringify = require("json-stable-stringify-without-jsonify");

//-----------------------------------------------------------------------------
// Helpers
Expand Down Expand Up @@ -182,6 +183,78 @@ describe("FlatConfigArray", () => {
assert.notStrictEqual(base[0].languageOptions.parserOptions, config.languageOptions.parserOptions, "parserOptions should be new object");
});

describe("Serialization of configs", () => {
it("should convert config into normalized JSON object", () => {

const configs = new FlatConfigArray([{
plugins: {
a: {},
b: {}
}
}]);

configs.normalizeSync();

const config = configs.getConfig("foo.js");
const expected = {
plugins: ["@", "a", "b"],
languageOptions: {
ecmaVersion: "latest",
sourceType: "module",
parser: "@/espree",
parserOptions: {}
},
processor: void 0
};
const actual = config.toJSON();

assert.deepStrictEqual(actual, expected);

assert.strictEqual(stringify(actual), stringify(expected));
});

it("should throw an error when config with parser object is normalized", () => {

const configs = new FlatConfigArray([{
languageOptions: {
parser: {
parse() { /* empty */ }
}
}
}]);

configs.normalizeSync();

const config = configs.getConfig("foo.js");

assert.throws(() => {
config.toJSON();
}, /Caching is not supported/u);

});

it("should throw an error when config with processor object is normalized", () => {

const configs = new FlatConfigArray([{
processor: {
preprocess() { /* empty */ },
postprocess() { /* empty */ }
}
}]);

configs.normalizeSync();

const config = configs.getConfig("foo.js");

assert.throws(() => {
config.toJSON();
}, /Caching is not supported/u);

});


});

describe("Special configs", () => {
it("eslint:recommended is replaced with an actual config", async () => {
const configs = new FlatConfigArray(["eslint:recommended"]);
Expand Down
44 changes: 27 additions & 17 deletions tests/lib/eslint/flat-eslint.js
View file
Open in desktop
Expand Up @@ -1396,7 +1396,7 @@ describe("FlatESLint", () => {
});

// Cannot be run properly until cache is implemented
xit("should run autofix even if files are cached without autofix results", async () => {
it("should run autofix even if files are cached without autofix results", async () => {
const baseOptions = {
cwd: path.join(fixtureDir, ".."),
overrideConfigFile: true,
Expand Down Expand Up @@ -1470,7 +1470,7 @@ describe("FlatESLint", () => {
});
});

xdescribe("cache", () => {
describe("cache", () => {

/**
* helper method to delete a file without caring about exceptions
Expand Down Expand Up @@ -1609,11 +1609,15 @@ describe("FlatESLint", () => {
assert(shell.test("-f", path.resolve(cwd, ".eslintcache")), "the cache for eslint was created at provided cwd");
});

it("should invalidate the cache if the configuration changed between executions", async () => {
assert(!shell.test("-f", path.resolve(".eslintcache")), "the cache for eslint does not exist");
it("should invalidate the cache if the overrideConfig changed between executions", async () => {
const cwd = getFixturePath("cache/src");
const cacheLocation = path.resolve(cwd, ".eslintcache");

assert(!shell.test("-f", cacheLocation), "the cache for eslint does not exist");

eslint = new FlatESLint({
overrideConfigFile: true,
cwd,

// specifying cache true the cache will be created
cache: true,
Expand All @@ -1627,24 +1631,26 @@ describe("FlatESLint", () => {
ignore: false
});

let spy = sinon.spy(fs, "readFileSync");
let spy = sinon.spy(fs.promises, "readFile");

let file = getFixturePath("cache/src", "test-file.js");
let file = path.join(cwd, "test-file.js");

file = fs.realpathSync(file);
const results = await eslint.lintFiles([file]);

for (const { errorCount, warningCount } of results) {
assert.strictEqual(errorCount + warningCount, 0, "the file passed without errors or warnings");
}
assert.strictEqual(spy.getCall(0).args[0], file, "the module read the file because is considered changed");
assert(shell.test("-f", path.resolve(".eslintcache")), "the cache for eslint was created");

assert(spy.calledWith(file), "ESLint should have read the file because it's considered changed");
assert(shell.test("-f", cacheLocation), "the cache for eslint should still exist");

// destroy the spy
sinon.restore();

eslint = new FlatESLint({
overrideConfigFile: true,
cwd,

// specifying cache true the cache will be created
cache: true,
Expand All @@ -1659,20 +1665,23 @@ describe("FlatESLint", () => {
});

// create a new spy
spy = sinon.spy(fs, "readFileSync");
spy = sinon.spy(fs.promises, "readFile");

const [cachedResult] = await eslint.lintFiles([file]);

assert.strictEqual(spy.getCall(0).args[0], file, "the module read the file because is considered changed because the config changed");
assert.strictEqual(cachedResult.errorCount, 1, "since configuration changed the cache was not used an one error was reported");
assert(shell.test("-f", path.resolve(".eslintcache")), "the cache for eslint was created");
assert(spy.calledWith(file), "ESLint should have read the file again because is considered changed because the config changed");
assert.strictEqual(cachedResult.errorCount, 1, "since configuration changed the cache was not used and one error was reported");
assert(shell.test("-f", cacheLocation), "The cache for ESLint should still exist (2)");
});

it("should remember the files from a previous run and do not operate on them if not changed", async () => {
assert(!shell.test("-f", path.resolve(".eslintcache")), "the cache for eslint does not exist");

const cwd = getFixturePath("cache/src");
const cacheLocation = path.resolve(cwd, ".eslintcache");

eslint = new FlatESLint({
overrideConfigFile: true,
cwd,

// specifying cache true the cache will be created
cache: true,
Expand All @@ -1686,22 +1695,23 @@ describe("FlatESLint", () => {
ignore: false
});

let spy = sinon.spy(fs, "readFileSync");
let spy = sinon.spy(fs.promises, "readFile");

let file = getFixturePath("cache/src", "test-file.js");

file = fs.realpathSync(file);

const result = await eslint.lintFiles([file]);

assert.strictEqual(spy.getCall(0).args[0], file, "the module read the file because is considered changed");
assert(shell.test("-f", path.resolve(".eslintcache")), "the cache for eslint was created");
assert(spy.calledWith(file), "the module read the file because is considered changed");
assert(shell.test("-f", cacheLocation), "the cache for eslint was created");

// destroy the spy
sinon.restore();

eslint = new FlatESLint({
overrideConfigFile: true,
cwd,

// specifying cache true the cache will be created
cache: true,
Expand All @@ -1716,7 +1726,7 @@ describe("FlatESLint", () => {
});

// create a new spy
spy = sinon.spy(fs, "readFileSync");
spy = sinon.spy(fs.promises, "readFile");

const cachedResult = await eslint.lintFiles([file]);

Expand Down