docs(rome_js_analyze): improve noCatchAssign docs (#4262)

crates/rome_js_analyze/src/semantic_analyzers/suspicious/no_catch_assign.rs

@@ -1,12 +1,17 @@
-use crate::{semantic_services::Semantic, JsRuleAction};
+use crate::semantic_services::Semantic;
 use rome_analyze::{context::RuleContext, declare_rule, Rule, RuleDiagnostic};
 use rome_console::markup;
 use rome_js_semantic::ReferencesExtensions;
 use rome_js_syntax::{JsCatchClause, JsSyntaxNode};
 use rome_rowan::AstNode;
 
 declare_rule! {
-    /// Disallow reassigning exceptions in catch clauses
+    /// Disallow reassigning exceptions in catch clauses.
+    ///
+    /// Assignment to a `catch` parameter can be misleading and confusing.
+    /// It is often unintended and indicative of a programmer error.
+    ///
+    /// Source: https://eslint.org/docs/latest/rules/no-ex-assign
     ///
     /// ## Examples
     ///
@@ -39,10 +44,13 @@ declare_rule! {
 }
 
 impl Rule for NoCatchAssign {
-    /// Why use [JsCatchClause] instead of [JsIdentifierAssignment] ? Because this could reduce search range.
-    /// We only compare the declaration of [JsCatchClause] with all descent [JsIdentifierAssignment] of its body.
+    // Why use [JsCatchClause] instead of [JsIdentifierAssignment] ?
+    // Because this could reduce search range.
+    // We only compare the declaration of [JsCatchClause] with all descent
+    // [JsIdentifierAssignment] of its body.
     type Query = Semantic<JsCatchClause>;
-    /// The first element of `State` is the reassignment of catch parameter, the second element of `State` is the declaration of catch clause.
+    // The first element of `State` is the reassignment of catch parameter,
+    // the second element of `State` is the declaration of catch clause.
     type State = (JsSyntaxNode, JsSyntaxNode);
     type Signals = Vec<Self::State>;
     type Options = ();
@@ -54,16 +62,9 @@ impl Rule for NoCatchAssign {
         catch_clause
             .declaration()
             .and_then(|decl| {
-                // catch_binding
-                // ## Example
-                // try {
-
-                // } catch (catch_binding) {
-                //          ^^^^^^^^^^^^^
-                // }
                 let catch_binding = decl.binding().ok()?;
-                // Only [JsIdentifierBinding] is allowed to use `model.all_references` now, so I need to make sure this is
-                // a [JsIdentifierBinding].
+                // Only [JsIdentifierBinding] is allowed to use `model.all_references` now,
+                // so there's need to make sure this is a [JsIdentifierBinding].
                 let identifier_binding = catch_binding
                     .as_any_js_binding()?
                     .as_js_identifier_binding()?;
@@ -81,24 +82,21 @@ impl Rule for NoCatchAssign {
 
     fn diagnostic(_ctx: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
         let (assignment, catch_binding_syntax) = state;
-        let diagnostic = RuleDiagnostic::new(
-            rule_category!(),
-            assignment.text_trimmed_range(),
-            markup! {
-                " Do not "<Emphasis>"reassign catch parameters."</Emphasis>""
-            },
+        Some(
+            RuleDiagnostic::new(
+                rule_category!(),
+                assignment.text_trimmed_range(),
+                markup! {
+                    "Reassigning a "<Emphasis>"catch parameter"</Emphasis>" is confusing."
+                },
+            )
+            .detail(
+                catch_binding_syntax.text_trimmed_range(),
+                markup! {
+                    "The "<Emphasis>"catch parameter"</Emphasis>" is declared here:"
+                },
+            )
+            .note("Use a local variable instead."),
         )
-        .detail(
-            catch_binding_syntax.text_trimmed_range(),
-            markup! {
-                "The catch parameter is declared here"
-            },
-        );
-
-        Some(diagnostic.note("Use a local variable instead."))
-    }
-
-    fn action(_: &RuleContext<Self>, _: &Self::State) -> Option<JsRuleAction> {
-        None
     }
 }

crates/rome_js_analyze/tests/specs/suspicious/noCatchAssign.js.snap

@@ -1,5 +1,6 @@
 ---
 source: crates/rome_js_analyze/tests/spec_tests.rs
+assertion_line: 91
 expression: noCatchAssign.js
 ---
 # Input
@@ -20,15 +21,15 @@ try {
 ```
 noCatchAssign.js:2:24 lint/suspicious/noCatchAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
-  !  Do not reassign catch parameters.
+  ! Reassigning a catch parameter is confusing.
   
     1 │ // invalid
   > 2 │ try { } catch (e) { e; e = 10; }
       │                        ^
     3 │ try {
     4 │ 
   
-  i The catch parameter is declared here
+  i The catch parameter is declared here:
   
     1 │ // invalid
   > 2 │ try { } catch (e) { e; e = 10; }
@@ -44,15 +45,15 @@ noCatchAssign.js:2:24 lint/suspicious/noCatchAssign ━━━━━━━━━
 ```
 noCatchAssign.js:6:3 lint/suspicious/noCatchAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
-  !  Do not reassign catch parameters.
+  ! Reassigning a catch parameter is confusing.
   
     5 │ } catch (error) {
   > 6 │   error = 100;
       │   ^^^^^
     7 │   {
     8 │     error = 10;
   
-  i The catch parameter is declared here
+  i The catch parameter is declared here:
   
     3 │ try {
     4 │ 
@@ -69,7 +70,7 @@ noCatchAssign.js:6:3 lint/suspicious/noCatchAssign ━━━━━━━━━
 ```
 noCatchAssign.js:8:5 lint/suspicious/noCatchAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
-  !  Do not reassign catch parameters.
+  ! Reassigning a catch parameter is confusing.
   
      6 │   error = 100;
      7 │   {
@@ -78,7 +79,7 @@ noCatchAssign.js:8:5 lint/suspicious/noCatchAssign ━━━━━━━━━
      9 │   }
     10 │ }
   
-  i The catch parameter is declared here
+  i The catch parameter is declared here:
   
     3 │ try {
     4 │ 

website/src/pages/lint/rules/index.mdx

@@ -481,7 +481,7 @@ Disallows using an async function as a Promise executor.
 	<a href="/lint/rules/noCatchAssign">noCatchAssign</a>
 	<span class="recommended">recommended</span>
 </h3>
-Disallow reassigning exceptions in catch clauses
+Disallow reassigning exceptions in catch clauses.
 </section>
 <section class="rule">
 <h3 data-toc-exclude id="noCommentText">

website/src/pages/lint/rules/noCatchAssign.md

@@ -7,7 +7,12 @@ parent: lint/rules/index
 
 > This rule is recommended by Rome.
 
-Disallow reassigning exceptions in catch clauses
+Disallow reassigning exceptions in catch clauses.
+
+Assignment to a `catch` parameter can be misleading and confusing.
+It is often unintended and indicative of a programmer error.
+
+Source: https://eslint.org/docs/latest/rules/no-ex-assign
 
 ## Examples
 
@@ -24,7 +29,7 @@ try {
 
 <pre class="language-text"><code class="language-text">suspicious/noCatchAssign.js:5:3 <a href="https://docs.rome.tools/lint/rules/noCatchAssign">lint/suspicious/noCatchAssign</a> ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
-<strong><span style="color: Tomato;">  </span></strong><strong><span style="color: Tomato;">✖</span></strong> <span style="color: Tomato;"> Do not </span><span style="color: Tomato;"><strong>reassign catch parameters.</strong></span>
+<strong><span style="color: Tomato;">  </span></strong><strong><span style="color: Tomato;">✖</span></strong> <span style="color: Tomato;">Reassigning a </span><span style="color: Tomato;"><strong>catch parameter</strong></span><span style="color: Tomato;"> is confusing.</span>
 
     <strong>3 │ </strong>} catch (e) {
     <strong>4 │ </strong>  e;
@@ -33,7 +38,7 @@ try {
     <strong>6 │ </strong>}
     <strong>7 │ </strong>
 
-<strong><span style="color: rgb(38, 148, 255);">  </span></strong><strong><span style="color: rgb(38, 148, 255);">ℹ</span></strong> <span style="color: rgb(38, 148, 255);">The catch parameter is declared here</span>
+<strong><span style="color: rgb(38, 148, 255);">  </span></strong><strong><span style="color: rgb(38, 148, 255);">ℹ</span></strong> <span style="color: rgb(38, 148, 255);">The </span><span style="color: rgb(38, 148, 255);"><strong>catch parameter</strong></span><span style="color: rgb(38, 148, 255);"> is declared here:</span>
 
     <strong>1 │ </strong>try {
     <strong>2 │ </strong>