feat(eslint-plugin): add new rule require-await

scottohara · scottohara · commit edf461497a08 · 2019-07-11T16:49:28.000+10:00
diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md
@@ -175,6 +175,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e
 | [`@typescript-eslint/prefer-string-starts-ends-with`](./docs/rules/prefer-string-starts-ends-with.md)     | Enforce the use of `String#startsWith` and `String#endsWith` instead of other equivalent methods of checking substrings                             |                    | :wrench: | :thought_balloon: |
 | [`@typescript-eslint/promise-function-async`](./docs/rules/promise-function-async.md)                     | Requires any function or method that returns a Promise to be marked async                                                                           |                    |          | :thought_balloon: |
 | [`@typescript-eslint/require-array-sort-compare`](./docs/rules/require-array-sort-compare.md)             | Enforce giving `compare` argument to `Array#sort`                                                                                                   |                    |          | :thought_balloon: |
+| [`@typescript-eslint/require-await`](./docs/rules/require-await.md)                                       | Disallow async functions which have no `await` expression                                                                                           |                    |          | :thought_balloon: |
 | [`@typescript-eslint/restrict-plus-operands`](./docs/rules/restrict-plus-operands.md)                     | When adding two variables, operands must both be of type number or of type string                                                                   |                    |          | :thought_balloon: |
 | [`@typescript-eslint/semi`](./docs/rules/semi.md)                                                         | Require or disallow semicolons instead of ASI                                                                                                       |                    | :wrench: |                   |
 | [`@typescript-eslint/strict-boolean-expressions`](./docs/rules/strict-boolean-expressions.md)             | Restricts the types allowed in boolean expressions                                                                                                  |                    |          | :thought_balloon: |
