typescript-eslint · kirkwaiblinger · Jul 18, 2024 · Jun 15, 2024 · Jun 15, 2024 · Jun 18, 2024
diff --git a/packages/eslint-plugin/docs/rules/return-await.mdx b/packages/eslint-plugin/docs/rules/return-await.mdx
@@ -21,23 +21,43 @@ The extended rule is named `return-await` instead of `no-return-await` because t
 ## Options
 
 ```ts
-type Options = 'in-try-catch' | 'always' | 'never';
+type Options =
+  | 'in-try-catch'
+  | 'always'
+  | 'error-handling-correctness-only'
+  | 'never';
 
 const defaultOptions: Options = 'in-try-catch';
 ```
 
+The options in this rule distinguish between "ordinary contexts" and "error-handling contexts".
+An error-handling context is anywhere where returning an unawaited promise would cause unexpected control flow regarding exceptions/rejections.
+See detailed examples in the sections for each option.
+
+- If you return a promise within a `try` block, it should be awaited in order to trigger subsequent `catch` or `finally` blocks as expected.
+- If you return a promise within a `catch` block, and there _is_ a `finally` block, it should be awaited in order to trigger the `finally` block as expected.
+- If you return a promise between a `using` or `await using` declaration and the end of its scope, it should be awaited, since it behaves equivalently to code wrapped in a `try` block followed by a `finally`.
+
+Ordinary contexts are anywhere else a promise may be returned.
+The choice of whether to await a returned promise in an ordinary context is mostly stylistic.
+
+With these terms defined, the options may be summarized as follows:
+
+|              Option               | Ordinary Context <br/> (stylistic preference 🎨) |        Error-Handling Context <br/> (catches bugs 🐛)        |                 Should I use this option?                  |
+| :-------------------------------: | :----------------------------------------------: | :----------------------------------------------------------: | :--------------------------------------------------------: |
+|             `always`              |             `return await promise;`              |                   `return await promise;`                    |                          ✅ Yes!                           |
+|          `in-try-catch`           |                `return promise;`                 |                   `return await promise;`                    |                          ✅ Yes!                           |
+| `error-handling-correctness-only` |                  don't care 🤷                   |                   `return await promise;`                    | 🟡 Okay to use, but the above options would be preferable. |
+|              `never`              |                `return promise;`                 | `return promise;` <br/> (⚠️ This behavior may be harmful ⚠️) |             ❌ No. This option is deprecated.              |
+
 ### `in-try-catch`
 
-In cases where returning an unawaited promise would cause unexpected error-handling control flow, the rule enforces that `await` must be used.
-Otherwise, the rule enforces that `await` must _not_ be used.
+In error-handling contexts, the rule enforces that returned promises must be awaited.
+In ordinary contexts, the rule enforces that returned promises _must not_ be awaited.
 
-Listing the error-handling cases exhaustively:
+This is a good option if you prefer the shorter `return promise` form for stylistic reasons, wherever it's safe to use.
 
-- if you `return` a promise within a `try`, then it must be `await`ed, since it will always be followed by a `catch` or `finally`.
-- if you `return` a promise within a `catch`, and there is _no_ `finally`, then it must _not_ be `await`ed.
-- if you `return` a promise within a `catch`, and there _is_ a `finally`, then it _must_ be `await`ed.
-- if you `return` a promise within a `finally`, then it must not be `await`ed.
-- if you `return` a promise between a `using` or `await using` declaration and the end of its scope, it must be `await`ed, since it behaves equivalently to code wrapped in a `try` block.
+Examples of code with `in-try-catch`:
 
 <Tabs>
 <TabItem value="❌ Incorrect">
