Permalink
Browse files

Fix: Make rule-tester strictly check messageId. (ref #9890) (#9908)

This change makes rule-tester check messageId directly by value provided instead
of the current behavior, which is to check the messageId's message value against
the message returned from the rule at runtime.
  • Loading branch information...
betaorbust authored and platinumazure committed Mar 22, 2018
1 parent ea6fb17 commit 0bc4a38352c3f21c56a50196dbeebfbf1567e2f2
@@ -228,16 +228,12 @@ var RuleTester = require("eslint").RuleTester;
var ruleTester = new RuleTester();
ruleTester.run("my-rule", rule, {
valid: ["bar", "baz"],
invalid: [
{
code: "foo",
errors: [
{
messageId: "avoidName",
data: {
name: "foo"
}
messageId: "avoidName"
}
]
}
View
@@ -198,6 +198,7 @@ function normalizeFixes(descriptor, sourceCode) {
* @param {(0|1|2)} options.severity Rule severity
* @param {(ASTNode|null)} options.node Node
* @param {string} options.message Error message
* @param {string} [options.messageId] The error message ID.
* @param {{start: SourceLocation, end: (SourceLocation|null)}} options.loc Start and end location
* @param {{text: string, range: (number[]|null)}} options.fix The fix object
* @param {string[]} options.sourceLines Source lines
View
@@ -487,36 +487,51 @@ class RuleTester {
/*
* Error object.
* This may have a message, node type, line, and/or
* This may have a message, messageId, data, node type, line, and/or
* column.
*/
if (error.message) {
if (hasOwnProperty(error, "message")) {
assert.ok(!hasOwnProperty(error, "messageId"), "Error should not specify both 'message' and a 'messageId'.");
assert.ok(!hasOwnProperty(error, "data"), "Error should not specify both 'data' and 'message'.");
assertMessageMatches(message.message, error.message);
}
if (error.messageId) {
const hOP = Object.hasOwnProperty.call.bind(Object.hasOwnProperty);
// verify that `error.message` is `undefined`
assert.strictEqual(error.message, void 0, "Error should not specify both a message and a messageId.");
if (!hOP(rule, "meta") || !hOP(rule.meta, "messages")) {
assert.fail("Rule must specify a messages hash in `meta`");
}
if (!hOP(rule.meta.messages, error.messageId)) {
} else if (hasOwnProperty(error, "messageId")) {
assert.ok(
hasOwnProperty(rule, "meta") && hasOwnProperty(rule.meta, "messages"),
"Error can not use 'messageId' if rule under test doesn't define 'meta.messages'."
);
if (!hasOwnProperty(rule.meta.messages, error.messageId)) {
const friendlyIDList = `[${Object.keys(rule.meta.messages).map(key => `'${key}'`).join(", ")}]`;
assert.fail(`Invalid messageId '${error.messageId}'. Expected one of ${friendlyIDList}.`);
assert(false, `Invalid messageId '${error.messageId}'. Expected one of ${friendlyIDList}.`);
}
let expectedMessage = rule.meta.messages[error.messageId];
if (error.data) {
expectedMessage = interpolate(expectedMessage, error.data);
assert.strictEqual(
error.messageId,
message.messageId,
`messageId '${message.messageId}' does not match expected messageId '${error.messageId}'.`
);
if (hasOwnProperty(error, "data")) {
/*
* if data was provided, then directly compare the returned message to a synthetic
* interpolated message using the same message ID and data provided in the test.
* See https://github.com/eslint/eslint/issues/9890 for context.
*/
const unformattedOriginalMessage = rule.meta.messages[error.messageId];
const rehydratedMessage = interpolate(unformattedOriginalMessage, error.data);
assert.strictEqual(
message.message,
rehydratedMessage,
`Hydrated message "${rehydratedMessage}" does not match "${message.message}"`
);
}
assertMessageMatches(message.message, expectedMessage);
}
assert.ok(
hasOwnProperty(error, "data") ? hasOwnProperty(error, "messageId") : true,
"Error must specify 'messageId' if 'data' is used."
);
if (error.type) {
assert.strictEqual(message.nodeType, error.type, `Error type should be ${error.type}, found ${message.nodeType}`);
}
@@ -0,0 +1,36 @@
"use strict";
module.exports.withMetaWithData = {
meta: {
messages: {
avoidFoo: "Avoid using variables named '{{ name }}'.",
unused: "An unused key"
}
},
create(context) {
return {
Identifier(node) {
if (node.name === "foo") {
context.report({
node,
messageId: "avoidFoo",
data: {
name: "foo"
}
});
}
}
};
}
};
module.exports.withMessageOnly = {
create(context) {
return {
Identifier(node) {
if (node.name === "foo") {
context.report({ node, message: "Avoid using variables named 'foo'."});
}
}
};
}
};
@@ -162,8 +162,8 @@ ruleTester.run("no-fallthrough", rule, {
}],
errors: [
{
message: errorsDefault.message,
type: errorsDefault.type,
message: errorsDefault[0].message,
type: errorsDefault[0].type,
line: 4,
column: 1
}
@@ -830,6 +830,81 @@ describe("RuleTester", () => {
}, "Test Scenarios for rule foo is invalid:\nCould not find any valid test scenarios");
});
// Nominal message/messageId use cases
it("should assert match if message provided in both test and result.", () => {
assert.throws(() => {
ruleTester.run("foo", require("../../fixtures/testers/rule-tester/messageId").withMessageOnly, {
valid: [],
invalid: [{ code: "foo", errors: [{ message: "something" }] }]
});
}, /Avoid using variables named/);
assert.doesNotThrow(() => {
ruleTester.run("foo", require("../../fixtures/testers/rule-tester/messageId").withMessageOnly, {
valid: [],
invalid: [{ code: "foo", errors: [{ message: "Avoid using variables named 'foo'." }] }]
});
});
});
it("should assert match between messageId if provided in both test and result.", () => {
assert.throws(() => {
ruleTester.run("foo", require("../../fixtures/testers/rule-tester/messageId").withMetaWithData, {
valid: [],
invalid: [{ code: "foo", errors: [{ messageId: "unused" }] }]
});
}, "messageId 'avoidFoo' does not match expected messageId 'unused'.");
assert.doesNotThrow(() => {
ruleTester.run("foo", require("../../fixtures/testers/rule-tester/messageId").withMetaWithData, {
valid: [],
invalid: [{ code: "foo", errors: [{ messageId: "avoidFoo" }] }]
});
});
});
it("should assert match between resulting message output if messageId and data provided in both test and result", () => {
assert.throws(() => {
ruleTester.run("foo", require("../../fixtures/testers/rule-tester/messageId").withMetaWithData, {
valid: [],
invalid: [{ code: "foo", errors: [{ messageId: "avoidFoo", data: { name: "notFoo" } }] }]
});
}, "Hydrated message \"Avoid using variables named 'notFoo'.\" does not match \"Avoid using variables named 'foo'.\"");
});
// messageId/message misconfiguration cases
it("should throw if user tests for both message and messageId", () => {
assert.throws(() => {
ruleTester.run("foo", require("../../fixtures/testers/rule-tester/messageId").withMetaWithData, {
valid: [],
invalid: [{ code: "foo", errors: [{ message: "something", messageId: "avoidFoo" }] }]
});
}, "Error should not specify both 'message' and a 'messageId'.");
});
it("should throw if user tests for messageId but the rule doesn't use the messageId meta syntax.", () => {
assert.throws(() => {
ruleTester.run("foo", require("../../fixtures/testers/rule-tester/messageId").withMessageOnly, {
valid: [],
invalid: [{ code: "foo", errors: [{ messageId: "avoidFoo" }] }]
});
}, "Error can not use 'messageId' if rule under test doesn't define 'meta.messages'");
});
it("should throw if user tests for messageId not listed in the rule's meta syntax.", () => {
assert.throws(() => {
ruleTester.run("foo", require("../../fixtures/testers/rule-tester/messageId").withMetaWithData, {
valid: [],
invalid: [{ code: "foo", errors: [{ messageId: "useFoo" }] }]
});
}, /Invalid messageId 'useFoo'/);
});
it("should throw if data provided without messageId.", () => {
assert.throws(() => {
ruleTester.run("foo", require("../../fixtures/testers/rule-tester/messageId").withMetaWithData, {
valid: [],
invalid: [{ code: "foo", errors: [{ data: "something" }] }]
});
}, "Error must specify 'messageId' if 'data' is used.");
});
describe("naming test cases", () => {
/**

0 comments on commit 0bc4a38

Please sign in to comment.