-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
New: Add context.report({ messageId }) (fixes #6740) #9165
Changes from all commits
de6a00b
304662b
809dd46
10e1e18
3db0b9a
6bdf97e
b12abec
90b1625
7e9a1b3
9f1b142
7a10989
1a756f6
15a069b
424aadd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,7 @@ | |
|
||
const assert = require("assert"); | ||
const ruleFixer = require("./util/rule-fixer"); | ||
const interpolate = require("./util/interpolate"); | ||
|
||
//------------------------------------------------------------------------------ | ||
// Typedefs | ||
|
@@ -41,7 +42,9 @@ function normalizeMultiArgReportCall() { | |
|
||
// If there is one argument, it is considered to be a new-style call already. | ||
if (arguments.length === 1) { | ||
return arguments[0]; | ||
|
||
// Shallow clone the object to avoid surprises if reusing the descriptor | ||
return Object.assign({}, arguments[0]); | ||
} | ||
|
||
// If the second argument is a string, the arguments are interpreted as [node, message, data, fix]. | ||
|
@@ -100,16 +103,7 @@ function normalizeReportLoc(descriptor) { | |
* @returns {string} The interpolated message for the descriptor | ||
*/ | ||
function normalizeMessagePlaceholders(descriptor) { | ||
if (!descriptor.data) { | ||
return descriptor.message; | ||
} | ||
return descriptor.message.replace(/\{\{\s*([^{}]+?)\s*\}\}/g, (fullMatch, term) => { | ||
if (term in descriptor.data) { | ||
return descriptor.data[term]; | ||
} | ||
|
||
return fullMatch; | ||
}); | ||
return interpolate(descriptor.message, descriptor.data); | ||
} | ||
|
||
/** | ||
|
@@ -216,6 +210,14 @@ function createProblem(options) { | |
source: options.sourceLines[options.loc.start.line - 1] || "" | ||
}; | ||
|
||
/* | ||
* If this isn’t in the conditional, some of the tests fail | ||
* because `messageId` is present in the problem object | ||
*/ | ||
if (options.messageId) { | ||
problem.messageId = options.messageId; | ||
} | ||
|
||
if (options.loc.end) { | ||
problem.endLine = options.loc.end.line; | ||
problem.endColumn = options.loc.end.column + 1; | ||
|
@@ -231,12 +233,13 @@ function createProblem(options) { | |
/** | ||
* Returns a function that converts the arguments of a `context.report` call from a rule into a reported | ||
* problem for the Node.js API. | ||
* @param {{ruleId: string, severity: number, sourceCode: SourceCode}} metadata Metadata for the reported problem | ||
* @param {{ruleId: string, severity: number, sourceCode: SourceCode, messageIds: Object}} metadata Metadata for the reported problem | ||
* @param {SourceCode} sourceCode The `SourceCode` instance for the text being linted | ||
* @returns {function(...args): { | ||
* ruleId: string, | ||
* severity: (0|1|2), | ||
* message: string, | ||
* message: (string|undefined), | ||
* messageId: (string|undefined), | ||
* line: number, | ||
* column: number, | ||
* endLine: (number|undefined), | ||
|
@@ -261,11 +264,29 @@ module.exports = function createReportTranslator(metadata) { | |
|
||
assertValidNodeInfo(descriptor); | ||
|
||
if (descriptor.messageId) { | ||
if (!metadata.messageIds) { | ||
throw new TypeError("context.report() called with a messageId, but no messages were present in the rule metadata."); | ||
} | ||
const id = descriptor.messageId; | ||
const messages = metadata.messageIds; | ||
|
||
if (descriptor.message) { | ||
throw new TypeError("context.report() called with a message and a messageId. Please only pass one."); | ||
} | ||
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]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you avoid mutating the descriptor here? I think it would be better to put the (Unlike #9165 (comment), this time I'm not concerned about mutating objects from rules, since I see you cloned the descriptor above. I just think not mutating a function's arguments in general is a good idea for code cleanliness, since it makes code easier to follow.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, this isn’t actually a function argument: // lib/report-translator.js:262
const descriptor = normalizeMultiArgReportCall.apply(null, arguments); There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oops, you're right. I still think it's slightly better to avoid mutation in general, but I don't feel strongly about it here, so there's no need to change it. |
||
} | ||
|
||
|
||
return createProblem({ | ||
ruleId: metadata.ruleId, | ||
severity: metadata.severity, | ||
node: descriptor.node, | ||
message: normalizeMessagePlaceholders(descriptor), | ||
messageId: descriptor.messageId, | ||
loc: normalizeReportLoc(descriptor), | ||
fix: normalizeFixes(descriptor, metadata.sourceCode), | ||
sourceLines: metadata.sourceCode.lines | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
/** | ||
* @fileoverview Interpolate keys from an object into a string with {{ }} markers. | ||
* @author Jed Fox | ||
*/ | ||
|
||
"use strict"; | ||
|
||
//------------------------------------------------------------------------------ | ||
// Public Interface | ||
//------------------------------------------------------------------------------ | ||
|
||
module.exports = (text, data) => { | ||
if (!data) { | ||
return text; | ||
} | ||
return text.replace(/\{\{\s*([^{}]+?)\s*\}\}/g, (fullMatch, term) => { | ||
if (term in data) { | ||
return data[term]; | ||
} | ||
|
||
// Preserve old behavior: If parameter name not provided, don't replace it. | ||
return fullMatch; | ||
}); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need an example of representing an error message with data in RuleTester?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