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.

Sign up for GitHub

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

Jump to bottom

Update: remove suggestion if it didn't provide a fix (fixes #13723) #13772

Merged
merged 1 commit into from Oct 23, 2020
Merged

Update: remove suggestion if it didn't provide a fix (fixes #13723) #13772

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions docs/developer-guide/working-with-rules.md
View file
Open in desktop
Expand Up @@ -382,6 +382,8 @@ Best practices for suggestions:
1. Don't try to do too much and suggest large refactors that could introduce a lot of breaking changes.
1. As noted above, don't try to conform to user-defined styles.

Suggestions are intended to provide fixes. ESLint will automatically remove the whole suggestion from the linting output if the suggestion's `fix` function returned `null` or an empty array/sequence.

#### Suggestion `messageId`s

Instead of using a `desc` key for suggestions a `messageId` can be used instead. This works the same way as `messageId`s for the overall error (see [messageIds](#messageIds)). Here is an example of how to use it in a rule:
Expand Down
22 changes: 13 additions & 9 deletions lib/linter/report-translator.js
View file
Open in desktop
Expand Up @@ -196,15 +196,19 @@ function mapSuggestions(descriptor, sourceCode, messages) {
return [];
}

return descriptor.suggest.map(suggestInfo => {
const computedDesc = suggestInfo.desc || messages[suggestInfo.messageId];

return {
...suggestInfo,
desc: interpolate(computedDesc, suggestInfo.data),
fix: normalizeFixes(suggestInfo, sourceCode)
};
});
return descriptor.suggest
.map(suggestInfo => {
const computedDesc = suggestInfo.desc || messages[suggestInfo.messageId];

return {
...suggestInfo,
desc: interpolate(computedDesc, suggestInfo.data),
fix: normalizeFixes(suggestInfo, sourceCode)
};
})

// Remove suggestions that didn't provide a fix
.filter(({ fix }) => fix);
}

/**
Expand Down
218 changes: 218 additions & 0 deletions tests/lib/linter/report-translator.js
View file
Open in desktop
Expand Up @@ -553,6 +553,224 @@ describe("createReportTranslator", () => {
}
);
});

it("should remove the whole suggestion if 'fix' function returned `null`.", () => {
const reportDescriptor = {
node,
loc: location,
message,
suggest: [{
desc: "A suggestion for the issue",
fix: () => null
}]
};

assert.deepStrictEqual(
translateReport(reportDescriptor),
{
ruleId: "foo-rule",
severity: 2,
message: "foo",
line: 2,
column: 1,
nodeType: "ExpressionStatement"
}
);
});

it("should remove the whole suggestion if 'fix' function returned an empty array.", () => {
const reportDescriptor = {
node,
loc: location,
message,
suggest: [{
desc: "A suggestion for the issue",
fix: () => []
}]
};

assert.deepStrictEqual(
translateReport(reportDescriptor),
{
ruleId: "foo-rule",
severity: 2,
message: "foo",
line: 2,
column: 1,
nodeType: "ExpressionStatement"
}
);
});

it("should remove the whole suggestion if 'fix' function returned an empty sequence.", () => {
const reportDescriptor = {
node,
loc: location,
message,
suggest: [{
desc: "A suggestion for the issue",
*fix() {}
}]
};

assert.deepStrictEqual(
translateReport(reportDescriptor),
{
ruleId: "foo-rule",
severity: 2,
message: "foo",
line: 2,
column: 1,
nodeType: "ExpressionStatement"
}
);
});

// This isn't offically supported, but autofix works the same way
it("should remove the whole suggestion if 'fix' function didn't return anything.", () => {
const reportDescriptor = {
node,
loc: location,
message,
suggest: [{
desc: "A suggestion for the issue",
fix() {}
}]
};

assert.deepStrictEqual(
translateReport(reportDescriptor),
{
ruleId: "foo-rule",
severity: 2,
message: "foo",
line: 2,
column: 1,
nodeType: "ExpressionStatement"
}
);
});

it("should keep suggestion before a removed suggestion.", () => {
const reportDescriptor = {
node,
loc: location,
message,
suggest: [{
desc: "Suggestion with a fix",
fix: () => ({ range: [1, 2], text: "foo" })
}, {
desc: "Suggestion without a fix",
fix: () => null
}]
};

assert.deepStrictEqual(
translateReport(reportDescriptor),
{
ruleId: "foo-rule",
severity: 2,
message: "foo",
line: 2,
column: 1,
nodeType: "ExpressionStatement",
suggestions: [{
desc: "Suggestion with a fix",
fix: { range: [1, 2], text: "foo" }
}]
}
);
});

it("should keep suggestion after a removed suggestion.", () => {
const reportDescriptor = {
node,
loc: location,
message,
suggest: [{
desc: "Suggestion without a fix",
fix: () => null
}, {
desc: "Suggestion with a fix",
fix: () => ({ range: [1, 2], text: "foo" })
}]
};

assert.deepStrictEqual(
translateReport(reportDescriptor),
{
ruleId: "foo-rule",
severity: 2,
message: "foo",
line: 2,
column: 1,
nodeType: "ExpressionStatement",
suggestions: [{
desc: "Suggestion with a fix",
fix: { range: [1, 2], text: "foo" }
}]
}
);
});

it("should remove multiple suggestions that didn't provide a fix and keep those that did.", () => {
const reportDescriptor = {
node,
loc: location,
message,
suggest: [{
desc: "Keep #1",
fix: () => ({ range: [1, 2], text: "foo" })
}, {
desc: "Remove #1",
fix() {
return null;
}
}, {
desc: "Keep #2",
fix: () => ({ range: [1, 2], text: "bar" })
}, {
desc: "Remove #2",
fix() {
return [];
}
}, {
desc: "Keep #3",
fix: () => ({ range: [1, 2], text: "baz" })
}, {
desc: "Remove #3",
*fix() {}
}, {
desc: "Keep #4",
fix: () => ({ range: [1, 2], text: "quux" })
}]
};

assert.deepStrictEqual(
translateReport(reportDescriptor),
{
ruleId: "foo-rule",
severity: 2,
message: "foo",
line: 2,
column: 1,
nodeType: "ExpressionStatement",
suggestions: [{
desc: "Keep #1",
fix: { range: [1, 2], text: "foo" }
}, {
desc: "Keep #2",
fix: { range: [1, 2], text: "bar" }
}, {
desc: "Keep #3",
fix: { range: [1, 2], text: "baz" }
}, {
desc: "Keep #4",
fix: { range: [1, 2], text: "quux" }
}]
}
);
});
});

describe("message interpolation", () => {
Expand Down