Docs: add note and example for extending the range of fix (refs #13706) (#13748)

mdjermanovic · web-flow · commit f885fe06a0a7 · 2021-10-08T17:38:34.000-07:00
* Chore: add tests for extending fix ranges (refs #13706) * Add note and example in working-with-rules
diff --git a/docs/developer-guide/working-with-rules.md b/docs/developer-guide/working-with-rules.md
@@ -352,6 +352,24 @@ Best practices for fixes:
 
     * This fixer can just select a quote type arbitrarily. If it guesses wrong, the resulting code will be automatically reported and fixed by the [`quotes`](/docs/rules/quotes.md) rule.
 
+Note: Making fixes as small as possible is a best practice, but in some cases it may be correct to extend the range of the fix in order to intentionally prevent other rules from making fixes in a surrounding range in the same pass. For instance, if replacement text declares a new variable, it can be useful to prevent other changes in the scope of the variable as they might cause name collisions.
+
+The following example replaces `node` and also ensures that no other fixes will be applied in the range of `node.parent` in the same pass:
+
+```js
+context.report({
+    node,
+    message,
+    *fix(fixer) {
+        yield fixer.replaceText(node, replacementText);
+
+        // extend range of the fix to the range of `node.parent`
+        yield fixer.insertTextBefore(node.parent, "");
+        yield fixer.insertTextAfter(node.parent, "");
+    }
+});
+```
+
 ### Providing Suggestions
 
 In some cases fixes aren't appropriate to be automatically applied, for example, if a fix potentially changes functionality or if there are multiple valid ways to fix a rule depending on the implementation intent (see the best practices for [applying fixes](#applying-fixes) listed above). In these cases, there is an alternative `suggest` option on `context.report()` that allows other tools, such as editors, to expose helpers for users to manually apply a suggestion.
diff --git a/tests/lib/linter/report-translator.js b/tests/lib/linter/report-translator.js
@@ -368,6 +368,35 @@ describe("createReportTranslator", () => {
             );
         });
 
+        it("should respect ranges of empty insertions when merging fixes to one.", () => {
+            const reportDescriptor = {
+                node,
+                loc: location,
+                message,
+                *fix() {
+                    yield { range: [4, 5], text: "cd" };
+                    yield { range: [2, 2], text: "" };
+                    yield { range: [7, 7], text: "" };
+                }
+            };
+
+            assert.deepStrictEqual(
+                translateReport(reportDescriptor),
+                {
+                    ruleId: "foo-rule",
+                    severity: 2,
+                    message: "foo",
+                    line: 2,
+                    column: 1,
+                    nodeType: "ExpressionStatement",
+                    fix: {
+                        range: [2, 7],
+                        text: "o\ncdar"
+                    }
+                }
+            );
+        });
+
         it("should pass through fixes if only one is present", () => {
             const reportDescriptor = {
                 node,
diff --git a/tests/lib/linter/rule-fixer.js b/tests/lib/linter/rule-fixer.js
@@ -30,6 +30,17 @@ describe("RuleFixer", () => {
 
         });
 
+        it("should allow inserting empty text", () => {
+
+            const result = ruleFixer.insertTextBefore({ range: [10, 20] }, "");
+
+            assert.deepStrictEqual(result, {
+                range: [10, 10],
+                text: ""
+            });
+
+        });
+
     });
 
     describe("insertTextBeforeRange", () => {
@@ -45,6 +56,17 @@ describe("RuleFixer", () => {
 
         });
 
+        it("should allow inserting empty text", () => {
+
+            const result = ruleFixer.insertTextBeforeRange([10, 20], "");
+
+            assert.deepStrictEqual(result, {
+                range: [10, 10],
+                text: ""
+            });
+
+        });
+
     });
 
     describe("insertTextAfter", () => {
@@ -60,6 +82,17 @@ describe("RuleFixer", () => {
 
         });
 
+        it("should allow inserting empty text", () => {
+
+            const result = ruleFixer.insertTextAfter({ range: [10, 20] }, "");
+
+            assert.deepStrictEqual(result, {
+                range: [20, 20],
+                text: ""
+            });
+
+        });
+
     });
 
     describe("insertTextAfterRange", () => {
@@ -75,6 +108,17 @@ describe("RuleFixer", () => {
 
         });
 
+        it("should allow inserting empty text", () => {
+
+            const result = ruleFixer.insertTextAfterRange([10, 20], "");
+
+            assert.deepStrictEqual(result, {
+                range: [20, 20],
+                text: ""
+            });
+
+        });
+
     });
 
     describe("removeAfter", () => {
