Conversation
🦋 Changeset detectedLatest commit: 6a0ae2a The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
bot
added
A-CLI
kedevked
force-pushed
the
feature/add-no-rethrow-without-cause
branch
2 times, most recently
from
388088a to
51381cd
Compare
WalkthroughAdds a new BIOME nursery lint rule Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
Comment |
kedevked
force-pushed
the
feature/add-no-rethrow-without-cause
branch
from
51381cd to
87f4646
Compare
kedevked
changed the title
CodSpeed Performance ReportMerging #7602 will not alter performanceComparing Summary
Footnotes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (10)
crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rsis excluded by!**/migrate/eslint_any_rule_to_biome.rsand included by**crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_js_analyze/src/lint/nursery.rsis excluded by!**/nursery.rsand included by**crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.valid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/valid.js.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (10)
.changeset/dull-yaks-poke.md(1 hunks)crates/biome_js_analyze/src/lint/nursery/no_rethrow_without_cause.rs(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/invalid.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.invalid.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.invalid.options.json(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.valid.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.valid.options.json(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/valid.js(1 hunks)crates/biome_rule_options/src/lib.rs(1 hunks)crates/biome_rule_options/src/no_rethrow_without_cause.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.valid.jscrates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/invalid.jscrates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.invalid.jscrates/biome_js_analyze/src/lint/nursery/no_rethrow_without_cause.rscrates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.invalid.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/valid.jscrates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.valid.options.json
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.valid.jscrates/biome_rule_options/src/no_rethrow_without_cause.rscrates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/invalid.jscrates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.invalid.jscrates/biome_js_analyze/src/lint/nursery/no_rethrow_without_cause.rscrates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.invalid.options.jsoncrates/biome_rule_options/src/lib.rscrates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/valid.jscrates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.valid.options.json
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.valid.jscrates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/invalid.jscrates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.invalid.jscrates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.invalid.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/valid.jscrates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.valid.options.json
.changeset/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
.changeset/*.md: In changeset files, only use #### or ##### headers; avoid other header levels
Changeset descriptions should use past tense for what you did (e.g., "Added...")
Describe current Biome behavior in present tense within changesets (e.g., "Biome now supports...")
For bug fixes in changesets, start with a link to the issue (e.g., "Fixed #1234: ...")
When referencing rules or assists in changesets, include links to their documentation pages
Include a minimal code block in the changeset when applicable to demonstrate the change
End every sentence in the changeset description with a period
Files:
.changeset/dull-yaks-poke.md
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format all Rust source files before committing (just f)
Files:
crates/biome_rule_options/src/no_rethrow_without_cause.rscrates/biome_js_analyze/src/lint/nursery/no_rethrow_without_cause.rscrates/biome_rule_options/src/lib.rs
🧠 Learnings (10)
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/crates/biome_rule_options/lib/**/*.rs : Options types must implement serialization/deserialization and schema support using derives: `Serialize`, `Deserialize`, `Deserializable`, and `#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]`
Applied to files:
crates/biome_rule_options/src/no_rethrow_without_cause.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/crates/biome_rule_options/lib/**/*.rs : Define per-rule options types in `biome_rule_options` crate (one file per rule, e.g., `lib/use_my_rule.rs`)
Applied to files:
crates/biome_rule_options/src/no_rethrow_without_cause.rscrates/biome_rule_options/src/lib.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : In declare_lint_rule! macros, set `version: "next"` for new or updated rules
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_rethrow_without_cause.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : When deprecating a rule, add `deprecated: "<reason>"` to `declare_lint_rule!`
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_rethrow_without_cause.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/{lint,assist}/**/*.rs : When banning globals (e.g., `noConsoleLog`), check the semantic model to avoid false positives from locally shadowed bindings
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_rethrow_without_cause.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : If a rule returns a code action (implements `action`), add `fix_kind` in `declare_lint_rule!` and use `ctx.metadata().applicability()` when building the action
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_rethrow_without_cause.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : Code blocks in rule docs must specify language; invalid snippets require `expect_diagnostic`; use `options`/`full_options`/`use_options` markers as appropriate
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_rethrow_without_cause.rs
📚 Learning: 2025-08-11T11:48:27.774Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:27.774Z
Learning: Applies to crates/biome_formatter/biome_html_formatter/tests/specs/html/**/options.json : When non-default formatting options are needed for a test group, place an options.json (biome.json format) alongside the .html files in that folder
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.invalid.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.valid.options.json
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/crates/**/tests/specs/**/*.jsonc : Snapshot test `.jsonc` files must contain an array of code snippets (strings); these run in script mode (no ESM syntax)
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.invalid.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.valid.options.json
📚 Learning: 2025-08-05T14:43:29.581Z
Learnt from: dyc3
PR: biomejs/biome#7081
File: packages/@biomejs/biome/configuration_schema.json:7765-7781
Timestamp: 2025-08-05T14:43:29.581Z
Learning: The file `packages/biomejs/biome/configuration_schema.json` is auto-generated and should not be manually edited or reviewed for schema issues; any changes should be made at the code generation source.
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.invalid.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.valid.options.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (25)
- GitHub Check: Check Dependencies
- GitHub Check: Documentation
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Test Node.js API
- GitHub Check: Check JS Files
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_package)
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: autofix
| "noRethrowWithoutCause": { | ||
| "level": "error", | ||
| "options": { | ||
| "requireCatchParameter": false |
There was a problem hiding this comment.
Fix the invalid options toggle.
Both requireCatchParameter option files currently set the flag to false, so the “invalid” suite never runs with the stricter configuration we’re supposedly checking. Let’s switch this one back to true so the unhappy-path specs actually do some work, rather than sipping tea with the happy-path crowd.
- "requireCatchParameter": false
+ "requireCatchParameter": true📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "requireCatchParameter": false | |
| "requireCatchParameter": true |
🤖 Prompt for AI Agents
In
crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.invalid.options.json
around line 9, the requireCatchParameter flag is incorrectly set to false which
prevents the “invalid” suite from exercising the stricter rule; edit this JSON
file to set "requireCatchParameter": true so the invalid tests run with the
intended stricter configuration.
a87dec4 to
fe19426
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
crates/biome_js_analyze/src/lint/nursery/no_rethrow_without_cause.rs (2)
175-177: Consider flipping the condition for clarityThe double negative (
!options.require_catch_parameter) makes this a bit harder to follow. Consider checking the positive condition first.- if !options.require_catch_parameter { - return None; - } + if options.require_catch_parameter { + // This is the case `catch {}`. + // Any `throw` inside is a problem. + return Some(State::NoErrorBinding(throw_statement.range())); + } - // This is the case `catch {}`. - // Any `throw` inside is a problem. - Some(State::NoErrorBinding(throw_statement.range())) + None
139-164: Consider extracting the cause validation logicThe nested logic for checking the
causeproperty is quite deep and could benefit from extraction into a helper function for better readability and testability.You could extract this into a helper function like
has_valid_cause_property(obj_expr: &JsObjectExpression, catch_error_name: &str) -> boolto make the main logic cleaner.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (10)
crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rsis excluded by!**/migrate/eslint_any_rule_to_biome.rsand included by**crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_js_analyze/src/lint/nursery.rsis excluded by!**/nursery.rsand included by**crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.valid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/valid.js.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (10)
.changeset/dull-yaks-poke.md(1 hunks)crates/biome_js_analyze/src/lint/nursery/no_rethrow_without_cause.rs(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/invalid.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.invalid.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.invalid.options.json(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.valid.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.valid.options.json(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/valid.js(1 hunks)crates/biome_rule_options/src/lib.rs(1 hunks)crates/biome_rule_options/src/no_rethrow_without_cause.rs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.valid.js
🚧 Files skipped from review as they are similar to previous changes (3)
- crates/biome_rule_options/src/no_rethrow_without_cause.rs
- crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/valid.js
- .changeset/dull-yaks-poke.md
🧰 Additional context used
📓 Path-based instructions (4)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.valid.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.invalid.options.jsoncrates/biome_js_analyze/src/lint/nursery/no_rethrow_without_cause.rscrates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.invalid.jscrates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/invalid.js
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.valid.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.invalid.options.jsoncrates/biome_js_analyze/src/lint/nursery/no_rethrow_without_cause.rscrates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.invalid.jscrates/biome_rule_options/src/lib.rscrates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/invalid.js
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.valid.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.invalid.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.invalid.jscrates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/invalid.js
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format all Rust source files before committing (just f)
Files:
crates/biome_js_analyze/src/lint/nursery/no_rethrow_without_cause.rscrates/biome_rule_options/src/lib.rs
🧠 Learnings (8)
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/crates/**/tests/specs/**/*.jsonc : Snapshot test `.jsonc` files must contain an array of code snippets (strings); these run in script mode (no ESM syntax)
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.valid.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.invalid.options.json
📚 Learning: 2025-08-11T11:48:27.774Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:27.774Z
Learning: Applies to crates/biome_formatter/biome_html_formatter/tests/specs/html/**/options.json : When non-default formatting options are needed for a test group, place an options.json (biome.json format) alongside the .html files in that folder
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.valid.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.invalid.options.json
📚 Learning: 2025-08-05T14:43:29.581Z
Learnt from: dyc3
PR: biomejs/biome#7081
File: packages/@biomejs/biome/configuration_schema.json:7765-7781
Timestamp: 2025-08-05T14:43:29.581Z
Learning: The file `packages/biomejs/biome/configuration_schema.json` is auto-generated and should not be manually edited or reviewed for schema issues; any changes should be made at the code generation source.
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.valid.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.invalid.options.json
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : In declare_lint_rule! macros, set `version: "next"` for new or updated rules
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_rethrow_without_cause.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : When deprecating a rule, add `deprecated: "<reason>"` to `declare_lint_rule!`
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_rethrow_without_cause.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/{lint,assist}/**/*.rs : When banning globals (e.g., `noConsoleLog`), check the semantic model to avoid false positives from locally shadowed bindings
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_rethrow_without_cause.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : If a rule returns a code action (implements `action`), add `fix_kind` in `declare_lint_rule!` and use `ctx.metadata().applicability()` when building the action
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_rethrow_without_cause.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/crates/biome_rule_options/lib/**/*.rs : Define per-rule options types in `biome_rule_options` crate (one file per rule, e.g., `lib/use_my_rule.rs`)
Applied to files:
crates/biome_rule_options/src/lib.rs
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/nursery/no_rethrow_without_cause.rs (1)
crates/biome_analyze/src/rule.rs (3)
recommended(554-557)sources(569-572)same(241-246)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (25)
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_package)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Documentation
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Check Dependencies
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Test Node.js API
- GitHub Check: Check JS Files
- GitHub Check: autofix
🔇 Additional comments (3)
crates/biome_js_analyze/tests/specs/nursery/noRethrowWithoutCause/requireCatchParameter.invalid.options.json (1)
9-9: Set the invalid config back to requireCatchParameter true.
With this flag stuck on false, the “invalid” suite never actually tests the stricter catch-parameter requirement, so regressions can wander in unchallenged. Let’s flip it back to true so the unhappy path does its job.- "requireCatchParameter": false + "requireCatchParameter": truecrates/biome_js_analyze/src/lint/nursery/no_rethrow_without_cause.rs (2)
56-62: LGTM! Well-structured rule declarationGood use of the
declare_lint_rule!macro with appropriate metadata. The ESLint source reference is properly configured.Based on learnings
115-118: Direct rethrow still slips past the ruleWhen the catch block does a plain
throw err;, we're returningNoneand not flagging it as a violation. This means direct rethrows go undetected, which defeats the purpose of the lint rule. We should emitState::WithoutCausefor any non-newexpression instead of bailing out.Apply this fix to catch direct rethrows:
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; + return Some(State::WithoutCause(throw_statement.range())); };
|
Personally I would suggest |
kedevked
force-pushed
the
feature/add-no-rethrow-without-cause
branch
from
e0d5184 to
03285cd
Compare
kedevked
changed the title
Done |
I concur, that's better than the ones i suggested in the original issue |
ematipico
left a comment
ematipico
left a comment
There was a problem hiding this comment.
The implementation looks good, however, the documentation and tests need some work. I pointed out the areas to address
.changeset/dull-yaks-poke.md
Outdated
| --- | ||
| "@biomejs/biome": patch | ||
| --- | ||
| Added the lint rule `useErrorCause`. |
There was a problem hiding this comment.
| Added the lint rule `useErrorCause`. | |
| Added the nursery lint rule `useErrorCause`. |
.changeset/dull-yaks-poke.md
Outdated
| - `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 |
There was a problem hiding this comment.
| #### Invalid | |
| **Invalid examples**: |
.changeset/dull-yaks-poke.md
Outdated
| } | ||
| ``` | ||
|
|
||
| #### Valid |
There was a problem hiding this comment.
| #### Valid | |
| **Valid examples:** |
.changeset/dull-yaks-poke.md
Outdated
| } | ||
| ``` | ||
|
|
||
| #### `requireCatchParameter: false` |
There was a problem hiding this comment.
| #### `requireCatchParameter: false` | |
| **Valid example** when `requireCatchParameter` is `false`: |
| /// ``` | ||
| /// |
There was a problem hiding this comment.
The documentation doesn't explain and test the option requireCatchParameter. See the format for options: https://github.com/biomejs/biome/blob/main/crates/biome_analyze/CONTRIBUTING.md#general-structure
| @@ -0,0 +1,13 @@ | |||
|
|
|||
There was a problem hiding this comment.
Notice that this doesn't emit any diagnostics. Add /* should emit diagnostics */ at the beginning of the file and fix the issue
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
crates/biome_js_analyze/src/lint/nursery/use_error_cause.rs (1)
161-164: Critical: Direct rethrows are not being flagged.The code returns
Nonefor non-newexpressions, which means direct rethrows likethrow err;won't emit diagnostics. This contradicts the rule's core purpose: ensuring errors are wrapped with acause.A past review comment indicated this was addressed in commit fe19426, but the current code still has the problematic pattern. The test file
requireCatchParameter.invalid.jsexpects these cases to emit diagnostics, creating an inconsistency.Apply this diff to flag direct rethrows:
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; + return Some(State::WithoutCause(throw_statement.range())); };crates/biome_js_analyze/tests/specs/nursery/useErrorCause/requireCatchParameter.invalid.js (1)
1-1: Minor wording inconsistency.The comment says "should generate diagnostics", but the past review comment requested "should emit diagnostics". Whilst functionally equivalent, consistency with the requested wording would be clearer.
Apply this diff:
-/* should generate diagnostics */ +/* should emit diagnostics */
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_js_analyze/tests/specs/nursery/useErrorCause/invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useErrorCause/requireCatchParameter.invalid.js.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (4)
.changeset/dull-yaks-poke.md(1 hunks)crates/biome_js_analyze/src/lint/nursery/use_error_cause.rs(1 hunks)crates/biome_js_analyze/tests/specs/nursery/useErrorCause/invalid.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/useErrorCause/requireCatchParameter.invalid.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/biome_js_analyze/tests/specs/nursery/useErrorCause/invalid.js
- .changeset/dull-yaks-poke.md
🧰 Additional context used
📓 Path-based instructions (5)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/src/lint/nursery/use_error_cause.rscrates/biome_js_analyze/tests/specs/nursery/useErrorCause/requireCatchParameter.invalid.js
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/src/lint/nursery/use_error_cause.rscrates/biome_js_analyze/tests/specs/nursery/useErrorCause/requireCatchParameter.invalid.js
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Before committing, format Rust and TOML files (e.g., via
just f/just format)
Files:
crates/biome_js_analyze/src/lint/nursery/use_error_cause.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Document rules, assists, and options via inline rustdoc in Rust source
Files:
crates/biome_js_analyze/src/lint/nursery/use_error_cause.rs
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_js_analyze/tests/specs/nursery/useErrorCause/requireCatchParameter.invalid.js
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/nursery/use_error_cause.rs (1)
packages/@biomejs/backend-jsonrpc/src/workspace.ts (1)
UseErrorCauseOptions(8253-8258)
🔇 Additional comments (1)
crates/biome_js_analyze/src/lint/nursery/use_error_cause.rs (1)
64-100: Excellent documentation of therequireCatchParameteroption.The option is now thoroughly documented with clear examples showing both the default behaviour and how to disable it. This addresses the previous review feedback.
| try { | ||
| throw new Error("Original error"); | ||
| } catch (err) { | ||
| throw err; | ||
| } | ||
|
|
||
|
|
||
| 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; | ||
| } | ||
| } |
There was a problem hiding this comment.
Test expectations don't match implementation behaviour.
Lines 6 and 28 contain direct rethrows (throw err;), which the test expects to flag. However, the current implementation at lines 161-164 of use_error_cause.rs returns None for non-new expressions, so these cases won't emit diagnostics.
This contradicts both the rule's purpose and a past review comment that claimed this was fixed in commit fe19426. Either the implementation needs fixing (which it does), or these test cases should be moved to a valid file.
kedevked
force-pushed
the
feature/add-no-rethrow-without-cause
branch
from
dced3c4 to
4183114
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/biome_js_analyze/src/lint/nursery/use_error_cause.rs (1)
11-15: Consider clarifying the rule description.The phrasing "Disallow rethrowing caught errors without wrapping them" might suggest direct rethrows (
throw err;) are forbidden, but the rule actually only checks wrapped errors. Consider: "When rethrowing a caught error by wrapping it in a new Error, ensure the original error is preserved using thecauseproperty."
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/biome_js_analyze/src/lint/nursery/use_error_cause.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/src/lint/nursery/use_error_cause.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/src/lint/nursery/use_error_cause.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Before committing, format Rust and TOML files (e.g., via
just f/just format)
Files:
crates/biome_js_analyze/src/lint/nursery/use_error_cause.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Document rules, assists, and options via inline rustdoc in Rust source
Files:
crates/biome_js_analyze/src/lint/nursery/use_error_cause.rs
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/nursery/use_error_cause.rs (1)
packages/@biomejs/backend-jsonrpc/src/workspace.ts (1)
UseErrorCauseOptions(8273-8278)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: Documentation
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: End-to-end tests
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Test Node.js API
- GitHub Check: Check JS Files
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_js_analyze)
🔇 Additional comments (8)
crates/biome_js_analyze/src/lint/nursery/use_error_cause.rs (8)
1-9: Imports look good.Clean set of imports for the rule implementation.
124-133: State enum is well-designed.Clear variants for each diagnostic scenario, with appropriate range tracking.
152-168: Scope checking logic is sound.Correctly identifies nested functions, methods, and classes to avoid false positives when throw statements are in independent error contexts.
178-181: Direct rethrow handling is correct.Returning
Nonefor direct rethrows (throw err;) is appropriate, as this rule focuses on wrapped errors. The inline comment clearly explains the intent.
202-230: Property iteration handles edge cases defensively.The loop continues when finding an incorrect
causeto check for potential duplicates, which prevents crashes on malformed code.
241-247: Option handling is correct.The
requireCatchParametercheck properly gates theNoErrorBindingdiagnostic, allowing empty catch blocks when the option is disabled.
250-293: Diagnostics are clear and helpful.The messages guide users to fix the issue, and
ShadowedCausehelpfully shows where the original binding is declared.
301-324: Binding validation logic is solid and edge-case tests cover botherr.message(property-access) and mismatched identifier (error) scenarios.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/biome_js_analyze/src/lint/nursery/use_error_cause.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/src/lint/nursery/use_error_cause.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/src/lint/nursery/use_error_cause.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Before committing, format Rust and TOML files (e.g., via
just f/just format)
Files:
crates/biome_js_analyze/src/lint/nursery/use_error_cause.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Document rules, assists, and options via inline rustdoc in Rust source
Files:
crates/biome_js_analyze/src/lint/nursery/use_error_cause.rs
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/nursery/use_error_cause.rs (1)
packages/@biomejs/backend-jsonrpc/src/workspace.ts (1)
UseErrorCauseOptions(8273-8278)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Documentation
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Check JS Files
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Check Dependencies
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Test Node.js API
- GitHub Check: autofix
🔇 Additional comments (5)
crates/biome_js_analyze/src/lint/nursery/use_error_cause.rs (5)
1-9: LGTM – imports are appropriate.Standard dependencies for a semantic lint rule.
10-121: Well-documented rule with clear examples.The option documentation (lines 76-112) properly addresses the earlier review feedback.
123-132: State enum variants are appropriate.
177-180: Verify whether direct rethrows should be flagged.A past review comment (marked as addressed in commit fe19426) indicated that
throw err;should returnState::WithoutCauserather thanNone, yet the current code still returnsNone. This means direct rethrows go unflagged.If the rule's intent is to require wrapping all rethrows (not just detecting missing cause in wrapped throws), this is a critical regression. If direct rethrows are intentionally allowed, the past review comment was incorrect.
Could you clarify the intended behaviour and verify whether the fix was inadvertently reverted?
249-292: Diagnostic messages are clear and helpful.
ematipico
force-pushed
the
feature/add-no-rethrow-without-cause
branch
from
946f9cf to
4511120
Compare
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>
4 participants
Summary
This Pull Request introduces a new lint rule,
noRethrowWithoutCause(based on ESLint'spreserve-caught-error, which discourages rethrowing a caught error without properly wrapping it in a newErrorobject and providing the original error as thecauseproperty.Addresses initial proposal and discussion from: #7469