diff --git a/packages/eslint-plugin/docs/rules/require-await.md b/packages/eslint-plugin/docs/rules/require-await.md
@@ -0,0 +1,49 @@
+# Disallow async functions which have no await expression (@typescript-eslint/require-await)
+
+Asynchronous functions that don’t use `await` might not need to be asynchronous functions and could be the unintentional result of refactoring.
+
+## Rule Details
+
+The `@typescript-eslint/require-await` rule extends the `require-await` rule from ESLint core, and allows for cases where the additional typing information can prevent false positives that would otherwise trigger the rule.
+
+One example is when a function marked as `async` returns a value that is:
+
+1. already a promise; or
+2. the result of calling another `async` function
+
+```typescript
+async function numberOne(): Promise<number> {
+  return Promise.resolve(1);
+}
+
+async function getDataFromApi(endpoint: string): Promise<Response> {
+  return fetch(endpoint);
+}
+```
+
+In the above examples, the core `require-await` triggers the following warnings:
+
+```
+async function 'numberOne' has no 'await' expression
+async function 'getDataFromApi' has no 'await' expression
+```
+
+One way to resolve these errors is to remove the `async` keyword. However doing so can cause a conflict with the [`@typescript-eslint/promise-function-async`](https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/promise-function-async.md) rule (if enabled), which requires any function returning a promise to be marked as `async`.
+
+Another way to resolve these errors is to add an `await` keyword to the return statements. However doing so can cause a conflict with the [`no-return-await`](https://eslint.org/docs/rules/no-return-await) rule (if enabled), which warns against using `return await` since the return value of an `async` function is always wrapped in `Promise.resolve` anyway.
+
+With the additional typing information available in Typescript code, this extension to the `require-await` rule is able to look at the _actual_ return types of an `async` function (before being implicitly wrapped in `Promise.resolve`), and avoid the need for an `await` expression when the return value is already a promise.
+
+See the [ESLint documentation](https://eslint.org/docs/rules/require-await) for more details on the `require-await` rule.
+
+## Rule Changes
+
+```cjson
+{
+    // note you must disable the base rule as it can report incorrect errors
+    "require-await": "off",
+    "@typescript-eslint/require-await": "error"
+}
+```
+
+<sup>Taken with ❤️ [from ESLint core](https://github.com/eslint/eslint/blob/master/docs/rules/require-await.md)</sup>
diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts
@@ -51,6 +51,7 @@ import preferRegexpExec from './prefer-regexp-exec';
 import preferStringStartsEndsWith from './prefer-string-starts-ends-with';
 import promiseFunctionAsync from './promise-function-async';
 import requireArraySortCompare from './require-array-sort-compare';
+import requireAwait from './require-await';
 import restrictPlusOperands from './restrict-plus-operands';
 import semi from './semi';
 import strictBooleanExpressions from './strict-boolean-expressions';
@@ -113,6 +114,7 @@ export default {
   'prefer-string-starts-ends-with': preferStringStartsEndsWith,
   'promise-function-async': promiseFunctionAsync,
   'require-array-sort-compare': requireArraySortCompare,
+  'require-await': requireAwait,
   'restrict-plus-operands': restrictPlusOperands,
   semi: semi,
   'strict-boolean-expressions': strictBooleanExpressions,
diff --git a/packages/eslint-plugin/src/rules/require-await.ts b/packages/eslint-plugin/src/rules/require-await.ts
@@ -0,0 +1,138 @@
+import {
+  TSESTree,
+  TSESLint,
+  AST_NODE_TYPES,
+} from '@typescript-eslint/experimental-utils';
+import baseRule from 'eslint/lib/rules/require-await';
+import * as tsutils from 'tsutils';
+import ts from 'typescript';
+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: {
+    type: 'suggestion',
+    docs: {
+      description: 'Disallow async functions which have no `await` expression',
+      category: 'Best Practices',
+      recommended: false,
+    },
+    schema: baseRule.meta.schema,
+    messages: baseRule.meta.messages,
+  },
+  defaultOptions: [],
+  create(context) {
+    const rules = baseRule.create(context);
+    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,
+    ) {
+      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);
+          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,
+    ) {
+      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;
+      }
+    }
+
+    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: TSESTree.ReturnStatement) {
+        if (!scopeInfo) {
+          return;
+        }
+
+        const { expression } = parserServices.esTreeNodeToTSNodeMap.get(
+          node,
+        ) as ts.ReturnStatement;
+        if (!expression) {
+          return;
+        }
+
+        const type = checker.getTypeAtLocation(expression);
+        if (tsutils.isThenableType(checker, expression, type)) {
+          scopeInfo.returnsPromise = true;
+        }
+      },
+
+      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
@@ -0,0 +1,123 @@
+import rule from '../../src/rules/require-await';
+import { RuleTester, getFixturesRootDir } from '../RuleTester';
+
+const rootDir = getFixturesRootDir();
+
+const ruleTester = new RuleTester({
+  parserOptions: {
+    ecmaVersion: 2018,
+    tsconfigRootDir: rootDir,
+    project: './tsconfig.json',
+  },
+  parser: '@typescript-eslint/parser',
+});
+
+const noAwaitFunctionDeclaration: any = {
+  message: "Async function 'numberOne' has no 'await' expression.",
+};
+
+const noAwaitFunctionExpression: any = {
+  message: "Async function has no 'await' expression.",
+};
+
+const noAwaitAsyncFunctionExpression: any = {
+  message: "Async arrow function has no 'await' expression.",
+};
+
+ruleTester.run('require-await', rule, {
+  valid: [
+    {
+      // Non-async function declaration
+      code: `function numberOne(): number {
+        return 1;
+      }`,
+    },
+    {
+      // Non-async function expression
+      code: `const numberOne = function(): number {
+        return 1;
+      }`,
+    },
+    {
+      // Non-async arrow function expression
+      code: `const numberOne = (): number => 1;`,
+    },
+    {
+      // Async function declaration with await
+      code: `async function numberOne(): Promise<number> {
+        return await 1;
+      }`,
+    },
+    {
+      // Async function expression with await
+      code: `const numberOne = async function(): Promise<number> {
+        return await 1;
+      }`,
+    },
+    {
+      // Async arrow function expression with await
+      code: `const numberOne = async (): Promise<number> => await 1;`,
+    },
+    {
+      // Async function declaration with promise return
+      code: `async function numberOne(): Promise<number> {
+        return Promise.resolve(1);
+      }`,
+    },
+    {
+      // Async function expression with promise return
+      code: `const numberOne = async function(): Promise<number> {
+        return Promise.resolve(1);
+      }`,
+    },
+    /*{
+      // Async arrow function expression with promise return
+      code: `const numberOne = async (): Promise<number> => Promise.resolve(1);`,
+    },*/
+    {
+      // Async function declaration with async function return
+      code: `async function numberOne(): Promise<number> {
+        return getAsyncNumber(1);
+      }
+      async function getAsyncNumber(x: number): Promise<number> {
+        return Promise.resolve(x);
+      }`,
+    },
+    {
+      // Async function expression with async function return
+      code: `const numberOne = async function(): Promise<number> {
+        return getAsyncNumber(1);
+      }
+      const getAsyncNumber = async function(x: number): Promise<number> {
+        return Promise.resolve(x);
+      }`,
+    },
+    /*{
+      // Async arrow function expression with async function return
+      code: `const numberOne = async (): Promise<number> => getAsyncNumber(1);
+      const getAsyncNumber = async (x: number): Promise<number> => Promise.resolve(x);`,
+    },*/
+  ],
+
+  invalid: [
+    {
+      // Async function declaration with no await
+      code: `async function numberOne(): Promise<number> {
+        return 1;
+      }`,
+      errors: [noAwaitFunctionDeclaration],
+    },
+    {
+      // Async function expression with no await
+      code: `const numberOne = async function(): Promise<number> {
+        return 1;
+      }`,
+      errors: [noAwaitFunctionExpression],
+    },
+    {
+      // Async arrow function expression with no await
+      code: `const numberOne = async (): Promise<number> => 1;`,
+      errors: [noAwaitAsyncFunctionExpression],
+    },
+  ],
+});
diff --git a/packages/eslint-plugin/typings/eslint-rules.d.ts b/packages/eslint-plugin/typings/eslint-rules.d.ts
@@ -409,6 +409,29 @@ declare module 'eslint/lib/rules/no-extra-parens' {
   export = rule;
 }
 
+declare module 'eslint/lib/rules/require-await' {
+  import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils';
+
+  const rule: TSESLint.RuleModule<
+    never,
+    [],
+    {
+      FunctionDeclaration(node: TSESTree.FunctionDeclaration): void;
+      FunctionExpression(node: TSESTree.FunctionExpression): void;
+      ArrowFunctionExpression(node: TSESTree.ArrowFunctionExpression): void;
+      'FunctionDeclaration:exit'(node: TSESTree.FunctionDeclaration): void;
+      'FunctionExpression:exit'(node: TSESTree.FunctionExpression): void;
+      'ArrowFunctionExpression:exit'(
+        node: TSESTree.ArrowFunctionExpression,
+      ): void;
+      ReturnStatement(node: TSESTree.ReturnStatement): void;
+      AwaitExpression(node: TSESTree.AwaitExpression): void;
+      ForOfStatement(node: TSESTree.ForOfStatement): void;
+    }
+  >;
+  export = rule;
+}
+
 declare module 'eslint/lib/rules/semi' {
   import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils';
 
