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

feat!: assert suggestion messages are unique in rule testers #17532

Merged
merged 5 commits into from Dec 20, 2023
Merged

feat!: assert suggestion messages are unique in rule testers #17532

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f714f85
feat!: assert suggestion messages are unique in rule testers
JoshuaKGoldberg Sep 5, 2023
2d0a63f
Apply suggestions from code review
JoshuaKGoldberg Nov 5, 2023
7f7bb7c
Remove unnecessary . addition
JoshuaKGoldberg Nov 5, 2023
4322885
Remove unused getSuggestionMessage functions
JoshuaKGoldberg Nov 5, 2023
f695801
Update tests/lib/rule-tester/flat-rule-tester.js
JoshuaKGoldberg Nov 6, 2023
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
16 changes: 16 additions & 0 deletions lib/rule-tester/flat-rule-tester.js
View file
Open in desktop
Expand Up @@ -794,6 +794,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
View file
Open in desktop
Expand Up @@ -867,6 +867,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
View file
Open in desktop
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
View file
Open in desktop
Expand Up @@ -2236,6 +2236,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
View file
Open in desktop
Expand Up @@ -2220,6 +2220,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