From 72cab0264d45cc9cef4dd51f5c0d94c59379f561 Mon Sep 17 00:00:00 2001 From: Francesco Trotta Date: Tue, 11 Oct 2022 21:00:18 +0200 Subject: [PATCH 1/3] fix: ignore messages without a `ruleId` in `getRulesMetaForResults` * `FlatESLint.prototype.getRulesMetaForResults` will now ignore messages without a `ruleId` instead of crashing with a `TypeError`. * Unit tests for `getRulesMetaForResults` added for both `FlatESLint` and `ESLint`. * The unit test "should return one rule meta when there is a suppressed linting error" was ported to `FlatESLint`. This was originally introduced for `ESLint` in #15459. Fixes #16402 --- lib/eslint/flat-eslint.js | 4 +++ tests/lib/eslint/eslint.js | 30 +++++++++++++++++++++ tests/lib/eslint/flat-eslint.js | 46 +++++++++++++++++++++++++++++++++ 3 files changed, 80 insertions(+) diff --git a/lib/eslint/flat-eslint.js b/lib/eslint/flat-eslint.js index c1ea80a3062..8cbb7346f56 100644 --- a/lib/eslint/flat-eslint.js +++ b/lib/eslint/flat-eslint.js @@ -700,6 +700,10 @@ class FlatESLint { const allMessages = result.messages.concat(result.suppressedMessages); for (const { ruleId } of allMessages) { + if (!ruleId) { + continue; + } + const rule = getRuleFromConfig(ruleId, config); // ensure the rule exists diff --git a/tests/lib/eslint/eslint.js b/tests/lib/eslint/eslint.js index 74f26cfa896..f1486e3356e 100644 --- a/tests/lib/eslint/eslint.js +++ b/tests/lib/eslint/eslint.js @@ -5051,6 +5051,36 @@ describe("ESLint", () => { nodePlugin.rules["no-new-require"].meta ); }); + + it("should ignore messages not related to a rule", async () => { + const engine = new ESLint({ + useEslintrc: false, + overrideConfig: { rules: { "no-var": "warn" } }, + reportUnusedDisableDirectives: "warn" + }); + + const results = [ + ...await engine.lintText("syntax error"), + ...await engine.lintText("// eslint-disable-line no-var"), + ...await engine.lintText("", { filePath: "/.ignored.js", warnIgnored: true }) + ]; + const rulesMeta = engine.getRulesMetaForResults(results); + + assert.deepStrictEqual(rulesMeta, {}); + }); + + it("should return a non-empty value if some of the messages are related to a rule", async () => { + const engine = new ESLint({ + useEslintrc: false, + overrideConfig: { rules: { "no-var": "warn" } }, + reportUnusedDisableDirectives: "warn" + }); + + const results = await engine.lintText("// eslint-disable-line no-var\nvar foo;"); + const rulesMeta = engine.getRulesMetaForResults(results); + + assert.deepStrictEqual(rulesMeta, { "no-var": coreRules.get("no-var").meta }); + }); }); describe("outputFixes()", () => { diff --git a/tests/lib/eslint/flat-eslint.js b/tests/lib/eslint/flat-eslint.js index 6db10be8b23..cca664da72b 100644 --- a/tests/lib/eslint/flat-eslint.js +++ b/tests/lib/eslint/flat-eslint.js @@ -3663,6 +3663,22 @@ describe("FlatESLint", () => { assert.strictEqual(rulesMeta.semi, coreRules.get("semi").meta); }); + it("should return one rule meta when there is a suppressed linting error", async () => { + const engine = new FlatESLint({ + overrideConfigFile: true, + overrideConfig: { + rules: { + semi: 2 + } + } + }); + + const results = await engine.lintText("a // eslint-disable-line semi"); + const rulesMeta = engine.getRulesMetaForResults(results); + + assert.strictEqual(rulesMeta.semi, coreRules.get("semi").meta); + }); + it("should return multiple rule meta when there are multiple linting errors", async () => { const engine = new FlatESLint({ overrideConfigFile: true, @@ -3707,6 +3723,36 @@ describe("FlatESLint", () => { nodePlugin.rules["no-new-require"].meta ); }); + + it("should ignore messages not related to a rule", async () => { + const engine = new FlatESLint({ + overrideConfigFile: true, + overrideConfig: { rules: { "no-var": "warn" } }, + reportUnusedDisableDirectives: "warn" + }); + + const results = [ + ...await engine.lintText("syntax error"), + ...await engine.lintText("// eslint-disable-line no-var"), + ...await engine.lintText("", { filePath: "/.ignored.js", warnIgnored: true }) + ]; + const rulesMeta = engine.getRulesMetaForResults(results); + + assert.deepStrictEqual(rulesMeta, {}); + }); + + it("should return a non-empty value if some of the messages are related to a rule", async () => { + const engine = new FlatESLint({ + overrideConfigFile: true, + overrideConfig: { rules: { "no-var": "warn" } }, + reportUnusedDisableDirectives: "warn" + }); + + const results = await engine.lintText("// eslint-disable-line no-var\nvar foo;"); + const rulesMeta = engine.getRulesMetaForResults(results); + + assert.deepStrictEqual(rulesMeta, { "no-var": coreRules.get("no-var").meta }); + }); }); describe("outputFixes()", () => { From 61768c8164af1820c15382486ee9819c7d56a52c Mon Sep 17 00:00:00 2001 From: Francesco Trotta Date: Fri, 14 Oct 2022 20:38:50 +0200 Subject: [PATCH 2/3] Update unit tests as suggested --- tests/lib/eslint/eslint.js | 26 +++++++++++++++++++------- tests/lib/eslint/flat-eslint.js | 26 +++++++++++++++++++------- 2 files changed, 38 insertions(+), 14 deletions(-) diff --git a/tests/lib/eslint/eslint.js b/tests/lib/eslint/eslint.js index f1486e3356e..cffcef097a2 100644 --- a/tests/lib/eslint/eslint.js +++ b/tests/lib/eslint/eslint.js @@ -4987,6 +4987,7 @@ describe("ESLint", () => { const results = await engine.lintText("a"); const rulesMeta = engine.getRulesMetaForResults(results); + assert.strictEqual(Object.keys(rulesMeta).length, 1); assert.strictEqual(rulesMeta.semi, coreRules.get("semi").meta); }); @@ -5003,6 +5004,7 @@ describe("ESLint", () => { const results = await engine.lintText("a // eslint-disable-line semi"); const rulesMeta = engine.getRulesMetaForResults(results); + assert.strictEqual(Object.keys(rulesMeta).length, 1); assert.strictEqual(rulesMeta.semi, coreRules.get("semi").meta); }); @@ -5059,14 +5061,24 @@ describe("ESLint", () => { reportUnusedDisableDirectives: "warn" }); - const results = [ - ...await engine.lintText("syntax error"), - ...await engine.lintText("// eslint-disable-line no-var"), - ...await engine.lintText("", { filePath: "/.ignored.js", warnIgnored: true }) - ]; - const rulesMeta = engine.getRulesMetaForResults(results); + { + const results = await engine.lintText("syntax error"); + const rulesMeta = engine.getRulesMetaForResults(results); + + assert.deepStrictEqual(rulesMeta, {}); + } + { + const results = await engine.lintText("// eslint-disable-line no-var"); + const rulesMeta = engine.getRulesMetaForResults(results); + + assert.deepStrictEqual(rulesMeta, {}); + } + { + const results = await engine.lintText("", { filePath: "/.ignored.js", warnIgnored: true }); + const rulesMeta = engine.getRulesMetaForResults(results); - assert.deepStrictEqual(rulesMeta, {}); + assert.deepStrictEqual(rulesMeta, {}); + } }); it("should return a non-empty value if some of the messages are related to a rule", async () => { diff --git a/tests/lib/eslint/flat-eslint.js b/tests/lib/eslint/flat-eslint.js index cca664da72b..443f87aaf77 100644 --- a/tests/lib/eslint/flat-eslint.js +++ b/tests/lib/eslint/flat-eslint.js @@ -3660,6 +3660,7 @@ describe("FlatESLint", () => { const results = await engine.lintText("a", { filePath: "foo.js" }); const rulesMeta = engine.getRulesMetaForResults(results); + assert.strictEqual(Object.keys(rulesMeta).length, 1); assert.strictEqual(rulesMeta.semi, coreRules.get("semi").meta); }); @@ -3676,6 +3677,7 @@ describe("FlatESLint", () => { const results = await engine.lintText("a // eslint-disable-line semi"); const rulesMeta = engine.getRulesMetaForResults(results); + assert.strictEqual(Object.keys(rulesMeta).length, 1); assert.strictEqual(rulesMeta.semi, coreRules.get("semi").meta); }); @@ -3731,14 +3733,24 @@ describe("FlatESLint", () => { reportUnusedDisableDirectives: "warn" }); - const results = [ - ...await engine.lintText("syntax error"), - ...await engine.lintText("// eslint-disable-line no-var"), - ...await engine.lintText("", { filePath: "/.ignored.js", warnIgnored: true }) - ]; - const rulesMeta = engine.getRulesMetaForResults(results); + { + const results = await engine.lintText("syntax error"); + const rulesMeta = engine.getRulesMetaForResults(results); + + assert.deepStrictEqual(rulesMeta, {}); + } + { + const results = await engine.lintText("// eslint-disable-line no-var"); + const rulesMeta = engine.getRulesMetaForResults(results); + + assert.deepStrictEqual(rulesMeta, {}); + } + { + const results = await engine.lintText("", { filePath: "/.ignored.js", warnIgnored: true }); + const rulesMeta = engine.getRulesMetaForResults(results); - assert.deepStrictEqual(rulesMeta, {}); + assert.deepStrictEqual(rulesMeta, {}); + } }); it("should return a non-empty value if some of the messages are related to a rule", async () => { From 1d6060cbaacd6a170499bbe2972abf43aa2a75df Mon Sep 17 00:00:00 2001 From: Francesco Trotta Date: Sun, 16 Oct 2022 18:47:44 +0200 Subject: [PATCH 3/3] Explicitly ignore a file path in a unit test for `getRulesMetaForResults` --- tests/lib/eslint/eslint.js | 9 +++++++-- tests/lib/eslint/flat-eslint.js | 9 +++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/tests/lib/eslint/eslint.js b/tests/lib/eslint/eslint.js index cffcef097a2..87d8bcdcf9c 100644 --- a/tests/lib/eslint/eslint.js +++ b/tests/lib/eslint/eslint.js @@ -5057,7 +5057,12 @@ describe("ESLint", () => { it("should ignore messages not related to a rule", async () => { const engine = new ESLint({ useEslintrc: false, - overrideConfig: { rules: { "no-var": "warn" } }, + overrideConfig: { + ignorePatterns: "ignored.js", + rules: { + "no-var": "warn" + } + }, reportUnusedDisableDirectives: "warn" }); @@ -5074,7 +5079,7 @@ describe("ESLint", () => { assert.deepStrictEqual(rulesMeta, {}); } { - const results = await engine.lintText("", { filePath: "/.ignored.js", warnIgnored: true }); + const results = await engine.lintText("", { filePath: "ignored.js", warnIgnored: true }); const rulesMeta = engine.getRulesMetaForResults(results); assert.deepStrictEqual(rulesMeta, {}); diff --git a/tests/lib/eslint/flat-eslint.js b/tests/lib/eslint/flat-eslint.js index 443f87aaf77..202517c1352 100644 --- a/tests/lib/eslint/flat-eslint.js +++ b/tests/lib/eslint/flat-eslint.js @@ -3729,7 +3729,12 @@ describe("FlatESLint", () => { it("should ignore messages not related to a rule", async () => { const engine = new FlatESLint({ overrideConfigFile: true, - overrideConfig: { rules: { "no-var": "warn" } }, + ignorePatterns: "ignored.js", + overrideConfig: { + rules: { + "no-var": "warn" + } + }, reportUnusedDisableDirectives: "warn" }); @@ -3746,7 +3751,7 @@ describe("FlatESLint", () => { assert.deepStrictEqual(rulesMeta, {}); } { - const results = await engine.lintText("", { filePath: "/.ignored.js", warnIgnored: true }); + const results = await engine.lintText("", { filePath: "ignored.js", warnIgnored: true }); const rulesMeta = engine.getRulesMetaForResults(results); assert.deepStrictEqual(rulesMeta, {});