Skip to content

Commit

Permalink
feat!: assert suggestion messages are unique in rule testers (#17532)
Browse files Browse the repository at this point in the history
* feat!: assert suggestion messages are unique in rule testers

* Apply suggestions from code review

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* Remove unnecessary . addition

* Remove unused getSuggestionMessage functions

* Update tests/lib/rule-tester/flat-rule-tester.js

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

---------

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
  • Loading branch information
JoshuaKGoldberg and mdjermanovic committed Dec 20, 2023
1 parent e563c52 commit b3e0bb0
Show file tree
Hide file tree
Showing 5 changed files with 190 additions and 0 deletions.
16 changes: 16 additions & 0 deletions lib/rule-tester/flat-rule-tester.js
Expand Up @@ -853,6 +853,22 @@ class FlatRuleTester {
const result = runRuleForItem(item);
const messages = result.messages;

for (const message of messages) {
if (hasOwnProperty(message, "suggestions")) {

/** @type {Map<string, number>} */
const seenMessageIndices = new Map();

for (let i = 0; i < message.suggestions.length; i += 1) {
const suggestionMessage = message.suggestions[i].desc;
const previous = seenMessageIndices.get(suggestionMessage);

assert.ok(!seenMessageIndices.has(suggestionMessage), `Suggestion message '${suggestionMessage}' reported from suggestion ${i} was previously reported by suggestion ${previous}. Suggestion messages should be unique within an error.`);
seenMessageIndices.set(suggestionMessage, i);
}
}
}

if (typeof item.errors === "number") {

if (item.errors === 0) {
Expand Down
16 changes: 16 additions & 0 deletions lib/rule-tester/rule-tester.js
Expand Up @@ -845,6 +845,22 @@ class RuleTester {
const result = runRuleForItem(item);
const messages = result.messages;

for (const message of messages) {
if (hasOwnProperty(message, "suggestions")) {

/** @type {Map<string, number>} */
const seenMessageIndices = new Map();

for (let i = 0; i < message.suggestions.length; i += 1) {
const suggestionMessage = message.suggestions[i].desc;
const previous = seenMessageIndices.get(suggestionMessage);

assert.ok(!seenMessageIndices.has(suggestionMessage), `Suggestion message '${suggestionMessage}' reported from suggestion ${i} was previously reported by suggestion ${previous}. Suggestion messages should be unique within an error.`);
seenMessageIndices.set(suggestionMessage, i);
}
}
}

if (typeof item.errors === "number") {

if (item.errors === 0) {
Expand Down
92 changes: 92 additions & 0 deletions tests/fixtures/testers/rule-tester/suggestions.js
Expand Up @@ -59,6 +59,98 @@ module.exports.withMessageIds = {
}
};

module.exports.withDuplicateDescriptions = {
meta: {
hasSuggestions: true
},
create(context) {
return {
Identifier(node) {
if (node.name === "foo") {
context.report({
node,
message: "Avoid using identifiers name 'foo'.",
suggest: [{
desc: "Rename 'foo' to 'bar'",
fix: fixer => fixer.replaceText(node, "bar")
}, {
desc: "Rename 'foo' to 'bar'",
fix: fixer => fixer.replaceText(node, "baz")
}]
});
}
}
};
}
};

module.exports.withDuplicateMessageIdsNoData = {
meta: {
messages: {
avoidFoo: "Avoid using identifiers named '{{ name }}'.",
renameFoo: "Rename identifier"
},
hasSuggestions: true
},
create(context) {
return {
Identifier(node) {
if (node.name === "foo") {
context.report({
node,
messageId: "avoidFoo",
data: {
name: "foo"
},
suggest: [{
messageId: "renameFoo",
fix: fixer => fixer.replaceText(node, "bar")
}, {
messageId: "renameFoo",
fix: fixer => fixer.replaceText(node, "baz")
}]
});
}
}
};
}
};

module.exports.withDuplicateMessageIdsWithData = {
meta: {
messages: {
avoidFoo: "Avoid using identifiers named foo.",
renameFoo: "Rename identifier 'foo' to '{{ newName }}'"
},
hasSuggestions: true
},
create(context) {
return {
Identifier(node) {
if (node.name === "foo") {
context.report({
node,
messageId: "avoidFoo",
suggest: [{
messageId: "renameFoo",
data: {
newName: "bar"
},
fix: fixer => fixer.replaceText(node, "bar")
}, {
messageId: "renameFoo",
data: {
newName: "bar"
},
fix: fixer => fixer.replaceText(node, "baz")
}]
});
}
}
};
}
};

module.exports.withoutHasSuggestionsProperty = {
create(context) {
return {
Expand Down
33 changes: 33 additions & 0 deletions tests/lib/rule-tester/flat-rule-tester.js
Expand Up @@ -2313,6 +2313,39 @@ describe("FlatRuleTester", () => {
}, /Invalid suggestion property name 'outpt'/u);
});

it("should fail if a rule produces two suggestions with the same description", () => {
assert.throws(() => {
ruleTester.run("suggestions-with-duplicate-descriptions", require("../../fixtures/testers/rule-tester/suggestions").withDuplicateDescriptions, {
valid: [],
invalid: [
{ code: "var foo = bar;", errors: 1 }
]
});
}, "Suggestion message 'Rename 'foo' to 'bar'' reported from suggestion 1 was previously reported by suggestion 0. Suggestion messages should be unique within an error.");
});

it("should fail if a rule produces two suggestions with the same messageId without data", () => {
assert.throws(() => {
ruleTester.run("suggestions-with-duplicate-messageids-no-data", require("../../fixtures/testers/rule-tester/suggestions").withDuplicateMessageIdsNoData, {
valid: [],
invalid: [
{ code: "var foo = bar;", errors: 1 }
]
});
}, "Suggestion message 'Rename identifier' reported from suggestion 1 was previously reported by suggestion 0. Suggestion messages should be unique within an error.");
});

it("should fail if a rule produces two suggestions with the same messageId with data", () => {
assert.throws(() => {
ruleTester.run("suggestions-with-duplicate-messageids-with-data", require("../../fixtures/testers/rule-tester/suggestions").withDuplicateMessageIdsWithData, {
valid: [],
invalid: [
{ code: "var foo = bar;", errors: 1 }
]
});
}, "Suggestion message 'Rename identifier 'foo' to 'bar'' reported from suggestion 1 was previously reported by suggestion 0. Suggestion messages should be unique within an error.");
});

it("should throw an error if a rule that doesn't have `meta.hasSuggestions` enabled produces suggestions", () => {
assert.throws(() => {
ruleTester.run("suggestions-missing-hasSuggestions-property", require("../../fixtures/testers/rule-tester/suggestions").withoutHasSuggestionsProperty, {
Expand Down
33 changes: 33 additions & 0 deletions tests/lib/rule-tester/rule-tester.js
Expand Up @@ -2187,6 +2187,39 @@ describe("RuleTester", () => {
}, /Invalid suggestion property name 'outpt'/u);
});

it("should fail if a rule produces two suggestions with the same description", () => {
assert.throws(() => {
ruleTester.run("suggestions-with-duplicate-descriptions", require("../../fixtures/testers/rule-tester/suggestions").withDuplicateDescriptions, {
valid: [],
invalid: [
{ code: "var foo = bar;", errors: 1 }
]
});
}, "Suggestion message 'Rename 'foo' to 'bar'' reported from suggestion 1 was previously reported by suggestion 0. Suggestion messages should be unique within an error.");
});

it("should fail if a rule produces two suggestions with the same messageId without data", () => {
assert.throws(() => {
ruleTester.run("suggestions-with-duplicate-messageids-no-data", require("../../fixtures/testers/rule-tester/suggestions").withDuplicateMessageIdsNoData, {
valid: [],
invalid: [
{ code: "var foo = bar;", errors: 1 }
]
});
}, "Suggestion message 'Rename identifier' reported from suggestion 1 was previously reported by suggestion 0. Suggestion messages should be unique within an error.");
});

it("should fail if a rule produces two suggestions with the same messageId with data", () => {
assert.throws(() => {
ruleTester.run("suggestions-with-duplicate-messageids-with-data", require("../../fixtures/testers/rule-tester/suggestions").withDuplicateMessageIdsWithData, {
valid: [],
invalid: [
{ code: "var foo = bar;", errors: 1 }
]
});
}, "Suggestion message 'Rename identifier 'foo' to 'bar'' reported from suggestion 1 was previously reported by suggestion 0. Suggestion messages should be unique within an error.");
});

it("should throw an error if a rule that doesn't have `meta.hasSuggestions` enabled produces suggestions", () => {
assert.throws(() => {
ruleTester.run("suggestions-missing-hasSuggestions-property", require("../../fixtures/testers/rule-tester/suggestions").withoutHasSuggestionsProperty, {
Expand Down

0 comments on commit b3e0bb0

Please sign in to comment.