feat(eslint-plugin): [await-thenable] report unnecessary await using statements (#10209)

kirkwaiblinger · JoshuaKGoldberg · web-flow · commit 5b2ebcdea901 · 2024-11-04T12:27:51.000-05:00
* [await-thenable] report unnecessary await using statements

* continue, not return

* await using without init

* disable lint error

* exempt code ticks from upper-case heading test

Co-authored-by: Josh Goldberg ✨ &lt;git@joshuakgoldberg.com&gt;

---------

Co-authored-by: Josh Goldberg ✨ &lt;git@joshuakgoldberg.com&gt;
diff --git a/packages/eslint-plugin/docs/rules/await-thenable.mdx b/packages/eslint-plugin/docs/rules/await-thenable.mdx
@@ -49,7 +49,7 @@ This rule also inspects [`for await...of` statements](https://developer.mozilla.
 While `for await...of` can be used with synchronous iterables, and it will await each promise produced by the iterable, it is inadvisable to do so.
 There are some tiny nuances that you may want to consider.
 
-The biggest difference between using `for await...of` and using `for...of` (plus awaiting each result yourself) is error handling.
+The biggest difference between using `for await...of` and using `for...of` (apart from awaiting each result yourself) is error handling.
 When an error occurs within the loop body, `for await...of` does _not_ close the original sync iterable, while `for...of` does.
 For detailed examples of this, see the [MDN documentation on using `for await...of` with sync-iterables](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for-await...of#iterating_over_sync_iterables_and_generators).
 
@@ -120,6 +120,62 @@ async function validUseOfForAwaitOnAsyncIterable() {
 </TabItem>
 </Tabs>
 
+## Explicit Resource Management (`await using` Statements)
+
+This rule also inspects [`await using` statements](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-5-2.html#using-declarations-and-explicit-resource-management).
+If the disposable being used is not async-disposable, an `await using` statement is unnecessary.
+
+### Examples
+
+<Tabs>
+<TabItem value="❌ Incorrect">
+
+```ts
+function makeSyncDisposable(): Disposable {
+  return {
+    [Symbol.dispose](): void {
+      // Dispose of the resource
+    },
+  };
+}
+
+async function shouldNotAwait() {
+  await using resource = makeSyncDisposable();
+}
+```
+
+</TabItem>
+<TabItem value="✅ Correct">
+
+```ts
+function makeSyncDisposable(): Disposable {
+  return {
+    [Symbol.dispose](): void {
+      // Dispose of the resource
+    },
+  };
+}
+
+async function shouldNotAwait() {
+  using resource = makeSyncDisposable();
+}
+
+function makeAsyncDisposable(): AsyncDisposable {
+  return {
+    async [Symbol.asyncDispose](): Promise<void> {
+      // Dispose of the resource asynchronously
+    },
+  };
+}
+
+async function shouldAwait() {
+  await using resource = makeAsyncDisposable();
+}
+```
+
+</TabItem>
+</Tabs>
+
 ## When Not To Use It
 
 If you want to allow code to `await` non-Promise values.
diff --git a/packages/eslint-plugin/src/rules/await-thenable.ts b/packages/eslint-plugin/src/rules/await-thenable.ts
@@ -4,6 +4,7 @@ import * as tsutils from 'ts-api-utils';
 
 import {
   createRule,
+  getFixOrSuggest,
   getParserServices,
   isAwaitKeyword,
   isTypeAnyType,
@@ -15,8 +16,9 @@ import { getForStatementHeadLoc } from '../util/getForStatementHeadLoc';
 
 type MessageId =
   | 'await'
+  | 'awaitUsingOfNonAsyncDisposable'
   | 'convertToOrdinaryFor'
-  | 'forAwaitOfNonThenable'
+  | 'forAwaitOfNonAsyncIterable'
   | 'removeAwait';
 
 export default createRule<[], MessageId>({
@@ -31,8 +33,10 @@ export default createRule<[], MessageId>({
     hasSuggestions: true,
     messages: {
       await: 'Unexpected `await` of a non-Promise (non-"Thenable") value.',
+      awaitUsingOfNonAsyncDisposable:
+        'Unexpected `await using` of a value that is not async disposable.',
       convertToOrdinaryFor: 'Convert to an ordinary `for...of` loop.',
-      forAwaitOfNonThenable:
+      forAwaitOfNonAsyncIterable:
         'Unexpected `for await...of` of a value that is not async iterable.',
       removeAwait: 'Remove unnecessary `await`.',
     },
@@ -80,21 +84,21 @@ export default createRule<[], MessageId>({
           return;
         }
 
-        const asyncIteratorSymbol = tsutils
+        const hasAsyncIteratorSymbol = tsutils
           .unionTypeParts(type)
-          .map(t =>
-            tsutils.getWellKnownSymbolPropertyOfType(
-              t,
-              'asyncIterator',
-              checker,
-            ),
-          )
-          .find(symbol => symbol != null);
-
-        if (asyncIteratorSymbol == null) {
+          .some(
+            typePart =>
+              tsutils.getWellKnownSymbolPropertyOfType(
+                typePart,
+                'asyncIterator',
+                checker,
+              ) != null,
+          );
+
+        if (!hasAsyncIteratorSymbol) {
           context.report({
             loc: getForStatementHeadLoc(context.sourceCode, node),
-            messageId: 'forAwaitOfNonThenable',
+            messageId: 'forAwaitOfNonAsyncIterable',
             suggest: [
               // Note that this suggestion causes broken code for sync iterables
               // of promises, since the loop variable is not awaited.
@@ -112,6 +116,57 @@ export default createRule<[], MessageId>({
           });
         }
       },
+
+      'VariableDeclaration[kind="await using"]'(
+        node: TSESTree.VariableDeclaration,
+      ): void {
+        for (const declarator of node.declarations) {
+          const init = declarator.init;
+          if (init == null) {
+            continue;
+          }
+          const type = services.getTypeAtLocation(init);
+          if (isTypeAnyType(type)) {
+            continue;
+          }
+
+          const hasAsyncDisposeSymbol = tsutils
+            .unionTypeParts(type)
+            .some(
+              typePart =>
+                tsutils.getWellKnownSymbolPropertyOfType(
+                  typePart,
+                  'asyncDispose',
+                  checker,
+                ) != null,
+            );
+
+          if (!hasAsyncDisposeSymbol) {
+            context.report({
+              node: init,
+              messageId: 'awaitUsingOfNonAsyncDisposable',
+              // let the user figure out what to do if there's
+              // await using a = b, c = d, e = f;
+              // it's rare and not worth the complexity to handle.
+              ...getFixOrSuggest({
+                fixOrSuggest:
+                  node.declarations.length === 1 ? 'suggest' : 'none',
+
+                suggestion: {
+                  messageId: 'removeAwait',
+                  fix(fixer): TSESLint.RuleFix {
+                    const awaitToken = nullThrows(
+                      context.sourceCode.getFirstToken(node, isAwaitKeyword),
+                      NullThrowsReasons.MissingToken('await', 'await using'),
+                    );
+                    return fixer.remove(awaitToken);
+                  },
+                },
+              }),
+            });
+          }
+        }
+      },
     };
   },
 });
diff --git a/packages/eslint-plugin/src/rules/prefer-optional-chain-utils/analyzeChain.ts b/packages/eslint-plugin/src/rules/prefer-optional-chain-utils/analyzeChain.ts
@@ -413,11 +413,11 @@ function getReportDescriptor(
     },
     messageId: 'preferOptionalChain',
     ...getFixOrSuggest({
+      fixOrSuggest: useSuggestionFixer ? 'suggest' : 'fix',
       suggestion: {
         fix,
         messageId: 'optionalChainSuggest',
       },
-      useFix: !useSuggestionFixer,
     }),
   };
 
diff --git a/packages/eslint-plugin/src/rules/return-await.ts b/packages/eslint-plugin/src/rules/return-await.ts
@@ -6,6 +6,7 @@ import * as ts from 'typescript';
 
 import {
   createRule,
+  getFixOrSuggest,
   getParserServices,
   isAwaitExpression,
   isAwaitKeyword,
@@ -44,6 +45,7 @@ export default createRule({
       requiresTypeChecking: true,
     },
     fixable: 'code',
+    // eslint-disable-next-line eslint-plugin/require-meta-has-suggestions -- suggestions are exposed through a helper.
     hasSuggestions: true,
     messages: {
       disallowedPromiseAwait:
@@ -340,14 +342,17 @@ export default createRule({
             context.report({
               node,
               messageId: 'requiredPromiseAwait',
-              ...fixOrSuggest(useAutoFix, {
-                messageId: 'requiredPromiseAwaitSuggestion',
-                fix: fixer =>
-                  insertAwait(
-                    fixer,
-                    node,
-                    isHigherPrecedenceThanAwait(expression),
-                  ),
+              ...getFixOrSuggest({
+                fixOrSuggest: useAutoFix ? 'fix' : 'suggest',
+                suggestion: {
+                  messageId: 'requiredPromiseAwaitSuggestion',
+                  fix: fixer =>
+                    insertAwait(
+                      fixer,
+                      node,
+                      isHigherPrecedenceThanAwait(expression),
+                    ),
+                },
               }),
             });
           }
@@ -359,9 +364,12 @@ export default createRule({
             context.report({
               node,
               messageId: 'disallowedPromiseAwait',
-              ...fixOrSuggest(useAutoFix, {
-                messageId: 'disallowedPromiseAwaitSuggestion',
-                fix: fixer => removeAwait(fixer, node),
+              ...getFixOrSuggest({
+                fixOrSuggest: useAutoFix ? 'fix' : 'suggest',
+                suggestion: {
+                  messageId: 'disallowedPromiseAwaitSuggestion',
+                  fix: fixer => removeAwait(fixer, node),
+                },
               }),
             });
           }
@@ -446,12 +454,3 @@ function getConfiguration(option: Option): RuleConfiguration {
       };
   }
 }
-
-function fixOrSuggest<MessageId extends string>(
-  useFix: boolean,
-  suggestion: TSESLint.SuggestionReportDescriptor<MessageId>,
-):
-  | { fix: TSESLint.ReportFixFunction }
-  | { suggest: TSESLint.SuggestionReportDescriptor<MessageId>[] } {
-  return useFix ? { fix: suggestion.fix } : { suggest: [suggestion] };
-}
diff --git a/packages/eslint-plugin/src/util/getFixOrSuggest.ts b/packages/eslint-plugin/src/util/getFixOrSuggest.ts
@@ -1,13 +1,21 @@
 import type { TSESLint } from '@typescript-eslint/utils';
 
 export function getFixOrSuggest<MessageId extends string>({
+  fixOrSuggest,
   suggestion,
-  useFix,
 }: {
+  fixOrSuggest: 'fix' | 'none' | 'suggest';
   suggestion: TSESLint.SuggestionReportDescriptor<MessageId>;
-  useFix: boolean;
 }):
   | { fix: TSESLint.ReportFixFunction }
-  | { suggest: TSESLint.SuggestionReportDescriptor<MessageId>[] } {
-  return useFix ? { fix: suggestion.fix } : { suggest: [suggestion] };
+  | { suggest: TSESLint.SuggestionReportDescriptor<MessageId>[] }
+  | undefined {
+  switch (fixOrSuggest) {
+    case 'fix':
+      return { fix: suggestion.fix };
+    case 'none':
+      return undefined;
+    case 'suggest':
+      return { suggest: [suggestion] };
+  }
 }
diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/await-thenable.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/await-thenable.shot
diff --git a/packages/eslint-plugin/tests/docs.test.ts b/packages/eslint-plugin/tests/docs.test.ts
@@ -252,9 +252,10 @@ describe('Validating rule docs', () => {
         // Get all H2 headings objects as the other levels are variable by design.
         const headings = tokens.filter(tokenIsH2);
 
-        headings.forEach(heading =>
-          expect(heading.text).toBe(titleCase(heading.text)),
-        );
+        headings.forEach(heading => {
+          const nonCodeText = heading.text.replace(/`[^`]*`/g, '');
+          expect(nonCodeText).toBe(titleCase(nonCodeText));
+        });
       });
 
       const headings = tokens.filter(tokenIsHeading);
diff --git a/packages/eslint-plugin/tests/rules/await-thenable.test.ts b/packages/eslint-plugin/tests/rules/await-thenable.test.ts
