crates/biome_js_analyze/tests/specs/nursery/noUselessCatchBinding/invalid.js (1)

1-5: Broaden invalid coverage: underscore alias and destructuring.

Two common patterns worth checking:

• An “intentionally unused” underscore name still unused → should be reported.
• Destructured catch binding where no bound names are used → should be reported.

Proposed additions:

 try {
     // Do something
 } catch (unused) {
     // Do something
 }
+
+// underscore-named binding is still unused and should be reported
+try {
+    // Do something
+} catch (_e) {
+    // Do something
+}
+
+// destructured catch binding with unused property should be reported
+try {
+    // Do something
+} catch ({ message }) {
+    // Do something
+}

crates/biome_js_analyze/tests/specs/nursery/noUselessCatchBinding/invalid.ts (1)

1-5: Mirror JS invalid edge cases in TS.

Parity helps: underscore alias and unused destructuring.

Proposed additions:

 try {
     // Do something
 } catch (unused) {
     // Do something
 }
+
+// underscore-named binding is still unused and should be reported
+try {
+    // Do something
+} catch (_e) {
+    // Do something
+}
+
+// destructured catch binding with unused property should be reported
+try {
+    // Do something
+} catch ({ message }) {
+    // Do something
+}

crates/biome_js_analyze/tests/specs/nursery/noUselessCatchBinding/valid.ts (1)

1-13: Add positive cases: used destructuring and nested closure capture.

These ensure the rule respects real usage patterns (binding pattern usage; reference captured in closures).

Proposed additions:

 try {
     // Do something
 } catch {
     // Do something
 }
+
+// destructured catch binding with used property should be valid
+try {
+    // Do something
+} catch ({ message }) {
+    console.error(message);
+}
+
+// capturing inside a nested closure should count as usage
+try {
+    // Do something
+} catch (e) {
+    queueMicrotask(() => console.error(e));
+}

crates/biome_js_analyze/tests/specs/nursery/noUselessCatchBinding/valid.js (1)

9-13: Add two “no diagnostic” tests: closure-use and destructuring

I searched the test specs and couldn't find a destructured catch or a case where the catch binding is only used inside a nested closure. Please add these to strengthen the rule.

Files to update:

• crates/biome_js_analyze/tests/specs/nursery/noUselessCatchBinding/valid.js (and mirror to valid.ts if needed)

Suggested snippets to add:

try {
// Do something
} catch (err) {
const log = () => console.error(err);
log();
}

try {
// Do something
} catch ({ message }) {
console.log(message);
}

crates/biome_rule_options/src/no_useless_catch_binding.rs (1)

3-6: Tiny polish: document the options type

A brief doc comment helps downstream config generators and schema users.

Apply this diff:

 #[derive(Default, Clone, Debug, Deserialize, Deserializable, Eq, PartialEq, Serialize)]
 #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
 #[serde(rename_all = "camelCase", deny_unknown_fields, default)]
-pub struct NoUselessCatchBindingOptions {}
+/// Options for the `noUselessCatchBinding` rule.
+/// Currently empty; reserved for future extensions (e.g. allowlist of names).
+pub struct NoUselessCatchBindingOptions {}

crates/biome_js_analyze/src/lint/nursery/no_useless_catch_binding.rs (2)

60-64: Remove unnecessary clone

catch_binding_ident isn’t reused; the clone is redundant.

Apply this diff:

-        let catch_binding_any_ident = AnyJsIdentifierBinding::from(catch_binding_ident.clone());
+        let catch_binding_any_ident = AnyJsIdentifierBinding::from(catch_binding_ident);

---

10-41: Docs: reference the standard, not the proposal URL

Optional catch binding shipped in ES2019; linking to the proposal can age poorly. Suggest pointing to the ES2019 feature in the docstring.

Apply this diff:

