fix(eslint-plugin): [require-await] better handle nesting (#1193)

typescript-eslint · Nov 16, 2019 · eb83af1 · eb83af1
1 parent 9829dd3
commit eb83af1
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 103 deletions.
diff --git a/packages/eslint-plugin/src/rules/require-await.ts b/packages/eslint-plugin/src/rules/require-await.ts
@@ -1,6 +1,5 @@
 import {
   TSESTree,
-  TSESLint,
   AST_NODE_TYPES,
 } from '@typescript-eslint/experimental-utils';
 import baseRule from 'eslint/lib/rules/require-await';
@@ -11,11 +10,6 @@ import * as util from '../util';
 type Options = util.InferOptionsTypeFromRule<typeof baseRule>;
 type MessageIds = util.InferMessageIdsTypeFromRule<typeof baseRule>;
 
-interface ScopeInfo {
-  upper: ScopeInfo | null;
-  returnsPromise: boolean;
-}
-
 export default util.createRule<Options, MessageIds>({
   name: 'require-await',
   meta: {
@@ -35,82 +29,6 @@ export default util.createRule<Options, MessageIds>({
     const parserServices = util.getParserServices(context);
     const checker = parserServices.program.getTypeChecker();
 
-    let scopeInfo: ScopeInfo | null = null;
-
-    /**
-     * Push the scope info object to the stack.
-     *
-     * @returns {void}
-     */
-    function enterFunction(
-      node:
-        | TSESTree.FunctionDeclaration
-        | TSESTree.FunctionExpression
-        | TSESTree.ArrowFunctionExpression,
-    ): void {
-      scopeInfo = {
-        upper: scopeInfo,
-        returnsPromise: false,
-      };
-
-      switch (node.type) {
-        case AST_NODE_TYPES.FunctionDeclaration:
-          rules.FunctionDeclaration(node);
-          break;
-
-        case AST_NODE_TYPES.FunctionExpression:
-          rules.FunctionExpression(node);
-          break;
-
-        case AST_NODE_TYPES.ArrowFunctionExpression:
-          rules.ArrowFunctionExpression(node);
-
-          // If body type is not BlockStatment, we need to check the return type here
-          if (node.body.type !== AST_NODE_TYPES.BlockStatement) {
-            const expression = parserServices.esTreeNodeToTSNodeMap.get(
-              node.body,
-            );
-            scopeInfo.returnsPromise = isThenableType(expression);
-          }
-
-          break;
-      }
-    }
-
-    /**
-     * Pop the top scope info object from the stack.
-     * Passes through to the base rule if the function doesn't return a promise
-     *
-     * @param {ASTNode} node - The node exiting
-     * @returns {void}
-     */
-    function exitFunction(
-      node:
-        | TSESTree.FunctionDeclaration
-        | TSESTree.FunctionExpression
-        | TSESTree.ArrowFunctionExpression,
-    ): void {
-      if (scopeInfo) {
-        if (!scopeInfo.returnsPromise) {
-          switch (node.type) {
-            case AST_NODE_TYPES.FunctionDeclaration:
-              rules['FunctionDeclaration:exit'](node);
-              break;
-
-            case AST_NODE_TYPES.FunctionExpression:
-              rules['FunctionExpression:exit'](node);
-              break;
-
-            case AST_NODE_TYPES.ArrowFunctionExpression:
-              rules['ArrowFunctionExpression:exit'](node);
-              break;
-          }
-        }
-
-        scopeInfo = scopeInfo.upper;
-      }
-    }
-
     /**
      * Checks if the node returns a thenable type
      *
@@ -124,34 +42,41 @@ export default util.createRule<Options, MessageIds>({
     }
 
     return {
-      'FunctionDeclaration[async = true]': enterFunction,
-      'FunctionExpression[async = true]': enterFunction,
-      'ArrowFunctionExpression[async = true]': enterFunction,
-      'FunctionDeclaration[async = true]:exit': exitFunction,
-      'FunctionExpression[async = true]:exit': exitFunction,
-      'ArrowFunctionExpression[async = true]:exit': exitFunction,
-
-      ReturnStatement(node): void {
-        if (!scopeInfo) {
-          return;
+      'FunctionDeclaration[async = true]': rules.FunctionDeclaration,
+      'FunctionExpression[async = true]': rules.FunctionExpression,
+      'ArrowFunctionExpression[async = true]'(
+        node: TSESTree.ArrowFunctionExpression,
+      ): void {
+        rules.ArrowFunctionExpression(node);
+
+        // If body type is not BlockStatment, we need to check the return type here
+        if (node.body.type !== AST_NODE_TYPES.BlockStatement) {
+          const expression = parserServices.esTreeNodeToTSNodeMap.get(
+            node.body,
+          );
+          if (expression && isThenableType(expression)) {
+            // tell the base rule to mark the scope as having an await so it ignores it
+            rules.AwaitExpression(node as never);
+          }
         }
+      },
+      'FunctionDeclaration[async = true]:exit':
+        rules['FunctionDeclaration:exit'],
+      'FunctionExpression[async = true]:exit': rules['FunctionExpression:exit'],
+      'ArrowFunctionExpression[async = true]:exit':
+        rules['ArrowFunctionExpression:exit'],
+      AwaitExpression: rules.AwaitExpression,
+      ForOfStatement: rules.ForOfStatement,
 
+      ReturnStatement(node): void {
         const { expression } = parserServices.esTreeNodeToTSNodeMap.get<
           ts.ReturnStatement
         >(node);
-        if (!expression) {
-          return;
+        if (expression && isThenableType(expression)) {
+          // tell the base rule to mark the scope as having an await so it ignores it
+          rules.AwaitExpression(node as never);
         }
-
-        scopeInfo.returnsPromise = isThenableType(expression);
       },
-
-      AwaitExpression: rules.AwaitExpression as TSESLint.RuleFunction<
-        TSESTree.Node
-      >,
-      ForOfStatement: rules.ForOfStatement as TSESLint.RuleFunction<
-        TSESTree.Node
-      >,
     };
   },
 });
diff --git a/packages/eslint-plugin/tests/rules/require-await.test.ts b/packages/eslint-plugin/tests/rules/require-await.test.ts
@@ -128,6 +128,15 @@ ruleTester.run('require-await', rule, {
         return Promise.resolve(x);
       }`,
     },
+    // https://github.com/typescript-eslint/typescript-eslint/issues/1188
+    `
+async function testFunction(): Promise<void> {
+  await Promise.all([1, 2, 3].map(
+    // this should not trigger an error on the parent function
+    async value => Promise.resolve(value)
+  ))
+}
+    `,
   ],
 
   invalid: [