8000 feat(eslint-plugin): [only-throw-error] add option `allowRethrowing` by kirkwaiblinger · Pull Request #11075 · typescript-eslint/typescript-eslint · GitHub
[go: up one dir, main page]

Skip to content

feat(eslint-plugin): [only-throw-error] add option allowRethrowing #11075

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

Merged
6 changes: 6 additions & 0 deletions packages/eslint-plugin/docs/rules/only-throw-error.mdx
8000
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,11 @@ interface Options {
| string
)[];

/**
* Whether to allow rethrowing caught values that are not `Error` objects.
*/
allowRethrowing?: boolean;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Docs] The options mentions in this page don't explain why any of the options exist. I suppose that's existing/separate docs work for the other ones. But do you think it'd be reasonable to at least mention allowRethrowing in this PR?

If you'd rather them all tackled together as a followup, I wouldn't block.


/**
* Whether to always allow throwing values typed as `any`.
*/
Expand All @@ -136,6 +141,7 @@ interface Options {

const defaultOptions: Options = {
allow: [],
allowRethrowing: false,
allowThrowingAny: true,
allowThrowingUnknown: true,
};
Expand Down
43 changes: 16 additions & 27 deletions packages/eslint-plugin/src/rules/no-floating-promises.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,18 @@ import {
createRule,
getOperatorPrecedence,
getParserServices,
getStaticMemberAccessValue,
isBuiltinSymbolLike,
OperatorPrecedence,
readonlynessOptionsDefaults,
readonlynessOptionsSchema,
skipChainExpression,
typeMatchesSomeSpecifier,
} from '../util';
import {
parseCatchCall,
parseFinallyCall,
parseThenCall,
} from '../util/promiseUtils';

export type Options = [
{
Expand Down Expand Up @@ -336,38 +340,23 @@ export default createRule<Options, MessageId>({
// If the outer expression is a call, a `.catch()` or `.then()` with
// rejection handler handles the promise.

const { callee } = node;
if (callee.type === AST_NODE_TYPES.MemberExpression) {
const methodName = getStaticMemberAccessValue(callee, context);
const catchRejectionHandler =
methodName === 'catch' && node.arguments.length >= 1
? node.arguments[0]
: undefined;
if (catchRejectionHandler) {
if (isValidRejectionHandler(catchRejectionHandler)) {
const promiseHandlingMethodCall =
parseCatchCall(node, context) ?? parseThenCall(node, context);
if (promiseHandlingMethodCall != null) {
const onRejected = promiseHandlingMethodCall.onRejected;
if (onRejected != null) {
if (isValidRejectionHandler(onRejected)) {
return { isUnhandled: false };
}
return { isUnhandled: true, nonFunctionHandler: true };
}
return { isUnhandled: true };
}

const thenRejectionHandler =
methodName === 'then' && node.arguments.length >= 2
? node.arguments[1]
: undefined;
if (thenRejectionHandler) {
if (isValidRejectionHandler(thenRejectionHandler)) {
return { isUnhandled: false };
}
return { isUnhandled: true, nonFunctionHandler: true };
}
const promiseFinallyCall = parseFinallyCall(node, context);

// `x.finally()` is transparent to resolution of the promise, so check `x`.
// ("object" in this context is the `x` in `x.finally()`)
const promiseFinallyObject =
methodName === 'finally' ? callee.object : undefined;
if (promiseFinallyObject) {
return isUnhandledPromise(checker, promiseFinallyObject);
}
if (promiseFinallyCall != null) {
return isUnhandledPromise(checker, promiseFinallyCall.object);
}

// All other cases are unhandled.
Expand Down
74 changes: 74 additions & 0 deletions packages/eslint-plugin/src/rules/only-throw-error.ts
6D40
Original file line number Diff line number Diff line change
@@ -1,25 +1,30 @@
import type { TSESTree } from '@typescript-eslint/utils';

import { AST_NODE_TYPES } from '@typescript-eslint/utils';
import { isThenableType } from 'ts-api-utils';
import * as ts from 'typescript';

import type { TypeOrValueSpecifier } from '../util';

import {
createRule,
findVariable,
getParserServices,
isErrorLike,
isTypeAnyType,
isTypeUnknownType,
typeMatchesSomeSpecifier,
typeOrValueSpecifiersSchema,
nullThrows,
} from '../util';
import { parseCatchCall, parseThenCall } from '../util/promiseUtils';

export type MessageIds = 'object' | 'undef';

export type Options = [
{
allow?: TypeOrValueSpecifier[];
allowRethrowing?: boolean;
allowThrowingAny?: boolean;
allowThrowingUnknown?: boolean;
},
Expand Down Expand Up @@ -48,6 +53,11 @@ export default createRule<Options, MessageIds>({
...typeOrValueSpecifiersSchema,
description: 'Type specifiers that can be thrown.',
},
allowRethrowing: {
type: 'boolean',
description:
'Whether to allow rethrowing caught values that are not `Error` objects.',
},
allowThrowingAny: {
type: 'boolean',
description:
Expand All @@ -65,13 +75,73 @@ export default createRule<Options, MessageIds>({
defaultOptions: [
{
allow: [],
allowRethrowing: true,
allowThrowingAny: true,
allowThrowingUnknown: true,
},
],
create(context, [options]) {
const services = getParserServices(context);
const allow = options.allow;

function isRethrownError(node: TSESTree.Node): boolean {
if (node.type !== AST_NODE_TYPES.Identifier) {
return false;
}

const scope = context.sourceCode.getScope(node);

const smVariable = nullThrows(
findVariable(scope, node),
`Variable ${node.name} should exist in scope manager`,
);

const variableDefinitions = smVariable.defs.filter(
def => def.isVariableDefinition,
);
if (variableDefinitions.length !== 1) {
return false;
}
const def = smVariable.defs[0];

// try { /* ... */ } catch (x) { throw x; }
if (def.node.type === AST_NODE_TYPES.CatchClause) {
return true;
}

// promise.catch(x => { throw x; })
// promise.then(onFulfilled, x => { throw x; })
if (
def.node.type === AST_NODE_TYPES.ArrowFunctionExpression &&
def.node.params.length >= 1 &&
def.node.params[0] === def.name &&
def.node.parent.type === AST_NODE_TYPES.CallExpression
) {
const callExpression = def.node.parent;

const parsedPromiseHandlingCall =
parseCatchCall(callExpression, context) ??
parseThenCall(callExpression, context);
if (parsedPromiseHandlingCall != null) {
const { object, onRejected } = parsedPromiseHandlingCall;
if (onRejected === def.node) {
const tsObjectNode = services.esTreeNodeToTSNodeMap.get(
object,
) as ts.Expression;

// make sure we're actually dealing with a promise
if (
isThenableType(services.program.getTypeChecker(), tsObjectNode)
) {
return true;
}
}
}
}

return false;
}

function checkThrowArgument(node: TSESTree.Node): void {
if (
node.type === AST_NODE_TYPES.AwaitExpression ||
Expand All @@ -80,6 +150,10 @@ export default createRule<Options, MessageIds>({
return;
}

if (options.allowRethrowing && isRethrownError(node)) {
return;
}

const type = services.getTypeAtLocation(node);

if (typeMatchesSomeSpecifier(type, allow, services.program)) {
Expand Down
131 changes: 131 additions & 0 deletions packages/eslint-plugin/src/util/promiseUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
import type { TSESTree } from '@typescript-eslint/utils';
import type { RuleContext } from '@typescript-eslint/utils/ts-eslint';

import { AST_NODE_TYPES } from '@typescript-eslint/utils';

import { getStaticMemberAccessValue } from './misc';

/**
* Parses a syntactically possible `Promise.then()` call. Does not check the
* type of the callee.
*/
export function parseThenCall(
node: TSESTree.CallExpression,
context: RuleContext<string, unknown[]>,
):
| {
onFulfilled?: TSESTree.Expression | undefined;
onRejected?: TSESTree.Expression | undefined;
object: TSESTree.Expression;
}
| undefined {
if (node.callee.type === AST_NODE_TYPES.MemberExpression) {
const methodName = getStaticMemberAccessValue(node.callee, context);
if (methodName === 'then') {
if (node.arguments.length >= 1) {
if (node.arguments[0].type === AST_NODE_TYPES.SpreadElement) {
return {
object: node.callee.object,
};
}

if (node.arguments.length >= 2) {
if (node.arguments[1].type === AST_NODE_TYPES.SpreadElement) {
return {
object: node.callee.object,
onFulfilled: node.arguments[0],
};
}

return {
object: node.callee.object,
onFulfilled: node.arguments[0],
onRejected: node.arguments[1],
};
}
return {
object: node.callee.object,
onFulfilled: node.arguments[0],
};
}
return {
object: node.callee.object,
};
}
}

return undefined;
}

/**
* Parses a syntactically possible `Promise.catch()` call. Does not check the
* type of the callee.
*/
export function parseCatchCall(
node: TSESTree.CallExpression,
context: RuleContext<string, unknown[]>,
):
| {
onRejected?: TSESTree.Expression | undefined;
object: TSESTree.Expression;
}
| undefined {
if (node.callee.type === AST_NODE_TYPES.MemberExpression) {
const methodName = getStaticMemberAccessValue(node.callee, context);
if (methodName === 'catch') {
if (node.arguments.length >= 1) {
if (node.arguments[0].type === AST_NODE_TYPES.SpreadElement) {
return {
object: node.callee.object,
};
}

return {
object: node.callee.object,
onRejected: node.arguments[0],
};
}
return {
object: node.callee.object,
};
}
}

return undefined;
}

/**
* Parses a syntactically possible `Promise.finally()` call. Does not check the
* type of the callee.
*/
export function parseFinallyCall(
node: TSESTree.CallExpression,
context: RuleContext<string, unknown[]>,
):
| {
object: TSESTree.Expression;
onFinally?: TSESTree.Expression | undefined;
}
| undefined {
if (node.callee.type === AST_NODE_TYPES.MemberExpression) {
const methodName = getStaticMemberAccessValue(node.callee, context);
if (methodName === 'finally') {
if (node.arguments.length >= 1) {
if (node.arguments[0].type === AST_NODE_TYPES.SpreadElement) {
return {
object: node.callee.object,
};
}
return {
object: node.callee.object,
onFinally: node.arguments[0],
};
}
return {
object: node.callee.object,
};
}
}

return undefined;
}
Loading
0