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

fix(no-promise-reject): new Promises and throw statements are now also checked #848

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
4 changes: 1 addition & 3 deletions docs/rules/no-promise-reject.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,13 @@

<!-- end auto-generated rule header -->

This rule disallows use of `Promise.reject()`.
This rule disallows rejecting promises.

## Rule Details

It is useful when using an `Option` type (something like `{ value: T } | { error: Error }`)
for handling errors. In this case a promise should always resolve with an `Option` and never reject.

This rule should be used in conjunction with [`no-throw-statements`](./no-throw-statements.md).

### ❌ Incorrect

<!-- eslint-skip -->
Expand Down
63 changes: 61 additions & 2 deletions src/rules/no-promise-reject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@ import {
type NamedCreateRuleCustomMeta,
type RuleResult,
} from "#/utils/rule";
import { isIdentifier, isMemberExpression } from "#/utils/type-guards";
import { getEnclosingFunction, getEnclosingTryStatement } from "#/utils/tree";
import {
isFunctionLike,
isIdentifier,
isMemberExpression,
} from "#/utils/type-guards";

/**
* The name of this rule.
Expand Down Expand Up @@ -39,7 +44,7 @@ const defaultOptions: Options = [{}];
* The possible error messages.
*/
const errorMessages = {
generic: "Unexpected reject, return an error instead.",
generic: "Unexpected rejection, resolve an error instead.",
} as const;

/**
Expand Down Expand Up @@ -67,6 +72,7 @@ function checkCallExpression(
return {
context,
descriptors:
// TODO: Better Promise type detection.
isMemberExpression(node.callee) &&
isIdentifier(node.callee.object) &&
isIdentifier(node.callee.property) &&
Expand All @@ -77,12 +83,65 @@ function checkCallExpression(
};
}

/**
* Check if the given NewExpression is for a Promise and it has a callback that rejects.
*/
function checkNewExpression(
node: TSESTree.NewExpression,
context: Readonly<RuleContext<keyof typeof errorMessages, Options>>,
): RuleResult<keyof typeof errorMessages, Options> {
return {
context,
descriptors:
// TODO: Better Promise type detection.
isIdentifier(node.callee) &&
node.callee.name === "Promise" &&
node.arguments[0] !== undefined &&
isFunctionLike(node.arguments[0]) &&
node.arguments[0].params.length === 2
? [{ node: node.arguments[0].params[1]!, messageId: "generic" }]
: [],
};
}

/**
* Check if the given ThrowStatement violates this rule.
*/
function checkThrowStatement(
node: TSESTree.ThrowStatement,
context: Readonly<RuleContext<keyof typeof errorMessages, Options>>,
): RuleResult<keyof typeof errorMessages, Options> {
const enclosingFunction = getEnclosingFunction(node);
if (enclosingFunction?.async !== true) {
return { context, descriptors: [] };
}

const enclosingTryStatement = getEnclosingTryStatement(node);
if (
enclosingTryStatement === null ||
getEnclosingFunction(enclosingTryStatement) !== enclosingFunction ||
enclosingTryStatement.handler === null
) {
return {
context,
descriptors: [{ node, messageId: "generic" }],
};
}

return {
context,
descriptors: [],
};
}

// Create the rule.
export const rule = createRule<keyof typeof errorMessages, Options>(
name,
meta,
defaultOptions,
{
CallExpression: checkCallExpression,
NewExpression: checkNewExpression,
ThrowStatement: checkThrowStatement,
},
);
30 changes: 26 additions & 4 deletions src/utils/tree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
isTSTypeAnnotation,
isTSTypeLiteral,
isTSTypeReference,
isTryStatement,
isVariableDeclaration,
} from "./type-guards";

Expand Down Expand Up @@ -52,7 +53,21 @@ export function isInFunctionBody(
node: TSESTree.Node,
async?: boolean,
): boolean {
const functionNode = getAncestorOfType(
const functionNode = getEnclosingFunction(node);

return (
functionNode !== null &&
(async === undefined || functionNode.async === async)
);
}

/**
* Get the function the given node is in.
*
* Will return null if not in a function.
*/
export function getEnclosingFunction(node: TSESTree.Node) {
return getAncestorOfType(
(
n,
c,
Expand All @@ -62,10 +77,17 @@ export function isInFunctionBody(
| TSESTree.FunctionExpression => isFunctionLike(n) && n.body === c,
node,
);
}

return (
functionNode !== null &&
(async === undefined || functionNode.async === async)
/**
* Get the function the given node is in.
*
* Will return null if not in a function.
*/
export function getEnclosingTryStatement(node: TSESTree.Node) {
return getAncestorOfType(
(n, c): n is TSESTree.TryStatement => isTryStatement(n) && n.block === c,
node,
);
}

Expand Down
15 changes: 15 additions & 0 deletions src/utils/type-guards.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,12 @@ export function isThrowStatement(
return node.type === AST_NODE_TYPES.ThrowStatement;
}

export function isTryStatement(
node: TSESTree.Node,
): node is TSESTree.TryStatement {
return node.type === AST_NODE_TYPES.TryStatement;
}

export function isTSArrayType(
node: TSESTree.Node,
): node is TSESTree.TSArrayType {
Expand Down Expand Up @@ -440,3 +446,12 @@ export function isObjectConstructorType(type: Type | null): boolean {
export function isFunctionLikeType(type: Type | null): boolean {
return type !== null && type.getCallSignatures().length > 0;
}

export function isPromiseType(type: Type | null): boolean {
return (
type !== null &&
(((type.symbol as unknown) !== undefined &&
type.symbol.name === "Promise") ||
(isUnionType(type) && type.types.some(isPromiseType)))
);
}
Loading