From f885fe06a0a79d91fc72a132fd31edf9ef0502cd Mon Sep 17 00:00:00 2001 From: Milos Djermanovic Date: Sat, 9 Oct 2021 02:38:34 +0200 Subject: [PATCH] Docs: add note and example for extending the range of fix (refs #13706) (#13748) * Chore: add tests for extending fix ranges (refs #13706) * Add note and example in working-with-rules --- docs/developer-guide/working-with-rules.md | 18 +++++++++ tests/lib/linter/report-translator.js | 29 ++++++++++++++ tests/lib/linter/rule-fixer.js | 44 ++++++++++++++++++++++ 3 files changed, 91 insertions(+) diff --git a/docs/developer-guide/working-with-rules.md b/docs/developer-guide/working-with-rules.md index a0e9e4b2c2b..735439f685f 100644 --- 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 index 1235d555e05..6feabb31b96 100644 --- 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 index a70e735509c..7cc350b6152 100644 --- 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", () => {