feat(eslint-plugin): add suggestion to require-await to remove async keyword (#9718)

reduckted · web-flow · commit 478990faf521 · 2024-08-19T07:18:14.000-04:00
* Add suggestion to require-await to remove async keyword Fixes #9621 * Include suggestion when there is a return type annotation. * Restructure suggestion range calculation. * Suggestion will replace return type of `Promise<T>` with `T`. * use noFormat instead of suppressing lint errors. * Remove unnecessary ecmaVersion from tests and correct test that had auto-formatting applied. * Corrected comment. * Updated the copy of ESLint test cases. * Added better support for async generators. * Used nullThrows to avoid unnecessary conditionals. * Added additional changes to end of array instead of inserting at the start. * Use removeRange instead of replaceTextRange with empty replacement. * Change typeArguments null check. * Replaced incorrect type guard.
diff --git a/packages/eslint-plugin/src/rules/require-await.ts b/packages/eslint-plugin/src/rules/require-await.ts
@@ -1,5 +1,6 @@
 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, RuleFix } from '@typescript-eslint/utils/ts-eslint';
 import * as tsutils from 'ts-api-utils';
 import type * as ts from 'typescript';
 
@@ -8,6 +9,9 @@ import {
   getFunctionHeadLoc,
   getFunctionNameWithKind,
   getParserServices,
+  isStartOfExpressionStatement,
+  needsPrecedingSemicolon,
+  nullThrows,
   upperCaseFirst,
 } from '../util';
 
@@ -37,7 +41,9 @@ export default createRule({
     schema: [],
     messages: {
       missingAwait: "{{name}} has no 'await' expression.",
+      removeAsync: "Remove 'async'.",
     },
+    hasSuggestions: true,
   },
   defaultOptions: [],
   create(context) {
@@ -75,13 +81,128 @@ export default createRule({
         !isEmptyFunction(node) &&
         !(scopeInfo.isGen && scopeInfo.isAsyncYield)
       ) {
+        // 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 = nullThrows(
+          context.sourceCode.getFirstToken(
+            nodeWithAsyncKeyword,
+            token => token.value === 'async',
+          ),
+          'The node is an async function, so it must have an "async" token.',
+        );
+
+        const asyncRange: Readonly<AST.Range> = [
+          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 = 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 ? ';' : undefined },
+        ];
+
+        // If there's a return type annotation and it's a
+        // `Promise<T>`, 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<T>`,
+        // then we can change it to `Generator<T>`.
+        if (
+          node.returnType?.typeAnnotation.type ===
+          AST_NODE_TYPES.TSTypeReference
+        ) {
+          if (scopeInfo.isGen) {
+            if (hasTypeName(node.returnType.typeAnnotation, 'AsyncGenerator')) {
+              changes.push({
+                range: node.returnType.typeAnnotation.typeName.range,
+                replacement: 'Generator',
+              });
+            }
+          } else if (
+            hasTypeName(node.returnType.typeAnnotation, 'Promise') &&
+            node.returnType.typeAnnotation.typeArguments != null
+          ) {
+            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.push(
+              // Remove the closing angled bracket.
+              { range: closeAngle.range, replacement: undefined },
+              // Remove the "Promise" identifier
+              // and the opening angled bracket.
+              {
+                range: [
+                  node.returnType.typeAnnotation.typeName.range[0],
+                  openAngle.range[1],
+                ],
+                replacement: undefined,
+              },
+            );
+          }
+        }
+
         context.report({
           node,
           loc: getFunctionHeadLoc(node, context.sourceCode),
           messageId: 'missingAwait',
           data: {
             name: upperCaseFirst(getFunctionNameWithKind(node)),
           },
+          suggest: [
+            {
+              messageId: 'removeAsync',
+              fix: (fixer): RuleFix[] =>
+                changes.map(change =>
+                  change.replacement !== undefined
+                    ? fixer.replaceTextRange(change.range, change.replacement)
+                    : fixer.removeRange(change.range),
+                ),
+            },
+          ],
         });
       }
 
@@ -200,3 +321,13 @@ function expandUnionOrIntersectionType(type: ts.Type): ts.Type[] {
   }
   return [type];
 }
+
+function hasTypeName(
+  typeReference: TSESTree.TSTypeReference,
+  typeName: string,
+): boolean {
+  return (
+    typeReference.typeName.type === AST_NODE_TYPES.Identifier &&
+    typeReference.typeName.name === typeName
+  );
+}
diff --git 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
@@ -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
@@ -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<string, TSESTree.AST_NODE_TYPES | null> = {
+  __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
