diff --git a/lib/rules/no-deprecated-report-api.js b/lib/rules/no-deprecated-report-api.js index 38c768f0..88dbcf58 100644 --- a/lib/rules/no-deprecated-report-api.js +++ b/lib/rules/no-deprecated-report-api.js @@ -48,7 +48,7 @@ module.exports = { fix (fixer) { const openingParen = sourceCode.getTokenBefore(node.arguments[0]); const closingParen = sourceCode.getLastToken(node); - const reportInfo = utils.getReportInfo(node.arguments); + const reportInfo = utils.getReportInfo(node.arguments, context); if (!reportInfo) { return null; diff --git a/lib/rules/no-missing-placeholders.js b/lib/rules/no-missing-placeholders.js index c4cf742e..ae9d95fb 100644 --- a/lib/rules/no-missing-placeholders.js +++ b/lib/rules/no-missing-placeholders.js @@ -6,6 +6,7 @@ 'use strict'; const utils = require('../utils'); +const { getStaticValue } = require('eslint-utils'); // ------------------------------------------------------------------------------ // Rule Definition @@ -40,13 +41,17 @@ module.exports = { contextIdentifiers.has(node.callee.object) && node.callee.property.type === 'Identifier' && node.callee.property.name === 'report' ) { - const reportInfo = utils.getReportInfo(node.arguments); + const reportInfo = utils.getReportInfo(node.arguments, context); + if (!reportInfo || !reportInfo.message) { + return; + } + const messageStaticValue = getStaticValue(reportInfo.message, context.getScope()); if ( - reportInfo && - reportInfo.message && - reportInfo.message.type === 'Literal' && - typeof reportInfo.message.value === 'string' && + ( + (reportInfo.message.type === 'Literal' && typeof reportInfo.message.value === 'string') || + (messageStaticValue && typeof messageStaticValue.value === 'string') + ) && (!reportInfo.data || reportInfo.data.type === 'ObjectExpression') ) { // Same regex as the one ESLint uses @@ -54,7 +59,7 @@ module.exports = { const PLACEHOLDER_MATCHER = /\{\{\s*([^{}]+?)\s*\}\}/g; let match; - while ((match = PLACEHOLDER_MATCHER.exec(reportInfo.message.value))) { // eslint-disable-line no-extra-parens + while ((match = PLACEHOLDER_MATCHER.exec(reportInfo.message.value || messageStaticValue.value))) { // eslint-disable-line no-extra-parens const matchingProperty = reportInfo.data && reportInfo.data.properties.find(prop => utils.getKeyName(prop) === match[1]); diff --git a/lib/rules/no-unused-placeholders.js b/lib/rules/no-unused-placeholders.js index af7b78d1..28041914 100644 --- a/lib/rules/no-unused-placeholders.js +++ b/lib/rules/no-unused-placeholders.js @@ -6,6 +6,7 @@ 'use strict'; const utils = require('../utils'); +const { getStaticValue } = require('eslint-utils'); // ------------------------------------------------------------------------------ // Rule Definition @@ -40,16 +41,21 @@ module.exports = { contextIdentifiers.has(node.callee.object) && node.callee.property.type === 'Identifier' && node.callee.property.name === 'report' ) { - const reportInfo = utils.getReportInfo(node.arguments); + const reportInfo = utils.getReportInfo(node.arguments, context); + if (!reportInfo || !reportInfo.message) { + return; + } + const messageStaticValue = getStaticValue(reportInfo.message, context.getScope()); if ( - reportInfo && - reportInfo.message && - reportInfo.message.type === 'Literal' && - typeof reportInfo.message.value === 'string' && - reportInfo.data && reportInfo.data.type === 'ObjectExpression' + ( + (reportInfo.message.type === 'Literal' && typeof reportInfo.message.value === 'string') || + (messageStaticValue && typeof messageStaticValue.value === 'string') + ) && + reportInfo.data && + reportInfo.data.type === 'ObjectExpression' ) { - const message = reportInfo.message.value; + const message = reportInfo.message.value || messageStaticValue.value; // https://github.com/eslint/eslint/blob/2874d75ed8decf363006db25aac2d5f8991bd969/lib/linter.js#L986 const PLACEHOLDER_MATCHER = /\{\{\s*([^{}]+?)\s*\}\}/g; const placeholdersInMessage = new Set(); diff --git a/lib/rules/prefer-placeholders.js b/lib/rules/prefer-placeholders.js index ec58277e..54d7ff1c 100644 --- a/lib/rules/prefer-placeholders.js +++ b/lib/rules/prefer-placeholders.js @@ -6,6 +6,7 @@ 'use strict'; const utils = require('../utils'); +const { findVariable } = require('eslint-utils'); // ------------------------------------------------------------------------------ // Rule Definition @@ -26,6 +27,9 @@ module.exports = { create (context) { let contextIdentifiers; + const sourceCode = context.getSourceCode(); + const { scopeManager } = sourceCode; + // ---------------------------------------------------------------------- // Public // ---------------------------------------------------------------------- @@ -40,16 +44,42 @@ module.exports = { contextIdentifiers.has(node.callee.object) && node.callee.property.type === 'Identifier' && node.callee.property.name === 'report' ) { - const reportInfo = utils.getReportInfo(node.arguments); + const reportInfo = utils.getReportInfo(node.arguments, context); + + if (!reportInfo || !reportInfo.message) { + return; + } + + let messageNode = reportInfo.message; + + if (messageNode.type === 'Identifier') { + // See if we can find the variable declaration. + + const variable = findVariable( + scopeManager.acquire(messageNode) || scopeManager.globalScope, + messageNode + ); + + if ( + !variable || + !variable.defs || + !variable.defs[0] || + !variable.defs[0].node || + variable.defs[0].node.type !== 'VariableDeclarator' || + !variable.defs[0].node.init + ) { + return; + } + + messageNode = variable.defs[0].node.init; + } if ( - reportInfo && reportInfo.message && ( - (reportInfo.message.type === 'TemplateLiteral' && reportInfo.message.expressions.length) || - (reportInfo.message.type === 'BinaryExpression' && reportInfo.message.operator === '+') - ) + (messageNode.type === 'TemplateLiteral' && messageNode.expressions.length) || + (messageNode.type === 'BinaryExpression' && messageNode.operator === '+') ) { context.report({ - node: reportInfo.message, + node: messageNode, message: 'Use report message placeholders instead of string concatenation.', }); } diff --git a/lib/rules/prefer-replace-text.js b/lib/rules/prefer-replace-text.js index 3b7278b2..2e89466a 100644 --- a/lib/rules/prefer-replace-text.js +++ b/lib/rules/prefer-replace-text.js @@ -47,7 +47,7 @@ module.exports = { parent.parent.parent.type === 'CallExpression' && contextIdentifiers.has(parent.parent.parent.callee.object) && parent.parent.parent.callee.property.name === 'report' && - utils.getReportInfo(parent.parent.parent.arguments).fix === node; + utils.getReportInfo(parent.parent.parent.arguments, context).fix === node; funcInfo = { upper: funcInfo, diff --git a/lib/rules/report-message-format.js b/lib/rules/report-message-format.js index cef98cfb..a02dd3ee 100644 --- a/lib/rules/report-message-format.js +++ b/lib/rules/report-message-format.js @@ -5,6 +5,7 @@ 'use strict'; +const { getStaticValue } = require('eslint-utils'); const utils = require('../utils'); // ------------------------------------------------------------------------------ @@ -35,9 +36,11 @@ module.exports = { * @returns {void} */ function processMessageNode (message) { + const staticValue = getStaticValue(message, context.getScope()); if ( (message.type === 'Literal' && typeof message.value === 'string' && !pattern.test(message.value)) || - (message.type === 'TemplateLiteral' && message.quasis.length === 1 && !pattern.test(message.quasis[0].value.cooked)) + (message.type === 'TemplateLiteral' && message.quasis.length === 1 && !pattern.test(message.quasis[0].value.cooked)) || + (staticValue && !pattern.test(staticValue.value)) ) { context.report({ node: message, @@ -75,7 +78,7 @@ module.exports = { contextIdentifiers.has(node.callee.object) && node.callee.property.type === 'Identifier' && node.callee.property.name === 'report' ) { - const reportInfo = utils.getReportInfo(node.arguments); + const reportInfo = utils.getReportInfo(node.arguments, context); const message = reportInfo && reportInfo.message; if (!message) { diff --git a/lib/utils.js b/lib/utils.js index 829c16ad..73beb8cd 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -1,5 +1,7 @@ 'use strict'; +const { getStaticValue } = require('eslint-utils'); + /** * Determines whether a node is a 'normal' (i.e. non-async, non-generator) function expression. * @param {ASTNode} node The node in question @@ -260,8 +262,9 @@ module.exports = { /** * Gets information on a report, given the arguments passed to context.report(). * @param {ASTNode[]} reportArgs The arguments passed to context.report() + * @param {Context} context */ - getReportInfo (reportArgs) { + getReportInfo (reportArgs, context) { // If there is exactly one argument, the API expects an object. // Otherwise, if the second argument is a string, the arguments are interpreted as // ['node', 'message', 'data', 'fix']. @@ -287,15 +290,17 @@ module.exports = { let keys; + const secondArgStaticValue = getStaticValue(reportArgs[1], context.getScope()); if ( - (reportArgs[1].type === 'Literal' && typeof reportArgs[1].value === 'string') || + (secondArgStaticValue && typeof secondArgStaticValue.value === 'string') || reportArgs[1].type === 'TemplateLiteral' ) { keys = ['node', 'message', 'data', 'fix']; } else if ( reportArgs[1].type === 'ObjectExpression' || reportArgs[1].type === 'ArrayExpression' || - (reportArgs[1].type === 'Literal' && typeof reportArgs[1].value !== 'string') + (reportArgs[1].type === 'Literal' && typeof reportArgs[1].value !== 'string') || + (secondArgStaticValue && ['object', 'number'].includes(typeof secondArgStaticValue.value)) ) { keys = ['node', 'loc', 'message', 'data', 'fix']; } else { diff --git a/tests/lib/rules/no-deprecated-report-api.js b/tests/lib/rules/no-deprecated-report-api.js index 24a81272..66c4b80b 100644 --- a/tests/lib/rules/no-deprecated-report-api.js +++ b/tests/lib/rules/no-deprecated-report-api.js @@ -62,6 +62,24 @@ ruleTester.run('no-deprecated-report-api', rule, { } }; `, + // With object as variable. + ` + const OBJ = {node, message}; + module.exports = { + create(context) { + context.report(OBJ); + } + }; + `, + // With object as variable but cannot determine its value statically. + ` + const OBJ = getObj(); + module.exports = { + create(context) { + context.report(OBJ); + } + }; + `, ], invalid: [ @@ -161,6 +179,39 @@ ruleTester.run('no-deprecated-report-api', rule, { `, errors: [ERROR], }, + { + // With message string in variable. + code: ` + const MESSAGE = 'foo'; + module.exports = { + create(context) { + context.report(theNode, MESSAGE); + } + }; + `, + output: ` + const MESSAGE = 'foo'; + module.exports = { + create(context) { + context.report({node: theNode, message: MESSAGE}); + } + }; + `, + errors: [ERROR], + }, + { + // With message in variable but no autofix since we can't statically determine its type. + code: ` + const MESSAGE = getMessage(); + module.exports = { + create(context) { + context.report(theNode, MESSAGE); + } + }; + `, + output: null, + errors: [ERROR], + }, { code: ` module.exports = { @@ -198,6 +249,49 @@ ruleTester.run('no-deprecated-report-api', rule, { `, errors: [ERROR], }, + { + // Location in variable as number. + code: ` + const LOC = 5; + module.exports.create = context => { + context.report(theNode, LOC, foo, bar); + }; + `, + output: ` + const LOC = 5; + module.exports.create = context => { + context.report({node: theNode, loc: LOC, message: foo, data: bar}); + }; + `, + errors: [ERROR], + }, + { + // Location in variable as object. + code: ` + const LOC = { line: 1, column: 2 }; + module.exports.create = context => { + context.report(theNode, LOC, foo, bar); + }; + `, + output: ` + const LOC = { line: 1, column: 2 }; + module.exports.create = context => { + context.report({node: theNode, loc: LOC, message: foo, data: bar}); + }; + `, + errors: [ERROR], + }, + { + // Location in variable but no autofix since we can't statically determine its type. + code: ` + const LOC = getLoc(); + module.exports.create = context => { + context.report(theNode, LOC, foo, bar); + }; + `, + output: null, + errors: [ERROR], + }, { code: ` module.exports = { diff --git a/tests/lib/rules/no-missing-placeholders.js b/tests/lib/rules/no-missing-placeholders.js index c6574c9b..09b8ce36 100644 --- a/tests/lib/rules/no-missing-placeholders.js +++ b/tests/lib/rules/no-missing-placeholders.js @@ -17,8 +17,8 @@ const RuleTester = require('eslint').RuleTester; * @param {string} missingKey The placeholder that is missing * @returns {object} An expected error */ -function error (missingKey) { - return { type: 'Literal', message: `The placeholder {{${missingKey}}} does not exist.` }; +function error (missingKey, type = 'Literal') { + return { type, message: `The placeholder {{${missingKey}}} does not exist.` }; } // ------------------------------------------------------------------------------ @@ -114,6 +114,20 @@ ruleTester.run('no-missing-placeholders', rule, { } }; `, + // Message in variable. + ` + const MESSAGE = 'foo {{bar}}'; + module.exports = context => { + context.report(node, MESSAGE, { bar: 'baz' }); + }; + `, + // Message in variable but cannot statically determine its type. + ` + const MESSAGE = getMessage(); + module.exports = context => { + context.report(node, MESSAGE, { baz: 'qux' }); + }; + `, ], invalid: [ @@ -166,6 +180,16 @@ ruleTester.run('no-missing-placeholders', rule, { `, errors: [error('bar')], }, + { + // Message in variable. + code: ` + const MESSAGE = 'foo {{bar}}'; + module.exports = context => { + context.report(node, MESSAGE, { baz: 'qux' }); + }; + `, + errors: [error('bar', 'Identifier')], + }, { code: ` module.exports = context => { diff --git a/tests/lib/rules/no-unused-placeholders.js b/tests/lib/rules/no-unused-placeholders.js index febe8c4f..4646de9f 100644 --- a/tests/lib/rules/no-unused-placeholders.js +++ b/tests/lib/rules/no-unused-placeholders.js @@ -17,8 +17,8 @@ const RuleTester = require('eslint').RuleTester; * @param {string} unusedKey The placeholder that is unused * @returns {object} An expected error */ -function error (unusedKey) { - return { type: 'Literal', message: `The placeholder {{${unusedKey}}} is unused.` }; +function error (unusedKey, type = 'Literal') { + return { type, message: `The placeholder {{${unusedKey}}} is unused.` }; } // ------------------------------------------------------------------------------ @@ -88,6 +88,20 @@ ruleTester.run('no-unused-placeholders', rule, { context.report(node, 'foo {{bar}}', { bar: 'baz' }); }; `, + // With message as variable. + ` + const MESSAGE = 'foo {{bar}}'; + module.exports = context => { + context.report(node, MESSAGE, { bar: 'baz' }); + }; + `, + // With message as variable but cannot statically determine its type. + ` + const MESSAGE = getMessage(); + module.exports = context => { + context.report(node, MESSAGE, { bar: 'baz' }); + }; + `, ` module.exports = context => { context.report(node, { line: 1, column: 3 }, 'foo {{bar}}', { bar: 'baz' }); @@ -110,6 +124,22 @@ ruleTester.run('no-unused-placeholders', rule, { `, errors: [error('bar')], }, + { + // With message as variable. + code: ` + const MESSAGE = 'foo'; + module.exports = { + create(context) { + context.report({ + node, + message: MESSAGE, + data: { bar } + }); + } + }; + `, + errors: [error('bar', 'Identifier')], + }, { code: ` module.exports = { diff --git a/tests/lib/rules/prefer-placeholders.js b/tests/lib/rules/prefer-placeholders.js index 140634d4..ce7daedc 100644 --- a/tests/lib/rules/prefer-placeholders.js +++ b/tests/lib/rules/prefer-placeholders.js @@ -60,6 +60,24 @@ ruleTester.run('prefer-placeholders', rule, { } }; `, + // With message in variable. + ` + const MESSAGE = 'foo is bad.'; + module.exports = { + create(context) { + context.report(node, MESSAGE); + } + }; + `, + // With message in variable but cannot statically determine its value. + ` + const MESSAGE = getMessage(); + module.exports = { + create(context) { + context.report(node, MESSAGE); + } + }; + `, ], invalid: [ @@ -76,6 +94,21 @@ ruleTester.run('prefer-placeholders', rule, { `, errors: [ERROR], }, + { + // With message in variable. + code: ` + const MESSAGE = \`\${foo} is bad.\`; + module.exports = { + create(context) { + context.report({ + node, + message: MESSAGE + }); + } + }; + `, + errors: [ERROR], + }, { code: ` module.exports = { diff --git a/tests/lib/rules/report-message-format.js b/tests/lib/rules/report-message-format.js index 03813a0d..d93543d2 100644 --- a/tests/lib/rules/report-message-format.js +++ b/tests/lib/rules/report-message-format.js @@ -33,6 +33,30 @@ ruleTester.run('report-message-format', rule, { `, options: ['foo'], }, + { + // With message as variable. + code: ` + const MESSAGE = 'foo'; + module.exports = { + create(context) { + context.report(node, MESSAGE); + } + }; + `, + options: ['foo'], + }, + { + // With message as variable but cannot statically determine its type. + code: ` + const MESSAGE = getMessage(); + module.exports = { + create(context) { + context.report(node, MESSAGE); + } + }; + `, + options: ['foo'], + }, { code: ` module.exports = { @@ -140,6 +164,18 @@ ruleTester.run('report-message-format', rule, { `, options: ['foo'], }, + { + // With message as variable. + code: ` + const MESSAGE = 'bar'; + module.exports = { + create(context) { + context.report(node, MESSAGE); + } + }; + `, + options: ['foo'], + }, { code: ` module.exports = { diff --git a/tests/lib/utils.js b/tests/lib/utils.js index 8f5cb5e4..1c60a5cc 100644 --- a/tests/lib/utils.js +++ b/tests/lib/utils.js @@ -352,7 +352,8 @@ describe('utils', () => { `context.report(${args.join(', ')})`, { ecmaVersion: 6, loc: false, range: false } ).body[0].expression.arguments; - const reportInfo = utils.getReportInfo(parsedArgs); + const context = { getScope () {} }; // mock object + const reportInfo = utils.getReportInfo(parsedArgs, context); assert.deepEqual(reportInfo, CASES.get(args)(parsedArgs)); });