feat(no-throw-statements)!: replace option allowInAsyncFunctions with allowToRejectPromises (#839)

fix #838

Author: RebeccaStevens

docs/rules/no-throw-statements.md

@@ -28,8 +28,6 @@ throw new Error("Something went wrong.");
 
 ### ✅ Correct
 
-<!-- eslint-skip -->
-
 ```js
 /* eslint functional/no-throw-statements: "error" */
 
@@ -38,8 +36,6 @@ function divide(x, y) {
 }
 ```
 
-<!-- eslint-skip -->
-
 ```js
 /* eslint functional/no-throw-statements: "error" */
 
@@ -58,15 +54,15 @@ This rule accepts an options object of the following type:
 
 ```ts
 type Options = {
-  allowInAsyncFunctions: boolean;
+  allowToRejectPromises: boolean;
 };
 ```
 
 ### Default Options
 
 ```ts
 const defaults = {
-  allowInAsyncFunctions: false,
+  allowToRejectPromises: false,
 };
 ```
 
@@ -76,19 +72,19 @@ const defaults = {
 
 ```ts
 const recommendedAndLiteOptions = {
-  allowInAsyncFunctions: true,
+  allowToRejectPromises: true,
 };
 ```
 
-### `allowInAsyncFunctions`
+### `allowToRejectPromises`
 
-If true, throw statements will be allowed within async functions.\
+If true, throw statements will be allowed when they are used to reject a promise, such when in an async function.\
 This essentially allows throw statements to be used as return statements for errors.
 
 #### ✅ Correct
 
 ```js
-/* eslint functional/no-throw-statements: ["error", { "allowInAsyncFunctions": true }] */
+/* eslint functional/no-throw-statements: ["error", { "allowToRejectPromises": true }] */
 
 async function divide(x, y) {
   const [xv, yv] = await Promise.all([x, y]);

docs/rules/prefer-immutable-types.md

@@ -425,7 +425,7 @@ acceptsCallback((options: CallbackOptions) => {});
 
 ```ts
 export interface CallbackOptions {
-  prop: string;
+  readonly prop: string;
 }
 type Callback = (options: CallbackOptions) => void;
 type AcceptsCallback = (callback: Callback) => void;

eslint.config.js

@@ -30,7 +30,12 @@ export default rsEslint(
       },
     },
     formatters: true,
-    functional: "lite",
+    functional: {
+      functionalEnforcement: "lite",
+      overrides: {
+        "functional/no-throw-statements": "off",
+      },
+    },
     jsonc: true,
     markdown: {
       enableTypeRequiredRules: true,
@@ -50,9 +55,6 @@ export default rsEslint(
     rules: {
       // Some types say they have nonnullable properties, but they don't always.
       "ts/no-unnecessary-condition": "off",
-
-      // Temp
-      "functional/no-throw-statements": "off",
     },
   },
   {

src/configs/recommended.ts

@@ -66,7 +66,7 @@ const overrides = {
   [noThrowStatements.fullName]: [
     "error",
     {
-      allowInAsyncFunctions: true,
+      allowToRejectPromises: true,
     },
   ],
   [noTryStatements.fullName]: "off",

src/rules/no-throw-statements.ts

@@ -9,7 +9,11 @@ import {
   type RuleResult,
   createRule,
 } from "#/utils/rule";
-import { isInFunctionBody } from "#/utils/tree";
+import {
+  getEnclosingFunction,
+  getEnclosingTryStatement,
+  isInPromiseHandlerFunction,
+} from "#/utils/tree";
 
 /**
  * The name of this rule.
@@ -26,7 +30,7 @@ export const fullName: `${typeof ruleNameScope}/${typeof name}` = `${ruleNameSco
  */
 type Options = [
   {
-    allowInAsyncFunctions: boolean;
+    allowToRejectPromises: boolean;
   },
 ];
 
@@ -37,7 +41,7 @@ const schema: JSONSchema4[] = [
   {
     type: "object",
     properties: {
-      allowInAsyncFunctions: {
+      allowToRejectPromises: {
         type: "boolean",
       },
     },
@@ -50,7 +54,7 @@ const schema: JSONSchema4[] = [
  */
 const defaultOptions: Options = [
   {
-    allowInAsyncFunctions: false,
+    allowToRejectPromises: false,
   },
 ];
 
@@ -85,9 +89,29 @@ function checkThrowStatement(
   context: Readonly<RuleContext<keyof typeof errorMessages, Options>>,
   options: Readonly<Options>,
 ): RuleResult<keyof typeof errorMessages, Options> {
-  const [{ allowInAsyncFunctions }] = options;
+  const [{ allowToRejectPromises }] = options;
+
+  if (!allowToRejectPromises) {
+    return { context, descriptors: [{ node, messageId: "generic" }] };
+  }
+
+  if (isInPromiseHandlerFunction(node, context)) {
+    return { context, descriptors: [] };
+  }
+
+  const enclosingFunction = getEnclosingFunction(node);
+  if (enclosingFunction?.async !== true) {
+    return { context, descriptors: [{ node, messageId: "generic" }] };
+  }
 
-  if (!allowInAsyncFunctions || !isInFunctionBody(node, true)) {
+  const enclosingTryStatement = getEnclosingTryStatement(node);
+  if (
+    !(
+      enclosingTryStatement === null ||
+      getEnclosingFunction(enclosingTryStatement) !== enclosingFunction ||
+      enclosingTryStatement.handler === null
+    )
+  ) {
     return { context, descriptors: [{ node, messageId: "generic" }] };
   }
 

src/utils/tree.ts

@@ -6,7 +6,7 @@ import type { RuleContext } from "@typescript-eslint/utils/ts-eslint";
 
 import typescript from "#/conditional-imports/typescript";
 
-import type { BaseOptions } from "./rule";
+import { type BaseOptions, getTypeOfNode } from "./rule";
 import {
   isBlockStatement,
   isCallExpression,
@@ -20,6 +20,7 @@ import {
   isMethodDefinition,
   isObjectExpression,
   isProgram,
+  isPromiseType,
   isProperty,
   isTSInterfaceBody,
   isTSInterfaceHeritage,
@@ -127,6 +128,30 @@ export function isInReadonly(node: TSESTree.Node): boolean {
   return getReadonly(node) !== null;
 }
 
+/**
+ * Test if the given node is in a handler function callback of a promise.
+ */
+export function isInPromiseHandlerFunction<
+  Context extends RuleContext<string, BaseOptions>,
+>(node: TSESTree.Node, context: Context): boolean {
+  const functionNode = getAncestorOfType(
+    (n, c): n is TSESTree.FunctionLike => isFunctionLike(n) && n.body === c,
+    node,
+  );
+
+  if (
+    functionNode === null ||
+    !isCallExpression(functionNode.parent) ||
+    !isMemberExpression(functionNode.parent.callee) ||
+    !isIdentifier(functionNode.parent.callee.property)
+  ) {
+    return false;
+  }
+
+  const objectType = getTypeOfNode(functionNode.parent.callee.object, context);
+  return isPromiseType(objectType);
+}
+
 /**
  * Test if the given node is shallowly inside a `Readonly<{...}>`.
  */

tests/rules/no-throw-statements.test.ts

@@ -4,7 +4,7 @@ import { describe, expect, it } from "vitest";
 
 import { name, rule } from "#/rules/no-throw-statements";
 
-import { esLatestConfig } from "../utils/configs";
+import { esLatestConfig, typescriptConfig } from "../utils/configs";
 
 describe(name, () => {
   describe("javascript - es latest", () => {
@@ -57,5 +57,120 @@ describe(name, () => {
       });
       expect(invalidResult.messages).toMatchSnapshot();
     });
+
+    describe("options", () => {
+      describe("allowToRejectPromises", () => {
+        it("doesn't report throw statements in async functions", () => {
+          valid({
+            code: dedent`
+              async function foo() {
+                throw new Error();
+              }
+            `,
+            options: [{ allowToRejectPromises: true }],
+          });
+        });
+
+        it("doesn't report throw statements in try without catch in async functions", () => {
+          valid({
+            code: dedent`
+              async function foo() {
+                try {
+                  throw new Error("hello");
+                } finally {
+                  console.log("world");
+                }
+              }
+            `,
+            options: [{ allowToRejectPromises: true }],
+          });
+        });
+
+        it("reports throw statements in try with catch in async functions", () => {
+          const invalidResult = invalid({
+            code: dedent`
+              async function foo() {
+                try {
+                  throw new Error("hello world");
+                } catch (e) {
+                  console.log(e);
+                }
+              }
+            `,
+            errors: ["generic"],
+            options: [{ allowToRejectPromises: true }],
+          });
+          expect(invalidResult.messages).toMatchSnapshot();
+        });
+
+        it("reports throw statements in functions nested in async functions", () => {
+          const invalidResult = invalid({
+            code: dedent`
+              async function foo() {
+                function bar() {
+                  throw new Error();
+                }
+              }
+            `,
+            errors: ["generic"],
+            options: [{ allowToRejectPromises: true }],
+          });
+          expect(invalidResult.messages).toMatchSnapshot();
+        });
+      });
+    });
+  });
+
+  describe("typescript", () => {
+    const { valid, invalid } = createRuleTester({
+      name,
+      rule,
+      configs: typescriptConfig,
+    });
+
+    describe("options", () => {
+      describe("allowToRejectPromises", () => {
+        it("doesn't report throw statements in promise then handlers", () => {
+          valid({
+            code: dedent`
+              function foo() {
+                Promise.resolve().then(() => {
+                  throw new Error();
+                });
+              }
+            `,
+            options: [{ allowToRejectPromises: true }],
+          });
+        });
+
+        it("doesn't report throw statements in promise catch handlers", () => {
+          valid({
+            code: dedent`
+              function foo() {
+                Promise.resolve().catch(() => {
+                  throw new Error();
+                });
+              }
+            `,
+            options: [{ allowToRejectPromises: true }],
+          });
+        });
+
+        it("doesn't report throw statements in promise handlers", () => {
+          valid({
+            code: dedent`
+              function foo() {
+                Promise.resolve().then(() => {
+                  throw new Error();
+                }, () => {
+                  throw new Error();
+                });
+              }
+            `,
+            options: [{ allowToRejectPromises: true }],
+          });
+        });
+      });
+    });
   });
 });