diff --git a/lib/linter/report-translator.js b/lib/linter/report-translator.js index 7d2705206cd..41a43eadc3b 100644 --- a/lib/linter/report-translator.js +++ b/lib/linter/report-translator.js @@ -100,6 +100,22 @@ function normalizeReportLoc(descriptor) { return descriptor.node.loc; } +/** + * Clones the given fix object. + * @param {Fix|null} fix The fix to clone. + * @returns {Fix|null} Deep cloned fix object or `null` if `null` or `undefined` was passed in. + */ +function cloneFix(fix) { + if (!fix) { + return null; + } + + return { + range: [fix.range[0], fix.range[1]], + text: fix.text + }; +} + /** * Check that a fix has a valid range. * @param {Fix|null} fix The fix to validate. @@ -137,7 +153,7 @@ function mergeFixes(fixes, sourceCode) { return null; } if (fixes.length === 1) { - return fixes[0]; + return cloneFix(fixes[0]); } fixes.sort(compareFixesByRange); @@ -183,7 +199,7 @@ function normalizeFixes(descriptor, sourceCode) { } assertValidFix(fix); - return fix; + return cloneFix(fix); } /** diff --git a/tests/lib/linter/linter.js b/tests/lib/linter/linter.js index 1b8f93d410f..f86e98ba8e0 100644 --- a/tests/lib/linter/linter.js +++ b/tests/lib/linter/linter.js @@ -15917,6 +15917,171 @@ var a = "test2"; assert.strictEqual(suppressedMessages.length, 0); }); + + // https://github.com/eslint/eslint/issues/16716 + it("should receive unique range arrays in suggestions", () => { + const configs = [ + { + plugins: { + "test-processors": { + processors: { + "line-processor": (() => { + const blocksMap = new Map(); + + return { + preprocess(text, fileName) { + const lines = text.split("\n"); + + blocksMap.set(fileName, lines); + + return lines.map((line, index) => ({ + text: line, + filename: `${index}.js` + })); + }, + + postprocess(messageLists, fileName) { + const lines = blocksMap.get(fileName); + let rangeOffset = 0; + + // intentionaly mutates objects and arrays + messageLists.forEach((messages, index) => { + messages.forEach(message => { + message.line += index; + if (typeof message.endLine === "number") { + message.endLine += index; + } + if (message.fix) { + message.fix.range[0] += rangeOffset; + message.fix.range[1] += rangeOffset; + } + if (message.suggestions) { + message.suggestions.forEach(suggestion => { + suggestion.fix.range[0] += rangeOffset; + suggestion.fix.range[1] += rangeOffset; + }); + } + }); + rangeOffset += lines[index].length + 1; + }); + + return messageLists.flat(); + }, + + supportsAutofix: true + }; + })() + } + }, + + "test-rules": { + rules: { + "no-foo": { + meta: { + hasSuggestions: true, + messages: { + unexpected: "Don't use 'foo'.", + replaceWithBar: "Replace with 'bar'", + replaceWithBaz: "Replace with 'baz'" + } + + }, + create(context) { + return { + Identifier(node) { + const { range } = node; + + if (node.name === "foo") { + context.report({ + node, + messageId: "unexpected", + suggest: [ + { + messageId: "replaceWithBar", + fix: () => ({ range, text: "bar" }) + }, + { + messageId: "replaceWithBaz", + fix: () => ({ range, text: "baz" }) + } + ] + }); + } + } + }; + } + } + } + } + } + }, + { + files: ["**/*.txt"], + processor: "test-processors/line-processor" + }, + { + files: ["**/*.js"], + rules: { + "test-rules/no-foo": 2 + } + } + ]; + + const result = linter.verifyAndFix( + "var a = 5;\nvar foo;\nfoo = a;", + configs, + { filename: "a.txt" } + ); + + assert.deepStrictEqual(result.messages, [ + { + ruleId: "test-rules/no-foo", + severity: 2, + message: "Don't use 'foo'.", + line: 2, + column: 5, + nodeType: "Identifier", + messageId: "unexpected", + endLine: 2, + endColumn: 8, + suggestions: [ + { + messageId: "replaceWithBar", + fix: { range: [15, 18], text: "bar" }, + desc: "Replace with 'bar'" + }, + { + messageId: "replaceWithBaz", + fix: { range: [15, 18], text: "baz" }, + desc: "Replace with 'baz'" + } + ] + }, + { + ruleId: "test-rules/no-foo", + severity: 2, + message: "Don't use 'foo'.", + line: 3, + column: 1, + nodeType: "Identifier", + messageId: "unexpected", + endLine: 3, + endColumn: 4, + suggestions: [ + { + messageId: "replaceWithBar", + fix: { range: [20, 23], text: "bar" }, + desc: "Replace with 'bar'" + }, + { + messageId: "replaceWithBaz", + fix: { range: [20, 23], text: "baz" }, + desc: "Replace with 'baz'" + } + ] + } + ]); + }); }); }); diff --git a/tests/lib/linter/report-translator.js b/tests/lib/linter/report-translator.js index 6feabb31b96..18c62f20729 100644 --- a/tests/lib/linter/report-translator.js +++ b/tests/lib/linter/report-translator.js @@ -1091,4 +1091,183 @@ describe("createReportTranslator", () => { } }); }); + + // https://github.com/eslint/eslint/issues/16716 + describe("unique `fix` and `fix.range` objects", () => { + const range = [0, 3]; + const fix = { range, text: "baz" }; + const additionalRange = [4, 7]; + const additionalFix = { range: additionalRange, text: "qux" }; + + it("should deep clone returned fix object", () => { + const translatedReport = translateReport({ + node, + messageId: "testMessage", + fix: () => fix + }); + + assert.deepStrictEqual(translatedReport.fix, fix); + assert.notStrictEqual(translatedReport.fix, fix); + assert.notStrictEqual(translatedReport.fix.range, fix.range); + }); + + it("should create a new fix object with a new range array when `fix()` returns an array with a single item", () => { + const translatedReport = translateReport({ + node, + messageId: "testMessage", + fix: () => [fix] + }); + + assert.deepStrictEqual(translatedReport.fix, fix); + assert.notStrictEqual(translatedReport.fix, fix); + assert.notStrictEqual(translatedReport.fix.range, fix.range); + }); + + it("should create a new fix object with a new range array when `fix()` returns an array with multiple items", () => { + const translatedReport = translateReport({ + node, + messageId: "testMessage", + fix: () => [fix, additionalFix] + }); + + assert.notStrictEqual(translatedReport.fix, fix); + assert.notStrictEqual(translatedReport.fix.range, fix.range); + assert.notStrictEqual(translatedReport.fix, additionalFix); + assert.notStrictEqual(translatedReport.fix.range, additionalFix.range); + }); + + it("should create a new fix object with a new range array when `fix()` generator yields a single item", () => { + const translatedReport = translateReport({ + node, + messageId: "testMessage", + *fix() { + yield fix; + } + }); + + assert.deepStrictEqual(translatedReport.fix, fix); + assert.notStrictEqual(translatedReport.fix, fix); + assert.notStrictEqual(translatedReport.fix.range, fix.range); + }); + + it("should create a new fix object with a new range array when `fix()` generator yields multiple items", () => { + const translatedReport = translateReport({ + node, + messageId: "testMessage", + *fix() { + yield fix; + yield additionalFix; + } + }); + + assert.notStrictEqual(translatedReport.fix, fix); + assert.notStrictEqual(translatedReport.fix.range, fix.range); + assert.notStrictEqual(translatedReport.fix, additionalFix); + assert.notStrictEqual(translatedReport.fix.range, additionalFix.range); + }); + + it("should deep clone returned suggestion fix object", () => { + const translatedReport = translateReport({ + node, + messageId: "testMessage", + suggest: [{ + messageId: "suggestion1", + fix: () => fix + }] + }); + + assert.deepStrictEqual(translatedReport.suggestions[0].fix, fix); + assert.notStrictEqual(translatedReport.suggestions[0].fix, fix); + assert.notStrictEqual(translatedReport.suggestions[0].fix.range, fix.range); + }); + + it("should create a new fix object with a new range array when suggestion `fix()` returns an array with a single item", () => { + const translatedReport = translateReport({ + node, + messageId: "testMessage", + suggest: [{ + messageId: "suggestion1", + fix: () => [fix] + }] + }); + + assert.deepStrictEqual(translatedReport.suggestions[0].fix, fix); + assert.notStrictEqual(translatedReport.suggestions[0].fix, fix); + assert.notStrictEqual(translatedReport.suggestions[0].fix.range, fix.range); + }); + + it("should create a new fix object with a new range array when suggestion `fix()` returns an array with multiple items", () => { + const translatedReport = translateReport({ + node, + messageId: "testMessage", + suggest: [{ + messageId: "suggestion1", + fix: () => [fix, additionalFix] + }] + }); + + assert.notStrictEqual(translatedReport.suggestions[0].fix, fix); + assert.notStrictEqual(translatedReport.suggestions[0].fix.range, fix.range); + assert.notStrictEqual(translatedReport.suggestions[0].fix, additionalFix); + assert.notStrictEqual(translatedReport.suggestions[0].fix.range, additionalFix.range); + }); + + it("should create a new fix object with a new range array when suggestion `fix()` generator yields a single item", () => { + const translatedReport = translateReport({ + node, + messageId: "testMessage", + suggest: [{ + messageId: "suggestion1", + *fix() { + yield fix; + } + }] + }); + + assert.deepStrictEqual(translatedReport.suggestions[0].fix, fix); + assert.notStrictEqual(translatedReport.suggestions[0].fix, fix); + assert.notStrictEqual(translatedReport.suggestions[0].fix.range, fix.range); + }); + + it("should create a new fix object with a new range array when suggestion `fix()` generator yields multiple items", () => { + const translatedReport = translateReport({ + node, + messageId: "testMessage", + suggest: [{ + messageId: "suggestion1", + *fix() { + yield fix; + yield additionalFix; + } + }] + }); + + assert.notStrictEqual(translatedReport.suggestions[0].fix, fix); + assert.notStrictEqual(translatedReport.suggestions[0].fix.range, fix.range); + assert.notStrictEqual(translatedReport.suggestions[0].fix, additionalFix); + assert.notStrictEqual(translatedReport.suggestions[0].fix.range, additionalFix.range); + }); + + it("should create different instances of range arrays when suggestions reuse the same instance", () => { + const translatedReport = translateReport({ + node, + messageId: "testMessage", + suggest: [ + { + messageId: "suggestion1", + fix: () => ({ range, text: "baz" }) + }, + { + messageId: "suggestion2", + data: { interpolated: "'interpolated value'" }, + fix: () => ({ range, text: "qux" }) + } + ] + }); + + assert.deepStrictEqual(translatedReport.suggestions[0].fix.range, range); + assert.deepStrictEqual(translatedReport.suggestions[1].fix.range, range); + assert.notStrictEqual(translatedReport.suggestions[0].fix.range, translatedReport.suggestions[1].fix.range); + }); + }); });