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.

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

Breaking: require rules to provide report messages (fixes #10011) #10057

Merged
merged 1 commit into from Mar 22, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 8 additions & 11 deletions lib/report-translator.js
Expand Up @@ -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.
Expand Down Expand Up @@ -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.");
Expand All @@ -268,15 +261,19 @@ 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.");
}


return createProblem({
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),
Expand Down
14 changes: 14 additions & 0 deletions tests/lib/linter.js
Expand Up @@ -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", () => {
Expand Down
9 changes: 9 additions & 0 deletions tests/lib/report-translator.js
Expand Up @@ -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.", () => {
Expand Down