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
eslint · Mar 11, 2021 · f62ec8d · f62ec8d
1 parent 0eecad2
commit f62ec8d
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 0 deletions.
diff --git 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
@@ -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"
+                );
+            }
+        });
     });
 });