feat(eslint-plugin): add rule `use-unknown-in-catch-callback-variable…

…s` (#8383) * Add rule use-unknown-in-catch-callback-variables * delete PR comment reminders * address code coverage gaps * fix spell checker * fix broken link * little bit of simplifying * simplify arrow function parentheses check * Your -> The Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com> * Clean up some comments and simplify a bit * add to strict config * Add examples to docs * Improve handling of destructuring parameters * tweaks * docs changes * Improve wording for "when not to use this" Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com> * improve wording and fix mistake * remove dead return * address istanbul-ignore * add bizarre syntax test cases * Improve main error message wording Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com> * utility for rest parameter * tweaks * fix casing * restore older versions of configs * config update * fix internal violations * Regenerate configs --------- Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com>
typescript-eslint · Mar 17, 2024 · 6d68020 · 6d68020
1 parent ce79c55
commit 6d68020
Show file tree

Hide file tree

Showing 24 changed files with 1,235 additions and 13 deletions.
diff --git a/packages/eslint-plugin/docs/rules/use-unknown-in-catch-callback-variable.md b/packages/eslint-plugin/docs/rules/use-unknown-in-catch-callback-variable.md
@@ -0,0 +1,80 @@
+---
+description: 'Enforce typing arguments in `.catch()` callbacks as `unknown`.'
+---
+
+> 🛑 This file is source code, not the primary documentation location! 🛑
+>
+> See **https://typescript-eslint.io/rules/use-unknown-in-catch-callback-variable** for documentation.
+
+This rule enforces that you always use the `unknown` type for the parameter of a `Promise.prototype.catch()` callback.
+
+<!--tabs-->
+
+### ❌ Incorrect
+
+```ts
+Promise.reject(new Error('I will reject!')).catch(err => {
+  console.log(err);
+});
+
+Promise.reject(new Error('I will reject!')).catch((err: any) => {
+  console.log(err);
+});
+
+Promise.reject(new Error('I will reject!')).catch((err: Error) => {
+  console.log(err);
+});
+```
+
+### ✅ Correct
+
+```ts
+Promise.reject(new Error('I will reject!')).catch((err: unknown) => {
+  console.log(err);
+});
+```
+
+<!--/tabs-->
+
+The reason for this rule is to enable programmers to impose constraints on `Promise` error handling analogously to what TypeScript provides for ordinary exception handling.
+
+For ordinary exceptions, TypeScript treats the `catch` variable as `any` by default. However, `unknown` would be a more accurate type, so TypeScript [introduced the `useUnknownInCatchVariables` compiler option](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-4.html#defaulting-to-the-unknown-type-in-catch-variables---useunknownincatchvariables) to treat the `catch` variable as `unknown` instead.
+
+```ts
+try {
+  throw x;
+} catch (err) {
+  // err has type 'any' with useUnknownInCatchVariables: false
+  // err has type 'unknown' with useUnknownInCatchVariables: true
+}
+```
+
+The Promise analog of the `try-catch` block, [`Promise.prototype.catch()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/catch), is not affected by the `useUnknownInCatchVariables` compiler option, and its "`catch` variable" will always have the type `any`.
+
+```ts
+Promise.reject(x).catch(err => {
+  // err has type 'any' regardless of `useUnknownInCatchVariables`
+});
+```
+
+However, you can still provide an explicit type annotation, which lets you achieve the same effect as the `useUnknownInCatchVariables` option does for synchronous `catch` variables.
+
+```ts
+Promise.reject(x).catch((err: unknown) => {
+  // err has type 'unknown'
+});
+```
+
+:::info
+There is actually a way to have the `catch()` callback variable use the `unknown` type _without_ an explicit type annotation at the call sites, but it has the drawback that it involves overriding global type declarations.
+For example, the library [better-TypeScript-lib](https://github.com/uhyo/better-typescript-lib) sets this up globally for your project (see [the relevant lines in the better-TypeScript-lib source code](https://github.com/uhyo/better-typescript-lib/blob/c294e177d1cc2b1d1803febf8192a4c83a1fe028/lib/lib.es5.d.ts#L635) for details on how).
+
+For further reading on this, you may also want to look into
+[the discussion in the proposal for this rule](https://github.com/typescript-eslint/typescript-eslint/issues/7526#issuecomment-1690600813) and [this TypeScript issue on typing catch callback variables as unknown](https://github.com/microsoft/TypeScript/issues/45602).
+:::
+
+## When Not To Use It
+
+If your codebase is not yet able to enable `useUnknownInCatchVariables`, it likely would be similarly difficult to enable this rule.
+
+If you have modified the global type declarations in order to make `catch()` callbacks use the `unknown` type without an explicit type annotation, you do not need this rule.
diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts
@@ -155,5 +155,6 @@ export = {
     '@typescript-eslint/typedef': 'error',
     '@typescript-eslint/unbound-method': 'error',
     '@typescript-eslint/unified-signatures': 'error',
+    '@typescript-eslint/use-unknown-in-catch-callback-variable': 'error',
   },
 } satisfies ClassicConfig.Config;
diff --git a/packages/eslint-plugin/src/configs/disable-type-checked.ts b/packages/eslint-plugin/src/configs/disable-type-checked.ts
@@ -62,5 +62,6 @@ export = {
     '@typescript-eslint/strict-boolean-expressions': 'off',
     '@typescript-eslint/switch-exhaustiveness-check': 'off',
     '@typescript-eslint/unbound-method': 'off',
+    '@typescript-eslint/use-unknown-in-catch-callback-variable': 'off',
   },
 } satisfies ClassicConfig.Config;
diff --git a/packages/eslint-plugin/src/configs/strict-type-checked-only.ts b/packages/eslint-plugin/src/configs/strict-type-checked-only.ts
@@ -65,5 +65,6 @@ export = {
       },
     ],
     '@typescript-eslint/unbound-method': 'error',
+    '@typescript-eslint/use-unknown-in-catch-callback-variable': 'error',
   },
 } satisfies ClassicConfig.Config;
diff --git a/packages/eslint-plugin/src/configs/strict-type-checked.ts b/packages/eslint-plugin/src/configs/strict-type-checked.ts
@@ -98,5 +98,6 @@ export = {
     '@typescript-eslint/triple-slash-reference': 'error',
     '@typescript-eslint/unbound-method': 'error',
     '@typescript-eslint/unified-signatures': 'error',
+    '@typescript-eslint/use-unknown-in-catch-callback-variable': 'error',
   },
 } satisfies ClassicConfig.Config;
diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts
@@ -141,6 +141,7 @@ import typeAnnotationSpacing from './type-annotation-spacing';
 import typedef from './typedef';
 import unboundMethod from './unbound-method';
 import unifiedSignatures from './unified-signatures';
+import useUnknownInCatchCallbackVariable from './use-unknown-in-catch-callback-variable';
 
 export default {
   'adjacent-overload-signatures': adjacentOverloadSignatures,
@@ -284,4 +285,5 @@ export default {
   typedef: typedef,
   'unbound-method': unboundMethod,
   'unified-signatures': unifiedSignatures,
+  'use-unknown-in-catch-callback-variable': useUnknownInCatchCallbackVariable,
 } satisfies Linter.PluginRules;
diff --git a/packages/eslint-plugin/src/rules/no-misused-promises.ts b/packages/eslint-plugin/src/rules/no-misused-promises.ts
@@ -3,7 +3,11 @@ import { AST_NODE_TYPES } from '@typescript-eslint/utils';
 import * as tsutils from 'ts-api-utils';
 import * as ts from 'typescript';
 
-import { createRule, getParserServices } from '../util';
+import {
+  createRule,
+  getParserServices,
+  isRestParameterDeclaration,
+} from '../util';
 
 type Options = [
   {
@@ -549,7 +553,7 @@ function voidFunctionArguments(
 
         // If this is a array 'rest' parameter, check all of the argument indices
         // from the current argument to the end.
-        if (decl && ts.isParameter(decl) && decl.dotDotDotToken) {
+        if (decl && isRestParameterDeclaration(decl)) {
           if (checker.isArrayType(type)) {
             // Unwrap 'Array<MaybeVoidFunction>' to 'MaybeVoidFunction',
             // so that we'll handle it in the same way as a non-rest

diff --git a/packages/eslint-plugin/src/rules/no-unsafe-argument.ts b/packages/eslint-plugin/src/rules/no-unsafe-argument.ts
@@ -1,10 +1,11 @@
 import type { TSESTree } from '@typescript-eslint/utils';
 import { AST_NODE_TYPES } from '@typescript-eslint/utils';
-import * as ts from 'typescript';
+import type * as ts from 'typescript';
 
 import {
   createRule,
   getParserServices,
+  isRestParameterDeclaration,
   isTypeAnyArrayType,
   isTypeAnyType,
   isUnsafeAssignment,
@@ -59,7 +60,7 @@ class FunctionSignature {
       const type = checker.getTypeOfSymbolAtLocation(param, tsNode);
 
       const decl = param.getDeclarations()?.[0];
-      if (decl && ts.isParameter(decl) && decl.dotDotDotToken) {
+      if (decl && isRestParameterDeclaration(decl)) {
         // is a rest param
         if (checker.isArrayType(type)) {
           restType = {