From 0d3a854db08a0e9b572215cb0a7a38b63f063664 Mon Sep 17 00:00:00 2001 From: Teddy Katz Date: Fri, 1 Sep 2017 15:43:15 -0400 Subject: [PATCH] Chore: avoid mutating report descriptors in report-translator (#9189) --- lib/report-translator.js | 108 +++++++++++++++++++-------------------- 1 file changed, 54 insertions(+), 54 deletions(-) diff --git a/lib/report-translator.js b/lib/report-translator.js index fe59d9a75b5..9c4ab8b9a9f 100644 --- a/lib/report-translator.js +++ b/lib/report-translator.js @@ -41,7 +41,7 @@ function normalizeMultiArgReportCall() { // If there is one argument, it is considered to be a new-style call already. if (arguments.length === 1) { - return Object.assign({}, arguments[0]); + return arguments[0]; } // If the second argument is a string, the arguments are interpreted as [node, message, data, fix]. @@ -67,7 +67,7 @@ function normalizeMultiArgReportCall() { /** * Asserts that either a loc or a node was provided, and the node is valid if it was provided. * @param {MessageDescriptor} descriptor A descriptor to validate - * @returns {MessageDescriptor} The same descriptor + * @returns {void} * @throws AssertionError if neither a node nor a loc was provided, or if the node is not an object */ function assertValidNodeInfo(descriptor) { @@ -76,46 +76,39 @@ function assertValidNodeInfo(descriptor) { } else { assert(descriptor.loc, "Node must be provided when reporting error if location is not provided"); } - - return descriptor; } /** * Normalizes a MessageDescriptor to always have a `loc` with `start` and `end` properties - * @param {MessageDescriptor} descriptor A descriptor for the report from a rule. This descriptor may be mutated - * by this function. - * @returns {MessageDescriptor} The updated MessageDescriptor that infers the `start` and `end` properties from - * the `node` of the original descriptor, or infers the `start` from the `loc` of the original descriptor. + * @param {MessageDescriptor} descriptor A descriptor for the report from a rule. + * @returns {{start: Location, end: (Location|null)}} An updated location that infers the `start` and `end` properties + * from the `node` of the original descriptor, or infers the `start` from the `loc` of the original descriptor. */ function normalizeReportLoc(descriptor) { if (descriptor.loc) { if (descriptor.loc.start) { - return descriptor; + return descriptor.loc; } - return Object.assign(descriptor, { loc: { start: descriptor.loc, end: null } }); + return { start: descriptor.loc, end: null }; } - - return Object.assign(descriptor, { loc: descriptor.node.loc }); + return descriptor.node.loc; } /** * Interpolates data placeholders in report messages - * @param {MessageDescriptor} descriptor The report message descriptor. This descriptor may be mutated - * by this function. - * @returns {MessageDescriptor} An new descriptor with a message containing the interpolated data + * @param {MessageDescriptor} descriptor The report message descriptor. + * @returns {string} The interpolated message for the descriptor */ function normalizeMessagePlaceholders(descriptor) { if (!descriptor.data) { - return descriptor; + return descriptor.message; } - return Object.assign(descriptor, { - message: descriptor.message.replace(/\{\{\s*([^{}]+?)\s*\}\}/g, (fullMatch, term) => { - if (term in descriptor.data) { - return descriptor.data[term]; - } - - return fullMatch; - }) + return descriptor.message.replace(/\{\{\s*([^{}]+?)\s*\}\}/g, (fullMatch, term) => { + if (term in descriptor.data) { + return descriptor.data[term]; + } + + return fullMatch; }); } @@ -134,7 +127,7 @@ function compareFixesByRange(a, b) { * Merges the given fixes array into one. * @param {Fix[]} fixes The fixes to merge. * @param {SourceCode} sourceCode The source code object to get the text between fixes. - * @returns {void} + * @returns {{text: string, range: [number, number]}} The merged fixes */ function mergeFixes(fixes, sourceCode) { if (fixes.length === 0) { @@ -169,14 +162,13 @@ function mergeFixes(fixes, sourceCode) { /** * Gets one fix object from the given descriptor. * If the descriptor retrieves multiple fixes, this merges those to one. - * @param {MessageDescriptor} descriptor The report descriptor. This descriptor may be mutated - * by this function. + * @param {MessageDescriptor} descriptor The report descriptor. * @param {SourceCode} sourceCode The source code object to get text between fixes. - * @returns {MessageDescriptor} The updated descriptor. + * @returns {({text: string, range: [number, number]}|null)} The fix for the descriptor */ function normalizeFixes(descriptor, sourceCode) { if (typeof descriptor.fix !== "function") { - return Object.assign(descriptor, { fix: null }); + return null; } // @type {null | Fix | Fix[] | IterableIterator} @@ -184,16 +176,22 @@ function normalizeFixes(descriptor, sourceCode) { // Merge to one. if (fix && Symbol.iterator in fix) { - return Object.assign(descriptor, { fix: mergeFixes(Array.from(fix), sourceCode) }); + return mergeFixes(Array.from(fix), sourceCode); } - return Object.assign(descriptor, { fix }); + return fix; } /** * Creates information about the report from a descriptor - * @param {MessageDescriptor} descriptor The message descriptor - * @param {string} ruleId The rule ID of the problem - * @param {(0|1|2)} severity The severity of the problem + * @param {{ + * ruleId: string, + * severity: (0|1|2), + * node: (ASTNode|null), + * message: string, + * loc: {start: SourceLocation, end: (SourceLocation|null)}, + * fix: ({text: string, range: [number, number]}|null), + * sourceLines: string[] + * }} options Information about the problem * @returns {function(...args): { * ruleId: string, * severity: (0|1|2), @@ -207,23 +205,24 @@ function normalizeFixes(descriptor, sourceCode) { * fix: ({text: string, range: [number, number]}|null) * }} Information about the report */ -function createProblemFromDescriptor(descriptor, ruleId, severity) { +function createProblem(options) { const problem = { - ruleId, - severity, - message: descriptor.message, - line: descriptor.loc.start.line, - column: descriptor.loc.start.column + 1, - nodeType: descriptor.node && descriptor.node.type || null + ruleId: options.ruleId, + severity: options.severity, + message: options.message, + line: options.loc.start.line, + column: options.loc.start.column + 1, + nodeType: options.node && options.node.type || null, + source: options.sourceLines[options.loc.start.line - 1] || "" }; - if (descriptor.loc.end) { - problem.endLine = descriptor.loc.end.line; - problem.endColumn = descriptor.loc.end.column + 1; + if (options.loc.end) { + problem.endLine = options.loc.end.line; + problem.endColumn = options.loc.end.column + 1; } - if (descriptor.fix) { - problem.fix = descriptor.fix; + if (options.fix) { + problem.fix = options.fix; } return problem; @@ -261,14 +260,15 @@ module.exports = function createReportTranslator(metadata) { const descriptor = normalizeMultiArgReportCall.apply(null, arguments); assertValidNodeInfo(descriptor); - normalizeReportLoc(descriptor); - normalizeMessagePlaceholders(descriptor); - normalizeFixes(descriptor, metadata.sourceCode); - - const problem = createProblemFromDescriptor(descriptor, metadata.ruleId, metadata.severity); - - problem.source = metadata.sourceCode.lines[problem.line - 1] || ""; - return problem; + return createProblem({ + ruleId: metadata.ruleId, + severity: metadata.severity, + node: descriptor.node, + message: normalizeMessagePlaceholders(descriptor), + loc: normalizeReportLoc(descriptor), + fix: normalizeFixes(descriptor, metadata.sourceCode), + sourceLines: metadata.sourceCode.lines + }); }; };