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

feat!: Update shouldUseFlatConfig and CLI so flat config is default #17748

Merged
merged 13 commits into from
Dec 20, 2023
Merged

feat!: Update shouldUseFlatConfig and CLI so flat config is default #17748

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
d0dea8f
feat!: Update shouldUseFlatConfig so flat config is default
nzakas Nov 10, 2023
df69890
Fix tests
nzakas Nov 10, 2023
208d2bc
Update CLI
nzakas Nov 10, 2023
99e5f75
Fix test
nzakas Nov 10, 2023
8ac03fa
Fix tests
nzakas Nov 13, 2023
728b4bc
Fix lint warning
nzakas Nov 13, 2023
7b359b0
Update bin tests to search for eslint.config.js
nzakas Nov 27, 2023
4fc794c
Update tests/lib/cli.js
nzakas Nov 27, 2023
bbf0119
Update tests/lib/cli.js
nzakas Nov 27, 2023
8b30806
Update tests/lib/cli.js
nzakas Nov 27, 2023
265105f
Merge tests for deprecation warnings
nzakas Nov 27, 2023
35ccf30
Update lib/cli.js
nzakas Dec 8, 2023
4816e25
Update tests/lib/cli.js
nzakas Dec 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Update CLI
  • Loading branch information
@nzakas
nzakas committed Dec 20, 2023
commit 208d2bcbfd624a18294aa3122f41e56bdf0f62a4
8 changes: 6 additions & 2 deletions lib/cli.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,10 +304,10 @@ const cli = {
* Executes the CLI based on an array of arguments that is passed in.
* @param {string|Array|Object} args The arguments to process.
* @param {string} [text] The text to lint (used for TTY).
* @param {boolean} [allowFlatConfig] Whether or not to allow flat config.
* @param {boolean} [allowFlatConfig=true] Whether or not to allow flat config.
* @returns {Promise<number>} The exit code for the operation.
*/
async execute(args, text, allowFlatConfig) {
async execute(args, text, allowFlatConfig = true) {
if (Array.isArray(args)) {
debug("CLI args: %o", args.slice(2));
}
Expand All @@ -323,6 +323,10 @@ const cli = {

debug("Using flat config?", usingFlatConfig);

if (!usingFlatConfig) {
nzakas marked this conversation as resolved.
Show resolved Hide resolved
process.emitWarning("You are using an eslintrc configuration file, which is deprecated and support will be removed in v10.0.0. Please migrate to an eslint.config.js file. See https://eslint.org/docs/latest/use/configure/migration-guide for details.", "ESLintRCWarning");
}

const CLIOptions = createCLIOptions(usingFlatConfig);

/** @type {ParsedCLIOptions} */
Expand Down
45 changes: 34 additions & 11 deletions tests/lib/cli.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1480,7 +1480,7 @@ describe("cli", () => {

describe("when given a config file", () => {
it("should load the specified config file", async () => {
const configPath = getFixturePath(".eslintrc");
const configPath = getFixturePath("eslint.config.js");
const filePath = getFixturePath("passing.js");

await cli.execute(`--config ${configPath} ${filePath}`);
Expand All @@ -1498,7 +1498,7 @@ describe("cli", () => {
const filePath = getFixturePath("globals-browser.js");
const code = `--config ${configPath} ${filePath}`;

const exit = await cli.execute(code);
const exit = await cli.execute(code, null, false);

assert.strictEqual(exit, 0);
});
Expand All @@ -1510,7 +1510,7 @@ describe("cli", () => {
const filePath = getFixturePath("globals-node.js");
const code = `--config ${configPath} ${filePath}`;

const exit = await cli.execute(code);
const exit = await cli.execute(code, null, false);

assert.strictEqual(exit, 0);
});
Expand All @@ -1522,7 +1522,7 @@ describe("cli", () => {
const filePath = getFixturePath("globals-nashorn.js");
const code = `--config ${configPath} ${filePath}`;

const exit = await cli.execute(code);
const exit = await cli.execute(code, null, false);

assert.strictEqual(exit, 0);
});
Expand All @@ -1534,7 +1534,7 @@ describe("cli", () => {
const filePath = getFixturePath("globals-webextensions.js");
const code = `--config ${configPath} ${filePath}`;

const exit = await cli.execute(code);
const exit = await cli.execute(code, null, false);

assert.strictEqual(exit, 0);
});
Expand All @@ -1549,7 +1549,7 @@ describe("cli", () => {
const code = `--rulesdir ${rulesPath} --config ${configPath} --no-ignore ${filePath}`;

await stdAssert.rejects(async () => {
const exit = await cli.execute(code);
const exit = await cli.execute(code, null, false);

assert.strictEqual(exit, 2);
}, /Error while loading rule 'custom-rule': Boom!/u);
Expand All @@ -1561,7 +1561,7 @@ describe("cli", () => {
const filePath = getFixturePath("rules", "test", "test-custom-rule.js");
const code = `--rulesdir ${rulesPath} --config ${configPath} --no-ignore ${filePath}`;

await cli.execute(code);
await cli.execute(code, null, false);

assert.isTrue(log.info.calledOnce);
assert.isTrue(log.info.neverCalledWith(""));
Expand All @@ -1573,7 +1573,7 @@ describe("cli", () => {
const configPath = getFixturePath("rules", "multi-rulesdirs.json");
const filePath = getFixturePath("rules", "test-multi-rulesdirs.js");
const code = `--rulesdir ${rulesPath} --rulesdir ${rulesPath2} --config ${configPath} --no-ignore ${filePath}`;
const exit = await cli.execute(code);
const exit = await cli.execute(code, null, false);

const call = log.info.getCall(0);

Expand All @@ -1591,17 +1591,40 @@ describe("cli", () => {
describe("when executing with no-eslintrc flag", () => {
it("should ignore a local config file", async () => {
const filePath = getFixturePath("eslintrc", "quotes.js");
const exit = await cli.execute(`--no-eslintrc --no-ignore ${filePath}`);
const exit = await cli.execute(`--no-eslintrc --no-ignore ${filePath}`, null, false);

assert.isTrue(log.info.notCalled);
assert.strictEqual(exit, 0);
});
});

describe("when using an eslintrc config file", () => {

let processStub;

beforeEach(() => {
processStub = sinon.stub(process, "emitWarning");
});

afterEach(() => {
processStub.restore();
});

it("should emit deprecation warning", async () => {
const configPath = getFixturePath(".eslintrc");
const filePath = getFixturePath("passing.js");

await cli.execute(`--config ${configPath} ${filePath}`, null, false);
Copy link
Member

Choose a reason for hiding this comment

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

After these changes, npm test will be logging more than 100 deprecation warnings until we drop eslintrc in v10.

Can we avoid that?

allowFlatConfig = false is passed only from tests, so we could update the condition in lib/cli.js to avoid logging warnings while running tests:

-if (!usingFlatConfig) {
+if (allowFlatConfig && !usingFlatConfig) {
    process.emitWarning("You are using an eslintrc configuration file, which is deprecated and support will be removed in v10.0.0. Please migrate to an eslint.config.js file. See https://eslint.org/docs/latest/use/configure/migration-guide for details.", "ESLintRCWarning");
}

Then, we could update this test this way to make it work:

Suggested change
await cli.execute(`--config ${configPath} ${filePath}`, null, false);
const localCLI = proxyquire("../../lib/cli", {
"./eslint/flat-eslint": { shouldUseFlatConfig: () => Promise.resolve(false) }
});
await localCLI.execute(`--config ${configPath} ${filePath}`);

Or, perhaps even better, merge this test into the test for ESLINT_USE_FLAT_CONFIG=false as that's anyway the most realistic test:

it(`should not use it when ESLINT_USE_FLAT_CONFIG=false even if an eslint.config.js is present:${configType}`, async () => {
    process.env.ESLINT_USE_FLAT_CONFIG = "false";
    process.cwd = getFixturePath;

    const exitCode = await cli.execute(`--no-ignore --ext .js ${getFixturePath("files")}`, null, useFlatConfig);

    assert.strictEqual(exitCode, 0);

+   if (useFlatConfig) {
+       assert.strictEqual(processStub.callCount, 1, "calls `process.emitWarning()` once");
+       assert.strictEqual(processStub.getCall(0).args[1], "ESLintRCWarning");
+   }
});

That would leave npm test with zero eslintrc deprecation warnings (assuming #17797 will be merged; those tests pass allowFlatConfig = true but mistakenly still work in eslintc mode, so they're mistakenly logging deprecation warnings).

Copy link
Member Author

Choose a reason for hiding this comment

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

Good suggestion, I went with the second approach.


assert.strictEqual(processStub.callCount, 1, "calls `process.emitWarning()` once");
assert.strictEqual(processStub.getCall(0).args[1], "ESLintRCWarning");
});
});

describe("when executing without no-eslintrc flag", () => {
it("should load a local config file", async () => {
const filePath = getFixturePath("eslintrc", "quotes.js");
const exit = await cli.execute(`--no-ignore ${filePath}`);
const exit = await cli.execute(`--no-ignore ${filePath}`, null, false);

assert.isTrue(log.info.calledOnce);
assert.strictEqual(exit, 1);
Expand All @@ -1615,7 +1638,7 @@ describe("cli", () => {
getFixturePath("globals-node.js")
];

await cli.execute(`--no-eslintrc --config ./packages/js/src/configs/eslint-recommended.js --no-ignore ${files.join(" ")}`);
await cli.execute(`--no-eslintrc --config ./packages/js/src/configs/eslint-recommended.js --no-ignore ${files.join(" ")}`, null, false);

assert.strictEqual(log.info.args[0][0].split("\n").length, 10);
});
Expand Down