refactor(rome_js_analyze): enhance useDefaultParameterLast

CHANGELOG.md

@@ -50,6 +50,12 @@
 
 - Fix a crash in the `NoParameterAssign` rule that occurred when there was a bogus binding. [#4323](https://github.com/rome/tools/issues/4323)
 
+- Improve the diagnostic and the code action of [`useDefaultParameterLast`](https://docs.rome.tools/lint/rules/usedefaultparameterlast/).
+
+  The diagnostic now reports the last required parameter which should precede optional and default parameters.
+
+  The code action now removes any whitespace between the parameter name and its initialization.
+
 - The rules [`useExhaustiveDependencies`](https://docs.rome.tools/lint/rules/useexhaustivedependencies/) and [`useHookAtTopLevel`](https://docs.rome.tools/lint/rules/usehookattoplevel/) accept a different shape of options
 
   Old configuration
@@ -72,7 +78,7 @@
   	}
   }
   ```
-  
+
   New configuration
 
   ```json

crates/rome_js_analyze/src/analyzers/style/use_default_parameter_last.rs

@@ -8,7 +8,7 @@ use rome_rowan::{declare_node_union, AstNode, BatchMutationExt, Direction};
 use crate::JsRuleAction;
 
 declare_rule! {
-    /// Enforce default function parameters and optional parameters to be last.
+    /// Enforce default function parameters and optional function parameters to be last.
     ///
     /// Default and optional parameters that precede a required parameter cannot be omitted at call site.
     ///
@@ -60,8 +60,8 @@ declare_node_union! {
 }
 
 impl AnyFormalParameter {
-    pub(crate) fn is_optional(&self) -> bool {
-        self.question_mark_token().is_some() || self.initializer().is_some()
+    pub(crate) fn is_required(&self) -> bool {
+        self.question_mark_token().is_none() && self.initializer().is_none()
     }
 
     pub(crate) fn initializer(&self) -> Option<JsInitializerClause> {
@@ -95,19 +95,22 @@ impl Rule for UseDefaultParameterLast {
 
     fn run(ctx: &RuleContext<Self>) -> Self::Signals {
         let formal_param = ctx.query();
-        if formal_param.is_optional() {
-            let next_required_param = formal_param
-                .syntax()
-                .siblings(Direction::Next)
-                .filter_map(AnyFormalParameter::cast)
-                .find(|x| !x.is_optional());
-            next_required_param
-        } else {
-            None
+        if formal_param.is_required() {
+            return None;
         }
+        let last_required_param = formal_param
+            .syntax()
+            .siblings(Direction::Next)
+            .filter_map(AnyFormalParameter::cast)
+            .filter(|x| x.is_required())
+            .last();
+        last_required_param
     }
 
-    fn diagnostic(ctx: &RuleContext<Self>, required_param: &Self::State) -> Option<RuleDiagnostic> {
+    fn diagnostic(
+        ctx: &RuleContext<Self>,
+        last_required_param: &Self::State,
+    ) -> Option<RuleDiagnostic> {
         let formal_param = ctx.query();
         let param_kind = if formal_param.question_mark_token().is_some() {
             "optional"
@@ -118,12 +121,12 @@ impl Rule for UseDefaultParameterLast {
             rule_category!(),
             formal_param.range(),
             markup! {
-                "The "<Emphasis>{param_kind}" parameter"</Emphasis>" should follow the "<Emphasis>"required parameter"</Emphasis>" or should be a "<Emphasis>"required parameter"</Emphasis>"."
+                "This "<Emphasis>{param_kind}" parameter"</Emphasis>" should follow the last "<Emphasis>"required parameter"</Emphasis>" or should be a "<Emphasis>"required parameter"</Emphasis>"."
             },
         ).detail(
-            required_param.range(),
+            last_required_param.range(),
             markup! {
-                "The "<Emphasis>"required parameter"</Emphasis>" is here:"
+                "The last "<Emphasis>"required parameter"</Emphasis>" is here:"
             },
         ).note(
             markup! {
@@ -150,18 +153,27 @@ impl Rule for UseDefaultParameterLast {
             mutation.remove_token(question_mark);
         } else {
             let initializer = opt_param.initializer()?;
-            let first_token = initializer.syntax().first_token()?;
-            let last_token = initializer.syntax().last_token()?;
-            let prev_token = first_token.prev_token()?;
-            let new_prev_token = prev_token.with_trailing_trivia_pieces(
-                prev_token
+            let first_initializer_token = initializer.syntax().first_token()?;
+            let last_initializer_token = initializer.syntax().last_token()?;
+            let prev_initializer_token = first_initializer_token.prev_token()?;
+            let trailing_trivia_count = prev_initializer_token.trailing_trivia().pieces().count();
+            let last_trailing_non_space = prev_initializer_token
+                .trailing_trivia()
+                .pieces()
+                .rev()
+                .position(|p| !p.is_newline() && !p.is_whitespace())
+                .unwrap_or(trailing_trivia_count);
+            let new_prev_initializer_token = prev_initializer_token.with_trailing_trivia_pieces(
+                prev_initializer_token
                     .trailing_trivia()
                     .pieces()
-                    .chain(first_token.leading_trivia().pieces())
-                    .chain(last_token.trailing_trivia().pieces())
+                    .take(trailing_trivia_count - last_trailing_non_space)
+                    .chain(first_initializer_token.leading_trivia().pieces())
+                    .chain(last_initializer_token.trailing_trivia().pieces())
                     .collect::<Vec<_>>(),
             );
-            mutation.replace_token_discard_trivia(prev_token, new_prev_token);
+            mutation
+                .replace_token_discard_trivia(prev_initializer_token, new_prev_initializer_token);
             mutation.remove_node(initializer);
         }
         Some(JsRuleAction {

crates/rome_js_analyze/tests/specs/style/useDefaultParameterLast/invalid.js

@@ -4,4 +4,6 @@ export function g(a, b = 0, c) {}
 
 export function g(a, b /* before */ = /* mid */ 0/* after */) {}
 
-export function g(a, b /* before */ = /* mid */ 0 /* after */ /* after comma */, c) {}
+export function g(a, b /* before */ = /* mid */ 0 /* after */ /* after comma */, c) {}
+
+export function u(a=5, b, c) {}

crates/rome_js_analyze/tests/specs/style/useDefaultParameterLast/invalid.js.snap

@@ -1,5 +1,6 @@
 ---
 source: crates/rome_js_analyze/tests/spec_tests.rs
+assertion_line: 100
 expression: invalid.js
 ---
 # Input
@@ -11,20 +12,23 @@ export function g(a, b = 0, c) {}
 export function g(a, b /* before */ = /* mid */ 0/* after */) {}
 
 export function g(a, b /* before */ = /* mid */ 0 /* after */ /* after comma */, c) {}
+
+export function u(a=5, b, c) {}
+
 ```
 
 # Diagnostics
 ```
 invalid.js:1:19 lint/style/useDefaultParameterLast  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
-  ! The default parameter should follow the required parameter or should be a required parameter.
+  ! This default parameter should follow the last required parameter or should be a required parameter.
 
   > 1 │ export function f(a = 0, b) {}
       │                   ^^^^^
     2 │ 
     3 │ export function g(a, b = 0, c) {}
 
-  i The required parameter is here:
+  i The last required parameter is here:
 
   > 1 │ export function f(a = 0, b) {}
       │                          ^
@@ -36,14 +40,14 @@ invalid.js:1:19 lint/style/useDefaultParameterLast  FIXABLE  ━━━━━━
   i Suggested fix: Turn the parameter into a required parameter.
 
     1 │ export·function·f(a·=·0,·b)·{}
-      │                     ---       
+      │                    ----       
 
 ```
 
 ```
 invalid.js:3:22 lint/style/useDefaultParameterLast  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
-  ! The default parameter should follow the required parameter or should be a required parameter.
+  ! This default parameter should follow the last required parameter or should be a required parameter.
 
     1 │ export function f(a = 0, b) {}
     2 │ 
@@ -52,7 +56,7 @@ invalid.js:3:22 lint/style/useDefaultParameterLast  FIXABLE  ━━━━━━
     4 │ 
     5 │ export function g(a, b /* before */ = /* mid */ 0/* after */) {}
 
-  i The required parameter is here:
+  i The last required parameter is here:
 
     1 │ export function f(a = 0, b) {}
     2 │ 
@@ -66,36 +70,65 @@ invalid.js:3:22 lint/style/useDefaultParameterLast  FIXABLE  ━━━━━━
   i Suggested fix: Turn the parameter into a required parameter.
 
     3 │ export·function·g(a,·b·=·0,·c)·{}
-      │                        ---       
+      │                       ----       
 
 ```
 
 ```
 invalid.js:7:22 lint/style/useDefaultParameterLast  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
-  ! The default parameter should follow the required parameter or should be a required parameter.
+  ! This default parameter should follow the last required parameter or should be a required parameter.
 
     5 │ export function g(a, b /* before */ = /* mid */ 0/* after */) {}
     6 │ 
   > 7 │ export function g(a, b /* before */ = /* mid */ 0 /* after */ /* after comma */, c) {}
       │                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+    8 │ 
+    9 │ export function u(a=5, b, c) {}
 
-  i The required parameter is here:
+  i The last required parameter is here:
 
     5 │ export function g(a, b /* before */ = /* mid */ 0/* after */) {}
     6 │ 
   > 7 │ export function g(a, b /* before */ = /* mid */ 0 /* after */ /* after comma */, c) {}
       │                                                                                  ^
+    8 │ 
+    9 │ export function u(a=5, b, c) {}
 
   i A default parameter that precedes a required parameter cannot be omitted at call site.
 
   i Suggested fix: Turn the parameter into a required parameter.
 
-    5 5 │   export function g(a, b /* before */ = /* mid */ 0/* after */) {}
-    6 6 │   
-    7   │ - export·function·g(a,·b·/*·before·*/·=·/*·mid·*/·0·/*·after·*/·/*·after·comma·*/,·c)·{}
-      7 │ + export·function·g(a,·b·/*·before·*/··/*·after·*/·/*·after·comma·*/,·c)·{}
+    7 │ export·function·g(a,·b·/*·before·*/·=·/*·mid·*/·0·/*·after·*/·/*·after·comma·*/,·c)·{}
+      │                                     --------------                                    
+
+```
+
+```
+invalid.js:9:19 lint/style/useDefaultParameterLast  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! This default parameter should follow the last required parameter or should be a required parameter.
+
+     7 │ export function g(a, b /* before */ = /* mid */ 0 /* after */ /* after comma */, c) {}
+     8 │ 
+   > 9 │ export function u(a=5, b, c) {}
+       │                   ^^^
+    10 │ 
+
+  i The last required parameter is here:
+
+     7 │ export function g(a, b /* before */ = /* mid */ 0 /* after */ /* after comma */, c) {}
+     8 │ 
+   > 9 │ export function u(a=5, b, c) {}
+       │                           ^
+    10 │ 
+
+  i A default parameter that precedes a required parameter cannot be omitted at call site.
+
+  i Suggested fix: Turn the parameter into a required parameter.
 
+    9 │ export·function·u(a=5,·b,·c)·{}
+      │                    --          
 
 ```