diff --git a/.changeset/dull-yaks-poke.md b/.changeset/dull-yaks-poke.md new file mode 100644 index 000000000000..2232dd8dd98f --- /dev/null +++ b/.changeset/dull-yaks-poke.md @@ -0,0 +1,59 @@ +--- +"@biomejs/biome": patch +--- +Added the nursery lint rule `useErrorCause`. + +This rule enforces that errors caught in a `catch` clause are not rethrown without wrapping them in a new `Error` object and specifying the original error as the `cause`. This helps preserve the error’s stack trace and context for better debugging. + +It can be configured with the following option: + +- `requireCatchParameter`: (default: `true`) + - When `true`, the rule requires that `catch` clauses have a parameter. If a `throw` statement appears inside a `catch` clause without a parameter, it will be flagged. + +**Invalid examples**: + +```js +try { + foo(); +} catch { + throw new Error("fail"); +} +``` + +```js +try { + foo(); +} catch (err) { + throw new Error(err.message); +} +``` + +**Valid examples:** + +```js +try { + foo(); +} catch (err) { + throw new Error("fail", { cause: err }); +} +``` + +```js +try { + foo(); +} catch (error) { + throw new Error("Something went wrong", { cause: error }); +} +``` + +**Valid example** when `requireCatchParameter` is `false`: + +Valid: + +```js +try { + foo(); +} catch { + throw new Error("fail"); +} +``` diff --git a/crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rs b/crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rs index 4296416c1071..fde938638f67 100644 --- a/crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rs +++ b/crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rs @@ -2477,6 +2477,18 @@ pub(crate) fn migrate_eslint_any_rule( .get_or_insert(Default::default()); rule.set_level(rule.level().max(rule_severity.into())); } + "preserve-caught-error" => { + if !options.include_nursery { + results.add(eslint_name, eslint_to_biome::RuleMigrationResult::Nursery); + return false; + } + let group = rules.nursery.get_or_insert_with(Default::default); + let rule = group + .unwrap_group_as_mut() + .use_error_cause + .get_or_insert(Default::default()); + rule.set_level(rule.level().max(rule_severity.into())); + } "qwik/jsx-a" => { let group = rules.a11y.get_or_insert_with(Default::default); let rule = group diff --git a/crates/biome_configuration/src/analyzer/linter/rules.rs b/crates/biome_configuration/src/analyzer/linter/rules.rs index fa6cd00974d5..23996a2f3dc2 100644 --- a/crates/biome_configuration/src/analyzer/linter/rules.rs +++ b/crates/biome_configuration/src/analyzer/linter/rules.rs @@ -388,6 +388,7 @@ pub enum RuleName { UseDeprecatedReason, UseDestructuring, UseEnumInitializers, + UseErrorCause, UseErrorMessage, UseExhaustiveDependencies, UseExhaustiveSwitchCases, @@ -814,6 +815,7 @@ impl RuleName { Self::UseDeprecatedReason => "useDeprecatedReason", Self::UseDestructuring => "useDestructuring", Self::UseEnumInitializers => "useEnumInitializers", + Self::UseErrorCause => "useErrorCause", Self::UseErrorMessage => "useErrorMessage", Self::UseExhaustiveDependencies => "useExhaustiveDependencies", Self::UseExhaustiveSwitchCases => "useExhaustiveSwitchCases", @@ -1236,6 +1238,7 @@ impl RuleName { Self::UseDeprecatedReason => RuleGroup::Style, Self::UseDestructuring => RuleGroup::Nursery, Self::UseEnumInitializers => RuleGroup::Style, + Self::UseErrorCause => RuleGroup::Nursery, Self::UseErrorMessage => RuleGroup::Suspicious, Self::UseExhaustiveDependencies => RuleGroup::Correctness, Self::UseExhaustiveSwitchCases => RuleGroup::Nursery, @@ -1667,6 +1670,7 @@ impl std::str::FromStr for RuleName { "useDeprecatedReason" => Ok(Self::UseDeprecatedReason), "useDestructuring" => Ok(Self::UseDestructuring), "useEnumInitializers" => Ok(Self::UseEnumInitializers), + "useErrorCause" => Ok(Self::UseErrorCause), "useErrorMessage" => Ok(Self::UseErrorMessage), "useExhaustiveDependencies" => Ok(Self::UseExhaustiveDependencies), "useExhaustiveSwitchCases" => Ok(Self::UseExhaustiveSwitchCases), diff --git a/crates/biome_diagnostics_categories/src/categories.rs b/crates/biome_diagnostics_categories/src/categories.rs index daf743925acb..5d5577fe2736 100644 --- a/crates/biome_diagnostics_categories/src/categories.rs +++ b/crates/biome_diagnostics_categories/src/categories.rs @@ -213,6 +213,7 @@ define_categories! { "lint/nursery/useBiomeSuppressionComment": "https://biomejs.dev/linter/rules/use-biome-suppression-comment", "lint/nursery/useConsistentArrowReturn": "https://biomejs.dev/linter/rules/use-consistent-arrow-return", "lint/nursery/useConsistentGraphqlDescriptions": "https://biomejs.dev/linter/rules/use-consistent-graphql-descriptions", + "lint/nursery/useErrorCause": "https://biomejs.dev/linter/rules/use-error-cause", "lint/nursery/useConsistentObjectDefinition": "https://biomejs.dev/linter/rules/use-consistent-object-definition", "lint/nursery/useDeprecatedDate": "https://biomejs.dev/linter/rules/use-deprecated-date", "lint/nursery/useDestructuring": "https://biomejs.dev/linter/rules/use-destructuring", diff --git a/crates/biome_js_analyze/src/lint/nursery.rs b/crates/biome_js_analyze/src/lint/nursery.rs index ce852362cb77..be93ffd6eba1 100644 --- a/crates/biome_js_analyze/src/lint/nursery.rs +++ b/crates/biome_js_analyze/src/lint/nursery.rs @@ -46,6 +46,7 @@ pub mod use_array_sort_compare; pub mod use_await_thenable; pub mod use_consistent_arrow_return; pub mod use_destructuring; +pub mod use_error_cause; pub mod use_exhaustive_switch_cases; pub mod use_explicit_type; pub mod use_find; @@ -58,4 +59,4 @@ pub mod use_spread; pub mod use_vue_consistent_define_props_declaration; pub mod use_vue_define_macros_order; pub mod use_vue_multi_word_component_names; -declare_lint_group! { pub Nursery { name : "nursery" , rules : [self :: no_ambiguous_anchor_text :: NoAmbiguousAnchorText , self :: no_before_interactive_script_outside_document :: NoBeforeInteractiveScriptOutsideDocument , self :: no_continue :: NoContinue , self :: no_deprecated_imports :: NoDeprecatedImports , self :: no_duplicated_spread_props :: NoDuplicatedSpreadProps , self :: no_empty_source :: NoEmptySource , self :: no_equals_to_null :: NoEqualsToNull , self :: no_excessive_lines_per_file :: NoExcessiveLinesPerFile , self :: no_floating_promises :: NoFloatingPromises , self :: no_for_in :: NoForIn , self :: no_import_cycles :: NoImportCycles , self :: no_increment_decrement :: NoIncrementDecrement , self :: no_jsx_literals :: NoJsxLiterals , self :: no_jsx_props_bind :: NoJsxPropsBind , self :: no_leaked_render :: NoLeakedRender , self :: no_misused_promises :: NoMisusedPromises , self :: no_multi_assign :: NoMultiAssign , self :: no_multi_str :: NoMultiStr , self :: no_next_async_client_component :: NoNextAsyncClientComponent , self :: no_parameters_only_used_in_recursion :: NoParametersOnlyUsedInRecursion , self :: no_proto :: NoProto , self :: no_react_forward_ref :: NoReactForwardRef , self :: no_return_assign :: NoReturnAssign , self :: no_script_url :: NoScriptUrl , self :: no_shadow :: NoShadow , self :: no_sync_scripts :: NoSyncScripts , self :: no_ternary :: NoTernary , self :: no_undeclared_env_vars :: NoUndeclaredEnvVars , self :: no_unknown_attribute :: NoUnknownAttribute , self :: no_unnecessary_conditions :: NoUnnecessaryConditions , self :: no_unresolved_imports :: NoUnresolvedImports , self :: no_unused_expressions :: NoUnusedExpressions , self :: no_useless_catch_binding :: NoUselessCatchBinding , self :: no_useless_undefined :: NoUselessUndefined , self :: no_vue_data_object_declaration :: NoVueDataObjectDeclaration , self :: no_vue_duplicate_keys :: NoVueDuplicateKeys , self :: no_vue_reserved_keys :: NoVueReservedKeys , self :: no_vue_reserved_props :: NoVueReservedProps , self :: no_vue_setup_props_reactivity_loss :: NoVueSetupPropsReactivityLoss , self :: use_array_sort_compare :: UseArraySortCompare , self :: use_await_thenable :: UseAwaitThenable , self :: use_consistent_arrow_return :: UseConsistentArrowReturn , self :: use_destructuring :: UseDestructuring , self :: use_exhaustive_switch_cases :: UseExhaustiveSwitchCases , self :: use_explicit_type :: UseExplicitType , self :: use_find :: UseFind , self :: use_max_params :: UseMaxParams , self :: use_qwik_method_usage :: UseQwikMethodUsage , self :: use_qwik_valid_lexical_scope :: UseQwikValidLexicalScope , self :: use_regexp_exec :: UseRegexpExec , self :: use_sorted_classes :: UseSortedClasses , self :: use_spread :: UseSpread , self :: use_vue_consistent_define_props_declaration :: UseVueConsistentDefinePropsDeclaration , self :: use_vue_define_macros_order :: UseVueDefineMacrosOrder , self :: use_vue_multi_word_component_names :: UseVueMultiWordComponentNames ,] } } +declare_lint_group! { pub Nursery { name : "nursery" , rules : [self :: no_ambiguous_anchor_text :: NoAmbiguousAnchorText , self :: no_before_interactive_script_outside_document :: NoBeforeInteractiveScriptOutsideDocument , self :: no_continue :: NoContinue , self :: no_deprecated_imports :: NoDeprecatedImports , self :: no_duplicated_spread_props :: NoDuplicatedSpreadProps , self :: no_empty_source :: NoEmptySource , self :: no_equals_to_null :: NoEqualsToNull , self :: no_excessive_lines_per_file :: NoExcessiveLinesPerFile , self :: no_floating_promises :: NoFloatingPromises , self :: no_for_in :: NoForIn , self :: no_import_cycles :: NoImportCycles , self :: no_increment_decrement :: NoIncrementDecrement , self :: no_jsx_literals :: NoJsxLiterals , self :: no_jsx_props_bind :: NoJsxPropsBind , self :: no_leaked_render :: NoLeakedRender , self :: no_misused_promises :: NoMisusedPromises , self :: no_multi_assign :: NoMultiAssign , self :: no_multi_str :: NoMultiStr , self :: no_next_async_client_component :: NoNextAsyncClientComponent , self :: no_parameters_only_used_in_recursion :: NoParametersOnlyUsedInRecursion , self :: no_proto :: NoProto , self :: no_react_forward_ref :: NoReactForwardRef , self :: no_return_assign :: NoReturnAssign , self :: no_script_url :: NoScriptUrl , self :: no_shadow :: NoShadow , self :: no_sync_scripts :: NoSyncScripts , self :: no_ternary :: NoTernary , self :: no_undeclared_env_vars :: NoUndeclaredEnvVars , self :: no_unknown_attribute :: NoUnknownAttribute , self :: no_unnecessary_conditions :: NoUnnecessaryConditions , self :: no_unresolved_imports :: NoUnresolvedImports , self :: no_unused_expressions :: NoUnusedExpressions , self :: no_useless_catch_binding :: NoUselessCatchBinding , self :: no_useless_undefined :: NoUselessUndefined , self :: no_vue_data_object_declaration :: NoVueDataObjectDeclaration , self :: no_vue_duplicate_keys :: NoVueDuplicateKeys , self :: no_vue_reserved_keys :: NoVueReservedKeys , self :: no_vue_reserved_props :: NoVueReservedProps , self :: no_vue_setup_props_reactivity_loss :: NoVueSetupPropsReactivityLoss , self :: use_array_sort_compare :: UseArraySortCompare , self :: use_await_thenable :: UseAwaitThenable , self :: use_consistent_arrow_return :: UseConsistentArrowReturn , self :: use_destructuring :: UseDestructuring , self :: use_error_cause :: UseErrorCause , self :: use_exhaustive_switch_cases :: UseExhaustiveSwitchCases , self :: use_explicit_type :: UseExplicitType , self :: use_find :: UseFind , self :: use_max_params :: UseMaxParams , self :: use_qwik_method_usage :: UseQwikMethodUsage , self :: use_qwik_valid_lexical_scope :: UseQwikValidLexicalScope , self :: use_regexp_exec :: UseRegexpExec , self :: use_sorted_classes :: UseSortedClasses , self :: use_spread :: UseSpread , self :: use_vue_consistent_define_props_declaration :: UseVueConsistentDefinePropsDeclaration , self :: use_vue_define_macros_order :: UseVueDefineMacrosOrder , self :: use_vue_multi_word_component_names :: UseVueMultiWordComponentNames ,] } } diff --git a/crates/biome_js_analyze/src/lint/nursery/use_error_cause.rs b/crates/biome_js_analyze/src/lint/nursery/use_error_cause.rs new file mode 100644 index 000000000000..c6f02c8bffa5 --- /dev/null +++ b/crates/biome_js_analyze/src/lint/nursery/use_error_cause.rs @@ -0,0 +1,335 @@ +use biome_analyze::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use biome_console::markup; +use biome_js_semantic::SemanticModel; +use biome_js_syntax::{AnyJsBindingPattern, JsCatchClause, JsThrowStatement}; +use biome_rowan::{AstNode, AstSeparatedList, TextRange}; +use biome_rule_options::use_error_cause::UseErrorCauseOptions; + +use crate::services::semantic::Semantic; + +declare_lint_rule! { + /// Enforce that `new Error()` is thrown with the original error as `cause`. + /// + /// When catching and rethrowing an error, it's recommended to wrap the original error in a new `Error` object to preserve the original error's stack trace and context. The original error should be passed as the `cause` property of the new `Error` object. + /// + /// This rule enforces that practice, helping to maintain a clear and traceable error propagation chain, which is crucial for effective debugging. + /// + /// ## Examples + /// + /// ### Invalid + /// + /// ```js,expect_diagnostic + /// try { + /// // ... + /// } catch (err) { + /// throw new Error(err.message); + /// } + /// ``` + /// + /// ```js,expect_diagnostic + /// try { + /// doSomething(); + /// } catch { + /// throw new Error("Something went wrong"); + /// } + /// ``` + /// + /// ```js,expect_diagnostic + /// try { + /// // ... + /// } catch ({ message }) { + /// throw new Error(message); + /// } + /// ``` + /// + /// Cause error is being shadowed by a closer scoped redeclaration. + /// ```js,expect_diagnostic + /// try { + /// doSomething(); + /// } catch (error) { + /// if (whatever) { + /// const error = anotherError; // This declaration shadows the caught error. + /// throw new Error("Something went wrong", { cause: error }); + /// } + /// } + /// ``` + /// + /// ### Valid + /// + /// ```js + /// try { + /// // ... + /// } catch (err) { + /// throw new Error("Something went wrong", { cause: err }); + /// } + /// + /// try { + /// throw "Not a rethrow, so it's ignored when nested"; + /// } catch (err) { + /// const fn = () => { + /// throw new Error("New unrelated error"); + /// } + /// fn(); + /// } + /// ``` + /// + /// ## Options + /// + /// The following options are available: + /// + /// ### `requireCatchParameter` + /// + /// If `true`, the rule will report a diagnostic for a `throw` statement inside an empty `catch {}` block, recommending that the error be caught in a parameter. + /// + /// Default: `true` + /// + /// ```json,options + /// { + /// "options": { + /// "requireCatchParameter": false + /// } + /// } + /// ``` + /// + /// This option is enabled by default, meaning the following code is considered invalid: + /// + /// ```js,expect_diagnostic + /// try { + /// doSomething(); + /// } catch { + /// throw new Error("Something went wrong"); + /// } + /// ``` + /// + /// To disable this check, you would set the option to `false`: + /// + /// ```js,use_options + /// try { + /// doSomething(); + /// } catch { + /// throw new Error("Something went wrong"); + /// } + /// ``` + /// + pub UseErrorCause { + version: "next", + name: "useErrorCause", + language: "js", + recommended: false, + sources: &[RuleSource::Eslint("preserve-caught-error").same()], + } +} + +#[derive(Debug, Clone, Copy)] +pub enum State { + WithoutCause(TextRange), + NoErrorBinding(TextRange), + ShadowedCause { + cause_range: TextRange, + catch_binding_range: TextRange, + }, + DestructuringBinding(TextRange), +} + +impl Rule for UseErrorCause { + type Query = Semantic; + type State = State; + type Signals = Option; + type Options = UseErrorCauseOptions; + + fn run(ctx: &RuleContext) -> Self::Signals { + let throw_statement = ctx.query(); + let options = ctx.options(); + let model = ctx.model(); + + let catch_clause = throw_statement + .syntax() + .ancestors() + .find_map(JsCatchClause::cast)?; + + let throw_syntax = throw_statement.syntax(); + for ancestor in throw_syntax.ancestors() { + // Stop traversing once we reach the catch clause itself + if ancestor == *catch_clause.syntax() { + break; + } + + // Check for function-like nodes that introduce a new scope + if biome_js_syntax::AnyJsFunction::can_cast(ancestor.kind()) + || biome_js_syntax::JsClassDeclaration::can_cast(ancestor.kind()) + || biome_js_syntax::JsClassExpression::can_cast(ancestor.kind()) + || biome_js_syntax::JsMethodObjectMember::can_cast(ancestor.kind()) + { + // The throw is inside a function/method/class defined within the catch block. + // This is likely an independent error and should be ignored by this rule. + return None; + } + } + + if let Some(catch_declaration) = catch_clause.declaration() { + let binding = catch_declaration.binding().ok()?; + match binding { + AnyJsBindingPattern::AnyJsBinding(catch_error_binding) => { + let identifier_binding = catch_error_binding.as_js_identifier_binding()?; + + let thrown_expression = throw_statement.argument().ok()?; + + let Some(new_expression) = thrown_expression.as_js_new_expression() else { + // Not a `new` expression, so the rule does not apply. This handles `throw e;` cases. + return None; + }; + + let Some(arguments) = new_expression.arguments() else { + return Some(State::WithoutCause(throw_statement.range())); + }; + let args = arguments.args(); + + if args.len() < 2 { + return Some(State::WithoutCause(throw_statement.range())); + } + + let Some(Ok(second_arg_expr)) = args.iter().nth(1) else { + return Some(State::WithoutCause(throw_statement.range())); + }; + let Some(expr) = second_arg_expr.as_any_js_expression() else { + return Some(State::WithoutCause(throw_statement.range())); + }; + let Some(obj_expr) = expr.as_js_object_expression() else { + return Some(State::WithoutCause(throw_statement.range())); + }; + + for member in obj_expr.members().iter().flatten() { + if let Some(prop) = member.as_js_property_object_member() { + let is_cause_prop = prop + .name() + .ok() + .and_then(|name_node| name_node.name()) + .is_some_and(|name| name == "cause"); + + if is_cause_prop && let Ok(value) = prop.value() { + match is_cause_value_correct_error( + &value, + identifier_binding, + model, + ) { + CauseValueCheckResult::Correct => return None, + CauseValueCheckResult::Shadowed => { + return Some(State::ShadowedCause { + cause_range: value.range(), + catch_binding_range: identifier_binding.range(), + }); + } + CauseValueCheckResult::Incorrect => { + // Continue checking other properties, another `cause` might be present. + // This is unlikely to be valid JS, but we handle it. + } + } + } + } + } + + // If no valid cause was found after checking all members, it's a violation. + Some(State::WithoutCause(throw_statement.range())) + } + AnyJsBindingPattern::JsArrayBindingPattern(_) + | AnyJsBindingPattern::JsObjectBindingPattern(_) => { + Some(State::DestructuringBinding(binding.range())) + } + } + } else { + if !options.require_catch_parameter { + return None; + } + // This is the case `catch {}`. + // Any `throw` inside is a problem. + Some(State::NoErrorBinding(throw_statement.range())) + } + } + + fn diagnostic(_ctx: &RuleContext, state: &Self::State) -> Option { + match state { + State::WithoutCause(range) => Some(RuleDiagnostic::new( + rule_category!(), + range, + markup! { + "The original error is not being passed to the new `Error` object.\ + Include the original error in the `cause` property to preserve it." + }, + )), + State::ShadowedCause { cause_range, catch_binding_range } => Some( + RuleDiagnostic::new( + rule_category!(), + cause_range, + markup! { + "The `cause` property is shadowing the original error from the `catch` clause." + }, + ) + .detail( + catch_binding_range, + "The original error is declared here.", + ), + ), + State::NoErrorBinding(range) => Some(RuleDiagnostic::new( + rule_category!(), + range, + markup! { + "The original error is being discarded because the `catch` clause doesn't have a parameter.\ + Specify an error object in the `catch` clause to access the original error." + }, + )), + State::DestructuringBinding(range) => Some(RuleDiagnostic::new( + rule_category!(), + range, + markup! { + "Destructuring the error in a `catch` clause is not recommended, \ + as it can lead to losing important information from the error object, \ + such as the stack trace.\ + Use a single variable to catch the error, and then access its properties." + }, + )), + } + } +} + +enum CauseValueCheckResult { + Correct, + Shadowed, + Incorrect, +} + +fn is_cause_value_correct_error( + value: &biome_js_syntax::AnyJsExpression, + catch_error_binding: &biome_js_syntax::JsIdentifierBinding, + model: &SemanticModel, +) -> CauseValueCheckResult { + let Some(cause_identifier_expr) = value.as_js_identifier_expression() else { + return CauseValueCheckResult::Incorrect; + }; + let Ok(cause_reference) = cause_identifier_expr.name() else { + return CauseValueCheckResult::Incorrect; + }; + + let Some(cause_binding) = model.binding(&cause_reference) else { + return CauseValueCheckResult::Incorrect; + }; + + let catch_binding = model.as_binding(catch_error_binding); + + if cause_binding == catch_binding { + CauseValueCheckResult::Correct + } else { + let cause_name = cause_identifier_expr + .name() + .ok() + .and_then(|n| n.value_token().ok()); + let catch_name = catch_error_binding.name_token().ok(); + + if cause_name.as_ref().map(|t| t.text_trimmed()) + == catch_name.as_ref().map(|t| t.text_trimmed()) + { + CauseValueCheckResult::Shadowed + } else { + CauseValueCheckResult::Incorrect + } + } +} diff --git a/crates/biome_js_analyze/tests/specs/nursery/useErrorCause/invalid.js b/crates/biome_js_analyze/tests/specs/nursery/useErrorCause/invalid.js new file mode 100644 index 000000000000..aceb9a313841 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/useErrorCause/invalid.js @@ -0,0 +1,58 @@ +/* should generate diagnostics */ + +try { + throw new Error("Original error"); +} catch (err) { + throw new Error("Wrapper error"); +} + + +try { + throw new Error("Original error"); +} catch (err) { + throw new Error(`Failed: ${err.message}`); +} + +try { + throw new Error("Original error"); +} catch (err) { + if (true) { + throw err; + } +} + +try { + doSomething(); +} catch { + throw new TypeError("Something went wrong"); +} + + +try { + throw new Error("Original error"); +} catch ({ message }) { + throw new Error(message); +} + + +try { + throw new Error("Original error"); +} catch (err) { + throw new Error(`Failed to process: ${err.message}`, { cause: err.message }); +} + +try { + doSomething(); +} catch (error) { + if (whatever) { + const error = anotherError; // This declaration is the problem. + throw new Error("Something went wrong", { cause: error }); + } +} + +try { + doSomething(); +} catch (err) { + throw new Error("", { cause: otherVar }); +} + diff --git a/crates/biome_js_analyze/tests/specs/nursery/useErrorCause/invalid.js.snap b/crates/biome_js_analyze/tests/specs/nursery/useErrorCause/invalid.js.snap new file mode 100644 index 000000000000..e52758397462 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/useErrorCause/invalid.js.snap @@ -0,0 +1,195 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: invalid.js +--- +# Input +```js +/* should generate diagnostics */ + +try { + throw new Error("Original error"); +} catch (err) { + throw new Error("Wrapper error"); +} + + +try { + throw new Error("Original error"); +} catch (err) { + throw new Error(`Failed: ${err.message}`); +} + +try { + throw new Error("Original error"); +} catch (err) { + if (true) { + throw err; + } +} + +try { + doSomething(); +} catch { + throw new TypeError("Something went wrong"); +} + + +try { + throw new Error("Original error"); +} catch ({ message }) { + throw new Error(message); +} + + +try { + throw new Error("Original error"); +} catch (err) { + throw new Error(`Failed to process: ${err.message}`, { cause: err.message }); +} + +try { + doSomething(); +} catch (error) { + if (whatever) { + const error = anotherError; // This declaration is the problem. + throw new Error("Something went wrong", { cause: error }); + } +} + +try { + doSomething(); +} catch (err) { + throw new Error("", { cause: otherVar }); +} + + +``` + +# Diagnostics +``` +invalid.js:6:3 lint/nursery/useErrorCause ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + i The original error is not being passed to the new `Error` object.Include the original error in the `cause` property to preserve it. + + 4 │ throw new Error("Original error"); + 5 │ } catch (err) { + > 6 │ throw new Error("Wrapper error"); + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 7 │ } + 8 │ + + i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information. + + +``` + +``` +invalid.js:13:3 lint/nursery/useErrorCause ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + i The original error is not being passed to the new `Error` object.Include the original error in the `cause` property to preserve it. + + 11 │ throw new Error("Original error"); + 12 │ } catch (err) { + > 13 │ throw new Error(`Failed: ${err.message}`); + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 14 │ } + 15 │ + + i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information. + + +``` + +``` +invalid.js:27:2 lint/nursery/useErrorCause ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + i The original error is being discarded because the `catch` clause doesn't have a parameter.Specify an error object in the `catch` clause to access the original error. + + 25 │ doSomething(); + 26 │ } catch { + > 27 │ throw new TypeError("Something went wrong"); + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 28 │ } + 29 │ + + i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information. + + +``` + +``` +invalid.js:33:10 lint/nursery/useErrorCause ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + i Destructuring the error in a `catch` clause is not recommended, as it can lead to losing important information from the error object, such as the stack trace.Use a single variable to catch the error, and then access its properties. + + 31 │ try { + 32 │ throw new Error("Original error"); + > 33 │ } catch ({ message }) { + │ ^^^^^^^^^^^ + 34 │ throw new Error(message); + 35 │ } + + i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information. + + +``` + +``` +invalid.js:41:3 lint/nursery/useErrorCause ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + i The original error is not being passed to the new `Error` object.Include the original error in the `cause` property to preserve it. + + 39 │ throw new Error("Original error"); + 40 │ } catch (err) { + > 41 │ throw new Error(`Failed to process: ${err.message}`, { cause: err.message }); + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 42 │ } + 43 │ + + i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information. + + +``` + +``` +invalid.js:49:58 lint/nursery/useErrorCause ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + i The `cause` property is shadowing the original error from the `catch` clause. + + 47 │ if (whatever) { + 48 │ const error = anotherError; // This declaration is the problem. + > 49 │ throw new Error("Something went wrong", { cause: error }); + │ ^^^^^ + 50 │ } + 51 │ } + + i The original error is declared here. + + 44 │ try { + 45 │ doSomething(); + > 46 │ } catch (error) { + │ ^^^^^ + 47 │ if (whatever) { + 48 │ const error = anotherError; // This declaration is the problem. + + i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information. + + +``` + +``` +invalid.js:56:2 lint/nursery/useErrorCause ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + i The original error is not being passed to the new `Error` object.Include the original error in the `cause` property to preserve it. + + 54 │ doSomething(); + 55 │ } catch (err) { + > 56 │ throw new Error("", { cause: otherVar }); + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 57 │ } + 58 │ + + i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information. + + +``` diff --git a/crates/biome_js_analyze/tests/specs/nursery/useErrorCause/noRequireCatchParameter.invalid.js b/crates/biome_js_analyze/tests/specs/nursery/useErrorCause/noRequireCatchParameter.invalid.js new file mode 100644 index 000000000000..df1e90831844 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/useErrorCause/noRequireCatchParameter.invalid.js @@ -0,0 +1,31 @@ +/* should generate diagnostics */ + +try { + throw new Error("Original error"); +} catch (err) { + throw new Error("Wrapper error"); +} + + +try { + throw new Error("Original error"); +} catch (err) { + throw new Error(`Failed: ${err.message}`); +} + + +try { + throw new Error("Original error"); +} catch (err) { + if (true) { + throw err; + } +} + + + +try { + throw new Error("Original error"); +} catch ({ message }) { + throw new Error(message); +} diff --git a/crates/biome_js_analyze/tests/specs/nursery/useErrorCause/noRequireCatchParameter.invalid.js.snap b/crates/biome_js_analyze/tests/specs/nursery/useErrorCause/noRequireCatchParameter.invalid.js.snap new file mode 100644 index 000000000000..116b43b1ebc1 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/useErrorCause/noRequireCatchParameter.invalid.js.snap @@ -0,0 +1,91 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: noRequireCatchParameter.invalid.js +--- +# Input +```js +/* should generate diagnostics */ + +try { + throw new Error("Original error"); +} catch (err) { + throw new Error("Wrapper error"); +} + + +try { + throw new Error("Original error"); +} catch (err) { + throw new Error(`Failed: ${err.message}`); +} + + +try { + throw new Error("Original error"); +} catch (err) { + if (true) { + throw err; + } +} + + + +try { + throw new Error("Original error"); +} catch ({ message }) { + throw new Error(message); +} + +``` + +# Diagnostics +``` +noRequireCatchParameter.invalid.js:6:3 lint/nursery/useErrorCause ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + i The original error is not being passed to the new `Error` object.Include the original error in the `cause` property to preserve it. + + 4 │ throw new Error("Original error"); + 5 │ } catch (err) { + > 6 │ throw new Error("Wrapper error"); + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 7 │ } + 8 │ + + i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information. + + +``` + +``` +noRequireCatchParameter.invalid.js:13:3 lint/nursery/useErrorCause ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + i The original error is not being passed to the new `Error` object.Include the original error in the `cause` property to preserve it. + + 11 │ throw new Error("Original error"); + 12 │ } catch (err) { + > 13 │ throw new Error(`Failed: ${err.message}`); + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 14 │ } + 15 │ + + i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information. + + +``` + +``` +noRequireCatchParameter.invalid.js:29:10 lint/nursery/useErrorCause ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + i Destructuring the error in a `catch` clause is not recommended, as it can lead to losing important information from the error object, such as the stack trace.Use a single variable to catch the error, and then access its properties. + + 27 │ try { + 28 │ throw new Error("Original error"); + > 29 │ } catch ({ message }) { + │ ^^^^^^^^^^^ + 30 │ throw new Error(message); + 31 │ } + + i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information. + + +``` diff --git a/crates/biome_js_analyze/tests/specs/nursery/useErrorCause/noRequireCatchParameter.invalid.options.json b/crates/biome_js_analyze/tests/specs/nursery/useErrorCause/noRequireCatchParameter.invalid.options.json new file mode 100644 index 000000000000..2c9c009084c2 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/useErrorCause/noRequireCatchParameter.invalid.options.json @@ -0,0 +1,15 @@ +{ + "$schema": "../../../../../../packages/@biomejs/biome/configuration_schema.json", + "linter": { + "rules": { + "nursery": { + "useErrorCause": { + "level": "error", + "options": { + "requireCatchParameter": false + } + } + } + } + } +} diff --git a/crates/biome_js_analyze/tests/specs/nursery/useErrorCause/noRequireCatchParameter.valid.js b/crates/biome_js_analyze/tests/specs/nursery/useErrorCause/noRequireCatchParameter.valid.js new file mode 100644 index 000000000000..08a2a9617cee --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/useErrorCause/noRequireCatchParameter.valid.js @@ -0,0 +1,15 @@ +// should not generate diagnostics + + +try { + doSomething(); +} catch { + throw new TypeError("Something went wrong"); +} + + +try { + doSomething(); +} catch (e) { + console.error(e); +} diff --git a/crates/biome_js_analyze/tests/specs/nursery/useErrorCause/noRequireCatchParameter.valid.js.snap b/crates/biome_js_analyze/tests/specs/nursery/useErrorCause/noRequireCatchParameter.valid.js.snap new file mode 100644 index 000000000000..9d3b997cb1a9 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/useErrorCause/noRequireCatchParameter.valid.js.snap @@ -0,0 +1,23 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: requireCatchParameter.valid.js +--- +# Input +```js +// should not generate diagnostics + + +try { + doSomething(); +} catch { + throw new TypeError("Something went wrong"); +} + + +try { + doSomething(); +} catch (e) { + console.error(e); +} + +``` diff --git a/crates/biome_js_analyze/tests/specs/nursery/useErrorCause/noRequireCatchParameter.valid.options.json b/crates/biome_js_analyze/tests/specs/nursery/useErrorCause/noRequireCatchParameter.valid.options.json new file mode 100644 index 000000000000..2c9c009084c2 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/useErrorCause/noRequireCatchParameter.valid.options.json @@ -0,0 +1,15 @@ +{ + "$schema": "../../../../../../packages/@biomejs/biome/configuration_schema.json", + "linter": { + "rules": { + "nursery": { + "useErrorCause": { + "level": "error", + "options": { + "requireCatchParameter": false + } + } + } + } + } +} diff --git a/crates/biome_js_analyze/tests/specs/nursery/useErrorCause/valid.js b/crates/biome_js_analyze/tests/specs/nursery/useErrorCause/valid.js new file mode 100644 index 000000000000..ca9879dec1ba --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/useErrorCause/valid.js @@ -0,0 +1,49 @@ +// should not generate diagnostics + +try { + throw new Error("Original error"); +} catch (err) { + throw new Error("Wrapper error", { cause: err }); +} + +try { + throw new Error("Original error"); +} catch (err) { + console.error("Caught error:", err); +} + + +class CustomError extends Error { + constructor(message, options) { + super(message, options); + this.name = "CustomError"; + } +} +try { + throw new Error("Original error"); +} catch (err) { + throw new CustomError("Custom failure", { cause: err }); +} + + +try { + throw new Error("Original error"); +} catch (err) { + throw new Error(`Failed to process: ${err.message}`, { cause: err }); +} + +try { + doSomething(); +} catch (e) { + console.error(e); +} + +try { +} catch (error) { + foo = { + bar() { + // This throw is not directly related to the caught error. + throw new Error("Something went wrong"); + } + }; +} diff --git a/crates/biome_js_analyze/tests/specs/nursery/useErrorCause/valid.js.snap b/crates/biome_js_analyze/tests/specs/nursery/useErrorCause/valid.js.snap new file mode 100644 index 000000000000..11bd140e3271 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/useErrorCause/valid.js.snap @@ -0,0 +1,57 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: valid.js +--- +# Input +```js +// should not generate diagnostics + +try { + throw new Error("Original error"); +} catch (err) { + throw new Error("Wrapper error", { cause: err }); +} + +try { + throw new Error("Original error"); +} catch (err) { + console.error("Caught error:", err); +} + + +class CustomError extends Error { + constructor(message, options) { + super(message, options); + this.name = "CustomError"; + } +} +try { + throw new Error("Original error"); +} catch (err) { + throw new CustomError("Custom failure", { cause: err }); +} + + +try { + throw new Error("Original error"); +} catch (err) { + throw new Error(`Failed to process: ${err.message}`, { cause: err }); +} + +try { + doSomething(); +} catch (e) { + console.error(e); +} + +try { +} catch (error) { + foo = { + bar() { + // This throw is not directly related to the caught error. + throw new Error("Something went wrong"); + } + }; +} + +``` diff --git a/crates/biome_rule_options/src/lib.rs b/crates/biome_rule_options/src/lib.rs index bd7143fd96de..cd6d81c4ba22 100644 --- a/crates/biome_rule_options/src/lib.rs +++ b/crates/biome_rule_options/src/lib.rs @@ -302,6 +302,7 @@ pub mod use_deprecated_date; pub mod use_deprecated_reason; pub mod use_destructuring; pub mod use_enum_initializers; +pub mod use_error_cause; pub mod use_error_message; pub mod use_exhaustive_dependencies; pub mod use_exhaustive_switch_cases; diff --git a/crates/biome_rule_options/src/use_error_cause.rs b/crates/biome_rule_options/src/use_error_cause.rs new file mode 100644 index 000000000000..4b68f7f6eb99 --- /dev/null +++ b/crates/biome_rule_options/src/use_error_cause.rs @@ -0,0 +1,19 @@ +use biome_deserialize_macros::{Deserializable, Merge}; +use serde::{Deserialize, Serialize}; + +/// Options for the `useErrorCause` rule. +#[derive(Clone, Debug, Deserialize, Deserializable, Eq, PartialEq, Serialize, Merge)] +#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] +#[serde(rename_all = "camelCase", deny_unknown_fields, default)] +pub struct UseErrorCauseOptions { + /// When set to `true`, the rule requires that `catch` clauses have a parameter. + pub require_catch_parameter: bool, +} + +impl Default for UseErrorCauseOptions { + fn default() -> Self { + Self { + require_catch_parameter: true, + } + } +} diff --git a/packages/@biomejs/backend-jsonrpc/src/workspace.ts b/packages/@biomejs/backend-jsonrpc/src/workspace.ts index f006794031f3..6dbd8ad8ac1a 100644 --- a/packages/@biomejs/backend-jsonrpc/src/workspace.ts +++ b/packages/@biomejs/backend-jsonrpc/src/workspace.ts @@ -2098,6 +2098,11 @@ See */ useDestructuring?: UseDestructuringConfiguration; /** + * Enforce that new Error() is thrown with the original error as cause. +See + */ + useErrorCause?: UseErrorCauseConfiguration; + /** * Require switch-case statements to be exhaustive. See */ @@ -3842,6 +3847,9 @@ export type UseDeprecatedDateConfiguration = export type UseDestructuringConfiguration = | RulePlainConfiguration | RuleWithUseDestructuringOptions; +export type UseErrorCauseConfiguration = + | RulePlainConfiguration + | RuleWithUseErrorCauseOptions; export type UseExhaustiveSwitchCasesConfiguration = | RulePlainConfiguration | RuleWithUseExhaustiveSwitchCasesOptions; @@ -5374,6 +5382,10 @@ export interface RuleWithUseDestructuringOptions { level: RulePlainConfiguration; options?: UseDestructuringOptions; } +export interface RuleWithUseErrorCauseOptions { + level: RulePlainConfiguration; + options?: UseErrorCauseOptions; +} export interface RuleWithUseExhaustiveSwitchCasesOptions { fix?: FixKind; level: RulePlainConfiguration; @@ -6752,6 +6764,15 @@ export interface UseDeprecatedDateOptions { argumentName?: string; } export type UseDestructuringOptions = {}; +/** + * Options for the `useErrorCause` rule. + */ +export interface UseErrorCauseOptions { + /** + * When set to `true`, the rule requires that `catch` clauses have a parameter. + */ + requireCatchParameter?: boolean; +} export type UseExhaustiveSwitchCasesOptions = {}; export type UseExplicitTypeOptions = {}; export type UseFindOptions = {}; @@ -7610,6 +7631,7 @@ export type Category = | "lint/nursery/useBiomeSuppressionComment" | "lint/nursery/useConsistentArrowReturn" | "lint/nursery/useConsistentGraphqlDescriptions" + | "lint/nursery/useErrorCause" | "lint/nursery/useConsistentObjectDefinition" | "lint/nursery/useDeprecatedDate" | "lint/nursery/useDestructuring" diff --git a/packages/@biomejs/biome/configuration_schema.json b/packages/@biomejs/biome/configuration_schema.json index 67283257fca6..a8c4e8dd0b49 100644 --- a/packages/@biomejs/biome/configuration_schema.json +++ b/packages/@biomejs/biome/configuration_schema.json @@ -5552,6 +5552,13 @@ { "type": "null" } ] }, + "useErrorCause": { + "description": "Enforce that new Error() is thrown with the original error as cause.\nSee ", + "anyOf": [ + { "$ref": "#/$defs/UseErrorCauseConfiguration" }, + { "type": "null" } + ] + }, "useExhaustiveSwitchCases": { "description": "Require switch-case statements to be exhaustive.\nSee ", "anyOf": [ @@ -9145,6 +9152,15 @@ "additionalProperties": false, "required": ["level"] }, + "RuleWithUseErrorCauseOptions": { + "type": "object", + "properties": { + "level": { "$ref": "#/$defs/RulePlainConfiguration" }, + "options": { "$ref": "#/$defs/UseErrorCauseOptions" } + }, + "additionalProperties": false, + "required": ["level"] + }, "RuleWithUseErrorMessageOptions": { "type": "object", "properties": { @@ -12148,6 +12164,24 @@ "type": "object", "additionalProperties": false }, + "UseErrorCauseConfiguration": { + "oneOf": [ + { "$ref": "#/$defs/RulePlainConfiguration" }, + { "$ref": "#/$defs/RuleWithUseErrorCauseOptions" } + ] + }, + "UseErrorCauseOptions": { + "description": "Options for the `useErrorCause` rule.", + "type": "object", + "properties": { + "requireCatchParameter": { + "description": "When set to `true`, the rule requires that `catch` clauses have a parameter.", + "type": "boolean", + "default": true + } + }, + "additionalProperties": false + }, "UseErrorMessageConfiguration": { "oneOf": [ { "$ref": "#/$defs/RulePlainConfiguration" },