New: Add new rule prefer-message-ids (#170)

eslint-community · Jul 26, 2021 · 95021dd · 95021dd
1 parent fcbb65c
commit 95021dd
Show file tree

Hide file tree

Showing 24 changed files with 298 additions and 56 deletions.
diff --git a/README.md b/README.md
@@ -57,6 +57,7 @@ Name | ✔️ | 🛠 | 💡 | Description
 [no-only-tests](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/no-only-tests.md) |  |  | 💡 | disallow the test case property `only`
 [no-unused-placeholders](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/no-unused-placeholders.md) | ✔️ |  |  | disallow unused placeholders in rule report messages
 [no-useless-token-range](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/no-useless-token-range.md) | ✔️ | 🛠 |  | disallow unnecessary calls to `sourceCode.getFirstToken()` and `sourceCode.getLastToken()`
+[prefer-message-ids](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/prefer-message-ids.md) |  |  |  | require using `messageId` instead of `message` to report rule violations
 [prefer-object-rule](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/prefer-object-rule.md) |  | 🛠 |  | disallow rule exports where the export is a function
 [prefer-output-null](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/prefer-output-null.md) |  | 🛠 |  | disallow invalid RuleTester test cases where the `output` matches the `code`
 [prefer-placeholders](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/prefer-placeholders.md) |  |  |  | require using placeholders for dynamic report messages

diff --git a/docs/rules/prefer-message-ids.md b/docs/rules/prefer-message-ids.md
@@ -0,0 +1,57 @@
+# Require using `messageId` instead of `message` to report rule violations (prefer-message-ids)
+
+When reporting a rule violation, it's preferred to provide the violation message with the `messageId` property instead of the `message` property. Message IDs provide the following benefits:
+
+* Rule violation messages can be stored in a central `meta.messages` object for convenient management
+* Rule violation messages do not need to be repeated in both the rule file and rule test file
+
+## Rule Details
+
+This rule catches usages of the `message` property when reporting a rule violation.
+
+Examples of **incorrect** code for this rule:
+
+```js
+/* eslint eslint-plugin/prefer-message-ids: error */
+
+module.exports = {
+  create (context) {
+    return {
+      CallExpression (node) {
+        context.report({
+          node,
+          message: 'Some error message.',
+        });
+      },
+    };
+  },
+};
+```
+
+Examples of **correct** code for this rule:
+
+```js
+/* eslint eslint-plugin/prefer-message-ids: error */
+
+module.exports = {
+  meta: {
+    messages: {
+      someMessageId: 'Some error message',
+    },
+  },
+  create (context) {
+    return {
+      CallExpression (node) {
+        context.report({
+          node,
+          messageId: 'someMessageId',
+        });
+      },
+    };
+  },
+};
+```
+
+## Further Reading
+
+* [messageIds API](https://eslint.org/docs/developer-guide/working-with-rules#messageids)
diff --git a/lib/rules/consistent-output.js b/lib/rules/consistent-output.js
@@ -26,6 +26,9 @@ module.exports = {
         enum: ['always', 'consistent'],
       },
     ],
+    messages: {
+      missingOutput: 'This test case should have an output assertion.',
+    },
   },
 
   create (context) {
@@ -48,7 +51,7 @@ module.exports = {
             casesWithoutOutput.forEach(testCase => {
               context.report({
                 node: testCase,
-                message: 'This test case should have an output assertion.',
+                messageId: 'missingOutput',
               });
             });
           }

diff --git a/lib/rules/meta-property-ordering.js b/lib/rules/meta-property-ordering.js
@@ -23,13 +23,15 @@ module.exports = {
       type: 'array',
       elements: { type: 'string' },
     }],
