Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from Oct 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 18 additions & 0 deletions docs/developer-guide/working-with-rules.md
Expand Up @@ -345,6 +345,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.
Expand Down
29 changes: 29 additions & 0 deletions tests/lib/linter/report-translator.js
Expand Up @@ -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,
Expand Down
44 changes: 44 additions & 0 deletions tests/lib/linter/rule-fixer.js
Expand Up @@ -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", () => {
Expand All @@ -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", () => {
Expand All @@ -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", () => {
Expand All @@ -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", () => {
Expand Down