diff --git a/packages/eslint-plugin/src/rules/require-await.ts b/packages/eslint-plugin/src/rules/require-await.ts index 32bee2f4cef..4688373dc38 100644 --- a/packages/eslint-plugin/src/rules/require-await.ts +++ b/packages/eslint-plugin/src/rules/require-await.ts @@ -1,16 +1,28 @@ import { - TSESTree, AST_NODE_TYPES, + TSESLint, + TSESTree, } from '@typescript-eslint/experimental-utils'; -import baseRule from 'eslint/lib/rules/require-await'; +import { + isArrowToken, + getFunctionNameWithKind, + isOpeningParenToken, +} from 'eslint-utils'; import * as tsutils from 'tsutils'; import * as ts from 'typescript'; import * as util from '../util'; -type Options = util.InferOptionsTypeFromRule; -type MessageIds = util.InferMessageIdsTypeFromRule; +interface ScopeInfo { + upper: ScopeInfo | null; + hasAwait: boolean; + hasAsync: boolean; +} +type FunctionNode = + | TSESTree.FunctionDeclaration + | TSESTree.FunctionExpression + | TSESTree.ArrowFunctionExpression; -export default util.createRule({ +export default util.createRule({ name: 'require-await', meta: { type: 'suggestion', @@ -21,20 +33,56 @@ export default util.createRule({ requiresTypeChecking: true, extendsBaseRule: true, }, - schema: baseRule.meta.schema, - messages: baseRule.meta.messages, + schema: [], + messages: { + missingAwait: "{{name}} has no 'await' expression.", + }, }, defaultOptions: [], create(context) { - const rules = baseRule.create(context); const parserServices = util.getParserServices(context); const checker = parserServices.program.getTypeChecker(); + const sourceCode = context.getSourceCode(); + let scopeInfo: ScopeInfo | null = null; + + /** + * Push the scope info object to the stack. + */ + function enterFunction(node: FunctionNode): void { + scopeInfo = { + upper: scopeInfo, + hasAwait: false, + hasAsync: node.async, + }; + } + + /** + * Pop the top scope info object from the stack. + * Also, it reports the function if needed. + */ + function exitFunction(node: FunctionNode): void { + /* istanbul ignore if */ if (!scopeInfo) { + // this shouldn't ever happen, as we have to exit a function after we enter it + return; + } + + if (node.async && !scopeInfo.hasAwait && !isEmptyFunction(node)) { + context.report({ + node, + loc: getFunctionHeadLoc(node, sourceCode), + messageId: 'missingAwait', + data: { + name: util.upperCaseFirst(getFunctionNameWithKind(node)), + }, + }); + } + + scopeInfo = scopeInfo.upper; + } + /** * Checks if the node returns a thenable type - * - * @param {ASTNode} node - The node to check - * @returns {boolean} */ function isThenableType(node: ts.Node): boolean { const type = checker.getTypeAtLocation(node); @@ -42,37 +90,115 @@ export default util.createRule({ return tsutils.isThenableType(checker, node, type); } + /** + * Marks the current scope as having an await + */ + function markAsHasAwait(): void { + if (!scopeInfo) { + return; + } + + scopeInfo.hasAwait = true; + } + return { - FunctionDeclaration: rules.FunctionDeclaration, - FunctionExpression: rules.FunctionExpression, - ArrowFunctionExpression: rules.ArrowFunctionExpression, - 'ArrowFunctionExpression[async = true]'( - node: TSESTree.ArrowFunctionExpression, + FunctionDeclaration: enterFunction, + FunctionExpression: enterFunction, + ArrowFunctionExpression: enterFunction, + 'FunctionDeclaration:exit': exitFunction, + 'FunctionExpression:exit': exitFunction, + 'ArrowFunctionExpression:exit': exitFunction, + + AwaitExpression: markAsHasAwait, + 'ForOfStatement[await = true]': markAsHasAwait, + + // check body-less async arrow function. + // ignore `async () => await foo` because it's obviously correct + 'ArrowFunctionExpression[async = true] > :not(BlockStatement, AwaitExpression)'( + node: Exclude< + TSESTree.Node, + TSESTree.BlockStatement | TSESTree.AwaitExpression + >, ): void { - // If body type is not BlockStatement, 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(); - } + const expression = parserServices.esTreeNodeToTSNodeMap.get(node); + if (expression && isThenableType(expression)) { + markAsHasAwait(); } }, - 'FunctionDeclaration:exit': rules['FunctionDeclaration:exit'], - 'FunctionExpression:exit': rules['FunctionExpression:exit'], - 'ArrowFunctionExpression:exit': rules['ArrowFunctionExpression:exit'], - AwaitExpression: rules.AwaitExpression, - ForOfStatement: rules.ForOfStatement, - ReturnStatement(node): void { + // short circuit early to avoid unnecessary type checks + if (!scopeInfo || scopeInfo.hasAwait || !scopeInfo.hasAsync) { + return; + } + const { expression } = parserServices.esTreeNodeToTSNodeMap.get(node); if (expression && isThenableType(expression)) { - // tell the base rule to mark the scope as having an await so it ignores it - rules.AwaitExpression(); + markAsHasAwait(); } }, }; }, }); + +function isEmptyFunction(node: FunctionNode): boolean { + return ( + node.body?.type === AST_NODE_TYPES.BlockStatement && + node.body.body.length === 0 + ); +} + +// https://github.com/eslint/eslint/blob/03a69dbe86d5b5768a310105416ae726822e3c1c/lib/rules/utils/ast-utils.js#L382-L392 +/** + * Gets the `(` token of the given function node. + */ +function getOpeningParenOfParams( + node: FunctionNode, + sourceCode: TSESLint.SourceCode, +): TSESTree.Token { + return util.nullThrows( + node.id + ? sourceCode.getTokenAfter(node.id, isOpeningParenToken) + : sourceCode.getFirstToken(node, isOpeningParenToken), + util.NullThrowsReasons.MissingToken('(', node.type), + ); +} + +// https://github.com/eslint/eslint/blob/03a69dbe86d5b5768a310105416ae726822e3c1c/lib/rules/utils/ast-utils.js#L1220-L1242 +/** + * Gets the location of the given function node for reporting. + */ +function getFunctionHeadLoc( + node: FunctionNode, + sourceCode: TSESLint.SourceCode, +): TSESTree.SourceLocation { + const parent = util.nullThrows( + node.parent, + util.NullThrowsReasons.MissingParent, + ); + let start = null; + let end = null; + + if (node.type === AST_NODE_TYPES.ArrowFunctionExpression) { + const arrowToken = util.nullThrows( + sourceCode.getTokenBefore(node.body, isArrowToken), + util.NullThrowsReasons.MissingToken('=>', node.type), + ); + + start = arrowToken.loc.start; + end = arrowToken.loc.end; + } else if ( + parent.type === AST_NODE_TYPES.Property || + parent.type === AST_NODE_TYPES.MethodDefinition + ) { + start = parent.loc.start; + end = getOpeningParenOfParams(node, sourceCode).loc.start; + } else { + start = node.loc.start; + end = getOpeningParenOfParams(node, sourceCode).loc.start; + } + + return { + start, + end, + }; +} diff --git a/packages/eslint-plugin/tests/rules/require-await.test.ts b/packages/eslint-plugin/tests/rules/require-await.test.ts index fff294f7ccb..66db5497980 100644 --- a/packages/eslint-plugin/tests/rules/require-await.test.ts +++ b/packages/eslint-plugin/tests/rules/require-await.test.ts @@ -12,20 +12,6 @@ const ruleTester = new RuleTester({ parser: '@typescript-eslint/parser', }); -// The rule has no messageId -// eslint-disable-next-line @typescript-eslint/no-explicit-any -const noAwaitFunctionDeclaration: any = { - message: "Async function 'numberOne' has no 'await' expression.", -}; -// eslint-disable-next-line @typescript-eslint/no-explicit-any -const noAwaitFunctionExpression: any = { - message: "Async function has no 'await' expression.", -}; -// eslint-disable-next-line @typescript-eslint/no-explicit-any -const noAwaitAsyncFunctionExpression: any = { - message: "Async arrow function has no 'await' expression.", -}; - ruleTester.run('require-await', rule, { valid: [ // Non-async function declaration @@ -126,19 +112,182 @@ async function testFunction(): Promise { code: `async function numberOne(): Promise { return 1; }`, - errors: [noAwaitFunctionDeclaration], + errors: [ + { + messageId: 'missingAwait', + data: { + name: "Async function 'numberOne'", + }, + }, + ], }, { // Async function expression with no await code: `const numberOne = async function(): Promise { return 1; }`, - errors: [noAwaitFunctionExpression], + errors: [ + { + messageId: 'missingAwait', + data: { + name: 'Async function', + }, + }, + ], }, { // Async arrow function expression with no await code: `const numberOne = async (): Promise => 1;`, - errors: [noAwaitAsyncFunctionExpression], + errors: [ + { + messageId: 'missingAwait', + data: { + name: 'Async arrow function', + }, + }, + ], + }, + { + // non-async function with await inside async function without await + code: + 'async function foo() { function nested() { await doSomething() } }', + errors: [ + { + messageId: 'missingAwait', + data: { + name: "Async function 'foo'", + }, + }, + ], + }, + ], +}); + +// base eslint tests +// https://github.com/eslint/eslint/blob/03a69dbe86d5b5768a310105416ae726822e3c1c/tests/lib/rules/require-await.js#L25-L132 +ruleTester.run('require-await', rule, { + valid: [ + 'async function foo() { await doSomething() }', + '(async function() { await doSomething() })', + 'async () => { await doSomething() }', + 'async () => await doSomething()', + '({ async foo() { await doSomething() } })', + 'class A { async foo() { await doSomething() } }', + '(class { async foo() { await doSomething() } })', + 'async function foo() { await (async () => { await doSomething() }) }', + + // empty functions are ok. + 'async function foo() {}', + 'async () => {}', + + // normal functions are ok. + 'function foo() { doSomething() }', + + // for-await-of + 'async function foo() { for await (x of xs); }', + + // global await + { + code: 'await foo()', + }, + { + code: ` + for await (let num of asyncIterable) { + console.log(num); + } + `, + }, + ], + invalid: [ + { + code: 'async function foo() { doSomething() }', + errors: [ + { + messageId: 'missingAwait', + data: { name: "Async function 'foo'" }, + }, + ], + }, + { + code: '(async function() { doSomething() })', + errors: [ + { + messageId: 'missingAwait', + data: { name: 'Async function' }, + }, + ], + }, + { + code: 'async () => { doSomething() }', + errors: [ + { + messageId: 'missingAwait', + data: { name: 'Async arrow function' }, + }, + ], + }, + { + code: 'async () => doSomething()', + errors: [ + { + messageId: 'missingAwait', + data: { name: 'Async arrow function' }, + }, + ], + }, + { + code: '({ async foo() { doSomething() } })', + errors: [ + { + messageId: 'missingAwait', + data: { name: "Async method 'foo'" }, + }, + ], + }, + { + code: 'class A { async foo() { doSomething() } }', + errors: [ + { + messageId: 'missingAwait', + data: { name: "Async method 'foo'" }, + }, + ], + }, + { + code: '(class { async foo() { doSomething() } })', + errors: [ + { + messageId: 'missingAwait', + data: { name: "Async method 'foo'" }, + }, + ], + }, + { + code: "(class { async ''() { doSomething() } })", + errors: [ + { + messageId: 'missingAwait', + data: { name: 'Async method' }, + }, + ], + }, + { + code: 'async function foo() { async () => { await doSomething() } }', + errors: [ + { + messageId: 'missingAwait', + data: { name: "Async function 'foo'" }, + }, + ], + }, + { + code: 'async function foo() { await (async () => { doSomething() }) }', + errors: [ + { + messageId: 'missingAwait', + data: { name: 'Async arrow function' }, + }, + ], }, ], }); diff --git a/packages/eslint-plugin/typings/eslint-rules.d.ts b/packages/eslint-plugin/typings/eslint-rules.d.ts index f64ebe424a9..9b730372c48 100644 --- a/packages/eslint-plugin/typings/eslint-rules.d.ts +++ b/packages/eslint-plugin/typings/eslint-rules.d.ts @@ -436,29 +436,6 @@ 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(): void; - ForOfStatement(node: TSESTree.ForOfStatement): void; - } - >; - export = rule; -} - declare module 'eslint/lib/rules/semi' { import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils';