From cf2c5a8297c12f1a5cb44a789b5fc201077179ca Mon Sep 17 00:00:00 2001 From: Francesco Trotta Date: Mon, 17 Oct 2022 14:04:35 +0200 Subject: [PATCH 1/4] fix: handle files with unspecified path in `getRulesMetaForResults` * In `getRulesMetaForResults`, files with an unspecified path are now treated as if they were located inside `cwd`. * In `getRulesMetaForResults`, when a result referencing a rule has no config, we will explicitly throw an error with a descriptive message. * Added top-level, internal functions `getPlaceholderPath` and `createExtraneousResultsError` to avoid code repetition. * Added two new unit tests. * Renamed an existing unit test to better disambiguate it from a new one. Also changed the assertion to check both error message and constructor. Fixes #16410 --- lib/eslint/flat-eslint.js | 48 ++++++++++++++++++++------- tests/lib/eslint/flat-eslint.js | 57 +++++++++++++++++++++++++++++++-- 2 files changed, 91 insertions(+), 14 deletions(-) diff --git a/lib/eslint/flat-eslint.js b/lib/eslint/flat-eslint.js index 479d5f14f2c..1bf84be38b9 100644 --- a/lib/eslint/flat-eslint.js +++ b/lib/eslint/flat-eslint.js @@ -161,6 +161,16 @@ function createRulesMeta(rules) { }, {}); } +/** + * Return the absolute path of a file named `"__placeholder__.js"` in a given directory. + * This is used as a replacement for a missing file path. + * @param {string} cwd An absolute directory path. + * @returns {string} The absolute path of a file named `"__placeholder__.js"` in the given directory. + */ +function getPlaceholderPath(cwd) { + return path.join(cwd, "__placeholder__.js"); +} + /** @type {WeakMap} */ const usedDeprecatedRulesCache = new WeakMap(); @@ -177,7 +187,7 @@ function getOrFindUsedDeprecatedRules(eslint, maybeFilePath) { } = privateMembers.get(eslint); const filePath = path.isAbsolute(maybeFilePath) ? maybeFilePath - : path.join(cwd, "__placeholder__.js"); + : getPlaceholderPath(cwd); const config = configs.getConfig(filePath); // Most files use the same config, so cache it. @@ -427,7 +437,7 @@ function verifyText({ * `config.extractConfig(filePath)` requires an absolute path, but `linter` * doesn't know CWD, so it gives `linter` an absolute path always. */ - const filePathToVerify = filePath === "" ? path.join(cwd, "__placeholder__.js") : filePath; + const filePathToVerify = filePath === "" ? getPlaceholderPath(cwd) : filePath; const { fixed, messages, output } = linter.verifyAndFix( text, configs, @@ -524,6 +534,14 @@ function *iterateRuleDeprecationWarnings(configs) { } } +/** + * Creates an error to be thrown when an array of results passed to `getRulesMetaForResults` was not created by the current engine. + * @returns {TypeError} An error object. + */ +function createExtraneousResultsError() { + return new TypeError("Results object was not created from this ESLint instance."); +} + //----------------------------------------------------------------------------- // Main API //----------------------------------------------------------------------------- @@ -660,7 +678,10 @@ class FlatESLint { } const resultRules = new Map(); - const { configs } = privateMembers.get(this); + const { + configs, + options: { cwd } + } = privateMembers.get(this); /* * We can only accurately return rules meta information for linting results if the @@ -669,7 +690,7 @@ class FlatESLint { * to let the user know we can't do anything here. */ if (!configs) { - throw new TypeError("Results object was not created from this ESLint instance."); + throw createExtraneousResultsError(); } for (const result of results) { @@ -678,13 +699,7 @@ class FlatESLint { * Normalize filename for . */ const filePath = result.filePath === "" - ? "__placeholder__.js" : result.filePath; - - /* - * All of the plugin and rule information is contained within the - * calculated config for the given file. - */ - const config = configs.getConfig(filePath); + ? getPlaceholderPath(cwd) : result.filePath; const allMessages = result.messages.concat(result.suppressedMessages); for (const { ruleId } of allMessages) { @@ -692,6 +707,15 @@ class FlatESLint { continue; } + /* + * All of the plugin and rule information is contained within the + * calculated config for the given file. + */ + const config = configs.getConfig(filePath); + + if (!config) { + throw createExtraneousResultsError(); + } const rule = getRuleFromConfig(ruleId, config); // ensure the rule exists @@ -1029,7 +1053,7 @@ class FlatESLint { const npmFormat = naming.normalizePackageName(normalizedFormatName, "eslint-formatter"); // TODO: This is pretty dirty...would be nice to clean up at some point. - formatterPath = ModuleResolver.resolve(npmFormat, path.join(cwd, "__placeholder__.js")); + formatterPath = ModuleResolver.resolve(npmFormat, getPlaceholderPath(cwd)); } catch { formatterPath = path.resolve(__dirname, "../", "cli-engine", "formatters", `${normalizedFormatName}.js`); } diff --git a/tests/lib/eslint/flat-eslint.js b/tests/lib/eslint/flat-eslint.js index 9e2b694f9c8..e7afaf4028c 100644 --- a/tests/lib/eslint/flat-eslint.js +++ b/tests/lib/eslint/flat-eslint.js @@ -3707,7 +3707,7 @@ describe("FlatESLint", () => { describe("getRulesMetaForResults()", () => { - it("should throw an error when results were not created from this instance", async () => { + it("should throw an error when this instance did not lint any files", async () => { const engine = new FlatESLint({ overrideConfigFile: true }); @@ -3744,7 +3744,42 @@ describe("FlatESLint", () => { "var err = doStuff();\nif (err) console.log('failed tests: ' + err);\nprocess.exit(1);\n" } ]); - }, /Results object was not created from this ESLint instance/u); + }, { + constructor: TypeError, + message: "Results object was not created from this ESLint instance." + }); + }); + + it("should throw an error when results were created from a different instance", async () => { + const engine1 = new FlatESLint({ + overrideConfigFile: true, + cwd: path.join(fixtureDir, "foo"), + overrideConfig: { + rules: { + semi: 2 + } + } + }); + const engine2 = new FlatESLint({ + overrideConfigFile: true, + cwd: path.join(fixtureDir, "bar"), + overrideConfig: { + rules: { + semi: 2 + } + } + }); + + const results1 = await engine1.lintText("1", { filePath: "file.js" }); + const results2 = await engine2.lintText("2", { filePath: "file.js" }); + + engine1.getRulesMetaForResults(results1); // should not throw an error + assert.throws(() => { + engine1.getRulesMetaForResults(results2); + }, { + constructor: TypeError, + message: "Results object was not created from this ESLint instance." + }); }); it("should return empty object when there are no linting errors", async () => { @@ -3880,6 +3915,24 @@ describe("FlatESLint", () => { assert.deepStrictEqual(rulesMeta, { "no-var": coreRules.get("no-var").meta }); }); + + it("should treat a result without `filePath` as if the file was located in `cwd`", async () => { + const engine = new FlatESLint({ + overrideConfigFile: true, + cwd: path.join(fixtureDir, "foo", "bar"), + ignorePatterns: "*/**", // ignore all subdirectories of `cwd` + overrideConfig: { + rules: { + eqeqeq: "warn" + } + } + }); + + const results = await engine.lintText("a==b"); + const rulesMeta = engine.getRulesMetaForResults(results); + + assert.deepStrictEqual(rulesMeta.eqeqeq, coreRules.get("eqeqeq").meta); + }); }); describe("outputFixes()", () => { From b756fda74655b8e6101b33b2e49720d5752f3e62 Mon Sep 17 00:00:00 2001 From: Francesco Trotta Date: Thu, 20 Oct 2022 12:26:49 +0200 Subject: [PATCH 2/4] Add a new unit test, move around an existing one --- tests/lib/eslint/flat-eslint.js | 49 +++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/tests/lib/eslint/flat-eslint.js b/tests/lib/eslint/flat-eslint.js index e7afaf4028c..1cdcc4cbcc9 100644 --- a/tests/lib/eslint/flat-eslint.js +++ b/tests/lib/eslint/flat-eslint.js @@ -3782,6 +3782,37 @@ describe("FlatESLint", () => { }); }); + it("should treat a result without `filePath` as if the file was located in `cwd`", async () => { + const engine = new FlatESLint({ + overrideConfigFile: true, + cwd: path.join(fixtureDir, "foo", "bar"), + ignorePatterns: "*/**", // ignore all subdirectories of `cwd` + overrideConfig: { + rules: { + eqeqeq: "warn" + } + } + }); + + const results = await engine.lintText("a==b"); + const rulesMeta = engine.getRulesMetaForResults(results); + + assert.deepStrictEqual(rulesMeta.eqeqeq, coreRules.get("eqeqeq").meta); + }); + + it("should not throw an error if a result without `filePath` contains an ignored file warning", async () => { + const engine = new FlatESLint({ + overrideConfigFile: true, + cwd: path.join(fixtureDir, "foo", "bar"), + ignorePatterns: "**" + }); + + const results = await engine.lintText("", { warnIgnored: true }); + const rulesMeta = engine.getRulesMetaForResults(results); + + assert.deepStrictEqual(rulesMeta, {}); + }); + it("should return empty object when there are no linting errors", async () => { const engine = new FlatESLint({ overrideConfigFile: true @@ -3915,24 +3946,6 @@ describe("FlatESLint", () => { assert.deepStrictEqual(rulesMeta, { "no-var": coreRules.get("no-var").meta }); }); - - it("should treat a result without `filePath` as if the file was located in `cwd`", async () => { - const engine = new FlatESLint({ - overrideConfigFile: true, - cwd: path.join(fixtureDir, "foo", "bar"), - ignorePatterns: "*/**", // ignore all subdirectories of `cwd` - overrideConfig: { - rules: { - eqeqeq: "warn" - } - } - }); - - const results = await engine.lintText("a==b"); - const rulesMeta = engine.getRulesMetaForResults(results); - - assert.deepStrictEqual(rulesMeta.eqeqeq, coreRules.get("eqeqeq").meta); - }); }); describe("outputFixes()", () => { From 31d8e86215b2c614e28a6688305f8a36e3d869f7 Mon Sep 17 00:00:00 2001 From: Francesco Trotta Date: Mon, 24 Oct 2022 09:32:37 +0200 Subject: [PATCH 3/4] Add a unit test with linted and ignored files --- tests/lib/eslint/flat-eslint.js | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/lib/eslint/flat-eslint.js b/tests/lib/eslint/flat-eslint.js index 1cdcc4cbcc9..e2e231aa9ed 100644 --- a/tests/lib/eslint/flat-eslint.js +++ b/tests/lib/eslint/flat-eslint.js @@ -3813,6 +3813,26 @@ describe("FlatESLint", () => { assert.deepStrictEqual(rulesMeta, {}); }); + it("should not throw an error if results contain linted files and one ignored file", async () => { + const engine = new FlatESLint({ + overrideConfigFile: true, + cwd: getFixturePath(), + ignorePatterns: "passing*", + overrideConfig: { + rules: { + "no-undef": 2, + semi: 1 + } + } + }); + + const results = await engine.lintFiles(["missing-semicolon.js", "passing.js", "undef.js"]); + const rulesMeta = engine.getRulesMetaForResults(results); + + assert.deepStrictEqual(rulesMeta["no-undef"], coreRules.get("no-undef").meta); + assert.deepStrictEqual(rulesMeta.semi, coreRules.get("semi").meta); + }); + it("should return empty object when there are no linting errors", async () => { const engine = new FlatESLint({ overrideConfigFile: true From 90b7df20f25d547a96691731619bc743ecd4e94c Mon Sep 17 00:00:00 2001 From: Francesco Trotta Date: Sat, 29 Oct 2022 13:45:39 +0200 Subject: [PATCH 4/4] Assert that there is an ignored file --- tests/lib/eslint/flat-eslint.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/lib/eslint/flat-eslint.js b/tests/lib/eslint/flat-eslint.js index e2e231aa9ed..5fb55c39556 100644 --- a/tests/lib/eslint/flat-eslint.js +++ b/tests/lib/eslint/flat-eslint.js @@ -3827,6 +3827,12 @@ describe("FlatESLint", () => { }); const results = await engine.lintFiles(["missing-semicolon.js", "passing.js", "undef.js"]); + + assert( + results.some(({ messages }) => messages.some(({ message, ruleId }) => !ruleId && message.startsWith("File ignored"))), + "At least one file should be ignored but none is." + ); + const rulesMeta = engine.getRulesMetaForResults(results); assert.deepStrictEqual(rulesMeta["no-undef"], coreRules.get("no-undef").meta);