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.

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

fix: use baseConfig constructor option in FlatESLint #16432

Merged
merged 2 commits into from Oct 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
33 changes: 14 additions & 19 deletions lib/eslint/flat-eslint.js
Expand Up @@ -262,24 +262,16 @@ function findFlatConfigFile(cwd) {
/**
* Load the config array from the given filename.
* @param {string} filePath The filename to load from.
* @param {Object} options Options to help load the config file.
* @param {string} options.basePath The base path for the config array.
* @param {boolean} options.shouldIgnore Whether to honor ignore patterns.
* @returns {Promise<FlatConfigArray>} The config array loaded from the config file.
* @returns {Promise<any>} The config loaded from the config file.
*/
async function loadFlatConfigFile(filePath, { basePath, shouldIgnore }) {
async function loadFlatConfigFile(filePath) {
debug(`Loading config from ${filePath}`);

const fileURL = pathToFileURL(filePath);

debug(`Config file URL is ${fileURL}`);

const module = await import(fileURL);

return new FlatConfigArray(module.default, {
basePath,
shouldIgnore
});
return (await import(fileURL)).default;
}

/**
Expand All @@ -290,6 +282,7 @@ async function loadFlatConfigFile(filePath, { basePath, shouldIgnore }) {
*/
async function calculateConfigArray(eslint, {
cwd,
baseConfig,
overrideConfig,
configFile,
ignore: shouldIgnore,
Expand Down Expand Up @@ -321,16 +314,18 @@ async function calculateConfigArray(eslint, {
basePath = path.resolve(path.dirname(configFilePath));
}

// load config array
let configs;

const configs = new FlatConfigArray(baseConfig || [], { basePath, shouldIgnore });

// load config file
if (configFilePath) {
configs = await loadFlatConfigFile(configFilePath, {
basePath,
shouldIgnore
});
} else {
configs = new FlatConfigArray([], { basePath, shouldIgnore });
const fileConfig = await loadFlatConfigFile(configFilePath);

if (Array.isArray(fileConfig)) {
configs.push(...fileConfig);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double checking: This may not be the best approach because basePath for a config file may not match the basePath that was passed in. Can you verify?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By basePath that was passed in, do you mean overrideConfigFile: <string> where the override config file is in another directory? We decided that in that case basePath should be cwd (#15683).

basePath is calculated and set here:

let configFilePath;
let basePath = cwd;
if (typeof configFile === "string") {
debug(`Override config file path is ${configFile}`);
configFilePath = path.resolve(cwd, configFile);
} else if (configFile !== false) {
debug("Searching for eslint.config.js");
configFilePath = await findFlatConfigFile(cwd);
if (!configFilePath) {
throw new Error("Could not find config file.");
}
basePath = path.resolve(path.dirname(configFilePath));
}

There are three cases:

  • Normal eslint.config.js lookup (overrideConfigFile: false, default): basePath = the directory where eslint.config.js was found.
  • Use an alternative config file (overrideConfigFile: <string>): basePath = cwd
  • Don't use any config files (overrideConfigFile: true): basePath = cwd

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. 👍 just wanted to double check.

} else {
configs.push(fileConfig);
}
}

// add in any configured defaults
Expand Down
5 changes: 5 additions & 0 deletions tests/fixtures/eslint.config_with_rules.js
@@ -0,0 +1,5 @@
module.exports = [{
rules: {
quotes: ["error", "single"]
}
}];
188 changes: 188 additions & 0 deletions tests/lib/eslint/flat-eslint.js
Expand Up @@ -4663,4 +4663,192 @@ describe("FlatESLint", () => {
});
});

describe("baseConfig", () => {
it("can be an object", async () => {
const eslint = new FlatESLint({
overrideConfigFile: true,
baseConfig: {
rules: {
semi: 2
}
}
});

const [{ messages }] = await eslint.lintText("foo");

assert.strictEqual(messages.length, 1);
assert.strictEqual(messages[0].ruleId, "semi");
});

it("can be an array", async () => {
const eslint = new FlatESLint({
overrideConfigFile: true,
baseConfig: [
{
rules: {
"no-var": 2
}
},
{
rules: {
semi: 2
}
}
]
});

const [{ messages }] = await eslint.lintText("var foo");

assert.strictEqual(messages.length, 2);
assert.strictEqual(messages[0].ruleId, "no-var");
assert.strictEqual(messages[1].ruleId, "semi");
});

it("should be inserted after default configs", async () => {
const eslint = new FlatESLint({
overrideConfigFile: true,
baseConfig: {
languageOptions: {
ecmaVersion: 5,
sourceType: "script"
}
}
});

const [{ messages }] = await eslint.lintText("let x");

/*
* if baseConfig was inserted before default configs,
* `ecmaVersion: "latest"` from default configs would overwrite
* `ecmaVersion: 5` from baseConfig, so this wouldn't be a parsing error.
*/

assert.strictEqual(messages.length, 1);
assert(messages[0].fatal, "Fatal error expected.");
});

it("should be inserted before configs from the config file", async () => {
const eslint = new FlatESLint({
cwd: getFixturePath(),
baseConfig: {
rules: {
strict: ["error", "global"]
},
languageOptions: {
sourceType: "script"
}
}
});

const [{ messages }] = await eslint.lintText("foo");

/*
* if baseConfig was inserted after configs from the config file,
* `strict: 0` from eslint.config.js wouldn't overwrite `strict: ["error", "global"]`
* from baseConfig, so there would be an error message from the `strict` rule.
*/

assert.strictEqual(messages.length, 0);
});

it("should be inserted before overrideConfig", async () => {
const eslint = new FlatESLint({
overrideConfigFile: true,
baseConfig: {
rules: {
semi: 2
}
},
overrideConfig: {
rules: {
semi: 1
}
}
});

const [{ messages }] = await eslint.lintText("foo");

assert.strictEqual(messages.length, 1);
assert.strictEqual(messages[0].ruleId, "semi");
assert.strictEqual(messages[0].severity, 1);
});

it("should be inserted before configs from the config file and overrideConfig", async () => {
const eslint = new FlatESLint({
overrideConfigFile: getFixturePath("eslint.config_with_rules.js"),
baseConfig: {
rules: {
quotes: ["error", "double"],
semi: "error"
}
},
overrideConfig: {
rules: {
quotes: "warn"
}
}
});

const [{ messages }] = await eslint.lintText('const foo = "bar"');

/*
* baseConfig: { quotes: ["error", "double"], semi: "error" }
* eslint.config_with_rules.js: { quotes: ["error", "single"] }
* overrideConfig: { quotes: "warn" }
*
* Merged config: { quotes: ["warn", "single"], semi: "error" }
*/

assert.strictEqual(messages.length, 2);
assert.strictEqual(messages[0].ruleId, "quotes");
assert.strictEqual(messages[0].severity, 1);
assert.strictEqual(messages[1].ruleId, "semi");
assert.strictEqual(messages[1].severity, 2);
});

it("when it has 'files' they should be intepreted as relative to the config file", async () => {

/*
* `fixtures/plugins` directory does not have a config file.
* It's parent directory `fixtures` does have a config file, so
* the base path will be `fixtures`, cwd will be `fixtures/plugins`
*/
const eslint = new FlatESLint({
cwd: getFixturePath("plugins"),
baseConfig: {
files: ["plugins/a.js"],
rules: {
semi: 2
}
}
});

const [{ messages }] = await eslint.lintText("foo", { filePath: getFixturePath("plugins/a.js") });

assert.strictEqual(messages.length, 1);
assert.strictEqual(messages[0].ruleId, "semi");
});

it("when it has 'ignores' they should be intepreted as relative to the config file", async () => {

/*
* `fixtures/plugins` directory does not have a config file.
* It's parent directory `fixtures` does have a config file, so
* the base path will be `fixtures`, cwd will be `fixtures/plugins`
*/
const eslint = new FlatESLint({
cwd: getFixturePath("plugins"),
baseConfig: {
ignores: ["plugins/a.js"]
}
});

const [{ messages }] = await eslint.lintText("foo", { filePath: getFixturePath("plugins/a.js"), warnIgnored: true });

assert.strictEqual(messages.length, 1);
assert.strictEqual(messages[0].severity, 1);
assert.match(messages[0].message, /ignored/u);
});
});

});