From af5886cb8d13c738d38cffddc8f0b3ba4e27ecfe Mon Sep 17 00:00:00 2001 From: Bryan Mishkin <698306+bmish@users.noreply.github.com> Date: Sun, 19 Jun 2022 21:42:55 -0400 Subject: [PATCH 1/2] breaking: update no-missing-placeholders and no-unused-placeholders to handle messageIds --- lib/rules/no-missing-placeholders.js | 42 +++++++-- lib/rules/no-unused-placeholders.js | 42 +++++++-- tests/lib/rules/no-missing-placeholders.js | 99 ++++++++++++++++++++++ tests/lib/rules/no-unused-placeholders.js | 90 ++++++++++++++++++++ 4 files changed, 263 insertions(+), 10 deletions(-) diff --git a/lib/rules/no-missing-placeholders.js b/lib/rules/no-missing-placeholders.js index e598e882..dfab457c 100644 --- a/lib/rules/no-missing-placeholders.js +++ b/lib/rules/no-missing-placeholders.js @@ -31,6 +31,7 @@ module.exports = { }, create(context) { + const sourceCode = context.getSourceCode(); let contextIdentifiers; // ---------------------------------------------------------------------- @@ -39,7 +40,6 @@ module.exports = { return { Program(ast) { - const sourceCode = context.getSourceCode(); contextIdentifiers = utils.getContextIdentifiers( sourceCode.scopeManager, ast @@ -57,10 +57,42 @@ module.exports = { return; } - const reportMessagesAndDataArray = utils - .collectReportViolationAndSuggestionData(reportInfo) - .filter((obj) => obj.message); - for (const { message, data } of reportMessagesAndDataArray) { + const info = utils.getRuleInfo(sourceCode); + const metaNode = info.meta; + const messagesNode = + metaNode && + metaNode.properties && + metaNode.properties.find( + (p) => p.type === 'Property' && utils.getKeyName(p) === 'messages' + ); + + const reportMessagesAndDataArray = + utils.collectReportViolationAndSuggestionData(reportInfo); + + // Check for any potential instances where we can use the messageId to fill in the message for convenience. + reportMessagesAndDataArray.forEach((obj) => { + if ( + !obj.message && + obj.messageId && + obj.messageId.type === 'Literal' + ) { + const correspondingMessage = + messagesNode && + messagesNode.value.properties && + messagesNode.value.properties.find( + (p) => + p.type === 'Property' && + utils.getKeyName(p) === obj.messageId.value + ); + if (correspondingMessage) { + obj.message = correspondingMessage.value; + } + } + }); + + for (const { message, data } of reportMessagesAndDataArray.filter( + (obj) => obj.message + )) { const messageStaticValue = getStaticValue( message, context.getScope() diff --git a/lib/rules/no-unused-placeholders.js b/lib/rules/no-unused-placeholders.js index c552bfee..d6547b1d 100644 --- a/lib/rules/no-unused-placeholders.js +++ b/lib/rules/no-unused-placeholders.js @@ -30,6 +30,7 @@ module.exports = { }, create(context) { + const sourceCode = context.getSourceCode(); let contextIdentifiers; // ---------------------------------------------------------------------- @@ -38,7 +39,6 @@ module.exports = { return { Program(ast) { - const sourceCode = context.getSourceCode(); contextIdentifiers = utils.getContextIdentifiers( sourceCode.scopeManager, ast @@ -56,10 +56,42 @@ module.exports = { return; } - const reportMessagesAndDataArray = utils - .collectReportViolationAndSuggestionData(reportInfo) - .filter((obj) => obj.message); - for (const { message, data } of reportMessagesAndDataArray) { + const info = utils.getRuleInfo(sourceCode); + const metaNode = info.meta; + const messagesNode = + metaNode && + metaNode.properties && + metaNode.properties.find( + (p) => p.type === 'Property' && utils.getKeyName(p) === 'messages' + ); + + const reportMessagesAndDataArray = + utils.collectReportViolationAndSuggestionData(reportInfo); + + // Check for any potential instances where we can use the messageId to fill in the message for convenience. + reportMessagesAndDataArray.forEach((obj) => { + if ( + !obj.message && + obj.messageId && + obj.messageId.type === 'Literal' + ) { + const correspondingMessage = + messagesNode && + messagesNode.value.properties && + messagesNode.value.properties.find( + (p) => + p.type === 'Property' && + utils.getKeyName(p) === obj.messageId.value + ); + if (correspondingMessage) { + obj.message = correspondingMessage.value; + } + } + }); + + for (const { message, data } of reportMessagesAndDataArray.filter( + (obj) => obj.message + )) { const messageStaticValue = getStaticValue( message, context.getScope() diff --git a/tests/lib/rules/no-missing-placeholders.js b/tests/lib/rules/no-missing-placeholders.js index 78d9a645..d9f31fba 100644 --- a/tests/lib/rules/no-missing-placeholders.js +++ b/tests/lib/rules/no-missing-placeholders.js @@ -113,6 +113,52 @@ ruleTester.run('no-missing-placeholders', rule, { } }; `, + // messageId but no placeholder. + ` + module.exports = { + meta: { + messages: { myMessageId: 'foo' } + }, + create(context) { + context.report({ node, messageId: 'myMessageId' }); + } + }; + `, + // messageId but the message property doesn't exist yet. + ` + module.exports = { + meta: { + messages: { } + }, + create(context) { + context.report({ node, messageId: 'myMessageId' }); + } + }; + `, + // messageId but no messages object doesn't exist yet. + ` + module.exports = { + meta: { }, + create(context) { + context.report({ node, messageId: 'myMessageId' }); + } + }; + `, + // messageId with correctly-used placeholder. + ` + module.exports = { + meta: { + messages: { myMessageId: 'foo {{bar}}' } + }, + create(context) { + context.report({ + node, + messageId: 'myMessageId', + data: { bar: 'baz' } + }); + } + }; + `, // Message in variable. ` const MESSAGE = 'foo {{bar}}'; @@ -145,6 +191,25 @@ ruleTester.run('no-missing-placeholders', rule, { } }; `, + // Suggestion with messageId + ` + module.exports = { + meta: { messages: { myMessageId: 'Remove {{functionName}}' } }, + create(context) { + context.report({ + node, + suggest: [ + { + messageId: 'myMessageId', + data: { + functionName: 'foo' + } + } + ] + }); + } + }; + `, ], invalid: [ @@ -269,6 +334,25 @@ ruleTester.run('no-missing-placeholders', rule, { `, errors: [error('bar')], }, + { + // Suggestion and messageId + code: ` + module.exports = { + meta: { messages: { myMessageId: 'foo {{bar}}' } }, + create(context) { + context.report({ + node, + suggest: [ + { + messageId: 'myMessageId', + } + ] + }); + } + }; + `, + errors: [error('bar')], + }, { // `create` in variable. code: ` @@ -283,5 +367,20 @@ ruleTester.run('no-missing-placeholders', rule, { `, errors: [error('hasOwnProperty')], }, + { + // messageId. + code: ` + module.exports = { + meta: { messages: { myMessageId: 'foo {{bar}}' } }, + create(context) { + context.report({ + node, + messageId: 'myMessageId' + }); + } + }; + `, + errors: [error('bar')], + }, ], }); diff --git a/tests/lib/rules/no-unused-placeholders.js b/tests/lib/rules/no-unused-placeholders.js index cabdbc2c..b915a5a8 100644 --- a/tests/lib/rules/no-unused-placeholders.js +++ b/tests/lib/rules/no-unused-placeholders.js @@ -122,6 +122,60 @@ ruleTester.run('no-unused-placeholders', rule, { } }; `, + // Suggestion with messageId + ` + module.exports = { + meta: { messages: { myMessageId: 'foo {{bar}}' } }, + create(context) { + context.report({ + node, + suggest: [ + { + messageId: 'myMessageId', + data: { 'bar': 'baz' } + } + ] + }); + } + }; + `, + // messageId but no placeholder. + ` + module.exports = { + meta: { + messages: { myMessageId: 'foo' } + }, + create(context) { + context.report({ node, messageId: 'myMessageId' }); + } + }; + `, + // messageId but the message property doesn't exist yet. + ` + module.exports = { + meta: { + messages: { } + }, + create(context) { + context.report({ node, messageId: 'myMessageId' }); + } + }; + `, + // messageId with correctly-used placeholder. + ` + module.exports = { + meta: { + messages: { myMessageId: 'foo {{bar}}' } + }, + create(context) { + context.report({ + node, + messageId: 'myMessageId', + data: { bar: 'baz' } + }); + } + }; + `, ], invalid: [ @@ -197,6 +251,22 @@ ruleTester.run('no-unused-placeholders', rule, { `, errors: [error('baz')], }, + { + // messageId. + code: ` + module.exports = { + meta: { messages: { myMessageId: 'foo' } }, + create(context) { + context.report({ + node, + messageId: 'myMessageId', + data: { bar } + }); + } + }; + `, + errors: [error('bar')], + }, { // Suggestion code: ` @@ -216,5 +286,25 @@ ruleTester.run('no-unused-placeholders', rule, { `, errors: [error('bar')], }, + { + // Suggestion and messageId + code: ` + module.exports = { + meta: { messages: { myMessageId: 'foo' } }, + create(context) { + context.report({ + node, + suggest: [ + { + messageId: 'myMessageId', + data: { bar } + } + ] + }); + } + }; + `, + errors: [error('bar')], + }, ], }); From d7461a34734fc0b8235d76f783966999ff2227ff Mon Sep 17 00:00:00 2001 From: Bryan Mishkin <698306+bmish@users.noreply.github.com> Date: Mon, 4 Jul 2022 10:57:39 -0400 Subject: [PATCH 2/2] use utils --- lib/rules/no-missing-message-ids.js | 2 +- lib/rules/no-missing-placeholders.js | 58 +++++++++------------- lib/rules/no-unused-placeholders.js | 58 +++++++++------------- tests/lib/rules/no-missing-placeholders.js | 12 ++++- tests/lib/rules/no-unused-placeholders.js | 17 +++++++ 5 files changed, 76 insertions(+), 71 deletions(-) diff --git a/lib/rules/no-missing-message-ids.js b/lib/rules/no-missing-message-ids.js index f10c9507..ac7318dc 100644 --- a/lib/rules/no-missing-message-ids.js +++ b/lib/rules/no-missing-message-ids.js @@ -71,7 +71,7 @@ module.exports = { values.forEach((val) => { if ( val.type === 'Literal' && - val.value !== null && + typeof val.value === 'string' && val.value !== '' && !utils.getMessageIdNodeById( val.value, diff --git a/lib/rules/no-missing-placeholders.js b/lib/rules/no-missing-placeholders.js index dfab457c..6523d2a8 100644 --- a/lib/rules/no-missing-placeholders.js +++ b/lib/rules/no-missing-placeholders.js @@ -32,18 +32,16 @@ module.exports = { create(context) { const sourceCode = context.getSourceCode(); + const { scopeManager } = sourceCode; + let contextIdentifiers; - // ---------------------------------------------------------------------- - // Public - // ---------------------------------------------------------------------- + const ruleInfo = utils.getRuleInfo(sourceCode); + const messagesNode = utils.getMessagesNode(ruleInfo, scopeManager); return { Program(ast) { - contextIdentifiers = utils.getContextIdentifiers( - sourceCode.scopeManager, - ast - ); + contextIdentifiers = utils.getContextIdentifiers(scopeManager, ast); }, CallExpression(node) { if ( @@ -57,38 +55,30 @@ module.exports = { return; } - const info = utils.getRuleInfo(sourceCode); - const metaNode = info.meta; - const messagesNode = - metaNode && - metaNode.properties && - metaNode.properties.find( - (p) => p.type === 'Property' && utils.getKeyName(p) === 'messages' - ); - const reportMessagesAndDataArray = utils.collectReportViolationAndSuggestionData(reportInfo); - // Check for any potential instances where we can use the messageId to fill in the message for convenience. - reportMessagesAndDataArray.forEach((obj) => { - if ( - !obj.message && - obj.messageId && - obj.messageId.type === 'Literal' - ) { - const correspondingMessage = - messagesNode && - messagesNode.value.properties && - messagesNode.value.properties.find( - (p) => - p.type === 'Property' && - utils.getKeyName(p) === obj.messageId.value + if (messagesNode) { + // Check for any potential instances where we can use the messageId to fill in the message for convenience. + reportMessagesAndDataArray.forEach((obj) => { + if ( + !obj.message && + obj.messageId && + obj.messageId.type === 'Literal' && + typeof obj.messageId.value === 'string' + ) { + const correspondingMessage = utils.getMessageIdNodeById( + obj.messageId.value, + ruleInfo, + scopeManager, + context.getScope() ); - if (correspondingMessage) { - obj.message = correspondingMessage.value; + if (correspondingMessage) { + obj.message = correspondingMessage.value; + } } - } - }); + }); + } for (const { message, data } of reportMessagesAndDataArray.filter( (obj) => obj.message diff --git a/lib/rules/no-unused-placeholders.js b/lib/rules/no-unused-placeholders.js index d6547b1d..61f17adb 100644 --- a/lib/rules/no-unused-placeholders.js +++ b/lib/rules/no-unused-placeholders.js @@ -31,18 +31,16 @@ module.exports = { create(context) { const sourceCode = context.getSourceCode(); + const { scopeManager } = sourceCode; + let contextIdentifiers; - // ---------------------------------------------------------------------- - // Public - // ---------------------------------------------------------------------- + const ruleInfo = utils.getRuleInfo(sourceCode); + const messagesNode = utils.getMessagesNode(ruleInfo, scopeManager); return { Program(ast) { - contextIdentifiers = utils.getContextIdentifiers( - sourceCode.scopeManager, - ast - ); + contextIdentifiers = utils.getContextIdentifiers(scopeManager, ast); }, CallExpression(node) { if ( @@ -56,38 +54,30 @@ module.exports = { return; } - const info = utils.getRuleInfo(sourceCode); - const metaNode = info.meta; - const messagesNode = - metaNode && - metaNode.properties && - metaNode.properties.find( - (p) => p.type === 'Property' && utils.getKeyName(p) === 'messages' - ); - const reportMessagesAndDataArray = utils.collectReportViolationAndSuggestionData(reportInfo); - // Check for any potential instances where we can use the messageId to fill in the message for convenience. - reportMessagesAndDataArray.forEach((obj) => { - if ( - !obj.message && - obj.messageId && - obj.messageId.type === 'Literal' - ) { - const correspondingMessage = - messagesNode && - messagesNode.value.properties && - messagesNode.value.properties.find( - (p) => - p.type === 'Property' && - utils.getKeyName(p) === obj.messageId.value + if (messagesNode) { + // Check for any potential instances where we can use the messageId to fill in the message for convenience. + reportMessagesAndDataArray.forEach((obj) => { + if ( + !obj.message && + obj.messageId && + obj.messageId.type === 'Literal' && + typeof obj.messageId.value === 'string' + ) { + const correspondingMessage = utils.getMessageIdNodeById( + obj.messageId.value, + ruleInfo, + scopeManager, + context.getScope() ); - if (correspondingMessage) { - obj.message = correspondingMessage.value; + if (correspondingMessage) { + obj.message = correspondingMessage.value; + } } - } - }); + }); + } for (const { message, data } of reportMessagesAndDataArray.filter( (obj) => obj.message diff --git a/tests/lib/rules/no-missing-placeholders.js b/tests/lib/rules/no-missing-placeholders.js index d9f31fba..fb2aafb6 100644 --- a/tests/lib/rules/no-missing-placeholders.js +++ b/tests/lib/rules/no-missing-placeholders.js @@ -124,7 +124,7 @@ ruleTester.run('no-missing-placeholders', rule, { } }; `, - // messageId but the message property doesn't exist yet. + // messageId but the message doesn't exist in `meta.messages`. ` module.exports = { meta: { @@ -135,7 +135,7 @@ ruleTester.run('no-missing-placeholders', rule, { } }; `, - // messageId but no messages object doesn't exist yet. + // messageId but no `meta.messages`. ` module.exports = { meta: { }, @@ -144,6 +144,14 @@ ruleTester.run('no-missing-placeholders', rule, { } }; `, + // messageId but no `meta`. + ` + module.exports = { + create(context) { + context.report({ node, messageId: 'myMessageId' }); + } + }; + `, // messageId with correctly-used placeholder. ` module.exports = { diff --git a/tests/lib/rules/no-unused-placeholders.js b/tests/lib/rules/no-unused-placeholders.js index b915a5a8..8679d0c4 100644 --- a/tests/lib/rules/no-unused-placeholders.js +++ b/tests/lib/rules/no-unused-placeholders.js @@ -161,6 +161,23 @@ ruleTester.run('no-unused-placeholders', rule, { } }; `, + // messageId but no `meta.messages`. + ` + module.exports = { + meta: {}, + create(context) { + context.report({ node, messageId: 'myMessageId' }); + } + }; + `, + // messageId but no `meta`. + ` + module.exports = { + create(context) { + context.report({ node, messageId: 'myMessageId' }); + } + }; + `, // messageId with correctly-used placeholder. ` module.exports = {