diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index a4d7be3ce5d..ce069a2aff6 100644 --- 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 new file mode 100644 index 00000000000..cbc86681727 --- /dev/null +++ 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 { + return Promise.resolve(1); +} + +async function getDataFromApi(endpoint: string): Promise { + 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" +} +``` + +Taken with ❤️ [from ESLint core](https://github.com/eslint/eslint/blob/master/docs/rules/require-await.md) diff --git a/packages/eslint-plugin/src/configs/all.json b/packages/eslint-plugin/src/configs/all.json index 10831d4b246..cf3f61c69cd 100644 --- a/packages/eslint-plugin/src/configs/all.json +++ b/packages/eslint-plugin/src/configs/all.json @@ -62,6 +62,7 @@ "@typescript-eslint/prefer-string-starts-ends-with": "error", "@typescript-eslint/promise-function-async": "error", "@typescript-eslint/require-array-sort-compare": "error", + "@typescript-eslint/require-await": "error", "@typescript-eslint/restrict-plus-operands": "error", "semi": "off", "@typescript-eslint/semi": "error", diff --git a/packages/eslint-plugin/src/configs/recommended.json b/packages/eslint-plugin/src/configs/recommended.json index 0e6d6d9f665..b3d488683d3 100644 --- a/packages/eslint-plugin/src/configs/recommended.json +++ b/packages/eslint-plugin/src/configs/recommended.json @@ -7,6 +7,7 @@ "camelcase": "off", "@typescript-eslint/camelcase": "error", "@typescript-eslint/class-name-casing": "error", + "@typescript-eslint/consistent-type-definitions": "error", "@typescript-eslint/explicit-function-return-type": "warn", "@typescript-eslint/explicit-member-accessibility": "error", "indent": "off", diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 73f34ae380f..48280664075 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -52,6 +52,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'; @@ -115,6 +116,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 new file mode 100644 index 00000000000..cbdd08791d9 --- /dev/null +++ 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; +type MessageIds = util.InferMessageIdsTypeFromRule; + +interface ScopeInfo { + upper: ScopeInfo | null; + returnsPromise: boolean; +} + +export default util.createRule({ + 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< + ts.ReturnStatement + >(node); + 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 new file mode 100644 index 00000000000..0b5f299e579 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/require-await.test.ts @@ -0,0 +1,114 @@ +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 { + return await 1; + }`, + }, + { + // Async function expression with await + code: `const numberOne = async function(): Promise { + return await 1; + }`, + }, + { + // Async arrow function expression with await + code: `const numberOne = async (): Promise => await 1;`, + }, + { + // Async function declaration with promise return + code: `async function numberOne(): Promise { + return Promise.resolve(1); + }`, + }, + { + // Async function expression with promise return + code: `const numberOne = async function(): Promise { + return Promise.resolve(1); + }`, + }, + { + // Async function declaration with async function return + code: `async function numberOne(): Promise { + return getAsyncNumber(1); + } + async function getAsyncNumber(x: number): Promise { + return Promise.resolve(x); + }`, + }, + { + // Async function expression with async function return + code: `const numberOne = async function(): Promise { + return getAsyncNumber(1); + } + const getAsyncNumber = async function(x: number): Promise { + return Promise.resolve(x); + }`, + }, + ], + + invalid: [ + { + // Async function declaration with no await + code: `async function numberOne(): Promise { + return 1; + }`, + errors: [noAwaitFunctionDeclaration], + }, + { + // Async function expression with no await + code: `const numberOne = async function(): Promise { + return 1; + }`, + errors: [noAwaitFunctionExpression], + }, + { + // Async arrow function expression with no await + code: `const numberOne = async (): Promise => 1;`, + errors: [noAwaitAsyncFunctionExpression], + }, + ], +}); diff --git a/packages/eslint-plugin/typings/eslint-rules.d.ts b/packages/eslint-plugin/typings/eslint-rules.d.ts index df9131dae4d..85c05b0dfe2 100644 --- 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';