feat(rome_js_analyze): noUselessCatch

crates/rome_diagnostics_categories/src/categories.rs

@@ -106,6 +106,7 @@ define_dategories! {
     "lint/nursery/noGlobalObjectCalls": "https://docs.rome.tools/lint/rules/noGlobalObjectCalls",
     "lint/nursery/noPrototypeBuiltins": "https://docs.rome.tools/lint/rules/noPrototypeBuiltins",
     "lint/nursery/noSvgWithoutTitle": "https://docs.rome.tools/lint/rules/noSvgWithoutTitle",
+    "lint/nursery/noUselessCatch": "https://docs.rome.tools/lint/rules/noUselessCatch",
     // Insert new nursery rule here
 
     // performance

crates/rome_js_analyze/src/analyzers/nursery/no_useless_catch.rs

@@ -0,0 +1,116 @@
+use rome_analyze::{context::RuleContext, declare_rule, Ast, Rule, RuleDiagnostic};
+use rome_console::markup;
+use rome_js_syntax::{JsCatchClause, TextRange};
+use rome_rowan::{AstNode, AstNodeList};
+
+declare_rule! {
+    /// Disallow unnecessary `catch` clauses.
+    ///
+    /// A `catch` clause that only rethrows the original error is redundant,
+    /// and has no effect on the runtime behavior of the program.
+    /// These redundant clauses can be a source of confusion and code bloat,
+    /// so it’s better to disallow these unnecessary `catch` clauses.
+    ///
+    /// Source: https://eslint.org/docs/latest/rules/no-useless-catch
+    ///
+    /// ## Examples
+    ///
+    /// ### Invalid
+    ///
+    /// ```js,expect_diagnostic
+    /// try {
+    ///     doSomething();
+    /// } catch(e) {
+    ///     throw e;
+    /// }
+    /// ```
+    /// ```js,expect_diagnostic
+    /// try {
+    ///     doSomething();
+    /// } catch(e) {
+    ///     throw e;
+    /// } finally {
+    ///     doCleanUp();
+    /// }
+    /// ```
+    /// ## Valid
+    ///
+    /// ```js
+    /// try {
+    ///     doSomething();
+    /// } catch(e) {
+    ///     doSomethingWhenCatch();
+    ///     throw e;
+    /// }
+    /// ```
+    ///
+    /// ```js
+    /// try {
+    ///     doSomething();
+    /// } catch(e) {
+    ///     handleError(e);
+    /// }
+    /// ```
+    ///
+    pub(crate) NoUselessCatch {
+        version: "next",
+        name: "noUselessCatch",
+        recommended: true,
+    }
+}
+
+impl Rule for NoUselessCatch {
+    type Query = Ast<JsCatchClause>;
+    type State = TextRange;
+    type Signals = Option<Self::State>;
+    type Options = ();
+
+    fn run(ctx: &RuleContext<Self>) -> Self::Signals {
+        let binding = ctx.query();
+
+        let catch_body = binding.body().ok()?;
+        let body_statements = catch_body.statements();
+
+        // We need guarantees that body_statements has only one `throw` statement.
+        if body_statements.len() != 1 {
+            return None;
+        }
+
+        let catch_declaration = binding.declaration()?;
+        let catch_binding_err = catch_declaration
+            .binding()
+            .ok()?
+            .as_any_js_binding()?
+            .as_js_identifier_binding()?
+            .name_token()
+            .ok()?;
+        let catch_err_name = catch_binding_err.text();
+
+        let first_statement = body_statements.first()?;
+        let js_throw_statement = first_statement.as_js_throw_statement()?;
+        let throw_ident = js_throw_statement
+            .argument()
+            .ok()?
+            .as_js_identifier_expression()?
+            .text();
+
+        if throw_ident.eq(catch_err_name) {
+            Some(js_throw_statement.syntax().text_trimmed_range())
+        } else {
+            None
+        }
+    }
+
+    fn diagnostic(_: &RuleContext<Self>, text_range: &Self::State) -> Option<RuleDiagnostic> {
+        Some(
+            RuleDiagnostic::new(
+                rule_category!(),
+                text_range,
+                markup!("The "<Emphasis>"catch"</Emphasis>" clause that only rethrows the original error is redundant."),
+            )
+            .note(markup!(
+                "These unnecessary "<Emphasis>"catch"</Emphasis>" clauses can be confusing. It is recommended to remove them."
+            )),
+        )
+    }
+}

crates/rome_js_analyze/tests/specs/nursery/noUselessCatch/invalid.js

@@ -0,0 +1,13 @@
+try {
+	doSomethingThatMightThrow();
+} catch (e) {
+	throw e;
+}
+
+try {
+	doSomethingThatMightThrow();
+} catch (e) {
+	throw e;
+} finally {
+	cleanUp();
+}

crates/rome_js_analyze/tests/specs/nursery/noUselessCatch/invalid.js.snap

@@ -0,0 +1,58 @@
+---
+source: crates/rome_js_analyze/tests/spec_tests.rs
+expression: invalid.js
+---
+# Input
+```js
+try {
+	doSomethingThatMightThrow();
+} catch (e) {
+	throw e;
+}
+
+try {
+	doSomethingThatMightThrow();
+} catch (e) {
+	throw e;
+} finally {
+	cleanUp();
+}
+
+```
+
+# Diagnostics
+```
+invalid.js:4:2 lint/nursery/noUselessCatch ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! The catch clause that only rethrows the original error is redundant.
+
+    2 │ 	doSomethingThatMightThrow();
+    3 │ } catch (e) {
+  > 4 │ 	throw e;
+      │ 	^^^^^^^^
+    5 │ }
+    6 │ 
+
+  i These unnecessary catch clauses can be confusing. It is recommended to remove them.
+
+
+```
+
+```
+invalid.js:10:2 lint/nursery/noUselessCatch ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! The catch clause that only rethrows the original error is redundant.
+
+     8 │ 	doSomethingThatMightThrow();
+     9 │ } catch (e) {
+  > 10 │ 	throw e;
+       │ 	^^^^^^^^
+    11 │ } finally {
+    12 │ 	cleanUp();
+
+  i These unnecessary catch clauses can be confusing. It is recommended to remove them.
+
+
+```
+
+

crates/rome_js_analyze/tests/specs/nursery/noUselessCatch/valid.js

@@ -0,0 +1,8 @@
+/* should not generate diagnostics */
+
+try {
+	doSomethingThatMightThrow();
+} catch (e) {
+	doSomethingBeforeRethrow();
+	throw e;
+}

crates/rome_js_analyze/tests/specs/nursery/noUselessCatch/valid.js.snap

@@ -0,0 +1,18 @@
+---
+source: crates/rome_js_analyze/tests/spec_tests.rs
+expression: valid.js
+---
+# Input
+```js
+/* should not generate diagnostics */
+
+try {
+	doSomethingThatMightThrow();
+} catch (e) {
+	doSomethingBeforeRethrow();
+	throw e;
+}
+
+```
+
+