Skip to content
Permalink
Browse files

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

  • Loading branch information
mdjermanovic committed Oct 23, 2020
1 parent 5183b14 commit bfddcedace5587d662c840c2edf33062b54a178e
Showing with 233 additions and 9 deletions.
  1. +2 −0 docs/developer-guide/working-with-rules.md
  2. +13 −9 lib/linter/report-translator.js
  3. +218 −0 tests/lib/linter/report-translator.js
@@ -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:
@@ -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);
}

/**
@@ -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", () => {

0 comments on commit bfddced

Please sign in to comment.
You can’t perform that action at this time.