@@ -169,7 +189,9 @@ async function validInTryCatch7() {
 
 ### `always`
 
-Requires that all returned promises are `await`ed.
+Requires that all returned promises be awaited.
+
+This is a good option if you like the consistency of simply always awaiting promises, or prefer not having to consider the distinction between error-handling contexts and ordinary contexts.
 
 Examples of code with `always`:
 
@@ -214,9 +236,49 @@ async function validAlways3() {
 </TabItem>
 </Tabs>
 
+### `error-handling-correctness-only`
+
+In error-handling contexts, the rule enforces that returned promises must be awaited.
+In ordinary contexts, the rule does not enforce any particular behavior around whether returned promises are awaited.
+
+This is a good option if you only want to benefit from rule's ability to catch control flow bugs in error-handling contexts, but don't want to enforce a particular style otherwise.
+
+:::info
+We recommend you configure either `in-try-catch` or `always` instead of this option.
+While the choice of whether to await promises outside of error-handling contexts is mostly stylistic, it's generally best to be consistent.
+:::
+
+Examples of additional correct code with `error-handling-correctness-only`:
+
+<Tabs>
+<TabItem value="✅ Correct">
+
+```ts option='"error-handling-correctness-only"'
+async function asyncFunction(): Promise<void> {
+  if (Math.random() < 0.5) {
+    return await Promise.resolve();
+  } else {
+    return Promise.resolve();
+  }
+}
+```
+
+</TabItem>
+</Tabs>
+
 ### `never`
 
-Disallows all `await`ing any returned promises.
+Disallows awaiting any returned promises.
+
+:::warning
+
+This option is deprecated and will be removed in a future major version of typescript-eslint.
+
+The `never` option introduces undesirable behavior in error-handling contexts.
+If you prefer to minimize returning awaited promises, consider instead using `in-try-catch` instead, which also generally bans returning awaited promises, but only where it is _safe_ not to await a promise.
+
+See more details at [typescript-eslint#9433](https://github.com/typescript-eslint/typescript-eslint/issues/9433).
+:::
 
 Examples of code with `never`:
 

diff --git a/packages/eslint-plugin/src/rules/return-await.ts b/packages/eslint-plugin/src/rules/return-await.ts
@@ -24,6 +24,12 @@ interface ScopeInfo {
   owningFunc: FunctionNode;
 }
 
+type Option =
+  | 'in-try-catch'
+  | 'always'
+  | 'never'
+  | 'error-handling-correctness-only';
+
 export default createRule({
   name: 'return-await',
   meta: {
@@ -50,7 +56,12 @@ export default createRule({
     schema: [
       {
         type: 'string',
-        enum: ['in-try-catch', 'always', 'never'],
+        enum: [
+          'in-try-catch',
+          'always',
+          'never',
+          'error-handling-correctness-only',
+        ] satisfies Option[],
       },
     ],
   },
@@ -271,92 +282,70 @@ export default createRule({
       const type = checker.getTypeAtLocation(child);
       const isThenable = tsutils.isThenableType(checker, expression, type);
 
-      if (!isAwait && !isThenable) {
-        return;
-      }
-
-      if (isAwait && !isThenable) {
-        // any/unknown could be thenable; do not auto-fix
-        const useAutoFix = !(isTypeAnyType(type) || isTypeUnknownType(type));
-
-        context.report({
-          messageId: 'nonPromiseAwait',
-          node,
-          ...fixOrSuggest(useAutoFix, {
-            messageId: 'nonPromiseAwait',
-            fix: fixer => removeAwait(fixer, node),
-          }),
-        });
-        return;
-      }
+      // handle awaited _non_thenables
 
-      const affectsErrorHandling =
-        affectsExplicitErrorHandling(expression) ||
-        affectsExplicitResourceManagement(node);
-      const useAutoFix = !affectsErrorHandling;
+      if (!isThenable) {
+        if (isAwait) {
+          // any/unknown could be thenable; do not auto-fix
+          const useAutoFix = !(isTypeAnyType(type) || isTypeUnknownType(type));
 
-      if (option === 'always') {
-        if (!isAwait && isThenable) {
           context.report({
-            messageId: 'requiredPromiseAwait',
+            messageId: 'nonPromiseAwait',
             node,
             ...fixOrSuggest(useAutoFix, {
-              messageId: 'requiredPromiseAwaitSuggestion',
-              fix: fixer =>
-                insertAwait(
-                  fixer,
-                  node,
-                  isHigherPrecedenceThanAwait(expression),
-                ),
+              messageId: 'nonPromiseAwait',
+              fix: fixer => removeAwait(fixer, node),
             }),
           });
         }
-
         return;
       }
 
-      if (option === 'never') {
-        if (isAwait) {
-          context.report({
-            messageId: 'disallowedPromiseAwait',
-            node,
-            ...fixOrSuggest(useAutoFix, {
-              messageId: 'disallowedPromiseAwaitSuggestion',
-              fix: fixer => removeAwait(fixer, node),
-            }),
-          });
-        }
+      // At this point it's definitely a thenable.
 
-        return;
-      }
+      const affectsErrorHandling =
+        affectsExplicitErrorHandling(expression) ||
+        affectsExplicitResourceManagement(node);
+      const useAutoFix = !affectsErrorHandling;
 
-      if (option === 'in-try-catch') {
-        if (isAwait && !affectsErrorHandling) {
-          context.report({
-            messageId: 'disallowedPromiseAwait',
-            node,
-            ...fixOrSuggest(useAutoFix, {
-              messageId: 'disallowedPromiseAwaitSuggestion',
-              fix: fixer => removeAwait(fixer, node),
-            }),
-          });
-        } else if (!isAwait && affectsErrorHandling) {
-          context.report({
-            messageId: 'requiredPromiseAwait',
-            node,
-            ...fixOrSuggest(useAutoFix, {
-              messageId: 'requiredPromiseAwaitSuggestion',
-              fix: fixer =>
-                insertAwait(
-                  fixer,
-                  node,
-                  isHigherPrecedenceThanAwait(expression),
-                ),
-            }),
-          });
-        }
+      const ruleConfiguration = getConfiguration(option as Option);
 
-        return;
+      const shouldAwaitInCurrentContext = affectsErrorHandling
+        ? ruleConfiguration.errorHandlingContext
+        : ruleConfiguration.ordinaryContext;
+
+      switch (shouldAwaitInCurrentContext) {
+        case "don't-care":
+          break;
+        case 'await':
+          if (!isAwait) {
+            context.report({
+              messageId: 'requiredPromiseAwait',
+              node,
+              ...fixOrSuggest(useAutoFix, {
+                messageId: 'requiredPromiseAwaitSuggestion',
+                fix: fixer =>
+                  insertAwait(
+                    fixer,
+                    node,
+                    isHigherPrecedenceThanAwait(expression),
+                  ),
+              }),
+            });
+          }
+          break;
+        case 'no-await':
+          if (isAwait) {
+            context.report({
+              messageId: 'disallowedPromiseAwait',
+              node,
+              ...fixOrSuggest(useAutoFix, {
+                messageId: 'disallowedPromiseAwaitSuggestion',
+                fix: fixer => removeAwait(fixer, node),
+              }),
+            });
+          }
+          break;
       }
     }
 
@@ -406,6 +395,38 @@ export default createRule({
   },
 });
 
+type WhetherToAwait = 'await' | 'no-await' | "don't-care";
+
+interface RuleConfiguration {
+  ordinaryContext: WhetherToAwait;
+  errorHandlingContext: WhetherToAwait;
+}
+
+function getConfiguration(option: Option): RuleConfiguration {
+  switch (option) {
+    case 'always':
+      return {
+        ordinaryContext: 'await',
+        errorHandlingContext: 'await',
+      };
+    case 'never':
+      return {
+        ordinaryContext: 'no-await',
+        errorHandlingContext: 'no-await',
+      };
+    case 'error-handling-correctness-only':
+      return {
+        ordinaryContext: "don't-care",
+        errorHandlingContext: 'await',
+      };
+    case 'in-try-catch':
+      return {
+        ordinaryContext: 'no-await',
+        errorHandlingContext: 'await',
+      };
+  }
+}
+
 function fixOrSuggest<MessageId extends string>(
   useFix: boolean,
   suggestion: TSESLint.SuggestionReportDescriptor<MessageId>,

diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/return-await.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/return-await.shot