Breaking: Update no-missing-placeholders, no-unused-placeholders, prefer-message-ids, prefer-placeholders rules to also apply to suggestion messages (#196)

bmish · web-flow · commit adcd4201e6ff · 2021-09-27T11:19:48.000+08:00
diff --git a/lib/rules/no-missing-placeholders.js b/lib/rules/no-missing-placeholders.js
@@ -45,33 +45,36 @@ module.exports = {
           node.callee.property.type === 'Identifier' && node.callee.property.name === 'report'
         ) {
           const reportInfo = utils.getReportInfo(node.arguments, context);
-          if (!reportInfo || !reportInfo.message) {
+          if (!reportInfo) {
             return;
           }
 
-          const messageStaticValue = getStaticValue(reportInfo.message, context.getScope());
-          if (
-            (
-              (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
-            // https://github.com/eslint/eslint/blob/e5446449d93668ccbdb79d78cc69f165ce4fde07/lib/eslint.js#L990
-            const PLACEHOLDER_MATCHER = /{{\s*([^{}]+?)\s*}}/g;
-            let match;
+          const reportMessagesAndDataArray = utils.collectReportViolationAndSuggestionData(reportInfo).filter(obj => obj.message);
+          for (const { message, data } of reportMessagesAndDataArray) {
+            const messageStaticValue = getStaticValue(message, context.getScope());
+            if (
+              (
+                (message.type === 'Literal' && typeof message.value === 'string') ||
+                (messageStaticValue && typeof messageStaticValue.value === 'string')
+              ) &&
+              (!data || data.type === 'ObjectExpression')
+            ) {
+              // Same regex as the one ESLint uses
+              // https://github.com/eslint/eslint/blob/e5446449d93668ccbdb79d78cc69f165ce4fde07/lib/eslint.js#L990
+              const PLACEHOLDER_MATCHER = /{{\s*([^{}]+?)\s*}}/g;
+              let match;
 
-            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]);
+              while ((match = PLACEHOLDER_MATCHER.exec(message.value || messageStaticValue.value))) { // eslint-disable-line no-extra-parens
+                const matchingProperty = data &&
+                  data.properties.find(prop => utils.getKeyName(prop) === match[1]);
 
-              if (!matchingProperty) {
-                context.report({
-                  node: reportInfo.message,
-                  messageId: 'placeholderDoesNotExist',
-                  data: { missingKey: match[1] },
-                });
+                if (!matchingProperty) {
+                  context.report({
+                    node: message,
+                    messageId: 'placeholderDoesNotExist',
+                    data: { missingKey: match[1] },
+                  });
+                }
               }
             }
           }
diff --git a/lib/rules/no-unused-placeholders.js b/lib/rules/no-unused-placeholders.js
@@ -45,38 +45,41 @@ module.exports = {
             node.callee.property.type === 'Identifier' && node.callee.property.name === 'report'
         ) {
           const reportInfo = utils.getReportInfo(node.arguments, context);
-          if (!reportInfo || !reportInfo.message) {
+          if (!reportInfo) {
             return;
           }
 
-          const messageStaticValue = getStaticValue(reportInfo.message, context.getScope());
-          if (
-            (
-              (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 || messageStaticValue.value;
-            // https://github.com/eslint/eslint/blob/2874d75ed8decf363006db25aac2d5f8991bd969/lib/linter.js#L986
-            const PLACEHOLDER_MATCHER = /{{\s*([^{}]+?)\s*}}/g;
-            const placeholdersInMessage = new Set();
+          const reportMessagesAndDataArray = utils.collectReportViolationAndSuggestionData(reportInfo).filter(obj => obj.message);
+          for (const { message, data } of reportMessagesAndDataArray) {
+            const messageStaticValue = getStaticValue(message, context.getScope());
+            if (
+              (
+                (message.type === 'Literal' && typeof message.value === 'string') ||
+                (messageStaticValue && typeof messageStaticValue.value === 'string')
+              ) &&
+              data &&
+              data.type === 'ObjectExpression'
+            ) {
+              const messageValue = 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();
 
-            message.replace(PLACEHOLDER_MATCHER, (fullMatch, term) => {
-              placeholdersInMessage.add(term);
-            });
+              messageValue.replace(PLACEHOLDER_MATCHER, (fullMatch, term) => {
+                placeholdersInMessage.add(term);
+              });
 
-            reportInfo.data.properties.forEach(prop => {
-              const key = utils.getKeyName(prop);
-              if (!placeholdersInMessage.has(key)) {
-                context.report({
-                  node: reportInfo.message,
-                  messageId: 'placeholderUnused',
-                  data: { unusedKey: key },
-                });
-              }
-            });
+              data.properties.forEach(prop => {
+                const key = utils.getKeyName(prop);
+                if (!placeholdersInMessage.has(key)) {
+                  context.report({
+                    node: message,
+                    messageId: 'placeholderUnused',
+                    data: { unusedKey: key },
+                  });
+                }
+              });
+            }
           }
         }
       },
diff --git a/lib/rules/prefer-message-ids.js b/lib/rules/prefer-message-ids.js
@@ -68,14 +68,17 @@ module.exports = {
              node.callee.property.type === 'Identifier' && node.callee.property.name === 'report'
         ) {
           const reportInfo = utils.getReportInfo(node.arguments, context);
-          if (!reportInfo || !reportInfo.message) {
+          if (!reportInfo) {
             return;
           }
 
-          context.report({
-            node: reportInfo.message.parent,
-            messageId: 'foundMessage',
-          });
+          const reportMessagesAndDataArray = utils.collectReportViolationAndSuggestionData(reportInfo).filter(obj => obj.message);
+          for (const { message } of reportMessagesAndDataArray) {
+            context.report({
+              node: message.parent,
+              messageId: 'foundMessage',
+            });
+          }
         }
       },
     };
diff --git a/lib/rules/prefer-placeholders.js b/lib/rules/prefer-placeholders.js
@@ -49,42 +49,43 @@ module.exports = {
         ) {
           const reportInfo = utils.getReportInfo(node.arguments, context);
 
-          if (!reportInfo || !reportInfo.message) {
+          if (!reportInfo) {
             return;
           }
 
-          let messageNode = reportInfo.message;
+          const reportMessagesAndDataArray = utils.collectReportViolationAndSuggestionData(reportInfo).filter(obj => obj.message);
+          for (let { message: messageNode } of reportMessagesAndDataArray) {
+            if (messageNode.type === 'Identifier') {
+              // See if we can find the variable declaration.
 
-          if (messageNode.type === 'Identifier') {
-            // See if we can find the variable declaration.
+              const variable = findVariable(
+                scopeManager.acquire(messageNode) || scopeManager.globalScope,
+                messageNode
+              );
 
-            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 (
-              !variable ||
-              !variable.defs ||
-              !variable.defs[0] ||
-              !variable.defs[0].node ||
-              variable.defs[0].node.type !== 'VariableDeclarator' ||
-              !variable.defs[0].node.init
+              (messageNode.type === 'TemplateLiteral' && messageNode.expressions.length > 0) ||
+              (messageNode.type === 'BinaryExpression' && messageNode.operator === '+')
             ) {
-              return;
+              context.report({
+                node: messageNode,
+                messageId: 'usePlaceholders',
+              });
             }
-
-            messageNode = variable.defs[0].node.init;
-          }
-
-          if (
-            (messageNode.type === 'TemplateLiteral' && messageNode.expressions.length > 0) ||
-            (messageNode.type === 'BinaryExpression' && messageNode.operator === '+')
-          ) {
-            context.report({
-              node: messageNode,
-              messageId: 'usePlaceholders',
-            });
           }
         }
       },
diff --git a/lib/utils.js b/lib/utils.js
@@ -169,6 +169,17 @@ function getRuleExportsCJS (ast) {
     }, {});
 }
 
+/**
+ * Find the value of a property in an object by its property key name.
+ * @param {Object} obj
+ * @param {String} keyName
+ * @returns property value
+ */
+function findObjectPropertyValueByKeyName (obj, keyName) {
+  const property = obj.properties.find(prop => prop.key.type === 'Identifier' && prop.key.name === keyName);
+  return property ? property.value : undefined;
+}
+
 module.exports = {
   /**
   * Performs static analysis on an AST to try to determine the final value of `module.exports`.
@@ -385,4 +396,32 @@ module.exports = {
       `,\n${propertyText}`
     );
   },
+
+  /**
+   * Collect all context.report({...}) violation/suggestion-related nodes into a standardized array for convenience.
+   * @param {Object} reportInfo - Result of getReportInfo().
+   * @returns {messageId?: String, message?: String, data?: Object, fix?: Function}[]
+   */
+  collectReportViolationAndSuggestionData (reportInfo) {
+    return [
+      // Violation message
+      {
+        messageId: reportInfo.messageId,
+        message: reportInfo.message,
+        data: reportInfo.data,
+        fix: reportInfo.fix,
+      },
+      // Suggestion messages
+      ...((reportInfo.suggest && reportInfo.suggest.elements) || [])
+        .map(suggestObjNode => {
+          return {
+            messageId: findObjectPropertyValueByKeyName(suggestObjNode, 'messageId'),
+            message: findObjectPropertyValueByKeyName(suggestObjNode, 'desc'), // Note: suggestion message named `desc`
+            data: findObjectPropertyValueByKeyName(suggestObjNode, 'data'),
+            fix: findObjectPropertyValueByKeyName(suggestObjNode, 'fix'),
+          };
+        }
+        ),
+    ];
+  },
 };
diff --git a/tests/lib/rules/no-missing-placeholders.js b/tests/lib/rules/no-missing-placeholders.js
@@ -128,6 +128,24 @@ ruleTester.run('no-missing-placeholders', rule, {
         context.report(node, MESSAGE, { baz: 'qux' });
       };
     `,
+    // Suggestion with placeholder
+    `
+      module.exports = {
+        create(context) {
+          context.report({
+            node,
+            suggest: [
+              {
+                desc: 'Remove {{functionName}}',
+                data: {
+                  functionName: 'foo'
+                }
+              }
+            ]
+          });
+        }
+      };
+    `,
   ],
 
   invalid: [
@@ -212,5 +230,45 @@ ruleTester.run('no-missing-placeholders', rule, {
       `,
       errors: [error('bar')],
     },
+
+    {
+      // Suggestion (no `data`)
+      code: `
+        module.exports = {
+          create(context) {
+            context.report({
+              node,
+              suggest: [
+                {
+                  desc: 'Remove {{bar}}'
+                }
+              ]
+            });
+          }
+        };
+      `,
+      errors: [error('bar')],
+    },
+    {
+      // Suggestion (`data` but missing placeholder)
+      code: `
+        module.exports = {
+          create(context) {
+            context.report({
+              node,
+              suggest: [
+                {
+                  desc: 'Remove {{bar}}',
+                  data: {
+                    notBar: 'abc'
+                  }
+                }
+              ]
+            });
+          }
+        };
+      `,
+      errors: [error('bar')],
+    },
   ],
 });
diff --git a/tests/lib/rules/no-unused-placeholders.js b/tests/lib/rules/no-unused-placeholders.js
@@ -107,6 +107,22 @@ ruleTester.run('no-unused-placeholders', rule, {
         context.report(node, { line: 1, column: 3 }, 'foo {{bar}}', { bar: 'baz' });
       };
     `,
+    // Suggestion
+    `
+      module.exports = {
+        create(context) {
+          context.report({
+            node,
+            suggest: [
+              {
+                desc: 'foo {{bar}}',
+                data: { 'bar': 'baz' }
+              }
+            ]
+          });
+        }
+      };
+    `,
   ],
 
   invalid: [
@@ -168,5 +184,24 @@ ruleTester.run('no-unused-placeholders', rule, {
       `,
       errors: [error('baz')],
     },
+    {
+      // Suggestion
+      code: `
+        module.exports = {
+          create(context) {
+            context.report({
+              node,
+              suggest: [
+                {
+                  desc: 'foo',
+                  data: { bar }
+                }
+              ]
+            });
+          }
+        };
+      `,
+      errors: [error('bar')],
+    },
   ],
 });
diff --git a/tests/lib/rules/prefer-message-ids.js b/tests/lib/rules/prefer-message-ids.js
diff --git a/tests/lib/rules/prefer-placeholders.js b/tests/lib/rules/prefer-placeholders.js
diff --git a/tests/lib/utils.js b/tests/lib/utils.js