+    messages: {
+      inconsistentOrder: 'The meta properties should be placed in a consistent order: [{{order}}].',
+    },
   },
 
   create (context) {
     const sourceCode = context.getSourceCode();
     const info = getRuleInfo(sourceCode);
 
-    const message = 'The meta properties should be placed in a consistent order: [{{order}}].';
     const order = context.options[0] || ['type', 'docs', 'fixable', 'schema', 'messages'];
 
     const orderMap = new Map(order.map((name, i) => [name, i]));
@@ -67,7 +69,7 @@ module.exports = {
         for (const violatingProp of violatingProps) {
           context.report({
             node: violatingProp,
-            message,
+            messageId: 'inconsistentOrder',
             data: {
               order: knownProps.map(getKeyName).join(', '),
             },

diff --git a/lib/rules/no-deprecated-context-methods.js b/lib/rules/no-deprecated-context-methods.js
@@ -44,6 +44,9 @@ module.exports = {
     },
     fixable: 'code',
     schema: [],
+    messages: {
+      newFormat: 'Use `{{contextName}}.getSourceCode().{{replacement}}` instead of `{{contextName}}.{{original}}`.',
+    },
   },
 
   create (context) {
@@ -66,7 +69,7 @@ module.exports = {
             contextId =>
               context.report({
                 node: contextId.parent,
-                message: 'Use `{{contextName}}.getSourceCode().{{replacement}}` instead of `{{contextName}}.{{original}}`.',
+                messageId: 'newFormat',
                 data: {
                   contextName: contextId.name,
                   original: contextId.parent.property.name,

diff --git a/lib/rules/no-deprecated-report-api.js b/lib/rules/no-deprecated-report-api.js
@@ -21,6 +21,9 @@ module.exports = {
     },
     fixable: 'code', // or "code" or "whitespace"
     schema: [],
+    messages: {
+      useNewAPI: 'Use the new-style context.report() API.',
+    },
   },
 
   create (context) {
@@ -44,7 +47,7 @@ module.exports = {
         ) {
           context.report({
             node: node.callee.property,
-            message: 'Use the new-style context.report() API.',
+            messageId: 'useNewAPI',
             fix (fixer) {
               const openingParen = sourceCode.getTokenBefore(node.arguments[0]);
               const closingParen = sourceCode.getLastToken(node);

diff --git a/lib/rules/no-missing-placeholders.js b/lib/rules/no-missing-placeholders.js
@@ -22,6 +22,9 @@ module.exports = {
     },
     fixable: null,
     schema: [],
+    messages: {
+      placeholderDoesNotExist: 'The placeholder {{{{missingKey}}}} does not exist.',
+    },
   },
 
   create (context) {
@@ -66,7 +69,7 @@ module.exports = {
               if (!matchingProperty) {
                 context.report({
                   node: reportInfo.message,
-                  message: 'The placeholder {{{{missingKey}}}} does not exist.',
+                  messageId: 'placeholderDoesNotExist',
                   data: { missingKey: match[1] },
                 });
               }

diff --git a/lib/rules/no-unused-placeholders.js b/lib/rules/no-unused-placeholders.js
@@ -22,6 +22,9 @@ module.exports = {
     },
     fixable: null,
     schema: [],
+    messages: {
+      placeholderUnused: 'The placeholder {{{{unusedKey}}}} is unused.',
+    },
   },
 
   create (context) {
@@ -69,7 +72,7 @@ module.exports = {
               if (!placeholdersInMessage.has(key)) {
                 context.report({
                   node: reportInfo.message,
-                  message: 'The placeholder {{{{unusedKey}}}} is unused.',
+                  messageId: 'placeholderUnused',
                   data: { unusedKey: key },
                 });
               }

diff --git a/lib/rules/no-useless-token-range.js b/lib/rules/no-useless-token-range.js
@@ -21,6 +21,9 @@ module.exports = {
     },
     fixable: 'code',
     schema: [],
+    messages: {
+      useReplacement: "Use '{{replacementText}}' instead.",
+    },
   },
 
   create (context) {
@@ -125,7 +128,7 @@ module.exports = {
               sourceCode.text.slice(identifier.parent.parent.range[1], fullRangeAccess.range[1]);
             context.report({
               node: identifier.parent.parent,
-              message: "Use '{{replacementText}}' instead.",
+              messageId: 'useReplacement',
               data: { replacementText },
               fix (fixer) {
                 return fixer.replaceText(identifier.parent.parent, sourceCode.getText(identifier.parent.parent.arguments[0]));

diff --git a/lib/rules/prefer-message-ids.js b/lib/rules/prefer-message-ids.js
@@ -0,0 +1,54 @@
+'use strict';
+
+const utils = require('../utils');
+
+// ------------------------------------------------------------------------------
+// Rule Definition
+// ------------------------------------------------------------------------------
+
+module.exports = {
+  meta: {
+    type: 'problem',
+    docs: {
+      description: 'require using `messageId` instead of `message` to report rule violations',
+      category: 'Rules',
+      recommended: false,
+    },
+    fixable: null,
+    schema: [],
+    messages: {
+      foundMessage: 'Use `messageId` instead of `message`.',
+    },
+  },
+
+  create (context) {
+    let contextIdentifiers;
+
+    // ----------------------------------------------------------------------
+    // Public
+    // ----------------------------------------------------------------------
+
+    return {
+      Program (ast) {
+        contextIdentifiers = utils.getContextIdentifiers(context, ast);
+      },
+      CallExpression (node) {
+        if (
+          node.callee.type === 'MemberExpression' &&
+             contextIdentifiers.has(node.callee.object) &&
+             node.callee.property.type === 'Identifier' && node.callee.property.name === 'report'
+        ) {
+          const reportInfo = utils.getReportInfo(node.arguments, context);
+          if (!reportInfo || !reportInfo.message) {
+            return;
+          }
+
+          context.report({
+            node: reportInfo.message.parent,
+            messageId: 'foundMessage',
+          });
+        }
+      },
+    };
+  },
+};
diff --git a/lib/rules/prefer-output-null.js b/lib/rules/prefer-output-null.js
@@ -21,14 +21,16 @@ module.exports = {
     },
     fixable: 'code',
     schema: [],
+    messages: {
+      useOutputNull: 'Use `output: null` to assert that a test case is not autofixed.',
+    },
   },
 
   create (context) {
     // ----------------------------------------------------------------------
     // Public
     // ----------------------------------------------------------------------
 
-    const message = 'Use `output: null` to assert that a test case is not autofixed.';
     const sourceCode = context.getSourceCode();
 
     return {
@@ -54,7 +56,7 @@ module.exports = {
             if (output && sourceCode.getText(output.value) === sourceCode.getText(code.value)) {
               context.report({
                 node: output,
-                message,
+                messageId: 'useOutputNull',
                 fix: fixer => fixer.replaceText(output.value, 'null'),
               });
             }

diff --git a/lib/rules/prefer-placeholders.js b/lib/rules/prefer-placeholders.js
@@ -22,6 +22,9 @@ module.exports = {
     },
     fixable: null,
     schema: [],
+    messages: {
+      usePlaceholders: 'Use report message placeholders instead of string concatenation.',
+    },
   },
 
   create (context) {
@@ -80,7 +83,7 @@ module.exports = {
           ) {
             context.report({
               node: messageNode,
-              message: 'Use report message placeholders instead of string concatenation.',
+              messageId: 'usePlaceholders',
             });
           }
         }

diff --git a/lib/rules/prefer-replace-text.js b/lib/rules/prefer-replace-text.js
@@ -21,11 +21,13 @@ module.exports = {
     },
     fixable: null,
     schema: [],
+    messages: {
+      useReplaceText: 'Use replaceText instead of replaceTextRange.',
+    },
   },
 
   create (context) {
     const sourceCode = context.getSourceCode();
-    const message = 'Use replaceText instead of replaceTextRange.';
     let funcInfo = {
       upper: null,
       codePath: null,
@@ -74,7 +76,7 @@ module.exports = {
           if (isIdenticalNodeRange) {
             context.report({
               node,
-              message,
+              messageId: 'useReplaceText',
             });
           }
         }

diff --git a/lib/rules/report-message-format.js b/lib/rules/report-message-format.js
@@ -24,6 +24,9 @@ module.exports = {
     schema: [
       { type: 'string' },
     ],
+    messages: {
+      noMatch: "Report message does not match the pattern '{{pattern}}'.",
+    },
   },
 
   create (context) {
@@ -44,7 +47,7 @@ module.exports = {
       ) {
         context.report({
           node: message,
-          message: "Report message does not match the pattern '{{pattern}}'.",
+          messageId: 'noMatch',
           data: { pattern: context.options[0] || '' },
         });
       }

diff --git a/lib/rules/test-case-property-ordering.js b/lib/rules/test-case-property-ordering.js
@@ -24,13 +24,15 @@ module.exports = {
       type: 'array',
       elements: { type: 'string' },
     }],
+    messages: {
+      inconsistentOrder: 'The properties of a test case should be placed in a consistent order: [{{order}}].',
+    },
   },
 
   create (context) {
     // ----------------------------------------------------------------------
     // Public
     // ----------------------------------------------------------------------
-    const message = 'The properties of a test case should be placed in a consistent order: [{{order}}].';
     const order = context.options[0] || [
       'filename',
       'code',
@@ -63,7 +65,7 @@ module.exports = {
 
                   context.report({
                     node: properties[i],
-                    message,
+                    messageId: 'inconsistentOrder',
                     data: { order: orderMsg.join(', ') },
                     fix (fixer) {
                       return orderMsg.map((key, index) => {

diff --git a/lib/rules/test-case-shorthand-strings.js b/lib/rules/test-case-shorthand-strings.js
@@ -21,6 +21,9 @@ module.exports = {
     },
     fixable: 'code',
     schema: [{ enum: ['as-needed', 'never', 'consistent', 'consistent-as-needed'] }],
+    messages: {
+      useShorthand: 'Use {{preferred}} for this test case instead of {{actual}}.',
+    },
   },
 
   create (context) {
@@ -62,7 +65,7 @@ module.exports = {
       }[shorthandOption]).forEach(badCaseInfo => {
         context.report({
           node: badCaseInfo.node,
-          message: 'Use {{preferred}} for this test case instead of {{actual}}.',
+          messageId: 'useShorthand',
           data: {
             preferred: badCaseInfo.shorthand ? 'an object' : 'a string',
             actual: badCaseInfo.shorthand ? 'a string' : 'an object',