Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 6 additions & 10 deletions docs/rules/no-throw-statements.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ throw new Error("Something went wrong.");

### ✅ Correct

<!-- eslint-skip -->

```js
/* eslint functional/no-throw-statements: "error" */

Expand All @@ -38,8 +36,6 @@ function divide(x, y) {
}
```

<!-- eslint-skip -->

```js
/* eslint functional/no-throw-statements: "error" */

Expand All @@ -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,
};
```

Expand All @@ -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]);
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 @@ -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;
Expand Down
10 changes: 6 additions & 4 deletions eslint.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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",
},
},
{
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 @@ -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,
Expand All @@ -20,6 +20,7 @@ import {
isMethodDefinition,
isObjectExpression,
isProgram,
isPromiseType,
isProperty,
isTSInterfaceBody,
isTSInterfaceHeritage,
Expand Down Expand Up @@ -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<{...}>`.
*/
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 }],
});
});
});
});
});
});
Loading