diff --git a/internal/rules/only_throw_error/only_throw_error.go b/internal/rules/only_throw_error/only_throw_error.go index d2914cb0..84b44c56 100644 --- a/internal/rules/only_throw_error/only_throw_error.go +++ b/internal/rules/only_throw_error/only_throw_error.go @@ -20,6 +20,104 @@ func buildUndefMessage() rule.RuleMessage { } } +// isRethrownError checks if the node is a rethrown caught error. +// This handles: +// 1. try { } catch (e) { throw e; } +// 2. promise.catch(e => { throw e; }) +// 3. promise.then(onFulfilled, e => { throw e; }) +func isRethrownError(ctx rule.RuleContext, node *ast.Node) bool { + if !ast.IsIdentifier(node) { + return false + } + + // Get the declaration of the variable + decl := utils.GetDeclaration(ctx.TypeChecker, node) + if decl == nil { + return false + } + + // Case 1: try { } catch (e) { throw e; } + if ast.IsCatchClause(decl.Parent) { + return true + } + + // Case 2 & 3: promise.catch(e => { throw e; }) or promise.then(onFulfilled, e => { throw e; }) + // The declaration must be from a parameter of an arrow function + if !ast.IsParameter(decl) { + return false + } + + paramDecl := decl.AsParameterDeclaration() + + // Must not be a rest parameter (...e) + if paramDecl.DotDotDotToken != nil { + return false + } + + // The parameter must belong to an arrow function + funcNode := decl.Parent + if !ast.IsArrowFunction(funcNode) { + return false + } + + arrowFunc := funcNode.AsArrowFunction() + + // The parameter must be the first parameter + if len(arrowFunc.Parameters.Nodes) == 0 || arrowFunc.Parameters.Nodes[0] != decl { + return false + } + + // The arrow function must be a direct argument of a call expression + if funcNode.Parent == nil || !ast.IsCallExpression(funcNode.Parent) { + return false + } + + callExpr := funcNode.Parent.AsCallExpression() + + // Check if this is a .catch() or .then() call + if !ast.IsPropertyAccessExpression(callExpr.Expression) { + return false + } + + propAccess := callExpr.Expression.AsPropertyAccessExpression() + methodName := propAccess.Name().Text() + + // For .catch(e => { throw e; }), the arrow function must be the first argument (onRejected) + // For .then(onFulfilled, e => { throw e; }), the arrow function must be the second argument (onRejected) + isRejectionHandler := false + args := callExpr.Arguments.Nodes + + if methodName == "catch" { + // .catch(onRejected) + // First argument must be our arrow function and not preceded by spread + if len(args) >= 1 && args[0] == funcNode && !ast.IsSpreadElement(args[0]) { + isRejectionHandler = true + } + } else if methodName == "then" { + // .then(onFulfilled, onRejected) + // Second argument must be our arrow function + // Also check that neither first nor second argument is a spread element + if len(args) >= 2 && args[1] == funcNode { + if !ast.IsSpreadElement(args[0]) && !ast.IsSpreadElement(args[1]) { + isRejectionHandler = true + } + } + } + + if !isRejectionHandler { + return false + } + + // Verify that the object is actually a thenable (Promise) + objectNode := propAccess.Expression + objectType := ctx.TypeChecker.GetTypeAtLocation(objectNode) + if !utils.IsThenableType(ctx.TypeChecker, objectNode, objectType) { + return false + } + + return true +} + var OnlyThrowErrorRule = rule.Rule{ Name: "only-throw-error", Run: func(ctx rule.RuleContext, options any) rule.RuleListeners { @@ -47,6 +145,10 @@ var OnlyThrowErrorRule = rule.Rule{ return } + if opts.AllowRethrowing && isRethrownError(ctx, expr) { + return + } + if utils.IsErrorLike(ctx.Program, ctx.TypeChecker, t) { return } diff --git a/internal/rules/only_throw_error/only_throw_error_test.go b/internal/rules/only_throw_error/only_throw_error_test.go index a64f7821..3e77193b 100644 --- a/internal/rules/only_throw_error/only_throw_error_test.go +++ b/internal/rules/only_throw_error/only_throw_error_test.go @@ -155,6 +155,42 @@ throw new Map(); }, { Code: ` +try { +} catch (e) { + throw e; +} + `, + Options: rule_tester.OptionsFromJSON[OnlyThrowErrorOptions](`{"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: rule_tester.OptionsFromJSON[OnlyThrowErrorOptions](`{"allowRethrowing": true, "allowThrowingAny": false, "allowThrowingUnknown": false}`), + }, + { + Code: ` +Promise.reject('foo').catch(e => { + throw e; +}); + `, + Options: rule_tester.OptionsFromJSON[OnlyThrowErrorOptions](`{"allowRethrowing": true, "allowThrowingAny": false, "allowThrowingUnknown": false}`), + }, + { + Code: ` import { createError } from 'errors'; throw createError(); `, @@ -597,5 +633,101 @@ function *foo(): Generator { }, }, }, + { + Code: ` +let x = 1; +Promise.reject('foo').catch(e => { + throw x; +}); + `, + Options: rule_tester.OptionsFromJSON[OnlyThrowErrorOptions](`{"allowRethrowing": true, "allowThrowingAny": false, "allowThrowingUnknown": false}`), + Errors: []rule_tester.InvalidTestCaseError{ + { + MessageId: "object", + }, + }, + }, + { + Code: ` +Promise.reject('foo').catch((...e) => { + throw e; +}); + `, + Options: rule_tester.OptionsFromJSON[OnlyThrowErrorOptions](`{"allowRethrowing": true, "allowThrowingAny": false, "allowThrowingUnknown": false}`), + Errors: []rule_tester.InvalidTestCaseError{ + { + MessageId: "object", + }, + }, + }, + { + Code: ` +declare const x: any[]; +Promise.reject('foo').catch(...x, e => { + throw e; +}); + `, + Options: rule_tester.OptionsFromJSON[OnlyThrowErrorOptions](`{"allowRethrowing": true, "allowThrowingAny": false, "allowThrowingUnknown": false}`), + Errors: []rule_tester.InvalidTestCaseError{ + { + MessageId: "object", + }, + }, + }, + { + Code: ` +declare const x: any[]; +Promise.reject('foo').then(...x, e => { + throw e; +}); + `, + Options: rule_tester.OptionsFromJSON[OnlyThrowErrorOptions](`{"allowRethrowing": true, "allowThrowingAny": false, "allowThrowingUnknown": false}`), + Errors: []rule_tester.InvalidTestCaseError{ + { + MessageId: "object", + }, + }, + }, + { + Code: ` +declare const onFulfilled: any; +declare const x: any[]; +Promise.reject('foo').then(onFulfilled, ...x, e => { + throw e; +}); + `, + Options: rule_tester.OptionsFromJSON[OnlyThrowErrorOptions](`{"allowRethrowing": true, "allowThrowingAny": false, "allowThrowingUnknown": false}`), + Errors: []rule_tester.InvalidTestCaseError{ + { + MessageId: "object", + }, + }, + }, + { + Code: ` +Promise.reject('foo').then((...e) => { + throw e; +}); + `, + Options: rule_tester.OptionsFromJSON[OnlyThrowErrorOptions](`{"allowRethrowing": true, "allowThrowingAny": false, "allowThrowingUnknown": false}`), + Errors: []rule_tester.InvalidTestCaseError{ + { + MessageId: "object", + }, + }, + }, + { + Code: ` +Promise.reject('foo').then(e => { + throw globalThis; +}); + `, + Options: rule_tester.OptionsFromJSON[OnlyThrowErrorOptions](`{"allowRethrowing": true, "allowThrowingAny": false, "allowThrowingUnknown": false}`), + Errors: []rule_tester.InvalidTestCaseError{ + { + MessageId: "object", + }, + }, + }, }) } diff --git a/internal/rules/only_throw_error/options.go b/internal/rules/only_throw_error/options.go index d98c3da0..7b046ea5 100644 --- a/internal/rules/only_throw_error/options.go +++ b/internal/rules/only_throw_error/options.go @@ -9,6 +9,9 @@ type OnlyThrowErrorOptions struct { // An array of type or value specifiers which are allowed to be thrown Allow []OnlyThrowErrorOptionsAllowElem `json:"allow,omitempty"` + // Whether to allow rethrowing caught values that are not Error objects + AllowRethrowing bool `json:"allowRethrowing,omitempty"` + // Whether to allow throwing values typed as any AllowThrowingAny bool `json:"allowThrowingAny,omitempty"` @@ -32,6 +35,9 @@ func (j *OnlyThrowErrorOptions) UnmarshalJSON(value []byte) error { if v, ok := raw["allow"]; !ok || v == nil { plain.Allow = []OnlyThrowErrorOptionsAllowElem{} } + if v, ok := raw["allowRethrowing"]; !ok || v == nil { + plain.AllowRethrowing = true + } if v, ok := raw["allowThrowingAny"]; !ok || v == nil { plain.AllowThrowingAny = true } diff --git a/internal/rules/only_throw_error/schema.json b/internal/rules/only_throw_error/schema.json index 61093283..3cad9a5e 100644 --- a/internal/rules/only_throw_error/schema.json +++ b/internal/rules/only_throw_error/schema.json @@ -12,6 +12,11 @@ "default": [], "description": "An array of type or value specifiers which are allowed to be thrown" }, + "allowRethrowing": { + "type": "boolean", + "default": true, + "description": "Whether to allow rethrowing caught values that are not Error objects" + }, "allowThrowingAny": { "type": "boolean", "default": true,