From 2903ff582fa75ca88e62f7ea9affafc434c4295b Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Sat, 7 Dec 2024 16:14:00 -0700 Subject: [PATCH 01/13] wip --- .../src/rules/only-throw-error.ts | 111 ++++++++++++++++++ .../tests/rules/only-throw-error.test.ts | 64 ++++++++++ 2 files changed, 175 insertions(+) diff --git a/packages/eslint-plugin/src/rules/only-throw-error.ts b/packages/eslint-plugin/src/rules/only-throw-error.ts index 87f6a1e34152..47da19d692e9 100644 --- a/packages/eslint-plugin/src/rules/only-throw-error.ts +++ b/packages/eslint-plugin/src/rules/only-throw-error.ts @@ -1,14 +1,17 @@ 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, + isStaticMemberAccessOfValue, isTypeAnyType, isTypeUnknownType, typeMatchesSomeSpecifier, @@ -22,6 +25,7 @@ type Options = [ allow?: TypeOrValueSpecifier[]; allowThrowingAny?: boolean; allowThrowingUnknown?: boolean; + allowRethrowing?: boolean; }, ]; @@ -48,6 +52,10 @@ export default createRule({ ...typeOrValueSpecifiersSchema, description: 'Type specifiers that can be thrown.', }, + allowRethrowing: { + type: 'boolean', + description: 'Whether to allow rethrowing caught values', + }, allowThrowingAny: { type: 'boolean', description: @@ -65,6 +73,7 @@ export default createRule({ defaultOptions: [ { allow: [], + allowRethrowing: true, allowThrowingAny: true, allowThrowingUnknown: true, }, @@ -72,6 +81,104 @@ export default createRule({ 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 functionScope = scope.variableScope; + + const smVariable = findVariable(scope, node.name); + + if (smVariable == null) { + return false; + } + const variableDefinitions = smVariable.defs.filter( + def => def.isVariableDefinition, + ); + if (variableDefinitions.length !== 1) { + return false; + } + const def = smVariable.defs[0]; + + if (def.node.type === AST_NODE_TYPES.CatchClause) { + const isCatchClauseInSameFunctionScope = + context.sourceCode.getScope(def.node).variableScope === functionScope; + return isCatchClauseInSameFunctionScope; + } + + 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; + + // TODO extract to common code. + // promise.catch(x => { throw x; }) + if (callExpression.arguments.length >= 1) { + const firstArgument = callExpression.arguments[0]; + if ( + firstArgument.type !== AST_NODE_TYPES.SpreadElement && + firstArgument === def.node + ) { + const callee = callExpression.callee; + if ( + callee.type === AST_NODE_TYPES.MemberExpression && + isStaticMemberAccessOfValue(callee, context, 'catch') + ) { + const tsObjectNode = services.esTreeNodeToTSNodeMap.get( + callee.object, + ); + if ( + isThenableType( + services.program.getTypeChecker(), + tsObjectNode as ts.Expression, + ) + ) { + return true; + } + } + } + } + + // TODO extract to common code. + // promise.then(onFulfilled, (x) => { throw x; }) + if (callExpression.arguments.length >= 2) { + const firstTwoArgs = callExpression.arguments.slice(0, 2); + if ( + firstTwoArgs.every(arg => arg.type !== AST_NODE_TYPES.SpreadElement) + ) { + const secondArg = firstTwoArgs[1]; + if (secondArg === def.node) { + const callee = callExpression.callee; + if ( + callee.type === AST_NODE_TYPES.MemberExpression && + isStaticMemberAccessOfValue(callee, context, 'then') + ) { + const tsObjectNode = services.esTreeNodeToTSNodeMap.get( + callee.object, + ); + if ( + isThenableType( + services.program.getTypeChecker(), + tsObjectNode as ts.Expression, + ) + ) { + return true; + } + } + } + } + } + } + + return false; + } + function checkThrowArgument(node: TSESTree.Node): void { if ( node.type === AST_NODE_TYPES.AwaitExpression || @@ -80,6 +187,10 @@ export default createRule({ return; } + if (options.allowRethrowing && isRethrownError(node)) { + return; + } + const type = services.getTypeAtLocation(node); if (typeMatchesSomeSpecifier(type, allow, services.program)) { diff --git a/packages/eslint-plugin/tests/rules/only-throw-error.test.ts b/packages/eslint-plugin/tests/rules/only-throw-error.test.ts index c7d52c6a1e1c..7e7792c2ee78 100644 --- a/packages/eslint-plugin/tests/rules/only-throw-error.test.ts +++ b/packages/eslint-plugin/tests/rules/only-throw-error.test.ts @@ -189,6 +189,46 @@ throw new Map(); }, ], }, + { + code: ` +try { +} catch (e) { + throw e; +} + `, + options: [ + { + allowRethrowing: true, + allowThrowingAny: false, + allowThrowingUnknown: false, + }, + ], + }, + { + code: ` +try { +} catch (eOuter) { + try { + if (Math.random() > 0.5) { + throw eOuter; + } + } catch (eInner) { + if (Math.random() > 0.5) { + throw eOuter; + } else { + throw eInner; + } + } +} + `, + options: [ + { + allowRethrowing: true, + allowThrowingAny: false, + allowThrowingUnknown: false, + }, + ], + }, ], invalid: [ { @@ -553,5 +593,29 @@ throw new UnknownError(); }, ], }, + { + code: ` +function foo() { + try { + } catch (e) { + function inner() { + throw e; + } + } +} + `, + errors: [ + { + messageId: 'object', + }, + ], + options: [ + { + allowRethrowing: true, + allowThrowingAny: false, + allowThrowingUnknown: false, + }, + ], + }, ], }); From 7d4332cda185dbc2b31e40112c5f73c93a9dfc27 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger <53019676+kirkwaiblinger@users.noreply.github.com> Date: Mon, 14 Apr 2025 18:10:50 -0600 Subject: [PATCH 02/13] progress --- packages/eslint-plugin/package.json | 2 +- .../src/rules/no-floating-promises.ts | 42 +++--- .../src/rules/only-throw-error.ts | 79 ++++------- .../eslint-plugin/src/util/promiseUtils.ts | 123 ++++++++++++++++++ .../tests/rules/only-throw-error.test.ts | 64 +++++++-- 5 files changed, 218 insertions(+), 92 deletions(-) create mode 100644 packages/eslint-plugin/src/util/promiseUtils.ts diff --git a/packages/eslint-plugin/package.json b/packages/eslint-plugin/package.json index 09f820ca8de3..9301dc20a271 100644 --- a/packages/eslint-plugin/package.json +++ b/packages/eslint-plugin/package.json @@ -56,7 +56,7 @@ "generate:breaking-changes": "tsx tools/generate-breaking-changes.mts", "generate:configs": "npx nx generate-configs repo", "lint": "npx nx lint", - "test": "vitest --run --config=$INIT_CWD/vitest.config.mts", + "test": "vitest --config=$INIT_CWD/vitest.config.mts", "test-single": "vitest --run --config=$INIT_CWD/vitest.config.mts", "check-types": "npx nx typecheck" }, diff --git a/packages/eslint-plugin/src/rules/no-floating-promises.ts b/packages/eslint-plugin/src/rules/no-floating-promises.ts index 8e91c1987b62..e8172470a218 100644 --- a/packages/eslint-plugin/src/rules/no-floating-promises.ts +++ b/packages/eslint-plugin/src/rules/no-floating-promises.ts @@ -18,6 +18,11 @@ import { skipChainExpression, typeMatchesSomeSpecifier, } from '../util'; +import { + parseCatchCall, + parseFinallyCall, + parseThenCall, +} from '../util/promiseUtils'; export type Options = [ { @@ -336,38 +341,23 @@ export default createRule({ // 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. diff --git a/packages/eslint-plugin/src/rules/only-throw-error.ts b/packages/eslint-plugin/src/rules/only-throw-error.ts index 8f869dcff47a..b3d8a23ccd43 100644 --- a/packages/eslint-plugin/src/rules/only-throw-error.ts +++ b/packages/eslint-plugin/src/rules/only-throw-error.ts @@ -1,5 +1,8 @@ import type { TSESTree } from '@typescript-eslint/utils'; +// eslint-disable-next-line @typescript-eslint/internal/no-typescript-estree-import -- just importing the type +import type { TSESTreeToTSNode } from '@typescript-eslint/typescript-estree'; + import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import { isThenableType } from 'ts-api-utils'; import * as ts from 'typescript'; @@ -11,12 +14,12 @@ import { findVariable, getParserServices, isErrorLike, - isStaticMemberAccessOfValue, isTypeAnyType, isTypeUnknownType, typeMatchesSomeSpecifier, typeOrValueSpecifiersSchema, } from '../util'; +import { parseCatchCall, parseThenCall } from '../util/promiseUtils'; export type MessageIds = 'object' | 'undef'; @@ -88,7 +91,6 @@ export default createRule({ } const scope = context.sourceCode.getScope(node); - const functionScope = scope.variableScope; const smVariable = findVariable(scope, node.name); @@ -103,12 +105,13 @@ export default createRule({ } const def = smVariable.defs[0]; + // catch (x) { throw x; } if (def.node.type === AST_NODE_TYPES.CatchClause) { - const isCatchClauseInSameFunctionScope = - context.sourceCode.getScope(def.node).variableScope === functionScope; - return isCatchClauseInSameFunctionScope; + 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 && @@ -117,60 +120,22 @@ export default createRule({ ) { const callExpression = def.node.parent; - // TODO extract to common code. - // promise.catch(x => { throw x; }) - if (callExpression.arguments.length >= 1) { - const firstArgument = callExpression.arguments[0]; - if ( - firstArgument.type !== AST_NODE_TYPES.SpreadElement && - firstArgument === def.node - ) { - const callee = callExpression.callee; + 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); + + // make sure we're actually dealing with a promise if ( - callee.type === AST_NODE_TYPES.MemberExpression && - isStaticMemberAccessOfValue(callee, context, 'catch') + isThenableType( + services.program.getTypeChecker(), + tsObjectNode satisfies TSESTreeToTSNode as ts.Expression, + ) ) { - const tsObjectNode = services.esTreeNodeToTSNodeMap.get( - callee.object, - ); - if ( - isThenableType( - services.program.getTypeChecker(), - tsObjectNode as ts.Expression, - ) - ) { - return true; - } - } - } - } - - // TODO extract to common code. - // promise.then(onFulfilled, (x) => { throw x; }) - if (callExpression.arguments.length >= 2) { - const firstTwoArgs = callExpression.arguments.slice(0, 2); - if ( - firstTwoArgs.every(arg => arg.type !== AST_NODE_TYPES.SpreadElement) - ) { - const secondArg = firstTwoArgs[1]; - if (secondArg === def.node) { - const callee = callExpression.callee; - if ( - callee.type === AST_NODE_TYPES.MemberExpression && - isStaticMemberAccessOfValue(callee, context, 'then') - ) { - const tsObjectNode = services.esTreeNodeToTSNodeMap.get( - callee.object, - ); - if ( - isThenableType( - services.program.getTypeChecker(), - tsObjectNode as ts.Expression, - ) - ) { - return true; - } - } + return true; } } } diff --git a/packages/eslint-plugin/src/util/promiseUtils.ts b/packages/eslint-plugin/src/util/promiseUtils.ts new file mode 100644 index 000000000000..fd6de51ab6c8 --- /dev/null +++ b/packages/eslint-plugin/src/util/promiseUtils.ts @@ -0,0 +1,123 @@ +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, +): + | { + 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; +} + +export function parseCatchCall( + node: TSESTree.CallExpression, + context: RuleContext, +): + | { + 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; +} + +export function parseFinallyCall( + node: TSESTree.CallExpression, + context: RuleContext, +): + | { + 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; +} diff --git a/packages/eslint-plugin/tests/rules/only-throw-error.test.ts b/packages/eslint-plugin/tests/rules/only-throw-error.test.ts index 7e7792c2ee78..9d991fa87799 100644 --- a/packages/eslint-plugin/tests/rules/only-throw-error.test.ts +++ b/packages/eslint-plugin/tests/rules/only-throw-error.test.ts @@ -229,6 +229,20 @@ try { }, ], }, + { + code: ` +Promise.reject('foo').catch(e => { + throw e; +}); + `, + options: [ + { + allowRethrowing: true, + allowThrowingAny: false, + allowThrowingUnknown: false, + }, + ], + }, ], invalid: [ { @@ -595,14 +609,48 @@ throw new UnknownError(); }, { code: ` -function foo() { - try { - } catch (e) { - function inner() { - throw e; - } - } -} +let x = 1; +Promise.reject('foo').catch(e => { + throw x; +}); + `, + errors: [ + { + messageId: 'object', + }, + ], + options: [ + { + allowRethrowing: true, + allowThrowingAny: false, + allowThrowingUnknown: false, + }, + ], + }, + { + code: ` +Promise.reject('foo').catch((...e) => { + throw e; +}); + `, + errors: [ + { + messageId: 'object', + }, + ], + options: [ + { + allowRethrowing: true, + allowThrowingAny: false, + allowThrowingUnknown: false, + }, + ], + }, + { + code: ` +Promise.reject('foo').then((...e) => { + throw e; +}); `, errors: [ { From c3cfb66fff5091287733ae5f2ef5e6e8a06a0143 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger <53019676+kirkwaiblinger@users.noreply.github.com> Date: Mon, 14 Apr 2025 18:21:47 -0600 Subject: [PATCH 03/13] comment --- packages/eslint-plugin/src/rules/only-throw-error.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/only-throw-error.ts b/packages/eslint-plugin/src/rules/only-throw-error.ts index b3d8a23ccd43..dd042bda2ab1 100644 --- a/packages/eslint-plugin/src/rules/only-throw-error.ts +++ b/packages/eslint-plugin/src/rules/only-throw-error.ts @@ -105,7 +105,7 @@ export default createRule({ } const def = smVariable.defs[0]; - // catch (x) { throw x; } + // try { /* ... */ } catch (x) { throw x; } if (def.node.type === AST_NODE_TYPES.CatchClause) { return true; } From 0b13b1d0cdeee27a478a2fe7f77940550ac0b77c Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger <53019676+kirkwaiblinger@users.noreply.github.com> Date: Mon, 14 Apr 2025 19:33:48 -0600 Subject: [PATCH 04/13] snapshots --- packages/eslint-plugin/src/rules/no-floating-promises.ts | 1 - .../tests/schema-snapshots/only-throw-error.shot | 6 ++++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-floating-promises.ts b/packages/eslint-plugin/src/rules/no-floating-promises.ts index e8172470a218..a19d057c545c 100644 --- a/packages/eslint-plugin/src/rules/no-floating-promises.ts +++ b/packages/eslint-plugin/src/rules/no-floating-promises.ts @@ -10,7 +10,6 @@ import { createRule, getOperatorPrecedence, getParserServices, - getStaticMemberAccessValue, isBuiltinSymbolLike, OperatorPrecedence, readonlynessOptionsDefaults, diff --git a/packages/eslint-plugin/tests/schema-snapshots/only-throw-error.shot b/packages/eslint-plugin/tests/schema-snapshots/only-throw-error.shot index 8727d31475d7..8480328ea869 100644 --- a/packages/eslint-plugin/tests/schema-snapshots/only-throw-error.shot +++ b/packages/eslint-plugin/tests/schema-snapshots/only-throw-error.shot @@ -100,6 +100,10 @@ }, "type": "array" }, + "allowRethrowing": { + "description": "Whether to allow rethrowing caught values", + "type": "boolean" + }, "allowThrowingAny": { "description": "Whether to always allow throwing values typed as `any`.", "type": "boolean" @@ -136,6 +140,8 @@ type Options = [ } | string )[]; + /** Whether to allow rethrowing caught values */ + allowRethrowing?: boolean; /** Whether to always allow throwing values typed as `any`. */ allowThrowingAny?: boolean; /** Whether to always allow throwing values typed as `unknown`. */ From 29f21609a1e6270988b621db85b98516ce4b9017 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger <53019676+kirkwaiblinger@users.noreply.github.com> Date: Thu, 1 May 2025 10:14:28 -0600 Subject: [PATCH 05/13] cov --- .../src/rules/only-throw-error.ts | 9 ++- .../tests/rules/no-floating-promises.test.ts | 48 +++++++++++ .../tests/rules/only-throw-error.test.ts | 80 +++++++++++++++++++ 3 files changed, 133 insertions(+), 4 deletions(-) diff --git a/packages/eslint-plugin/src/rules/only-throw-error.ts b/packages/eslint-plugin/src/rules/only-throw-error.ts index dd042bda2ab1..763d9b4239f2 100644 --- a/packages/eslint-plugin/src/rules/only-throw-error.ts +++ b/packages/eslint-plugin/src/rules/only-throw-error.ts @@ -18,6 +18,7 @@ import { isTypeUnknownType, typeMatchesSomeSpecifier, typeOrValueSpecifiersSchema, + nullThrows, } from '../util'; import { parseCatchCall, parseThenCall } from '../util/promiseUtils'; @@ -92,11 +93,11 @@ export default createRule({ const scope = context.sourceCode.getScope(node); - const smVariable = findVariable(scope, node.name); + const smVariable = nullThrows( + findVariable(scope, node), + `Variable ${node.name} should exist in scope manager`, + ); - if (smVariable == null) { - return false; - } const variableDefinitions = smVariable.defs.filter( def => def.isVariableDefinition, ); diff --git a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts index 609c091eaae5..f976bbe821ec 100644 --- a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts @@ -5493,5 +5493,53 @@ await (>{}); }, ], }, + { + code: ` +Promise.reject('foo').then(); + `, + errors: [ + { + messageId: 'floatingVoid', + suggestions: [ + { + messageId: 'floatingFixVoid', + output: ` +void Promise.reject('foo').then(); + `, + }, + { + messageId: 'floatingFixAwait', + output: ` +await Promise.reject('foo').then(); + `, + }, + ], + }, + ], + }, + { + code: ` +Promise.reject('foo').finally(); + `, + errors: [ + { + messageId: 'floatingVoid', + suggestions: [ + { + messageId: 'floatingFixVoid', + output: ` +void Promise.reject('foo').finally(); + `, + }, + { + messageId: 'floatingFixAwait', + output: ` +await Promise.reject('foo').finally(); + `, + }, + ], + }, + ], + }, ], }); diff --git a/packages/eslint-plugin/tests/rules/only-throw-error.test.ts b/packages/eslint-plugin/tests/rules/only-throw-error.test.ts index 9d991fa87799..a85d65cd85e5 100644 --- a/packages/eslint-plugin/tests/rules/only-throw-error.test.ts +++ b/packages/eslint-plugin/tests/rules/only-throw-error.test.ts @@ -648,8 +648,88 @@ Promise.reject('foo').catch((...e) => { }, { code: ` +declare const x: any[]; +Promise.reject('foo').catch(...x, e => { + throw e; +}); + `, + errors: [ + { + messageId: 'object', + }, + ], + options: [ + { + allowRethrowing: true, + allowThrowingAny: false, + allowThrowingUnknown: false, + }, + ], + }, + { + code: ` +declare const x: any[]; +Promise.reject('foo').then(...x, e => { + throw e; +}); + `, + errors: [ + { + messageId: 'object', + }, + ], + options: [ + { + allowRethrowing: true, + allowThrowingAny: false, + allowThrowingUnknown: false, + }, + ], + }, + { + code: ` +declare const onFulfilled: any; +declare const x: any[]; +Promise.reject('foo').then(onFulfilled, ...x, e => { + throw e; +}); + `, + errors: [ + { + messageId: 'object', + }, + ], + options: [ + { + allowRethrowing: true, + allowThrowingAny: false, + allowThrowingUnknown: false, + }, + ], + }, + { + code: ` Promise.reject('foo').then((...e) => { throw e; +}); + `, + errors: [ + { + messageId: 'object', + }, + ], + options: [ + { + allowRethrowing: true, + allowThrowingAny: false, + allowThrowingUnknown: false, + }, + ], + }, + { + code: ` +Promise.reject('foo').then(e => { + throw globalThis; }); `, errors: [ From a4728558c14166d270ff02a5f873108449500ca6 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger <53019676+kirkwaiblinger@users.noreply.github.com> Date: Thu, 1 May 2025 10:16:25 -0600 Subject: [PATCH 06/13] cov2 --- .../tests/rules/no-floating-promises.test.ts | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts index f976bbe821ec..2485771ef234 100644 --- a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts @@ -5541,5 +5541,29 @@ await Promise.reject('foo').finally(); }, ], }, + { + code: ` +Promise.reject('foo').then(...[], () => {}); + `, + errors: [ + { + messageId: 'floatingVoid', + suggestions: [ + { + messageId: 'floatingFixVoid', + output: ` +void Promise.reject('foo').then(...[], () => {}); + `, + }, + { + messageId: 'floatingFixAwait', + output: ` +await Promise.reject('foo').then(...[], () => {}); + `, + }, + ], + }, + ], + }, ], }); From 4febd749b8d8632436ce0681a2817c2278e3ac82 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger <53019676+kirkwaiblinger@users.noreply.github.com> Date: Thu, 1 May 2025 10:26:16 -0600 Subject: [PATCH 07/13] docs --- .vscode/extensions.json | 3 ++- packages/eslint-plugin/docs/rules/only-throw-error.mdx | 8 ++++++++ packages/eslint-plugin/src/rules/only-throw-error.ts | 5 +++-- .../tests/schema-snapshots/only-throw-error.shot | 4 ++-- 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/.vscode/extensions.json b/.vscode/extensions.json index d4d111dbe6e4..8889d778d4e5 100644 --- a/.vscode/extensions.json +++ b/.vscode/extensions.json @@ -6,7 +6,8 @@ "esbenp.prettier-vscode", "streetsidesoftware.code-spell-checker", "tlent.jest-snapshot-language-support", - "mrmlnc.vscode-json5" + "mrmlnc.vscode-json5", + "unifiedjs.vscode-mdx" ], "unwantedRecommendations": ["hookyqr.beautify", "dbaeumer.jshint"] } diff --git a/packages/eslint-plugin/docs/rules/only-throw-error.mdx b/packages/eslint-plugin/docs/rules/only-throw-error.mdx index 49d980a127a2..b23bdc0809eb 100644 --- a/packages/eslint-plugin/docs/rules/only-throw-error.mdx +++ b/packages/eslint-plugin/docs/rules/only-throw-error.mdx @@ -123,6 +123,13 @@ interface Options { | string )[]; + /** + * Whether to allow rethrowing caught values that are not `Error` objects. + * + * @default false + */ + allowRethrowing?: boolean; + /** * Whether to always allow throwing values typed as `any`. */ @@ -136,6 +143,7 @@ interface Options { const defaultOptions: Options = { allow: [], + allowRethrowing: false, allowThrowingAny: true, allowThrowingUnknown: true, }; diff --git a/packages/eslint-plugin/src/rules/only-throw-error.ts b/packages/eslint-plugin/src/rules/only-throw-error.ts index 763d9b4239f2..a262e6d4b3fc 100644 --- a/packages/eslint-plugin/src/rules/only-throw-error.ts +++ b/packages/eslint-plugin/src/rules/only-throw-error.ts @@ -27,9 +27,9 @@ export type MessageIds = 'object' | 'undef'; export type Options = [ { allow?: TypeOrValueSpecifier[]; + allowRethrowing?: boolean; allowThrowingAny?: boolean; allowThrowingUnknown?: boolean; - allowRethrowing?: boolean; }, ]; @@ -58,7 +58,8 @@ export default createRule({ }, allowRethrowing: { type: 'boolean', - description: 'Whether to allow rethrowing caught values', + description: + 'Whether to allow rethrowing caught values that are not `Error` objects.', }, allowThrowingAny: { type: 'boolean', diff --git a/packages/eslint-plugin/tests/schema-snapshots/only-throw-error.shot b/packages/eslint-plugin/tests/schema-snapshots/only-throw-error.shot index 8480328ea869..9bcf6d14e382 100644 --- a/packages/eslint-plugin/tests/schema-snapshots/only-throw-error.shot +++ b/packages/eslint-plugin/tests/schema-snapshots/only-throw-error.shot @@ -101,7 +101,7 @@ "type": "array" }, "allowRethrowing": { - "description": "Whether to allow rethrowing caught values", + "description": "Whether to allow rethrowing caught values that are not `Error` objects.", "type": "boolean" }, "allowThrowingAny": { @@ -140,7 +140,7 @@ type Options = [ } | string )[]; - /** Whether to allow rethrowing caught values */ + /** Whether to allow rethrowing caught values that are not `Error` objects. */ allowRethrowing?: boolean; /** Whether to always allow throwing values typed as `any`. */ allowThrowingAny?: boolean; From 22b249ad89f21a36e389a858fc81c6927622d2f9 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger <53019676+kirkwaiblinger@users.noreply.github.com> Date: Thu, 1 May 2025 10:28:45 -0600 Subject: [PATCH 08/13] revert unrelated change --- packages/eslint-plugin/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/package.json b/packages/eslint-plugin/package.json index cb7855fd803f..79aa9f997102 100644 --- a/packages/eslint-plugin/package.json +++ b/packages/eslint-plugin/package.json @@ -56,7 +56,7 @@ "generate:breaking-changes": "tsx tools/generate-breaking-changes.mts", "generate:configs": "npx nx generate-configs repo", "lint": "npx nx lint", - "test": "vitest --config=$INIT_CWD/vitest.config.mts", + "test": "vitest --run --config=$INIT_CWD/vitest.config.mts", "test-single": "vitest --run --config=$INIT_CWD/vitest.config.mts", "check-types": "npx nx typecheck" }, From e2da1c9d12c763425a6d6819127d68a1d33e8c8a Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger <53019676+kirkwaiblinger@users.noreply.github.com> Date: Thu, 1 May 2025 10:33:53 -0600 Subject: [PATCH 09/13] more comments --- packages/eslint-plugin/src/util/promiseUtils.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/util/promiseUtils.ts b/packages/eslint-plugin/src/util/promiseUtils.ts index fd6de51ab6c8..3cbbec7a6fa6 100644 --- a/packages/eslint-plugin/src/util/promiseUtils.ts +++ b/packages/eslint-plugin/src/util/promiseUtils.ts @@ -6,7 +6,7 @@ import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import { getStaticMemberAccessValue } from './misc'; /** - * Parses a syntactically possible `Promise.then` call. Does not check the + * Parses a syntactically possible `Promise.then()` call. Does not check the * type of the callee. */ export function parseThenCall( @@ -57,6 +57,10 @@ export function parseThenCall( 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, @@ -90,6 +94,10 @@ export function parseCatchCall( 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, From dd71c120a3b63e1cc53346e4115a6fe7a9ab7676 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger <53019676+kirkwaiblinger@users.noreply.github.com> Date: Thu, 1 May 2025 10:55:53 -0600 Subject: [PATCH 10/13] fixup --- .../tests/rules/no-floating-promises.test.ts | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts index 2485771ef234..a983a40f1368 100644 --- a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts @@ -5543,6 +5543,30 @@ await Promise.reject('foo').finally(); }, { code: ` +Promise.reject('foo').finally(...[], () => {}); + `, + errors: [ + { + messageId: 'floatingVoid', + suggestions: [ + { + messageId: 'floatingFixVoid', + output: ` +void Promise.reject('foo').finally(...[], () => {}); + `, + }, + { + messageId: 'floatingFixAwait', + output: ` +await Promise.reject('foo').finally(...[], () => {}); + `, + }, + ], + }, + ], + }, + { + code: ` Promise.reject('foo').then(...[], () => {}); `, errors: [ From e583c088bfab0f31feb1bc98af9dc4c040211a74 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger <53019676+kirkwaiblinger@users.noreply.github.com> Date: Sun, 4 May 2025 23:17:47 -0600 Subject: [PATCH 11/13] Update packages/eslint-plugin/docs/rules/only-throw-error.mdx MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Josh Goldberg ✨ --- packages/eslint-plugin/docs/rules/only-throw-error.mdx | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/only-throw-error.mdx b/packages/eslint-plugin/docs/rules/only-throw-error.mdx index b23bdc0809eb..e30f67a0869b 100644 --- a/packages/eslint-plugin/docs/rules/only-throw-error.mdx +++ b/packages/eslint-plugin/docs/rules/only-throw-error.mdx @@ -125,8 +125,6 @@ interface Options { /** * Whether to allow rethrowing caught values that are not `Error` objects. - * - * @default false */ allowRethrowing?: boolean; From 3237b223a61ba2ad5c2f9cb3e9f3af84b0c78001 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger <53019676+kirkwaiblinger@users.noreply.github.com> Date: Sun, 4 May 2025 23:27:00 -0600 Subject: [PATCH 12/13] simplify type assertion --- packages/eslint-plugin/src/rules/only-throw-error.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/eslint-plugin/src/rules/only-throw-error.ts b/packages/eslint-plugin/src/rules/only-throw-error.ts index a262e6d4b3fc..8d00c09254dd 100644 --- a/packages/eslint-plugin/src/rules/only-throw-error.ts +++ b/packages/eslint-plugin/src/rules/only-throw-error.ts @@ -128,14 +128,13 @@ export default createRule({ if (parsedPromiseHandlingCall != null) { const { object, onRejected } = parsedPromiseHandlingCall; if (onRejected === def.node) { - const tsObjectNode = services.esTreeNodeToTSNodeMap.get(object); + const tsObjectNode = services.esTreeNodeToTSNodeMap.get( + object, + ) as ts.Expression; // make sure we're actually dealing with a promise if ( - isThenableType( - services.program.getTypeChecker(), - tsObjectNode satisfies TSESTreeToTSNode as ts.Expression, - ) + isThenableType(services.program.getTypeChecker(), tsObjectNode) ) { return true; } From 786c589d7e07b5db00f87f8c13f4dce4cb123058 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Mon, 5 May 2025 11:28:52 -0400 Subject: [PATCH 13/13] fix lint and an unrelated change --- .vscode/extensions.json | 3 +-- packages/eslint-plugin/src/rules/only-throw-error.ts | 3 --- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/.vscode/extensions.json b/.vscode/extensions.json index 8889d778d4e5..d4d111dbe6e4 100644 --- a/.vscode/extensions.json +++ b/.vscode/extensions.json @@ -6,8 +6,7 @@ "esbenp.prettier-vscode", "streetsidesoftware.code-spell-checker", "tlent.jest-snapshot-language-support", - "mrmlnc.vscode-json5", - "unifiedjs.vscode-mdx" + "mrmlnc.vscode-json5" ], "unwantedRecommendations": ["hookyqr.beautify", "dbaeumer.jshint"] } diff --git a/packages/eslint-plugin/src/rules/only-throw-error.ts b/packages/eslint-plugin/src/rules/only-throw-error.ts index 8d00c09254dd..187e6ac5e5b1 100644 --- a/packages/eslint-plugin/src/rules/only-throw-error.ts +++ b/packages/eslint-plugin/src/rules/only-throw-error.ts @@ -1,8 +1,5 @@ import type { TSESTree } from '@typescript-eslint/utils'; -// eslint-disable-next-line @typescript-eslint/internal/no-typescript-estree-import -- just importing the type -import type { TSESTreeToTSNode } from '@typescript-eslint/typescript-estree'; - import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import { isThenableType } from 'ts-api-utils'; import * as ts from 'typescript';