-    /// This rule enforces removing unnecessary catch bindings in accordance with ECMAScript 2019.
-    /// See also: [Optional catch binding](https://tc39.es/proposal-optional-catch-binding)
+    /// This rule enforces removing unnecessary catch bindings in accordance with ECMAScript 2019.
+    /// See also: the ECMAScript 2019 “optional catch binding” feature in the language specification.

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/*.rs : For new JavaScript lint rules generated by `just new-js-lintrule`, implement the rule in the generated file under `biome_js_analyze/lib/src/lint/nursery/`

Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : When banning globals (e.g., `console.log`), consult the semantic model to avoid false positives on locally shadowed identifiers

Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/tests/specs/**/{invalid*,valid*}.{js,jsx,ts,tsx,css,graphql,jsonc} : Place snapshot test cases under `tests/specs/<group>/<ruleName>/` using files prefixed with `invalid` and `valid`

Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/*.rs : For new JavaScript lint rules generated by `just new-js-lintrule`, implement the rule in the generated file under `biome_js_analyze/lib/src/lint/nursery/`

Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/tests/quick_test.rs : Use `biome_js_analyze/tests/quick_test.rs` for quick, ad-hoc testing; un-ignore the test and adjust the rule filter as needed

Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_rule_options/lib/**/*.rs : Define per-rule options in the `biome_rule_options` crate under `lib/`, with serde- and schemars-compatible derives and `#[serde(rename_all = "camelCase", deny_unknown_fields, default)]`

Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : In `declare_lint_rule!` declarations, set `version: "next"`

Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : Use `domains` in `declare_lint_rule!` when applicable; recommended rules with domains enable only when the domain is active

Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : If a rule provides a code action, add `fix_kind` in `declare_lint_rule!` and use `ctx.action_category(ctx.category(), ctx.group())` and `ctx.metadata().applicability()` when constructing actions

crates/biome_rule_options/src/lib.rs (1)

208-210: LGTM: placement alongside no_useless_catch is sensible.

Export order/readability remain consistent with neighbouring modules.

crates/biome_js_analyze/tests/specs/nursery/noUselessCatchBinding/valid.js (1)

3-7: LGTM: positive control where the binding is used

This is a good “no diagnostic” case covering a direct reference inside the catch block.

crates/biome_js_analyze/src/lint/nursery/no_useless_catch_binding.rs (2)

86-96: Verify trivia/comment preservation in the fix

Removing the entire JsCatchDeclaration will drop any inline comments around the parameter. That’s acceptable for an unsafe fix, but let’s lock behaviour with a snapshot.

Add an invalid test like:

try {} catch (/*leading*/ unused /*trailing*/) {}

and ensure the fixed output is either:

• catch {} (comments dropped), or
• /*leading*//*trailing*/ catch {} (comments preserved),
  consistently across JS/TS.

---

57-68: Handle destructured catch bindings too
The noUselessCatchBinding rule currently only flags simple identifier bindings and will silently skip any destructured parameters (e.g. catch ({ message }) {}). I checked the spec files under
crates/biome_js_analyze/tests/specs/nursery/noUselessCatchBinding and found no tests for destructured catch bindings, so this gap isn’t covered yet.

• The code bails out at as_js_identifier_binding(), ignoring patterns.
• We should collect every AnyJsIdentifierBinding within a destructured pattern and only signal when all are unused.

Shall I draft a patch to:

1. Extract all identifiers from a binding pattern, and
2. Signal only if each one is unused?

crates/biome_js_analyze/tests/specs/nursery/noUselessCatchBinding/invalid.ts (1)

1-1: Fix file header: this is an invalid fixture

Invalid fixtures should declare that they generate diagnostics.

-// should not generate diagnostics
+// should generate diagnostics

crates/biome_js_analyze/tests/specs/nursery/noUselessCatchBinding/valid.ts (1)

3-17: Add a few more “valid” edge cases (alias, nested, rest)

These help guard against false positives with more complex patterns.

Add at the end of the file:

+try { /* ... */ } catch ({ used: alias }: any) { console.error(alias); }
+try { /* ... */ } catch ({ nested: { used } }: any) { console.error(used); }
+try { /* ... */ } catch ({ ...rest }: any) { console.error(rest); }

