Repository: not-an-aardvark/eslint-plugin-eslint-plugin (js)

Issue:
Breaking: Message rules should be updated to apply to suggestion messages
These should all be updated to consider both autofixer/suggestion messages:

- no-missing-placeholders
- no-unused-placeholders
- prefer-message-ids
- prefer-placeholders

Pull request:
Breaking: Update `no-missing-placeholders`, `no-unused-placeholders`, `prefer-message-ids`, `prefer-placeholders` rules to also apply to suggestion messages
Rules should apply to both violation messages and suggestion messages.

Fixes #191.

Part of v4 release (#120).

Patch:
diff --git a/lib/rules/no-missing-placeholders.js b/lib/rules/no-missing-placeholders.js
index 77aca1b..1f66c2b 100644
--- 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
index 83c38a4..9cfc158 100644
--- 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
index 8fddd35..e6a0dc5 100644
--- 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
index 21cf23a..666d23b 100644
--- 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
index f07d499..8ed164b 100644
--- 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'),
+          };
+        }
+        ),
+    ];
+  },
 };


Test patch:
diff --git a/tests/lib/rules/no-missing-placeholders.js b/tests/lib/rules/no-missing-placeholders.js
index 09b8ce3..997f236 100644
--- 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
index 4646de9..87b6f06 100644
--- 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
index 2dc5343..952509f 100644
--- a/tests/lib/rules/prefer-message-ids.js
+++ b/tests/lib/rules/prefer-message-ids.js
@@ -29,6 +29,14 @@ ruleTester.run('prefer-message-ids', rule, {
         }
       };
     `,
+    // Suggestion
+    `
+      module.exports = {
+        create(context) {
+          context.report({ node, suggest: [{messageId:'foo'}] });
+        }
+      };
+    `,
     {
       // ESM
       code: `
@@ -102,6 +110,17 @@ ruleTester.run('prefer-message-ids', rule, {
       `,
       errors: [{ messageId: 'foundMessage', type: 'Property' }],
     },
+    {
+      // Suggestion
+      code: `
+        module.exports = {
+          create(context) {
+            context.report({ node, suggest: [{desc:'foo'}] });
+          }
+        };
+      `,
+      errors: [{ messageId: 'foundMessage', type: 'Property' }],
+    },
     {
       // ESM
       code: `
diff --git a/tests/lib/rules/prefer-placeholders.js b/tests/lib/rules/prefer-placeholders.js
index 2de61b8..1bf9f40 100644
--- a/tests/lib/rules/prefer-placeholders.js
+++ b/tests/lib/rules/prefer-placeholders.js
@@ -17,8 +17,6 @@ const RuleTester = require('eslint').RuleTester;
 // ------------------------------------------------------------------------------
 
 const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 6 } });
-const ERROR = { messageId: 'usePlaceholders' };
-
 
 ruleTester.run('prefer-placeholders', rule, {
   valid: [
@@ -78,6 +76,44 @@ ruleTester.run('prefer-placeholders', rule, {
         }
       };
     `,
+    // Suggestion (message + data)
+    `
+      module.exports = {
+        create(context) {
+          context.report({
+            node,
+            suggest: [
+              {
+                message: '{{foo}} is bad.',
+                data: { foo },
+              }
+            ]
+          });
+        }
+      };
+    `,
+    // Suggestion (message)
+    `
+      module.exports = {
+        create(context) {
+          context.report({
+            node,
+            suggest: [ { message: 'hello world' }]
+          });
+        }
+      };
+    `,
+    // Suggestion (messageId)
+    `
+      module.exports = {
+        create(context) {
+          context.report({
+            node,
+            suggest: [ { messageId: 'myMessageId' }]
+          });
+        }
+      };
+    `,
   ],
 
   invalid: [
@@ -92,7 +128,23 @@ ruleTester.run('prefer-placeholders', rule, {
           }
         };
       `,
-      errors: [ERROR],
+      errors: [{ messageId: 'usePlaceholders', type: 'TemplateLiteral' }],
+    },
+    {
+      // Suggestion
+      code: `
+        module.exports = {
+          create(context) {
+            context.report({
+              node,
+              suggest: [
+                { desc: \`\${foo} is bad.\` }
+              ]
+            });
+          }
+        };
+      `,
+      errors: [{ messageId: 'usePlaceholders', type: 'TemplateLiteral' }],
     },
     {
       // With message in variable.
@@ -107,7 +159,7 @@ ruleTester.run('prefer-placeholders', rule, {
           }
         };
       `,
-      errors: [ERROR],
+      errors: [{ messageId: 'usePlaceholders', type: 'TemplateLiteral' }],
     },
     {
       code: `
@@ -120,7 +172,7 @@ ruleTester.run('prefer-placeholders', rule, {
           }
         };
       `,
-      errors: [ERROR],
+      errors: [{ messageId: 'usePlaceholders', type: 'BinaryExpression' }],
     },
     {
       code: `
@@ -130,7 +182,7 @@ ruleTester.run('prefer-placeholders', rule, {
           }
         };
       `,
-      errors: [ERROR],
+      errors: [{ messageId: 'usePlaceholders', type: 'TemplateLiteral' }],
     },
   ],
 });
diff --git a/tests/lib/utils.js b/tests/lib/utils.js
index ac97bd3..ea12dde 100644
--- a/tests/lib/utils.js
+++ b/tests/lib/utils.js
@@ -421,4 +421,54 @@ describe('utils', () => {
       });
     });
   });
+
+  describe('collectReportViolationAndSuggestionData', () => {
+    const CASES = [
+      {
+        code: `
+          context.report({
+            node: {},
+            message: "message1",
+            messageId: "messageId1",
+            data: { foo: 'hello' },
+            fix(fixer) {},
+            suggest: [{
+              desc: "message2",
+              messageId: "messageId2",
+              data: { bar: 'world' },
+              fix(fixer) {},
+            }]
+          });
+        `,
+        shouldMatch: [
+          {
+            message: { type: 'Literal', value: 'message1' },
+            messageId: { type: 'Literal', value: 'messageId1' },
+            data: { type: 'ObjectExpression', properties: [{ key: { name: 'foo' } }] },
+            fix: { type: 'FunctionExpression' },
+          },
+          {
+            message: { type: 'Literal', value: 'message2' },
+            messageId: { type: 'Literal', value: 'messageId2' },
+            data: { type: 'ObjectExpression', properties: [{ key: { name: 'bar' } }] },
+            fix: { type: 'FunctionExpression' },
+          },
+        ],
+      },
+    ];
+
+    it('behaves correctly', () => {
+      for (const testCase of CASES) {
+        const ast = espree.parse(testCase.code, { ecmaVersion: 6, range: true });
+        const context = { getScope () {} }; // mock object
+        const reportNode = ast.body[0].expression;
+        const reportInfo = utils.getReportInfo(reportNode.arguments, context);
+        const data = utils.collectReportViolationAndSuggestionData(reportInfo);
+        assert(
+          lodash.isMatch(data, testCase.shouldMatch),
+          `Expected \n${inspect(data)}\nto match\n${inspect(testCase.shouldMatch)}`
+        );
+      }
+    });
+  });
 });