From f62ec8d30d925e70e4d0d40640857c587ac2e116 Mon Sep 17 00:00:00 2001 From: Jacob Bandes-Storch Date: Thu, 11 Mar 2021 14:05:17 -0600 Subject: [PATCH] Update: throw error when fix range is invalid (#14142) * Do nothing when fix range is invalid * Move validation to rule-fixer and add error logging * Move validation to report-translator * Update from review * Test ranges with undefined; test fix merging * Assert & test missing range --- lib/linter/report-translator.js | 17 ++++++++++++++ tests/lib/linter/report-translator.js | 33 +++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/lib/linter/report-translator.js b/lib/linter/report-translator.js index bed5af81e5d..75005c16e58 100644 --- a/lib/linter/report-translator.js +++ b/lib/linter/report-translator.js @@ -115,6 +115,17 @@ function normalizeReportLoc(descriptor) { return descriptor.node.loc; } +/** + * Check that a fix has a valid range. + * @param {Fix|null} fix The fix to validate. + * @returns {void} + */ +function assertValidFix(fix) { + if (fix) { + assert(fix.range && typeof fix.range[0] === "number" && typeof fix.range[1] === "number", `Fix has invalid range: ${JSON.stringify(fix, null, 2)}`); + } +} + /** * Compares items in a fixes array by range. * @param {Fix} a The first message. @@ -133,6 +144,10 @@ function compareFixesByRange(a, b) { * @returns {{text: string, range: number[]}} The merged fixes */ function mergeFixes(fixes, sourceCode) { + for (const fix of fixes) { + assertValidFix(fix); + } + if (fixes.length === 0) { return null; } @@ -181,6 +196,8 @@ function normalizeFixes(descriptor, sourceCode) { if (fix && Symbol.iterator in fix) { return mergeFixes(Array.from(fix), sourceCode); } + + assertValidFix(fix); return fix; } diff --git a/tests/lib/linter/report-translator.js b/tests/lib/linter/report-translator.js index 6c7252b48f6..c6dd242e330 100644 --- a/tests/lib/linter/report-translator.js +++ b/tests/lib/linter/report-translator.js @@ -1028,5 +1028,38 @@ describe("createReportTranslator", () => { "Node must be provided when reporting error if location is not provided" ); }); + + it("should throw an error if fix range is invalid", () => { + assert.throws( + () => translateReport({ node, messageId: "testMessage", fix: () => ({ text: "foo" }) }), + "Fix has invalid range" + ); + + for (const badRange of [[0], [0, null], [null, 0], [void 0, 1], [0, void 0], [void 0, void 0], []]) { + assert.throws( + // eslint-disable-next-line no-loop-func + () => translateReport( + { node, messageId: "testMessage", fix: () => ({ range: badRange, text: "foo" }) } + ), + "Fix has invalid range" + ); + + assert.throws( + // eslint-disable-next-line no-loop-func + () => translateReport( + { + node, + messageId: "testMessage", + fix: () => [ + { range: [0, 0], text: "foo" }, + { range: badRange, text: "bar" }, + { range: [1, 1], text: "baz" } + ] + } + ), + "Fix has invalid range" + ); + } + }); }); });