diff --git a/docs/developer-guide/working-with-rules.md b/docs/developer-guide/working-with-rules.md index f23afb16bfb1..4967ef2fda60 100644 --- a/docs/developer-guide/working-with-rules.md +++ b/docs/developer-guide/working-with-rules.md @@ -190,6 +190,8 @@ Instead of typing out messages in both the `context.report()` call and your test This allows you to avoid retyping error messages. It also prevents errors reported in different sections of your rule from having out-of-date messages. +If the `data` property is included, it is checked for strict deep equality with the resulting runtime error's `data` field. However, if the `data` property is omitted, only the `messageId` will be checked. + ```js {% raw %} // in your rule @@ -226,14 +228,34 @@ var rule = require("../../../lib/rules/my-rule"); var RuleTester = require("eslint").RuleTester; var ruleTester = new RuleTester(); + ruleTester.run("my-rule", rule, { valid: ["bar", "baz"], + invalid: [ + { + code: "foo", + errors: [ + { + // Here we are asserting that the avoideName message + // was used, but not checking against the data object + // used to format it. This form is also used when the + // message has no formatting options and is just a string. + messageId: "avoidName" + } + ] + } + ] +}); +ruleTester.run("my-rule", rule, { + valid: ["bar", "baz"], invalid: [ { code: "foo", errors: [ { + // Here we're asserting that both the messageId "avoidName" + // was used as well as the exact data used to format it. messageId: "avoidName", data: { name: "foo" diff --git a/lib/report-translator.js b/lib/report-translator.js index 7893a1f7ad23..23054c121e77 100644 --- a/lib/report-translator.js +++ b/lib/report-translator.js @@ -198,6 +198,8 @@ 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 {Object} [options.data] The error message data. * @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 @@ -222,6 +224,10 @@ function createProblem(options) { problem.messageId = options.messageId; } + if (options.data) { + problem.data = options.data; + } + if (options.loc.end) { problem.endLine = options.loc.end.line; problem.endColumn = options.loc.end.column + 1; @@ -271,13 +277,13 @@ module.exports = function createReportTranslator(metadata) { descriptor.message = messages[id]; } - return createProblem({ ruleId: metadata.ruleId, severity: metadata.severity, node: descriptor.node, message: normalizeMessagePlaceholders(descriptor), messageId: descriptor.messageId, + data: descriptor.data, loc: normalizeReportLoc(descriptor), fix: normalizeFixes(descriptor, metadata.sourceCode), sourceLines: metadata.sourceCode.lines diff --git a/lib/rules/no-undef.js b/lib/rules/no-undef.js index c8347d50d1aa..4fbd81859daf 100644 --- a/lib/rules/no-undef.js +++ b/lib/rules/no-undef.js @@ -63,7 +63,7 @@ module.exports = { context.report({ node: identifier, message: "'{{name}}' is not defined.", - data: identifier + data: { name: identifier.name } }); }); } diff --git a/lib/rules/no-unused-vars.js b/lib/rules/no-unused-vars.js index 1d0cef85628f..6164a38cadb8 100644 --- a/lib/rules/no-unused-vars.js +++ b/lib/rules/no-unused-vars.js @@ -632,7 +632,9 @@ module.exports = { message: unusedVar.references.some(ref => ref.isWrite()) ? getAssignedMessage() : getDefinedMessage(unusedVar), - data: unusedVar + data: { + name: unusedVar.name + } }); } } diff --git a/lib/testers/rule-tester.js b/lib/testers/rule-tester.js index de218a875f53..bca9353e1699 100644 --- a/lib/testers/rule-tester.js +++ b/lib/testers/rule-tester.js @@ -47,8 +47,7 @@ const lodash = require("lodash"), ajv = require("../util/ajv"), Linter = require("../linter"), Environments = require("../config/environments"), - SourceCodeFixer = require("../util/source-code-fixer"), - interpolate = require("../util/interpolate"); + SourceCodeFixer = require("../util/source-code-fixer"); //------------------------------------------------------------------------------ // Private Members @@ -487,34 +486,36 @@ 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.strictEqual(hasOwnProperty(error, "messageId"), false, "Error should not specify both 'message' and a 'messageId'."); + assert.strictEqual(hasOwnProperty(error, "data"), false, "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}.`); } - - let expectedMessage = rule.meta.messages[error.messageId]; - - if (error.data) { - expectedMessage = interpolate(expectedMessage, error.data); + assert.strictEqual( + error.messageId, + message.messageId, + `messageId mismatch saw '${message.messageId}' and expected '${error.messageId}'.` + ); + if (hasOwnProperty(error, "data")) { + assert.ok(hasOwnProperty(message, "data"), "Error referrs to messageId data, but none provided by rule."); + assert.deepStrictEqual( + error.data, + message.data, + `Data properties do not match. Saw \`${util.inspect(message.data, false)}\` and expected \`${util.inspect(error.data, false)}\`` + ); } - - assertMessageMatches(message.message, expectedMessage); } if (error.type) { diff --git a/tests/fixtures/testers/rule-tester/messageId.js b/tests/fixtures/testers/rule-tester/messageId.js new file mode 100644 index 000000000000..aeeddc01c47c --- /dev/null +++ b/tests/fixtures/testers/rule-tester/messageId.js @@ -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.withoutMetaWithoutData = { + create(context) { + return { + Identifier(node) { + if (node.name === "foo") { + context.report({ node, message: "Avoid using variables named 'foo'."}); + } + } + }; + } +}; diff --git a/tests/lib/cli-engine.js b/tests/lib/cli-engine.js index 48bc82f10b8b..b83dedf55df0 100644 --- a/tests/lib/cli-engine.js +++ b/tests/lib/cli-engine.js @@ -351,6 +351,7 @@ describe("CLIEngine", () => { ruleId: "no-undef", severity: 2, message: "'foo' is not defined.", + data: { name: "foo" }, line: 1, column: 11, endLine: 1, @@ -1395,6 +1396,10 @@ describe("CLIEngine", () => { column: 9, line: 2, message: "Expected '===' and instead saw '=='.", + data: { + actualOperator: "==", + expectedOperator: "===" + }, nodeType: "BinaryExpression", ruleId: "eqeqeq", severity: 2, @@ -1416,6 +1421,7 @@ describe("CLIEngine", () => { endColumn: 21, endLine: 1, message: "'foo' is not defined.", + data: { name: "foo" }, nodeType: "Identifier", ruleId: "no-undef", severity: 2, diff --git a/tests/lib/report-translator.js b/tests/lib/report-translator.js index f1e00ed3da55..89b7dbb28bd6 100644 --- a/tests/lib/report-translator.js +++ b/tests/lib/report-translator.js @@ -54,7 +54,7 @@ describe("createReportTranslator", () => { describe("old-style call with location", () => { it("should extract the location correctly", () => { assert.deepStrictEqual( - translateReport(node, location, message, {}), + translateReport(node, location, message), { ruleId: "foo-rule", severity: 2, @@ -71,7 +71,7 @@ describe("createReportTranslator", () => { describe("old-style call without location", () => { it("should use the start location and end location of the node", () => { assert.deepStrictEqual( - translateReport(node, message, {}), + translateReport(node, message), { ruleId: "foo-rule", severity: 2, @@ -336,6 +336,7 @@ describe("createReportTranslator", () => { ruleId: "foo-rule", severity: 2, message: "my message testing!", + data: ["!", "testing"], line: 42, column: 24, nodeType: "ExpressionStatement", @@ -358,6 +359,7 @@ describe("createReportTranslator", () => { severity: 2, ruleId: "foo-rule", message: "hello ExpressionStatement", + data: { dynamic: "ExpressionStatement" }, nodeType: "ExpressionStatement", line: 1, column: 4, @@ -373,6 +375,7 @@ describe("createReportTranslator", () => { severity: 2, ruleId: "foo-rule", message: "hello ExpressionStatement", + data: { dynamic: "ExpressionStatement" }, nodeType: "ExpressionStatement", line: 1, column: 4, @@ -558,6 +561,7 @@ describe("createReportTranslator", () => { severity: 2, ruleId: "foo-rule", message: "my message testing!", + data: ["!", "testing"], nodeType: "ExpressionStatement", line: 1, column: 1, diff --git a/tests/lib/rules/no-fallthrough.js b/tests/lib/rules/no-fallthrough.js index c2e48fadf619..4d8e4514ba31 100644 --- a/tests/lib/rules/no-fallthrough.js +++ b/tests/lib/rules/no-fallthrough.js @@ -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 } diff --git a/tests/lib/testers/rule-tester.js b/tests/lib/testers/rule-tester.js index d7a4bd51c301..d92b925d189a 100644 --- a/tests/lib/testers/rule-tester.js +++ b/tests/lib/testers/rule-tester.js @@ -830,6 +830,116 @@ 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").withoutMetaWithoutData, { + valid: [], + invalid: [{ code: "foo", errors: [{ message: "something" }] }] + }); + }, /Avoid using variables named/); + + assert.doesNotThrow(() => { + ruleTester.run("foo", require("../../fixtures/testers/rule-tester/messageId").withoutMetaWithoutData, { + valid: [], + invalid: [{ code: "foo", errors: [{ message: "Avoid using variables named 'foo'." }] }] + }); + }); + }); + + it("should assert match between messageId if provided in both test and result and test data is undefined.", () => { + assert.throws(() => { + ruleTester.run("foo", require("../../fixtures/testers/rule-tester/messageId").withMetaWithData, { + valid: [], + invalid: [{ code: "foo", errors: [{ messageId: "unused" }] }] + }); + }, "messageId mismatch saw 'avoidFoo' and expected '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 messageId and data if provided in both test and result.", () => { + const testConfigsData = [ + { name: "foo", bad: "data" }, + { name: "not foo" }, + { notName: "no foo" }, + { } + ]; + + testConfigsData.forEach(data => { + assert.throws(() => { + ruleTester.run("foo", require("../../fixtures/testers/rule-tester/messageId").withMetaWithData, { + valid: [], + invalid: [{ code: "foo", errors: [{ messageId: "avoidFoo", data }] }] + }); + }, /Data properties do not match. Saw `\{[^}]*}` and expected `\{[^}]*}`/); + }); + + assert.throws(() => { + ruleTester.run("foo", require("../../fixtures/testers/rule-tester/messageId").withMetaWithData, { + valid: [], + invalid: [{ code: "foo", errors: [{ messageId: "unused", data: { name: "foo" } }] }] + }); + }, "messageId mismatch saw 'avoidFoo' and expected 'unused'."); + + assert.doesNotThrow(() => { + ruleTester.run("foo", require("../../fixtures/testers/rule-tester/messageId").withMetaWithData, { + valid: [], + invalid: [{ code: "foo", errors: [{ messageId: "avoidFoo", data: { name: "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").withoutMetaWithoutData, { + 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 message as well as data.", () => { + assert.throws(() => { + ruleTester.run("foo", require("../../fixtures/testers/rule-tester/messageId").withoutMetaWithoutData, { + valid: [], + invalid: [{ code: "foo", errors: [{ message: "A failure message", data: { some: "data" } }] }] + }); + }, "Error should not specify both 'data' and 'message'."); + }); + + it("should not throw if user tests for messageId but does not include data as long as the test's messageId matches the result's messageId.", () => { + assert.doesNotThrow(() => { + ruleTester.run("foo", require("../../fixtures/testers/rule-tester/messageId").withMetaWithData, { + valid: [], + invalid: [{ code: "foo", errors: [{ messageId: "avoidFoo" }] }] + }); + }, "Message"); + }); + + it("should not throw if user tests for a messageId and data and the rule passes messageId and data if and only if messageIds match and data in the test is present in the result's data.", () => { + assert.doesNotThrow(() => { + ruleTester.run("foo", require("../../fixtures/testers/rule-tester/messageId").withMetaWithData, { + valid: [], + invalid: [{ code: "foo", errors: [{ messageId: "avoidFoo", data: { name: "foo" } }] }] + }); + }, "Message"); + }); + describe("naming test cases", () => { /**