Skip to content

Commit

Permalink
feat(no-throw-statements)!: replace option allowInAsyncFunctions wi…
Browse files Browse the repository at this point in the history
…th `allowToRejectPromises`
  • Loading branch information
RebeccaStevens committed Aug 2, 2024
1 parent e71976f commit 70176b1
Show file tree
Hide file tree
Showing 7 changed files with 186 additions and 20 deletions.
12 changes: 6 additions & 6 deletions docs/rules/no-throw-statements.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,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,
};
```

Expand All @@ -76,19 +76,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]);
Expand Down
2 changes: 1 addition & 1 deletion docs/rules/prefer-immutable-types.md
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ acceptsCallback((options: CallbackOptions) => {});

```ts
export interface CallbackOptions {
prop: string;
readonly prop: string;
}
type Callback = (options: CallbackOptions) => void;
type AcceptsCallback = (callback: Callback) => void;
Expand Down
10 changes: 6 additions & 4 deletions eslint.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,12 @@ export default rsEslint(
},
},
formatters: true,
functional: "lite",
functional: {
functionalEnforcement: "lite",
overrides: {
"functional/no-throw-statements": "off",
},
},
jsonc: true,
markdown: {
enableTypeRequiredRules: true,
Expand All @@ -56,9 +61,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",
},
},
{
Expand Down
2 changes: 1 addition & 1 deletion src/configs/recommended.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ const overrides = {
[noThrowStatements.fullName]: [
"error",
{
allowInAsyncFunctions: true,
allowToRejectPromises: true,
},
],
[noTryStatements.fullName]: "off",
Expand Down
36 changes: 30 additions & 6 deletions src/rules/no-throw-statements.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -26,7 +30,7 @@ export const fullName: `${typeof ruleNameScope}/${typeof name}` = `${ruleNameSco
*/
type Options = [
{
allowInAsyncFunctions: boolean;
allowToRejectPromises: boolean;
},
];

Expand All @@ -37,7 +41,7 @@ const schema: JSONSchema4[] = [
{
type: "object",
properties: {
allowInAsyncFunctions: {
allowToRejectPromises: {
type: "boolean",
},
},
Expand All @@ -50,7 +54,7 @@ const schema: JSONSchema4[] = [
*/
const defaultOptions: Options = [
{
allowInAsyncFunctions: false,
allowToRejectPromises: false,
},
];

Expand Down Expand Up @@ -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" }] };
}

Expand Down
27 changes: 26 additions & 1 deletion src/utils/tree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,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,
Expand All @@ -18,6 +18,7 @@ import {
isMethodDefinition,
isObjectExpression,
isProgram,
isPromiseType,
isProperty,
isTSInterfaceBody,
isTSInterfaceHeritage,
Expand Down Expand Up @@ -125,6 +126,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<{...}>`.
*/
Expand Down
117 changes: 116 additions & 1 deletion tests/rules/no-throw-statements.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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 }],
});
});
});
});
});
});

0 comments on commit 70176b1

Please sign in to comment.