crates/biome_js_analyze/tests/specs/nursery/noUselessCatchBinding/valid.js (1)

3-17: Mirror a couple more “valid” JS cases (alias, nested, rest)

Parities the TS suggestions to future-proof the rule.

Append:

+try { /* ... */ } catch ({ used: alias }) { console.error(alias); }
+try { /* ... */ } catch ({ nested: { used } }) { console.error(used); }
+try { /* ... */ } catch ({ ...rest }) { console.error(rest); }

crates/biome_js_analyze/tests/specs/nursery/noUselessCatchBinding/invalid.ts (1)

3-21: Add a couple more invalid TS patterns (rest, nested alias)

Strengthens coverage for patterns that can still be entirely unused.

Consider adding:

+try { /* ... */ } catch ({ ...rest }: any) { }
+try { /* ... */ } catch ({ nested: { unused } }: any) { }
+try { /* ... */ } catch ({ used: _unused }: any) { }

crates/biome_js_analyze/src/lint/nursery/no_useless_catch_binding.rs (2)

92-101: Minor: avoid allocation when checking bindings

You can stream the descendants and early-return; no need to collect into a Vec.

-        let mut idents: Vec<AnyJsIdentifierBinding> = Vec::new();
-        for descendant in catch_binding.syntax().descendants() {
-            if let Some(ident) = AnyJsIdentifierBinding::cast(descendant) {
-                idents.push(ident);
-            }
-        }
-        if idents.iter().all(|ident| is_unused(model, ident)) {
+        let all_unused = catch_binding
+            .syntax()
+            .descendants()
+            .filter_map(AnyJsIdentifierBinding::cast)
+            .all(|ident| is_unused(model, &ident));
+        if all_unused {
             return Some(());
         }

---

104-118: Diagnostic copy is clear; consider a short suggestion hint

Optional: a one-liner hint helps users understand the next step without opening the action panel.

             RuleDiagnostic::new(
                 rule_category!(),
                 catch_declaration.range(),
                 markup! {
-                    "This "<Emphasis>"catch binding"</Emphasis>" is unused."
+                    "This "<Emphasis>"catch binding"</Emphasis>" is unused; prefer "<Emphasis>"catch { }"</Emphasis>"."
                 },
             )

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/*.rs : For new JavaScript lint rules generated by `just new-js-lintrule`, implement the rule in the generated file under `biome_js_analyze/lib/src/lint/nursery/`

Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/*.rs : For new JavaScript lint rules generated by `just new-js-lintrule`, implement the rule in the generated file under `biome_js_analyze/lib/src/lint/nursery/`

Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : In `declare_lint_rule!` declarations, set `version: "next"`

Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : Use `domains` in `declare_lint_rule!` when applicable; recommended rules with domains enable only when the domain is active

Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : If a rule provides a code action, add `fix_kind` in `declare_lint_rule!` and use `ctx.action_category(ctx.category(), ctx.group())` and `ctx.metadata().applicability()` when constructing actions

Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : When porting from other linters, declare `sources: &[RuleSource::<...>]` in `declare_lint_rule!` using `.same()` or `.inspired()` as appropriate

Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : When banning globals (e.g., `console.log`), consult the semantic model to avoid false positives on locally shadowed identifiers

Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/tests/specs/**/{invalid*,valid*}.{js,jsx,ts,tsx,css,graphql,jsonc} : Place snapshot test cases under `tests/specs/<group>/<ruleName>/` using files prefixed with `invalid` and `valid`

Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : Mark code blocks in docs with a language and required properties: use `expect_diagnostic` for invalid examples (exactly one diagnostic), `options`/`full_options` for config blocks, and `use_options` for subsequent examples

Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/tests/quick_test.rs : Use `biome_js_analyze/tests/quick_test.rs` for quick, ad-hoc testing; un-ignore the test and adjust the rule filter as needed