From bfddcedace5587d662c840c2edf33062b54a178e Mon Sep 17 00:00:00 2001 From: Milos Djermanovic Date: Sat, 24 Oct 2020 01:40:21 +0200 Subject: [PATCH] Update: remove suggestion if it didn't provide a fix (fixes #13723) (#13772) --- docs/developer-guide/working-with-rules.md | 2 + lib/linter/report-translator.js | 22 ++- tests/lib/linter/report-translator.js | 218 +++++++++++++++++++++ 3 files changed, 233 insertions(+), 9 deletions(-) diff --git a/docs/developer-guide/working-with-rules.md b/docs/developer-guide/working-with-rules.md index 2b814e7a98b..89d37c483a4 100644 --- a/docs/developer-guide/working-with-rules.md +++ b/docs/developer-guide/working-with-rules.md @@ -382,6 +382,8 @@ Best practices for suggestions: 1. Don't try to do too much and suggest large refactors that could introduce a lot of breaking changes. 1. As noted above, don't try to conform to user-defined styles. +Suggestions are intended to provide fixes. ESLint will automatically remove the whole suggestion from the linting output if the suggestion's `fix` function returned `null` or an empty array/sequence. + #### Suggestion `messageId`s Instead of using a `desc` key for suggestions a `messageId` can be used instead. This works the same way as `messageId`s for the overall error (see [messageIds](#messageIds)). Here is an example of how to use it in a rule: diff --git a/lib/linter/report-translator.js b/lib/linter/report-translator.js index eef5165585b..bed5af81e5d 100644 --- a/lib/linter/report-translator.js +++ b/lib/linter/report-translator.js @@ -196,15 +196,19 @@ function mapSuggestions(descriptor, sourceCode, messages) { return []; } - return descriptor.suggest.map(suggestInfo => { - const computedDesc = suggestInfo.desc || messages[suggestInfo.messageId]; - - return { - ...suggestInfo, - desc: interpolate(computedDesc, suggestInfo.data), - fix: normalizeFixes(suggestInfo, sourceCode) - }; - }); + return descriptor.suggest + .map(suggestInfo => { + const computedDesc = suggestInfo.desc || messages[suggestInfo.messageId]; + + return { + ...suggestInfo, + desc: interpolate(computedDesc, suggestInfo.data), + fix: normalizeFixes(suggestInfo, sourceCode) + }; + }) + + // Remove suggestions that didn't provide a fix + .filter(({ fix }) => fix); } /** diff --git a/tests/lib/linter/report-translator.js b/tests/lib/linter/report-translator.js index a79400cf7e4..4d137b578ba 100644 --- a/tests/lib/linter/report-translator.js +++ b/tests/lib/linter/report-translator.js @@ -553,6 +553,224 @@ describe("createReportTranslator", () => { } ); }); + + it("should remove the whole suggestion if 'fix' function returned `null`.", () => { + const reportDescriptor = { + node, + loc: location, + message, + suggest: [{ + desc: "A suggestion for the issue", + fix: () => null + }] + }; + + assert.deepStrictEqual( + translateReport(reportDescriptor), + { + ruleId: "foo-rule", + severity: 2, + message: "foo", + line: 2, + column: 1, + nodeType: "ExpressionStatement" + } + ); + }); + + it("should remove the whole suggestion if 'fix' function returned an empty array.", () => { + const reportDescriptor = { + node, + loc: location, + message, + suggest: [{ + desc: "A suggestion for the issue", + fix: () => [] + }] + }; + + assert.deepStrictEqual( + translateReport(reportDescriptor), + { + ruleId: "foo-rule", + severity: 2, + message: "foo", + line: 2, + column: 1, + nodeType: "ExpressionStatement" + } + ); + }); + + it("should remove the whole suggestion if 'fix' function returned an empty sequence.", () => { + const reportDescriptor = { + node, + loc: location, + message, + suggest: [{ + desc: "A suggestion for the issue", + *fix() {} + }] + }; + + assert.deepStrictEqual( + translateReport(reportDescriptor), + { + ruleId: "foo-rule", + severity: 2, + message: "foo", + line: 2, + column: 1, + nodeType: "ExpressionStatement" + } + ); + }); + + // This isn't offically supported, but autofix works the same way + it("should remove the whole suggestion if 'fix' function didn't return anything.", () => { + const reportDescriptor = { + node, + loc: location, + message, + suggest: [{ + desc: "A suggestion for the issue", + fix() {} + }] + }; + + assert.deepStrictEqual( + translateReport(reportDescriptor), + { + ruleId: "foo-rule", + severity: 2, + message: "foo", + line: 2, + column: 1, + nodeType: "ExpressionStatement" + } + ); + }); + + it("should keep suggestion before a removed suggestion.", () => { + const reportDescriptor = { + node, + loc: location, + message, + suggest: [{ + desc: "Suggestion with a fix", + fix: () => ({ range: [1, 2], text: "foo" }) + }, { + desc: "Suggestion without a fix", + fix: () => null + }] + }; + + assert.deepStrictEqual( + translateReport(reportDescriptor), + { + ruleId: "foo-rule", + severity: 2, + message: "foo", + line: 2, + column: 1, + nodeType: "ExpressionStatement", + suggestions: [{ + desc: "Suggestion with a fix", + fix: { range: [1, 2], text: "foo" } + }] + } + ); + }); + + it("should keep suggestion after a removed suggestion.", () => { + const reportDescriptor = { + node, + loc: location, + message, + suggest: [{ + desc: "Suggestion without a fix", + fix: () => null + }, { + desc: "Suggestion with a fix", + fix: () => ({ range: [1, 2], text: "foo" }) + }] + }; + + assert.deepStrictEqual( + translateReport(reportDescriptor), + { + ruleId: "foo-rule", + severity: 2, + message: "foo", + line: 2, + column: 1, + nodeType: "ExpressionStatement", + suggestions: [{ + desc: "Suggestion with a fix", + fix: { range: [1, 2], text: "foo" } + }] + } + ); + }); + + it("should remove multiple suggestions that didn't provide a fix and keep those that did.", () => { + const reportDescriptor = { + node, + loc: location, + message, + suggest: [{ + desc: "Keep #1", + fix: () => ({ range: [1, 2], text: "foo" }) + }, { + desc: "Remove #1", + fix() { + return null; + } + }, { + desc: "Keep #2", + fix: () => ({ range: [1, 2], text: "bar" }) + }, { + desc: "Remove #2", + fix() { + return []; + } + }, { + desc: "Keep #3", + fix: () => ({ range: [1, 2], text: "baz" }) + }, { + desc: "Remove #3", + *fix() {} + }, { + desc: "Keep #4", + fix: () => ({ range: [1, 2], text: "quux" }) + }] + }; + + assert.deepStrictEqual( + translateReport(reportDescriptor), + { + ruleId: "foo-rule", + severity: 2, + message: "foo", + line: 2, + column: 1, + nodeType: "ExpressionStatement", + suggestions: [{ + desc: "Keep #1", + fix: { range: [1, 2], text: "foo" } + }, { + desc: "Keep #2", + fix: { range: [1, 2], text: "bar" } + }, { + desc: "Keep #3", + fix: { range: [1, 2], text: "baz" } + }, { + desc: "Keep #4", + fix: { range: [1, 2], text: "quux" } + }] + } + ); + }); }); describe("message interpolation", () => {