From 9f6584c955c03bea431dbeb2378417434ca7b29c Mon Sep 17 00:00:00 2001 From: Teddy Katz Date: Sun, 4 Mar 2018 18:31:01 -0500 Subject: [PATCH] Breaking: require rules to provide report messages (fixes #10011) This updates ESLint core to throw an error when a rule reports a problem without providing a report message. It is unlikely that many users were relying on the previous behavior, because a reported problem without a message would crash several formatters, including the default formatter. However, it is possible that someone was relying on this with the `json` formatter. --- lib/report-translator.js | 19 ++++++++----------- tests/lib/linter.js | 14 ++++++++++++++ tests/lib/report-translator.js | 9 +++++++++ 3 files changed, 31 insertions(+), 11 deletions(-) diff --git a/lib/report-translator.js b/lib/report-translator.js index 7893a1f7ad2..703060e74ac 100644 --- a/lib/report-translator.js +++ b/lib/report-translator.js @@ -113,15 +113,6 @@ function normalizeReportLoc(descriptor) { return descriptor.node.loc; } -/** - * Interpolates data placeholders in report messages - * @param {MessageDescriptor} descriptor The report message descriptor. - * @returns {string} The interpolated message for the descriptor - */ -function normalizeMessagePlaceholders(descriptor) { - return interpolate(descriptor.message, descriptor.data); -} - /** * Compares items in a fixes array by range. * @param {Fix} a The first message. @@ -255,6 +246,8 @@ module.exports = function createReportTranslator(metadata) { assertValidNodeInfo(descriptor); + let computedMessage; + if (descriptor.messageId) { if (!metadata.messageIds) { throw new TypeError("context.report() called with a messageId, but no messages were present in the rule metadata."); @@ -268,7 +261,11 @@ module.exports = function createReportTranslator(metadata) { if (!messages || !Object.prototype.hasOwnProperty.call(messages, id)) { throw new TypeError(`context.report() called with a messageId of '${id}' which is not present in the 'messages' config: ${JSON.stringify(messages, null, 2)}`); } - descriptor.message = messages[id]; + computedMessage = messages[id]; + } else if (descriptor.message) { + computedMessage = descriptor.message; + } else { + throw new TypeError("Missing `message` property in report() call; add a message that describes the linting problem."); } @@ -276,7 +273,7 @@ module.exports = function createReportTranslator(metadata) { ruleId: metadata.ruleId, severity: metadata.severity, node: descriptor.node, - message: normalizeMessagePlaceholders(descriptor), + message: interpolate(computedMessage, descriptor.data), messageId: descriptor.messageId, loc: normalizeReportLoc(descriptor), fix: normalizeFixes(descriptor, metadata.sourceCode), diff --git a/tests/lib/linter.js b/tests/lib/linter.js index 7c3ac678afb..2244ade3573 100644 --- a/tests/lib/linter.js +++ b/tests/lib/linter.js @@ -870,6 +870,20 @@ describe("Linter", () => { sinon.assert.calledTwice(spyLiteral); sinon.assert.calledOnce(spyBinaryExpression); }); + + it("should throw an error if a rule reports a problem without a message", () => { + linter.defineRule("invalid-report", context => ({ + Program(node) { + context.report({ node }); + } + })); + + assert.throws( + () => linter.verify("foo", { rules: { "invalid-report": "error" } }), + TypeError, + "Missing `message` property in report() call; add a message that describes the linting problem." + ); + }); }); describe("when config has shared settings for rules", () => { diff --git a/tests/lib/report-translator.js b/tests/lib/report-translator.js index f1e00ed3da5..d6d83893234 100644 --- a/tests/lib/report-translator.js +++ b/tests/lib/report-translator.js @@ -168,6 +168,15 @@ describe("createReportTranslator", () => { /^context\.report\(\) called with a messageId of '[^']+' which is not present in the 'messages' config:/ ); }); + it("should throw when no message is provided", () => { + const reportDescriptor = { node }; + + assert.throws( + () => translateReport(reportDescriptor), + TypeError, + "Missing `message` property in report() call; add a message that describes the linting problem." + ); + }); }); describe("combining autofixes", () => { it("should merge fixes to one if 'fix' function returns an array of fixes.", () => {