From e02e2ea88757f7464673372af57a94a93a6addce Mon Sep 17 00:00:00 2001 From: reduckted Date: Tue, 23 Jul 2024 21:49:36 +1000 Subject: [PATCH 01/14] Add suggestion to require-await to remove async keyword Fixes #9621 --- .../eslint-plugin/src/rules/require-await.ts | 67 +++- packages/eslint-plugin/src/util/index.ts | 2 + .../src/util/isStartOfExpressionStatement.ts | 22 + .../src/util/needsPrecedingSemiColon.ts | 125 ++++++ .../tests/rules/require-await.test.ts | 378 +++++++++++++++++- 5 files changed, 592 insertions(+), 2 deletions(-) create mode 100644 packages/eslint-plugin/src/util/isStartOfExpressionStatement.ts create mode 100644 packages/eslint-plugin/src/util/needsPrecedingSemiColon.ts diff --git a/packages/eslint-plugin/src/rules/require-await.ts b/packages/eslint-plugin/src/rules/require-await.ts index eed21f74f350..ba0e5891d0df 100644 --- a/packages/eslint-plugin/src/rules/require-await.ts +++ b/packages/eslint-plugin/src/rules/require-await.ts @@ -1,5 +1,10 @@ import type { TSESTree } from '@typescript-eslint/utils'; -import { AST_NODE_TYPES } from '@typescript-eslint/utils'; +import { AST_NODE_TYPES, AST_TOKEN_TYPES } from '@typescript-eslint/utils'; +import type { + AST, + ReportFixFunction, + RuleFix, +} from '@typescript-eslint/utils/ts-eslint'; import * as tsutils from 'ts-api-utils'; import type * as ts from 'typescript'; @@ -8,6 +13,8 @@ import { getFunctionHeadLoc, getFunctionNameWithKind, getParserServices, + isStartOfExpressionStatement, + needsPrecedingSemicolon, upperCaseFirst, } from '../util'; @@ -37,7 +44,9 @@ export default createRule({ schema: [], messages: { missingAwait: "{{name}} has no 'await' expression.", + removeAsync: "Remove 'async'.", }, + hasSuggestions: true, }, defaultOptions: [], create(context) { @@ -75,6 +84,61 @@ export default createRule({ !isEmptyFunction(node) && !(scopeInfo.isGen && scopeInfo.isAsyncYield) ) { + let fix: ReportFixFunction | undefined; + + // Only include a suggestion if the function's return type is + // inferred. If it has an explicit return type, then removing + // the `async` keyword will cause a compilation error. + if (!node.returnType) { + // If the function belongs to a method definition or + // property, then the function's range may not include the + // `async` keyword and we should look at the parent instead. + const nodeWithAsyncKeyword = + (node.parent.type === AST_NODE_TYPES.MethodDefinition && + node.parent.value === node) || + (node.parent.type === AST_NODE_TYPES.Property && + node.parent.method && + node.parent.value === node) + ? node.parent + : node; + + const asyncToken = context.sourceCode.getFirstToken( + nodeWithAsyncKeyword, + token => token.value === 'async', + ); + + let asyncRange: Readonly; + if (asyncToken) { + const nextTokenWithComments = context.sourceCode.getTokenAfter( + asyncToken, + { includeComments: true }, + ); + if (nextTokenWithComments) { + asyncRange = [ + asyncToken.range[0], + nextTokenWithComments.range[0], + ] as const; + } + } + + // Removing the `async` keyword can cause parsing errors if the current + // statement is relying on automatic semicolon insertion. If ASI is currently + // being used, then we should replace the `async` keyword with a semicolon. + let addSemiColon = false; + if (asyncToken) { + const nextToken = context.sourceCode.getTokenAfter(asyncToken); + addSemiColon = + nextToken?.type === AST_TOKEN_TYPES.Punctuator && + (nextToken.value === '[' || nextToken.value === '(') && + (nodeWithAsyncKeyword.type === AST_NODE_TYPES.MethodDefinition || + isStartOfExpressionStatement(nodeWithAsyncKeyword)) && + needsPrecedingSemicolon(context.sourceCode, nodeWithAsyncKeyword); + + fix = (fixer): RuleFix => + fixer.replaceTextRange(asyncRange, addSemiColon ? ';' : ''); + } + } + context.report({ node, loc: getFunctionHeadLoc(node, context.sourceCode), @@ -82,6 +146,7 @@ export default createRule({ data: { name: upperCaseFirst(getFunctionNameWithKind(node)), }, + suggest: fix ? [{ messageId: 'removeAsync', fix }] : [], }); } diff --git a/packages/eslint-plugin/src/util/index.ts b/packages/eslint-plugin/src/util/index.ts index 317fe1af66e3..fea330b3fb8b 100644 --- a/packages/eslint-plugin/src/util/index.ts +++ b/packages/eslint-plugin/src/util/index.ts @@ -12,8 +12,10 @@ export * from './getThisExpression'; export * from './getWrappingFixer'; export * from './isNodeEqual'; export * from './isNullLiteral'; +export * from './isStartOfExpressionStatement'; export * from './isUndefinedIdentifier'; export * from './misc'; +export * from './needsPrecedingSemiColon'; export * from './objectIterators'; export * from './scopeUtils'; export * from './types'; diff --git a/packages/eslint-plugin/src/util/isStartOfExpressionStatement.ts b/packages/eslint-plugin/src/util/isStartOfExpressionStatement.ts new file mode 100644 index 000000000000..76f920dfbeb8 --- /dev/null +++ b/packages/eslint-plugin/src/util/isStartOfExpressionStatement.ts @@ -0,0 +1,22 @@ +import type { TSESTree } from '@typescript-eslint/utils'; +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; + +// The following is copied from `eslint`'s source code. +// https://github.com/eslint/eslint/blob/3a4eaf921543b1cd5d1df4ea9dec02fab396af2a/lib/rules/utils/ast-utils.js#L1026-L1041 +// Could be export { isStartOfExpressionStatement } from 'eslint/lib/rules/utils/ast-utils' +/** + * Tests if a node appears at the beginning of an ancestor ExpressionStatement node. + * @param node The node to check. + * @returns Whether the node appears at the beginning of an ancestor ExpressionStatement node. + */ +export function isStartOfExpressionStatement(node: TSESTree.Node): boolean { + const start = node.range[0]; + let ancestor: TSESTree.Node | undefined = node; + + while ((ancestor = ancestor.parent) && ancestor.range[0] === start) { + if (ancestor.type === AST_NODE_TYPES.ExpressionStatement) { + return true; + } + } + return false; +} diff --git a/packages/eslint-plugin/src/util/needsPrecedingSemiColon.ts b/packages/eslint-plugin/src/util/needsPrecedingSemiColon.ts new file mode 100644 index 000000000000..5f9a94f551f4 --- /dev/null +++ b/packages/eslint-plugin/src/util/needsPrecedingSemiColon.ts @@ -0,0 +1,125 @@ +import type { TSESTree } from '@typescript-eslint/utils'; +import { AST_NODE_TYPES, AST_TOKEN_TYPES } from '@typescript-eslint/utils'; +import { + isClosingBraceToken, + isClosingParenToken, +} from '@typescript-eslint/utils/ast-utils'; +import type { SourceCode } from '@typescript-eslint/utils/ts-eslint'; + +// The following is adapted from `eslint`'s source code. +// https://github.com/eslint/eslint/blob/3a4eaf921543b1cd5d1df4ea9dec02fab396af2a/lib/rules/utils/ast-utils.js#L1043-L1132 +// Could be export { isStartOfExpressionStatement } from 'eslint/lib/rules/utils/ast-utils' + +const BREAK_OR_CONTINUE = new Set([ + AST_NODE_TYPES.BreakStatement, + AST_NODE_TYPES.ContinueStatement, +]); + +// Declaration types that must contain a string Literal node at the end. +const DECLARATIONS = new Set([ + AST_NODE_TYPES.ExportAllDeclaration, + AST_NODE_TYPES.ExportNamedDeclaration, + AST_NODE_TYPES.ImportDeclaration, +]); + +const IDENTIFIER_OR_KEYWORD = new Set([ + AST_NODE_TYPES.Identifier, + AST_TOKEN_TYPES.Keyword, +]); + +// Keywords that can immediately precede an ExpressionStatement node, mapped to the their node types. +const NODE_TYPES_BY_KEYWORD: Record = { + __proto__: null, + break: AST_NODE_TYPES.BreakStatement, + continue: AST_NODE_TYPES.ContinueStatement, + debugger: AST_NODE_TYPES.DebuggerStatement, + do: AST_NODE_TYPES.DoWhileStatement, + else: AST_NODE_TYPES.IfStatement, + return: AST_NODE_TYPES.ReturnStatement, + yield: AST_NODE_TYPES.YieldExpression, +}; + +/* + * Before an opening parenthesis, postfix `++` and `--` always trigger ASI; + * the tokens `:`, `;`, `{` and `=>` don't expect a semicolon, as that would count as an empty statement. + */ +const PUNCTUATORS = new Set([':', ';', '{', '=>', '++', '--']); + +/* + * Statements that can contain an `ExpressionStatement` after a closing parenthesis. + * DoWhileStatement is an exception in that it always triggers ASI after the closing parenthesis. + */ +const STATEMENTS = new Set([ + AST_NODE_TYPES.DoWhileStatement, + AST_NODE_TYPES.ForInStatement, + AST_NODE_TYPES.ForOfStatement, + AST_NODE_TYPES.ForStatement, + AST_NODE_TYPES.IfStatement, + AST_NODE_TYPES.WhileStatement, + AST_NODE_TYPES.WithStatement, +]); + +/** + * Determines whether an opening parenthesis `(`, bracket `[` or backtick ``` ` ``` needs to be preceded by a semicolon. + * This opening parenthesis or bracket should be at the start of an `ExpressionStatement`, a `MethodDefinition` or at + * the start of the body of an `ArrowFunctionExpression`. + * @param sourceCode The source code object. + * @param node A node at the position where an opening parenthesis or bracket will be inserted. + * @returns Whether a semicolon is required before the opening parenthesis or bracket. + */ +export function needsPrecedingSemicolon( + sourceCode: SourceCode, + node: TSESTree.Node, +): boolean { + const prevToken = sourceCode.getTokenBefore(node); + + if ( + !prevToken || + (prevToken.type === AST_TOKEN_TYPES.Punctuator && + PUNCTUATORS.has(prevToken.value)) + ) { + return false; + } + + const prevNode = sourceCode.getNodeByRangeIndex(prevToken.range[0]); + + if (!prevNode) { + return false; + } + + if (isClosingParenToken(prevToken)) { + return !STATEMENTS.has(prevNode.type); + } + + if (isClosingBraceToken(prevToken)) { + return ( + (prevNode.type === AST_NODE_TYPES.BlockStatement && + prevNode.parent.type === AST_NODE_TYPES.FunctionExpression && + prevNode.parent.parent.type !== AST_NODE_TYPES.MethodDefinition) || + (prevNode.type === AST_NODE_TYPES.ClassBody && + prevNode.parent.type === AST_NODE_TYPES.ClassExpression) || + prevNode.type === AST_NODE_TYPES.ObjectExpression + ); + } + + if (!prevNode.parent) { + return false; + } + + if (IDENTIFIER_OR_KEYWORD.has(prevToken.type)) { + if (BREAK_OR_CONTINUE.has(prevNode.parent.type)) { + return false; + } + + const keyword = prevToken.value; + const nodeType = NODE_TYPES_BY_KEYWORD[keyword]; + + return prevNode.type !== nodeType; + } + + if (prevToken.type === AST_TOKEN_TYPES.String) { + return !DECLARATIONS.has(prevNode.parent.type); + } + + return true; +} diff --git a/packages/eslint-plugin/tests/rules/require-await.test.ts b/packages/eslint-plugin/tests/rules/require-await.test.ts index d4b0d6c6d2f1..6bf084ef927e 100644 --- a/packages/eslint-plugin/tests/rules/require-await.test.ts +++ b/packages/eslint-plugin/tests/rules/require-await.test.ts @@ -277,6 +277,7 @@ async function numberOne(): Promise { data: { name: "Async function 'numberOne'", }, + suggestions: [], }, ], }, @@ -293,6 +294,7 @@ const numberOne = async function (): Promise { data: { name: "Async function 'numberOne'", }, + suggestions: [], }, ], }, @@ -305,6 +307,7 @@ const numberOne = async function (): Promise { data: { name: "Async arrow function 'numberOne'", }, + suggestions: [], }, ], }, @@ -323,6 +326,18 @@ const numberOne = async function (): Promise { data: { name: "Async function 'foo'", }, + suggestions: [ + { + messageId: 'removeAsync', + output: ` + function foo() { + function nested() { + await doSomething(); + } + } + `, + }, + ], }, ], }, @@ -353,6 +368,16 @@ async function* foo() { data: { name: "Async generator function 'foo'", }, + suggestions: [ + { + messageId: 'removeAsync', + output: ` +function* foo() { + yield 1; +} + `, + }, + ], }, ], }, @@ -368,6 +393,16 @@ const foo = async function* () { data: { name: "Async generator function 'foo'", }, + suggestions: [ + { + messageId: 'removeAsync', + output: ` +const foo = function* () { + console.log('bar'); +}; + `, + }, + ], }, ], }, @@ -383,6 +418,16 @@ async function* asyncGenerator() { data: { name: "Async generator function 'asyncGenerator'", }, + suggestions: [ + { + messageId: 'removeAsync', + output: ` +function* asyncGenerator() { + yield 1; +} + `, + }, + ], }, ], }, @@ -398,6 +443,16 @@ async function* asyncGenerator(source: Iterable) { data: { name: "Async generator function 'asyncGenerator'", }, + suggestions: [ + { + messageId: 'removeAsync', + output: ` +function* asyncGenerator(source: Iterable) { + yield* source; +} + `, + }, + ], }, ], }, @@ -418,6 +473,21 @@ async function* asyncGenerator(source: Iterable | AsyncIterable) { data: { name: "Async generator function 'asyncGenerator'", }, + suggestions: [ + { + messageId: 'removeAsync', + output: ` +function isAsyncIterable(value: unknown): value is AsyncIterable { + return true; +} +function* asyncGenerator(source: Iterable | AsyncIterable) { + if (!isAsyncIterable(source)) { + yield* source; + } +} + `, + }, + ], }, ], }, @@ -436,6 +506,19 @@ async function* asyncGenerator() { data: { name: "Async generator function 'asyncGenerator'", }, + suggestions: [ + { + messageId: 'removeAsync', + output: ` +function* syncGenerator() { + yield 1; +} +function* asyncGenerator() { + yield* syncGenerator(); +} + `, + }, + ], }, ], }, @@ -451,13 +534,23 @@ async function* asyncGenerator() { data: { name: "Async arrow function 'fn'", }, + suggestions: [ + { + messageId: 'removeAsync', + output: ` + const fn = () => { + using foo = new Bar(); + }; + `, + }, + ], }, ], }, ], }); // base eslint tests -// https://github.com/eslint/eslint/blob/03a69dbe86d5b5768a310105416ae726822e3c1c/tests/lib/rules/require-await.js#L25-L132 +// https://github.com/eslint/eslint/blob/3a4eaf921543b1cd5d1df4ea9dec02fab396af2a/tests/lib/rules/require-await.js#L25-L274 ruleTester.run('require-await', rule, { valid: [ ` @@ -542,6 +635,16 @@ for await (let num of asyncIterable) { { messageId: 'missingAwait', data: { name: "Async function 'foo'" }, + suggestions: [ + { + messageId: 'removeAsync', + output: ` + function foo() { + doSomething(); + } + `, + }, + ], }, ], }, @@ -555,6 +658,16 @@ for await (let num of asyncIterable) { { messageId: 'missingAwait', data: { name: 'Async function' }, + suggestions: [ + { + messageId: 'removeAsync', + output: ` + (function () { + doSomething(); + }); + `, + }, + ], }, ], }, @@ -568,6 +681,16 @@ for await (let num of asyncIterable) { { messageId: 'missingAwait', data: { name: 'Async arrow function' }, + suggestions: [ + { + messageId: 'removeAsync', + output: ` + () => { + doSomething(); + }; + `, + }, + ], }, ], }, @@ -577,6 +700,9 @@ for await (let num of asyncIterable) { { messageId: 'missingAwait', data: { name: 'Async arrow function' }, + suggestions: [ + { messageId: 'removeAsync', output: '() => doSomething();' }, + ], }, ], }, @@ -592,6 +718,18 @@ for await (let num of asyncIterable) { { messageId: 'missingAwait', data: { name: "Async method 'foo'" }, + suggestions: [ + { + messageId: 'removeAsync', + output: ` + ({ + foo() { + doSomething(); + }, + }); + `, + }, + ], }, ], }, @@ -607,6 +745,45 @@ for await (let num of asyncIterable) { { messageId: 'missingAwait', data: { name: "Async method 'foo'" }, + suggestions: [ + { + messageId: 'removeAsync', + output: ` + class A { + foo() { + doSomething(); + } + } + `, + }, + ], + }, + ], + }, + { + code: ` + class A { + public async foo() { + doSomething(); + } + } + `, + errors: [ + { + messageId: 'missingAwait', + data: { name: "Async method 'foo'" }, + suggestions: [ + { + messageId: 'removeAsync', + output: ` + class A { + public foo() { + doSomething(); + } + } + `, + }, + ], }, ], }, @@ -622,6 +799,18 @@ for await (let num of asyncIterable) { { messageId: 'missingAwait', data: { name: "Async method 'foo'" }, + suggestions: [ + { + messageId: 'removeAsync', + output: ` + (class { + foo() { + doSomething(); + } + }); + `, + }, + ], }, ], }, @@ -637,6 +826,18 @@ for await (let num of asyncIterable) { { messageId: 'missingAwait', data: { name: 'Async method' }, + suggestions: [ + { + messageId: 'removeAsync', + output: ` + (class { + ''() { + doSomething(); + } + }); + `, + }, + ], }, ], }, @@ -652,6 +853,18 @@ for await (let num of asyncIterable) { { messageId: 'missingAwait', data: { name: "Async function 'foo'" }, + suggestions: [ + { + messageId: 'removeAsync', + output: ` + function foo() { + async () => { + await doSomething(); + }; + } + `, + }, + ], }, ], }, @@ -667,6 +880,169 @@ for await (let num of asyncIterable) { { messageId: 'missingAwait', data: { name: 'Async arrow function' }, + suggestions: [ + { + messageId: 'removeAsync', + output: ` + async function foo() { + await (() => { + doSomething(); + }); + } + `, + }, + ], + }, + ], + }, + { + code: ` + const obj = { + async: async function foo() { + bar(); + }, + }; + `, + errors: [ + { + messageId: 'missingAwait', + data: { name: "Async method 'async'" }, + suggestions: [ + { + output: ` + const obj = { + async: function foo() { + bar(); + }, + }; + `, + messageId: 'removeAsync', + }, + ], + }, + ], + }, + { + // This test verifies that the async keyword and any following + // whitespace is removed, but not the following comments. + // eslint-disable-next-line @typescript-eslint/internal/plugin-test-formatting + code: ` + async /* test */ function foo() { + doSomething(); + } + `, + errors: [ + { + messageId: 'missingAwait', + data: { name: "Async function 'foo'" }, + suggestions: [ + { + output: ` + /* test */ function foo() { + doSomething(); + } + `, + messageId: 'removeAsync', + }, + ], + }, + ], + }, + { + code: ` + class A { + a = 0; + async [b]() { + return 0; + } + } + `, + languageOptions: { + parserOptions: { ecmaVersion: 2022 }, + }, + errors: [ + { + messageId: 'missingAwait', + data: { name: 'Async method' }, + suggestions: [ + { + output: ` + class A { + a = 0; + [b]() { + return 0; + } + } + `, + messageId: 'removeAsync', + }, + ], + }, + ], + }, + { + // This test must not have a semicolon after "foo". + // eslint-disable-next-line @typescript-eslint/internal/plugin-test-formatting + code: ` + foo + async () => { + return 0; + } + `, + languageOptions: { + parserOptions: { + ecmaVersion: 2022, + }, + }, + errors: [ + { + messageId: 'missingAwait', + data: { name: 'Async arrow function' }, + suggestions: [ + { + output: ` + foo + ;() => { + return 0; + } + `, + messageId: 'removeAsync', + }, + ], + }, + ], + }, + { + code: ` + class A { + foo() {} + async [bar]() { + baz; + } + } + `, + languageOptions: { + parserOptions: { + ecmaVersion: 2022, + }, + }, + errors: [ + { + messageId: 'missingAwait', + data: { name: 'Async method' }, + suggestions: [ + { + output: ` + class A { + foo() {} + [bar]() { + baz; + } + } + `, + messageId: 'removeAsync', + }, + ], }, ], }, From 9ba518507611a76eab705fb8020fc9923c864491 Mon Sep 17 00:00:00 2001 From: reduckted Date: Wed, 7 Aug 2024 19:53:06 +1000 Subject: [PATCH 02/14] Include suggestion when there is a return type annotation. --- .../eslint-plugin/src/rules/require-await.ts | 87 +++++++++---------- .../tests/rules/require-await.test.ts | 39 ++++++++- 2 files changed, 77 insertions(+), 49 deletions(-) diff --git a/packages/eslint-plugin/src/rules/require-await.ts b/packages/eslint-plugin/src/rules/require-await.ts index ba0e5891d0df..5c3710f09b16 100644 --- a/packages/eslint-plugin/src/rules/require-await.ts +++ b/packages/eslint-plugin/src/rules/require-await.ts @@ -86,57 +86,52 @@ export default createRule({ ) { let fix: ReportFixFunction | undefined; - // Only include a suggestion if the function's return type is - // inferred. If it has an explicit return type, then removing - // the `async` keyword will cause a compilation error. - if (!node.returnType) { - // If the function belongs to a method definition or - // property, then the function's range may not include the - // `async` keyword and we should look at the parent instead. - const nodeWithAsyncKeyword = - (node.parent.type === AST_NODE_TYPES.MethodDefinition && - node.parent.value === node) || - (node.parent.type === AST_NODE_TYPES.Property && - node.parent.method && - node.parent.value === node) - ? node.parent - : node; + // If the function belongs to a method definition or + // property, then the function's range may not include the + // `async` keyword and we should look at the parent instead. + const nodeWithAsyncKeyword = + (node.parent.type === AST_NODE_TYPES.MethodDefinition && + node.parent.value === node) || + (node.parent.type === AST_NODE_TYPES.Property && + node.parent.method && + node.parent.value === node) + ? node.parent + : node; - const asyncToken = context.sourceCode.getFirstToken( - nodeWithAsyncKeyword, - token => token.value === 'async', - ); + const asyncToken = context.sourceCode.getFirstToken( + nodeWithAsyncKeyword, + token => token.value === 'async', + ); - let asyncRange: Readonly; - if (asyncToken) { - const nextTokenWithComments = context.sourceCode.getTokenAfter( - asyncToken, - { includeComments: true }, - ); - if (nextTokenWithComments) { - asyncRange = [ - asyncToken.range[0], - nextTokenWithComments.range[0], - ] as const; - } + let asyncRange: Readonly; + if (asyncToken) { + const nextTokenWithComments = context.sourceCode.getTokenAfter( + asyncToken, + { includeComments: true }, + ); + if (nextTokenWithComments) { + asyncRange = [ + asyncToken.range[0], + nextTokenWithComments.range[0], + ] as const; } + } - // Removing the `async` keyword can cause parsing errors if the current - // statement is relying on automatic semicolon insertion. If ASI is currently - // being used, then we should replace the `async` keyword with a semicolon. - let addSemiColon = false; - if (asyncToken) { - const nextToken = context.sourceCode.getTokenAfter(asyncToken); - addSemiColon = - nextToken?.type === AST_TOKEN_TYPES.Punctuator && - (nextToken.value === '[' || nextToken.value === '(') && - (nodeWithAsyncKeyword.type === AST_NODE_TYPES.MethodDefinition || - isStartOfExpressionStatement(nodeWithAsyncKeyword)) && - needsPrecedingSemicolon(context.sourceCode, nodeWithAsyncKeyword); + // Removing the `async` keyword can cause parsing errors if the current + // statement is relying on automatic semicolon insertion. If ASI is currently + // being used, then we should replace the `async` keyword with a semicolon. + let addSemiColon = false; + if (asyncToken) { + const nextToken = context.sourceCode.getTokenAfter(asyncToken); + addSemiColon = + nextToken?.type === AST_TOKEN_TYPES.Punctuator && + (nextToken.value === '[' || nextToken.value === '(') && + (nodeWithAsyncKeyword.type === AST_NODE_TYPES.MethodDefinition || + isStartOfExpressionStatement(nodeWithAsyncKeyword)) && + needsPrecedingSemicolon(context.sourceCode, nodeWithAsyncKeyword); - fix = (fixer): RuleFix => - fixer.replaceTextRange(asyncRange, addSemiColon ? ';' : ''); - } + fix = (fixer): RuleFix => + fixer.replaceTextRange(asyncRange, addSemiColon ? ';' : ''); } context.report({ diff --git a/packages/eslint-plugin/tests/rules/require-await.test.ts b/packages/eslint-plugin/tests/rules/require-await.test.ts index 6bf084ef927e..940b3df14edf 100644 --- a/packages/eslint-plugin/tests/rules/require-await.test.ts +++ b/packages/eslint-plugin/tests/rules/require-await.test.ts @@ -277,7 +277,16 @@ async function numberOne(): Promise { data: { name: "Async function 'numberOne'", }, - suggestions: [], + suggestions: [ + { + output: ` +function numberOne(): Promise { + return 1; +} + `, + messageId: 'removeAsync', + }, + ], }, ], }, @@ -294,7 +303,16 @@ const numberOne = async function (): Promise { data: { name: "Async function 'numberOne'", }, - suggestions: [], + suggestions: [ + { + output: ` +const numberOne = function (): Promise { + return 1; +}; + `, + messageId: 'removeAsync', + }, + ], }, ], }, @@ -307,7 +325,12 @@ const numberOne = async function (): Promise { data: { name: "Async arrow function 'numberOne'", }, - suggestions: [], + suggestions: [ + { + output: 'const numberOne = (): Promise => 1;', + messageId: 'removeAsync', + }, + ], }, ], }, @@ -353,6 +376,16 @@ async function* foo(): void { data: { name: "Async generator function 'foo'", }, + suggestions: [ + { + output: ` +function* foo(): void { + doSomething(); +} + `, + messageId: 'removeAsync', + }, + ], }, ], }, From c48b00f9545193820eded3eb368bce7ba122020e Mon Sep 17 00:00:00 2001 From: reduckted Date: Wed, 7 Aug 2024 21:14:24 +1000 Subject: [PATCH 03/14] Restructure suggestion range calculation. --- .../eslint-plugin/src/rules/require-await.ts | 34 +++++++++---------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/packages/eslint-plugin/src/rules/require-await.ts b/packages/eslint-plugin/src/rules/require-await.ts index 5c3710f09b16..e5d4fc96894f 100644 --- a/packages/eslint-plugin/src/rules/require-await.ts +++ b/packages/eslint-plugin/src/rules/require-await.ts @@ -103,35 +103,33 @@ export default createRule({ token => token.value === 'async', ); - let asyncRange: Readonly; if (asyncToken) { const nextTokenWithComments = context.sourceCode.getTokenAfter( asyncToken, { includeComments: true }, ); + if (nextTokenWithComments) { - asyncRange = [ + const asyncRange: Readonly = [ asyncToken.range[0], nextTokenWithComments.range[0], ] as const; - } - } - // Removing the `async` keyword can cause parsing errors if the current - // statement is relying on automatic semicolon insertion. If ASI is currently - // being used, then we should replace the `async` keyword with a semicolon. - let addSemiColon = false; - if (asyncToken) { - const nextToken = context.sourceCode.getTokenAfter(asyncToken); - addSemiColon = - nextToken?.type === AST_TOKEN_TYPES.Punctuator && - (nextToken.value === '[' || nextToken.value === '(') && - (nodeWithAsyncKeyword.type === AST_NODE_TYPES.MethodDefinition || - isStartOfExpressionStatement(nodeWithAsyncKeyword)) && - needsPrecedingSemicolon(context.sourceCode, nodeWithAsyncKeyword); + // Removing the `async` keyword can cause parsing errors if the + // current statement is relying on automatic semicolon insertion. + // If ASI is currently being used, then we should replace the + // `async` keyword with a semicolon. + const nextToken = context.sourceCode.getTokenAfter(asyncToken); + const addSemiColon = + nextToken?.type === AST_TOKEN_TYPES.Punctuator && + (nextToken.value === '[' || nextToken.value === '(') && + (nodeWithAsyncKeyword.type === AST_NODE_TYPES.MethodDefinition || + isStartOfExpressionStatement(nodeWithAsyncKeyword)) && + needsPrecedingSemicolon(context.sourceCode, nodeWithAsyncKeyword); - fix = (fixer): RuleFix => - fixer.replaceTextRange(asyncRange, addSemiColon ? ';' : ''); + fix = (fixer): RuleFix => + fixer.replaceTextRange(asyncRange, addSemiColon ? ';' : ''); + } } context.report({ From d8b86a4ea2799fdfb3d0cddc9642173dba3ed277 Mon Sep 17 00:00:00 2001 From: reduckted Date: Wed, 7 Aug 2024 21:27:29 +1000 Subject: [PATCH 04/14] Suggestion will replace return type of `Promise` with `T`. --- .../eslint-plugin/src/rules/require-await.ts | 50 ++++++++++++++++++- .../tests/rules/require-await.test.ts | 32 ++++++++++-- 2 files changed, 77 insertions(+), 5 deletions(-) diff --git a/packages/eslint-plugin/src/rules/require-await.ts b/packages/eslint-plugin/src/rules/require-await.ts index e5d4fc96894f..492fead83868 100644 --- a/packages/eslint-plugin/src/rules/require-await.ts +++ b/packages/eslint-plugin/src/rules/require-await.ts @@ -127,8 +127,54 @@ export default createRule({ isStartOfExpressionStatement(nodeWithAsyncKeyword)) && needsPrecedingSemicolon(context.sourceCode, nodeWithAsyncKeyword); - fix = (fixer): RuleFix => - fixer.replaceTextRange(asyncRange, addSemiColon ? ';' : ''); + const changes = [ + { range: asyncRange, replacement: addSemiColon ? ';' : '' }, + ]; + + // If there's a return type annotation and it's a + // `Promise`, we can also change the return type + // annotation to just `T` as part of the suggestion. + if ( + node.returnType?.typeAnnotation.type === + AST_NODE_TYPES.TSTypeReference && + node.returnType.typeAnnotation.typeName.type === + AST_NODE_TYPES.Identifier && + node.returnType.typeAnnotation.typeName.name === 'Promise' && + node.returnType.typeAnnotation.typeArguments + ) { + const openAngle = context.sourceCode.getFirstToken( + node.returnType.typeAnnotation, + token => + token.type === AST_TOKEN_TYPES.Punctuator && + token.value === '<', + ); + const closeAngle = context.sourceCode.getLastToken( + node.returnType.typeAnnotation, + token => + token.type === AST_TOKEN_TYPES.Punctuator && + token.value === '>', + ); + if (openAngle && closeAngle) { + changes.unshift( + // Remove the closing angled bracket. + { range: closeAngle.range, replacement: '' }, + // Remove the "Promise" identifier + // and the opening angled bracket. + { + range: [ + node.returnType.typeAnnotation.typeName.range[0], + openAngle.range[1], + ], + replacement: '', + }, + ); + } + } + + fix = (fixer): RuleFix[] => + changes.map(change => + fixer.replaceTextRange(change.range, change.replacement), + ); } } diff --git a/packages/eslint-plugin/tests/rules/require-await.test.ts b/packages/eslint-plugin/tests/rules/require-await.test.ts index 940b3df14edf..3528ded00e51 100644 --- a/packages/eslint-plugin/tests/rules/require-await.test.ts +++ b/packages/eslint-plugin/tests/rules/require-await.test.ts @@ -280,7 +280,7 @@ async function numberOne(): Promise { suggestions: [ { output: ` -function numberOne(): Promise { +function numberOne(): number { return 1; } `, @@ -306,7 +306,7 @@ const numberOne = async function (): Promise { suggestions: [ { output: ` -const numberOne = function (): Promise { +const numberOne = function (): number { return 1; }; `, @@ -327,7 +327,33 @@ const numberOne = function (): Promise { }, suggestions: [ { - output: 'const numberOne = (): Promise => 1;', + output: 'const numberOne = (): number => 1;', + messageId: 'removeAsync', + }, + ], + }, + ], + }, + { + // Return type with nested generic type + code: ` +async function values(): Promise> { + return [1]; +} + `, + errors: [ + { + messageId: 'missingAwait', + data: { + name: "Async function 'values'", + }, + suggestions: [ + { + output: ` +function values(): Array { + return [1]; +} + `, messageId: 'removeAsync', }, ], From 0f75da369bdf05304f5fe884b35d6886bbf3c82e Mon Sep 17 00:00:00 2001 From: reduckted Date: Sat, 10 Aug 2024 16:59:25 +1000 Subject: [PATCH 05/14] use noFormat instead of suppressing lint errors. --- packages/eslint-plugin/tests/rules/require-await.test.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/require-await.test.ts b/packages/eslint-plugin/tests/rules/require-await.test.ts index 3528ded00e51..2693b6e97a66 100644 --- a/packages/eslint-plugin/tests/rules/require-await.test.ts +++ b/packages/eslint-plugin/tests/rules/require-await.test.ts @@ -1,4 +1,4 @@ -import { RuleTester } from '@typescript-eslint/rule-tester'; +import { noFormat, RuleTester } from '@typescript-eslint/rule-tester'; import rule from '../../src/rules/require-await'; import { getFixturesRootDir } from '../RuleTester'; @@ -984,8 +984,7 @@ for await (let num of asyncIterable) { { // This test verifies that the async keyword and any following // whitespace is removed, but not the following comments. - // eslint-disable-next-line @typescript-eslint/internal/plugin-test-formatting - code: ` + code: noFormat` async /* test */ function foo() { doSomething(); } @@ -1041,8 +1040,7 @@ for await (let num of asyncIterable) { }, { // This test must not have a semicolon after "foo". - // eslint-disable-next-line @typescript-eslint/internal/plugin-test-formatting - code: ` + code: noFormat` foo async () => { return 0; From 6808bb6a7578e99c027fcd8a795ae45b84794559 Mon Sep 17 00:00:00 2001 From: reduckted Date: Sat, 10 Aug 2024 17:14:52 +1000 Subject: [PATCH 06/14] Remove unnecessary ecmaVersion from tests and correct test that had auto-formatting applied. --- .../tests/rules/require-await.test.ts | 22 +++++-------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/require-await.test.ts b/packages/eslint-plugin/tests/rules/require-await.test.ts index 2693b6e97a66..5e1aa3a1c646 100644 --- a/packages/eslint-plugin/tests/rules/require-await.test.ts +++ b/packages/eslint-plugin/tests/rules/require-await.test.ts @@ -1007,17 +1007,15 @@ for await (let num of asyncIterable) { ], }, { - code: ` + // This test must not have a semicolon after "a = 0". + code: noFormat` class A { - a = 0; + a = 0 async [b]() { return 0; } } `, - languageOptions: { - parserOptions: { ecmaVersion: 2022 }, - }, errors: [ { messageId: 'missingAwait', @@ -1026,8 +1024,8 @@ for await (let num of asyncIterable) { { output: ` class A { - a = 0; - [b]() { + a = 0 + ;[b]() { return 0; } } @@ -1046,11 +1044,6 @@ for await (let num of asyncIterable) { return 0; } `, - languageOptions: { - parserOptions: { - ecmaVersion: 2022, - }, - }, errors: [ { messageId: 'missingAwait', @@ -1078,11 +1071,6 @@ for await (let num of asyncIterable) { } } `, - languageOptions: { - parserOptions: { - ecmaVersion: 2022, - }, - }, errors: [ { messageId: 'missingAwait', From 5cdcd4ce47719921560db0c236969ee245b0184f Mon Sep 17 00:00:00 2001 From: reduckted Date: Sat, 10 Aug 2024 17:15:19 +1000 Subject: [PATCH 07/14] Corrected comment. --- packages/eslint-plugin/tests/rules/require-await.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/tests/rules/require-await.test.ts b/packages/eslint-plugin/tests/rules/require-await.test.ts index 5e1aa3a1c646..5405fe197b60 100644 --- a/packages/eslint-plugin/tests/rules/require-await.test.ts +++ b/packages/eslint-plugin/tests/rules/require-await.test.ts @@ -335,7 +335,7 @@ const numberOne = function (): number { ], }, { - // Return type with nested generic type + // Return type with nested type argument code: ` async function values(): Promise> { return [1]; From 577281244520703d122affce652d571523d46949 Mon Sep 17 00:00:00 2001 From: reduckted Date: Sat, 10 Aug 2024 17:48:38 +1000 Subject: [PATCH 08/14] Updated the copy of ESLint test cases. --- .../tests/rules/require-await.test.ts | 78 ++++++++++++++++--- 1 file changed, 69 insertions(+), 9 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/require-await.test.ts b/packages/eslint-plugin/tests/rules/require-await.test.ts index 5405fe197b60..5118ef93fba9 100644 --- a/packages/eslint-plugin/tests/rules/require-await.test.ts +++ b/packages/eslint-plugin/tests/rules/require-await.test.ts @@ -134,14 +134,6 @@ async function testFunction(): Promise { async value => Promise.resolve(value), ), ); -} - `, - 'async function* run() {}', - ` -async function* run() { - await new Promise(resolve => setTimeout(resolve, 100)); - yield 'Hello'; - console.log('World'); } `, ` @@ -214,7 +206,6 @@ async function* test(source: MyType) { yield* source; } `, - 'const foo: () => void = async function* () {};', ` async function* foo(): Promise { return new Promise(res => res(\`hello\`)); @@ -391,6 +382,8 @@ function values(): Array { ], }, { + // Note: This code is considered a valid case in the ESLint tests, + // but because we have type information we can say that it's invalid. code: ` async function* foo(): void { doSomething(); @@ -441,6 +434,8 @@ function* foo() { ], }, { + // Note: This code is considered a valid case in the ESLint tests, + // but because we have type information we can say that it's invalid. code: ` const foo = async function* () { console.log('bar'); @@ -574,6 +569,33 @@ function* syncGenerator() { } function* asyncGenerator() { yield* syncGenerator(); +} + `, + }, + ], + }, + ], + }, + { + // Note: This code is considered a valid case in the ESLint tests, + // but because we have type information we can say that it's invalid. + code: ` +async function* asyncGenerator() { + yield* anotherAsyncGenerator(); // Unknown function. +} + `, + errors: [ + { + messageId: 'missingAwait', + data: { + name: "Async generator function 'asyncGenerator'", + }, + suggestions: [ + { + messageId: 'removeAsync', + output: ` +function* asyncGenerator() { + yield* anotherAsyncGenerator(); // Unknown function. } `, }, @@ -682,6 +704,44 @@ for await (let num of asyncIterable) { } `, }, + { + code: ` + async function* run() { + await new Promise(resolve => setTimeout(resolve, 100)); + yield 'Hello'; + console.log('World'); + } + `, + }, + { + code: 'async function* run() {}', + }, + { + code: 'const foo = async function* () {};', + }, + // Note: There are three test cases for async generators in the + // ESLint repository that are considered valid. Because we have + // type information, we can consider those cases to be invalid. + // There are test cases in here to confirm that they are invalid. + // + // See https://github.com/typescript-eslint/typescript-eslint/pull/1782 + // + // The cases are: + // + // 1. + // async function* run() { + // yield* anotherAsyncGenerator(); + // } + // + // 2. + // const foo = async function* () { + // console.log('bar'); + // }; + // + // 3. + // async function* run() { + // console.log('bar'); + // } ], invalid: [ { From 93c1dd05da4bd9c3173eb4091d8435487c5e5930 Mon Sep 17 00:00:00 2001 From: reduckted Date: Sat, 10 Aug 2024 20:31:02 +1000 Subject: [PATCH 09/14] Added better support for async generators. --- .../eslint-plugin/src/rules/require-await.ts | 87 ++++++++++++------- .../tests/rules/require-await.test.ts | 77 ++++++++++++++++ 2 files changed, 131 insertions(+), 33 deletions(-) diff --git a/packages/eslint-plugin/src/rules/require-await.ts b/packages/eslint-plugin/src/rules/require-await.ts index 492fead83868..b04043a5dbd9 100644 --- a/packages/eslint-plugin/src/rules/require-await.ts +++ b/packages/eslint-plugin/src/rules/require-await.ts @@ -134,40 +134,50 @@ export default createRule({ // If there's a return type annotation and it's a // `Promise`, we can also change the return type // annotation to just `T` as part of the suggestion. - if ( - node.returnType?.typeAnnotation.type === - AST_NODE_TYPES.TSTypeReference && - node.returnType.typeAnnotation.typeName.type === - AST_NODE_TYPES.Identifier && - node.returnType.typeAnnotation.typeName.name === 'Promise' && - node.returnType.typeAnnotation.typeArguments - ) { - const openAngle = context.sourceCode.getFirstToken( - node.returnType.typeAnnotation, - token => - token.type === AST_TOKEN_TYPES.Punctuator && - token.value === '<', - ); - const closeAngle = context.sourceCode.getLastToken( - node.returnType.typeAnnotation, - token => - token.type === AST_TOKEN_TYPES.Punctuator && - token.value === '>', - ); - if (openAngle && closeAngle) { - changes.unshift( - // Remove the closing angled bracket. - { range: closeAngle.range, replacement: '' }, - // Remove the "Promise" identifier - // and the opening angled bracket. - { - range: [ - node.returnType.typeAnnotation.typeName.range[0], - openAngle.range[1], - ], - replacement: '', - }, + // Alternatively, if the function is a generator and + // the return type annotation is `AsyncGenerator`, + // then we can change it to `Generator`. + if (node.returnType) { + if (scopeInfo.isGen) { + if ( + hasTypeName(node.returnType.typeAnnotation, 'AsyncGenerator') + ) { + changes.unshift({ + range: node.returnType.typeAnnotation.typeName.range, + replacement: 'Generator', + }); + } + } else if ( + hasTypeName(node.returnType.typeAnnotation, 'Promise') && + !!node.returnType.typeAnnotation.typeArguments + ) { + const openAngle = context.sourceCode.getFirstToken( + node.returnType.typeAnnotation, + token => + token.type === AST_TOKEN_TYPES.Punctuator && + token.value === '<', ); + const closeAngle = context.sourceCode.getLastToken( + node.returnType.typeAnnotation, + token => + token.type === AST_TOKEN_TYPES.Punctuator && + token.value === '>', + ); + if (openAngle && closeAngle) { + changes.unshift( + // Remove the closing angled bracket. + { range: closeAngle.range, replacement: '' }, + // Remove the "Promise" identifier + // and the opening angled bracket. + { + range: [ + node.returnType.typeAnnotation.typeName.range[0], + openAngle.range[1], + ], + replacement: '', + }, + ); + } } } @@ -304,3 +314,14 @@ function expandUnionOrIntersectionType(type: ts.Type): ts.Type[] { } return [type]; } + +function hasTypeName( + typeAnnotation: TSESTree.TypeNode, + typeName: string, +): typeAnnotation is TSESTree.TSTypeReference { + return ( + typeAnnotation.type === AST_NODE_TYPES.TSTypeReference && + typeAnnotation.typeName.type === AST_NODE_TYPES.Identifier && + typeAnnotation.typeName.name === typeName + ); +} diff --git a/packages/eslint-plugin/tests/rules/require-await.test.ts b/packages/eslint-plugin/tests/rules/require-await.test.ts index 5118ef93fba9..79fce65f7176 100644 --- a/packages/eslint-plugin/tests/rules/require-await.test.ts +++ b/packages/eslint-plugin/tests/rules/require-await.test.ts @@ -628,6 +628,83 @@ function* asyncGenerator() { }, ], }, + { + code: ` + // intentional TS error + async function* foo(): Promise { + yield 1; + } + `, + errors: [ + { + messageId: 'missingAwait', + data: { + name: "Async generator function 'foo'", + }, + suggestions: [ + { + messageId: 'removeAsync', + output: ` + // intentional TS error + function* foo(): Promise { + yield 1; + } + `, + }, + ], + }, + ], + }, + { + code: ` + async function* foo(): AsyncGenerator { + yield 1; + } + `, + errors: [ + { + messageId: 'missingAwait', + data: { + name: "Async generator function 'foo'", + }, + suggestions: [ + { + messageId: 'removeAsync', + output: ` + function* foo(): Generator { + yield 1; + } + `, + }, + ], + }, + ], + }, + { + code: ` + async function* foo(): AsyncGenerator { + yield 1; + } + `, + errors: [ + { + messageId: 'missingAwait', + data: { + name: "Async generator function 'foo'", + }, + suggestions: [ + { + messageId: 'removeAsync', + output: ` + function* foo(): Generator { + yield 1; + } + `, + }, + ], + }, + ], + }, ], }); // base eslint tests From 93f2e78c4e9862f28355dbf2ce3433b3a341b7b6 Mon Sep 17 00:00:00 2001 From: reduckted Date: Fri, 16 Aug 2024 22:10:39 +1000 Subject: [PATCH 10/14] Used nullThrows to avoid unnecessary conditionals. --- .../eslint-plugin/src/rules/require-await.ts | 180 +++++++++--------- 1 file changed, 91 insertions(+), 89 deletions(-) diff --git a/packages/eslint-plugin/src/rules/require-await.ts b/packages/eslint-plugin/src/rules/require-await.ts index b04043a5dbd9..4a4ddb61c844 100644 --- a/packages/eslint-plugin/src/rules/require-await.ts +++ b/packages/eslint-plugin/src/rules/require-await.ts @@ -1,10 +1,6 @@ import type { TSESTree } from '@typescript-eslint/utils'; import { AST_NODE_TYPES, AST_TOKEN_TYPES } from '@typescript-eslint/utils'; -import type { - AST, - ReportFixFunction, - RuleFix, -} from '@typescript-eslint/utils/ts-eslint'; +import type { AST, RuleFix } from '@typescript-eslint/utils/ts-eslint'; import * as tsutils from 'ts-api-utils'; import type * as ts from 'typescript'; @@ -15,6 +11,7 @@ import { getParserServices, isStartOfExpressionStatement, needsPrecedingSemicolon, + nullThrows, upperCaseFirst, } from '../util'; @@ -84,8 +81,6 @@ export default createRule({ !isEmptyFunction(node) && !(scopeInfo.isGen && scopeInfo.isAsyncYield) ) { - let fix: ReportFixFunction | undefined; - // If the function belongs to a method definition or // property, then the function's range may not include the // `async` keyword and we should look at the parent instead. @@ -98,93 +93,92 @@ export default createRule({ ? node.parent : node; - const asyncToken = context.sourceCode.getFirstToken( - nodeWithAsyncKeyword, - token => token.value === 'async', + const asyncToken = nullThrows( + context.sourceCode.getFirstToken( + nodeWithAsyncKeyword, + token => token.value === 'async', + ), + 'The node is an async function, so it must have an "async" token.', ); - if (asyncToken) { - const nextTokenWithComments = context.sourceCode.getTokenAfter( - asyncToken, - { includeComments: true }, - ); - - if (nextTokenWithComments) { - const asyncRange: Readonly = [ - asyncToken.range[0], - nextTokenWithComments.range[0], - ] as const; + const asyncRange: Readonly = [ + asyncToken.range[0], + nullThrows( + context.sourceCode.getTokenAfter(asyncToken, { + includeComments: true, + }), + 'There will always be a token after the "async" keyword.', + ).range[0], + ] as const; - // Removing the `async` keyword can cause parsing errors if the - // current statement is relying on automatic semicolon insertion. - // If ASI is currently being used, then we should replace the - // `async` keyword with a semicolon. - const nextToken = context.sourceCode.getTokenAfter(asyncToken); - const addSemiColon = - nextToken?.type === AST_TOKEN_TYPES.Punctuator && - (nextToken.value === '[' || nextToken.value === '(') && - (nodeWithAsyncKeyword.type === AST_NODE_TYPES.MethodDefinition || - isStartOfExpressionStatement(nodeWithAsyncKeyword)) && - needsPrecedingSemicolon(context.sourceCode, nodeWithAsyncKeyword); + // Removing the `async` keyword can cause parsing errors if the + // current statement is relying on automatic semicolon insertion. + // If ASI is currently being used, then we should replace the + // `async` keyword with a semicolon. + const nextToken = nullThrows( + context.sourceCode.getTokenAfter(asyncToken), + 'There will always be a token after the "async" keyword.', + ); + const addSemiColon = + nextToken.type === AST_TOKEN_TYPES.Punctuator && + (nextToken.value === '[' || nextToken.value === '(') && + (nodeWithAsyncKeyword.type === AST_NODE_TYPES.MethodDefinition || + isStartOfExpressionStatement(nodeWithAsyncKeyword)) && + needsPrecedingSemicolon(context.sourceCode, nodeWithAsyncKeyword); - const changes = [ - { range: asyncRange, replacement: addSemiColon ? ';' : '' }, - ]; + const changes = [ + { range: asyncRange, replacement: addSemiColon ? ';' : '' }, + ]; - // If there's a return type annotation and it's a - // `Promise`, we can also change the return type - // annotation to just `T` as part of the suggestion. - // Alternatively, if the function is a generator and - // the return type annotation is `AsyncGenerator`, - // then we can change it to `Generator`. - if (node.returnType) { - if (scopeInfo.isGen) { - if ( - hasTypeName(node.returnType.typeAnnotation, 'AsyncGenerator') - ) { - changes.unshift({ - range: node.returnType.typeAnnotation.typeName.range, - replacement: 'Generator', - }); - } - } else if ( - hasTypeName(node.returnType.typeAnnotation, 'Promise') && - !!node.returnType.typeAnnotation.typeArguments - ) { - const openAngle = context.sourceCode.getFirstToken( - node.returnType.typeAnnotation, - token => - token.type === AST_TOKEN_TYPES.Punctuator && - token.value === '<', - ); - const closeAngle = context.sourceCode.getLastToken( - node.returnType.typeAnnotation, - token => - token.type === AST_TOKEN_TYPES.Punctuator && - token.value === '>', - ); - if (openAngle && closeAngle) { - changes.unshift( - // Remove the closing angled bracket. - { range: closeAngle.range, replacement: '' }, - // Remove the "Promise" identifier - // and the opening angled bracket. - { - range: [ - node.returnType.typeAnnotation.typeName.range[0], - openAngle.range[1], - ], - replacement: '', - }, - ); - } - } + // If there's a return type annotation and it's a + // `Promise`, we can also change the return type + // annotation to just `T` as part of the suggestion. + // Alternatively, if the function is a generator and + // the return type annotation is `AsyncGenerator`, + // then we can change it to `Generator`. + if (node.returnType) { + if (scopeInfo.isGen) { + if (hasTypeName(node.returnType.typeAnnotation, 'AsyncGenerator')) { + changes.unshift({ + range: node.returnType.typeAnnotation.typeName.range, + replacement: 'Generator', + }); } - - fix = (fixer): RuleFix[] => - changes.map(change => - fixer.replaceTextRange(change.range, change.replacement), - ); + } else if ( + hasTypeName(node.returnType.typeAnnotation, 'Promise') && + !!node.returnType.typeAnnotation.typeArguments + ) { + const openAngle = nullThrows( + context.sourceCode.getFirstToken( + node.returnType.typeAnnotation, + token => + token.type === AST_TOKEN_TYPES.Punctuator && + token.value === '<', + ), + 'There are type arguments, so the angle bracket will exist.', + ); + const closeAngle = nullThrows( + context.sourceCode.getLastToken( + node.returnType.typeAnnotation, + token => + token.type === AST_TOKEN_TYPES.Punctuator && + token.value === '>', + ), + 'There are type arguments, so the angle bracket will exist.', + ); + changes.unshift( + // Remove the closing angled bracket. + { range: closeAngle.range, replacement: '' }, + // Remove the "Promise" identifier + // and the opening angled bracket. + { + range: [ + node.returnType.typeAnnotation.typeName.range[0], + openAngle.range[1], + ], + replacement: '', + }, + ); } } @@ -195,7 +189,15 @@ export default createRule({ data: { name: upperCaseFirst(getFunctionNameWithKind(node)), }, - suggest: fix ? [{ messageId: 'removeAsync', fix }] : [], + suggest: [ + { + messageId: 'removeAsync', + fix: (fixer): RuleFix[] => + changes.map(change => + fixer.replaceTextRange(change.range, change.replacement), + ), + }, + ], }); } From 6981669e56741a146a2fdb26609834add88efb03 Mon Sep 17 00:00:00 2001 From: reduckted Date: Fri, 16 Aug 2024 22:11:30 +1000 Subject: [PATCH 11/14] Added additional changes to end of array instead of inserting at the start. --- packages/eslint-plugin/src/rules/require-await.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/require-await.ts b/packages/eslint-plugin/src/rules/require-await.ts index 4a4ddb61c844..df2f180e38a7 100644 --- a/packages/eslint-plugin/src/rules/require-await.ts +++ b/packages/eslint-plugin/src/rules/require-await.ts @@ -139,7 +139,7 @@ export default createRule({ if (node.returnType) { if (scopeInfo.isGen) { if (hasTypeName(node.returnType.typeAnnotation, 'AsyncGenerator')) { - changes.unshift({ + changes.push({ range: node.returnType.typeAnnotation.typeName.range, replacement: 'Generator', }); @@ -166,7 +166,7 @@ export default createRule({ ), 'There are type arguments, so the angle bracket will exist.', ); - changes.unshift( + changes.push( // Remove the closing angled bracket. { range: closeAngle.range, replacement: '' }, // Remove the "Promise" identifier From 9200af7ee07160b27f7ae3f09443ba279f50de56 Mon Sep 17 00:00:00 2001 From: reduckted Date: Fri, 16 Aug 2024 22:21:45 +1000 Subject: [PATCH 12/14] Use removeRange instead of replaceTextRange with empty replacement. --- packages/eslint-plugin/src/rules/require-await.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/eslint-plugin/src/rules/require-await.ts b/packages/eslint-plugin/src/rules/require-await.ts index df2f180e38a7..72061632abb1 100644 --- a/packages/eslint-plugin/src/rules/require-await.ts +++ b/packages/eslint-plugin/src/rules/require-await.ts @@ -127,7 +127,7 @@ export default createRule({ needsPrecedingSemicolon(context.sourceCode, nodeWithAsyncKeyword); const changes = [ - { range: asyncRange, replacement: addSemiColon ? ';' : '' }, + { range: asyncRange, replacement: addSemiColon ? ';' : undefined }, ]; // If there's a return type annotation and it's a @@ -168,7 +168,7 @@ export default createRule({ ); changes.push( // Remove the closing angled bracket. - { range: closeAngle.range, replacement: '' }, + { range: closeAngle.range, replacement: undefined }, // Remove the "Promise" identifier // and the opening angled bracket. { @@ -176,7 +176,7 @@ export default createRule({ node.returnType.typeAnnotation.typeName.range[0], openAngle.range[1], ], - replacement: '', + replacement: undefined, }, ); } @@ -194,7 +194,9 @@ export default createRule({ messageId: 'removeAsync', fix: (fixer): RuleFix[] => changes.map(change => - fixer.replaceTextRange(change.range, change.replacement), + change.replacement !== undefined + ? fixer.replaceTextRange(change.range, change.replacement) + : fixer.removeRange(change.range), ), }, ], From cbd47985bf69781615601a109dba18d1dc064f7c Mon Sep 17 00:00:00 2001 From: reduckted Date: Fri, 16 Aug 2024 22:23:07 +1000 Subject: [PATCH 13/14] Change typeArguments null check. --- packages/eslint-plugin/src/rules/require-await.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/require-await.ts b/packages/eslint-plugin/src/rules/require-await.ts index 72061632abb1..b8d6db487b5a 100644 --- a/packages/eslint-plugin/src/rules/require-await.ts +++ b/packages/eslint-plugin/src/rules/require-await.ts @@ -146,7 +146,7 @@ export default createRule({ } } else if ( hasTypeName(node.returnType.typeAnnotation, 'Promise') && - !!node.returnType.typeAnnotation.typeArguments + node.returnType.typeAnnotation.typeArguments != null ) { const openAngle = nullThrows( context.sourceCode.getFirstToken( From 3a7065f9b73e1adec4261bc92be9cc648951e39b Mon Sep 17 00:00:00 2001 From: reduckted Date: Fri, 16 Aug 2024 22:29:05 +1000 Subject: [PATCH 14/14] Replaced incorrect type guard. --- packages/eslint-plugin/src/rules/require-await.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/eslint-plugin/src/rules/require-await.ts b/packages/eslint-plugin/src/rules/require-await.ts index b8d6db487b5a..e08b4a895a43 100644 --- a/packages/eslint-plugin/src/rules/require-await.ts +++ b/packages/eslint-plugin/src/rules/require-await.ts @@ -136,7 +136,10 @@ export default createRule({ // Alternatively, if the function is a generator and // the return type annotation is `AsyncGenerator`, // then we can change it to `Generator`. - if (node.returnType) { + if ( + node.returnType?.typeAnnotation.type === + AST_NODE_TYPES.TSTypeReference + ) { if (scopeInfo.isGen) { if (hasTypeName(node.returnType.typeAnnotation, 'AsyncGenerator')) { changes.push({ @@ -320,12 +323,11 @@ function expandUnionOrIntersectionType(type: ts.Type): ts.Type[] { } function hasTypeName( - typeAnnotation: TSESTree.TypeNode, + typeReference: TSESTree.TSTypeReference, typeName: string, -): typeAnnotation is TSESTree.TSTypeReference { +): boolean { return ( - typeAnnotation.type === AST_NODE_TYPES.TSTypeReference && - typeAnnotation.typeName.type === AST_NODE_TYPES.Identifier && - typeAnnotation.typeName.name === typeName + typeReference.typeName.type === AST_NODE_TYPES.Identifier && + typeReference.typeName.name === typeName ); }