Support ConditionalExpressions in dry-error-messages rule (#11917)

* Support ConditionalExpressions in dry-error-messages rule * tests
babel · Aug 5, 2020 · c0f6f03 · c0f6f03
1 parent 50b3262
commit c0f6f03
Show file tree

Hide file tree

Showing 5 changed files with 70 additions and 11 deletions.
diff --git a/eslint/babel-eslint-plugin-development-internal/src/rules/dry-error-messages.js b/eslint/babel-eslint-plugin-development-internal/src/rules/dry-error-messages.js
@@ -41,6 +41,25 @@ function findIdNode(node) {
   return null;
 }
 
+function findIdNodes(node) {
+  if (node.type === "ConditionalExpression") {
+    const consequent = findIdNode(node.consequent);
+    const alternate = findIdNode(node.alternate);
+
+    if (consequent && alternate) {
+      return [consequent, alternate];
+    }
+  }
+
+  const idNode = findIdNode(node);
+
+  if (idNode) {
+    return [idNode];
+  }
+
+  return null;
+}
+
 function findReference(node, scope) {
   let currentScope = scope;
 
@@ -128,11 +147,13 @@ export default {
         node,
       ) {
         const [, errorMsgNode] = node.arguments;
-        const nodeToCheck = findIdNode(errorMsgNode);
+        const nodesToCheck = findIdNodes(errorMsgNode);
 
         if (
-          nodeToCheck &&
-          referencesImportedBinding(nodeToCheck, getScope(), importedBindings)
+          Array.isArray(nodesToCheck) &&
+          nodesToCheck.every(node =>
+            referencesImportedBinding(node, getScope(), importedBindings),
+          )
         ) {
           return;
         }

diff --git a/eslint/babel-eslint-plugin-development-internal/test/rules/dry-error-messages.js b/eslint/babel-eslint-plugin-development-internal/test/rules/dry-error-messages.js
@@ -261,6 +261,14 @@ ruleTester.run("dry-error-messages", rule, {
       code: "this.raise(loc);",
       options: [{ errorModule: ERRORS_MODULE }],
     },
+
+    // Support ternary as second argument
+    {
+      filename: FILENAME,
+      code:
+        "import Errors, { NotErrors } from 'errorsModule'; this.raise(loc, a ? Errors.someErrorMessage : Errors.someOtherErrorMessage);",
+      options: [{ errorModule: ERRORS_MODULE }],
+    },
   ],
   invalid: [
     {
@@ -691,5 +699,43 @@ ruleTester.run("dry-error-messages", rule, {
         },
       ],
     },
+
+    // Should error if either part of a ternary isn't from error module
+    {
+      filename: FILENAME,
+      code:
+        "import Errors, { NotErrors } from 'errorsModule'; this.raise(loc, a ? Errors.someErrorMessage : 'hello');",
+      options: [{ errorModule: ERRORS_MODULE }],
+      errors: [
+        {
+          messageId: "mustBeImported",
+          data: { errorModule: ERRORS_MODULE },
+        },
+      ],
+    },
+    {
+      filename: FILENAME,
+      code:
+        "import Errors, { NotErrors } from 'errorsModule'; this.raise(loc, a ? 'hello' : Errors.someErrorMessage);",
+      options: [{ errorModule: ERRORS_MODULE }],
+      errors: [
+        {
+          messageId: "mustBeImported",
+          data: { errorModule: ERRORS_MODULE },
+        },
+      ],
+    },
+    {
+      filename: FILENAME,
+      code:
+        "import Errors, { NotErrors } from 'errorsModule'; this.raise(loc, a ? 'hello' : 'world');",
+      options: [{ errorModule: ERRORS_MODULE }],
+      errors: [
+        {
+          messageId: "mustBeImported",
+          data: { errorModule: ERRORS_MODULE },
+        },
+      ],
+    },
   ],
 });
diff --git a/packages/babel-parser/src/parser/lval.js b/packages/babel-parser/src/parser/lval.js
@@ -366,15 +366,13 @@ export default class LValParser extends NodeUtils {
             ? isStrictBindReservedWord(expr.name, this.inModule)
             : isStrictBindOnlyReservedWord(expr.name))
         ) {
-          /* eslint-disable @babel/development-internal/dry-error-messages */
           this.raise(
             expr.start,
             bindingType === BIND_NONE
               ? Errors.StrictEvalArguments
               : Errors.StrictEvalArgumentsBinding,
             expr.name,
           );
-          /* eslint-enable @babel/development-internal/dry-error-messages */
         }
 
         if (checkClashes) {
@@ -471,15 +469,13 @@ export default class LValParser extends NodeUtils {
         break;
 
       default: {
-        /* eslint-disable @babel/development-internal/dry-error-messages */
         this.raise(
           expr.start,
           bindingType === BIND_NONE
             ? Errors.InvalidLhs
             : Errors.InvalidLhsBinding,
           contextDescription,
         );
-        /* eslint-enable @babel/development-internal/dry-error-messages */
       }
     }
   }

diff --git a/packages/babel-parser/src/parser/statement.js b/packages/babel-parser/src/parser/statement.js
@@ -2042,15 +2042,13 @@ export default class StatementParser extends ExpressionParser {
     name: string,
   ): void {
     if (this.state.exportedIdentifiers.indexOf(name) > -1) {
-      /* eslint-disable @babel/development-internal/dry-error-messages */
       this.raise(
         node.start,
         name === "default"
           ? Errors.DuplicateDefaultExport
           : Errors.DuplicateExport,
         name,
       );
-      /* eslint-enable @babel/development-internal/dry-error-messages */
     }
     this.state.exportedIdentifiers.push(name);
   }

diff --git a/packages/babel-parser/src/tokenizer/index.js b/packages/babel-parser/src/tokenizer/index.js
@@ -420,14 +420,12 @@ export default class Tokenizer extends ParserErrors {
       // misleading
       this.expectPlugin("recordAndTuple");
       if (this.getPluginOption("recordAndTuple", "syntaxType") !== "hash") {
-        /* eslint-disable @babel/development-internal/dry-error-messages */
         throw this.raise(
           this.state.pos,
           next === charCodes.leftCurlyBrace
             ? Errors.RecordExpressionHashIncorrectStartSyntaxType
             : Errors.TupleExpressionHashIncorrectStartSyntaxType,
         );
-        /* eslint-enable @babel/development-internal/dry-error-messages */
       }
 
       if (next === charCodes.leftCurlyBrace) {