chore: add fix action, improve diagnostics, add more tests

Author: mdevils

crates/biome_configuration/src/analyzer/linter/rules.rs

@@ -1,8 +1,15 @@
+use crate::JsRuleAction;
 use crate::services::semantic::Semantic;
-use biome_analyze::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule};
+use biome_analyze::{
+    FixKind, Rule, RuleAction, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule,
+};
 use biome_console::markup;
-use biome_js_syntax::{JsCallExpression, global_identifier};
-use biome_rowan::{AstNode, AstSeparatedList, TextRange};
+use biome_js_factory::make;
+use biome_js_syntax::{
+    AnyJsCallArgument, AnyJsExpression, AnyJsObjectMember, JsCallArgumentList, JsCallExpression,
+    JsLanguage, JsSyntaxKind, T, global_identifier,
+};
+use biome_rowan::{AstNode, AstSeparatedList, BatchMutationExt, TextRange};
 
 declare_lint_rule! {
     /// Prefer object spread over `Object.assign()` when constructing new objects.
@@ -54,12 +61,13 @@ declare_lint_rule! {
             RuleSource::Eslint("prefer-object-spread"),
         ],
         recommended: false,
+        fix_kind: FixKind::Safe,
     }
 }
 
 impl Rule for UseObjectSpread {
     type Query = Semantic<JsCallExpression>;
-    type State = TextRange;
+    type State = RuleState;
     type Signals = Option<Self::State>;
     type Options = ();
 
@@ -76,33 +84,112 @@ impl Rule for UseObjectSpread {
         }
 
         let method = member_expr.member().ok()?;
-        if method.value_token().ok()?.text() != "assign" {
+        if method.value_token().ok()?.text_trimmed() != "assign" {
             return None;
         }
 
-        let args = node.arguments().ok()?;
-        let first_arg = args.args().first()?.ok()?;
+        let args = node.arguments().ok()?.args();
+
+        let first_arg = args.first()?.ok()?;
+
         let expression = first_arg.as_any_js_expression()?;
+        if !matches!(expression, AnyJsExpression::JsObjectExpression(_)) {
+            return None;
+        }
 
-        expression
-            .as_js_object_expression()
-            .and(Some(member_expr.range()))
+        if args
+            .iter()
+            .skip(1)
+            .any(|arg| matches!(arg, Ok(AnyJsCallArgument::JsSpread(_))))
+        {
+            // If there are any spread arguments, we cannot convert to object spread
+            return None;
+        }
+
+        Some(RuleState {
+            member_expr_range: member_expr.range(),
+            args,
+        })
     }
 
     fn diagnostic(_ctx: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
-        Some(
-            RuleDiagnostic::new(
-                rule_category!(),
-                state,
-                markup! {
-                    "Object spread should be used instead of "<Emphasis>"Object.assign"</Emphasis>
-                    " when constructing new objects."
-                },
-            )
-            .note(markup! {
+        Some(RuleDiagnostic::new(
+            rule_category!(),
+            state.member_expr_range,
+            markup! {
+                "Object spread syntax is more concise, readable, and performs better"
+                " than "<Emphasis>"Object.assign"</Emphasis>"."
+            },
+        ))
+    }
+
+    fn action(ctx: &RuleContext<Self>, state: &Self::State) -> Option<RuleAction<JsLanguage>> {
+        let args = &state.args;
+        let mut object_members = Vec::new();
+        let mut object_member_separators = Vec::new();
+
+        for arg in args.iter().flatten() {
+            let AnyJsCallArgument::AnyJsExpression(expression) = arg else {
+                return None;
+            };
+            match expression {
+                // Flatten nested object expressions.
+                AnyJsExpression::JsObjectExpression(object_expr) => {
+                    let object_member_list = object_expr.members();
+                    let mut separators = object_member_list
+                        .separators()
+                        .flatten()
+                        .collect::<Vec<_>>();
+                    let members = object_member_list.iter().flatten().collect::<Vec<_>>();
+                    // Keep original separators to preserve comments and whitespace.
+                    separators.resize_with(members.len(), || make::token(T![,]));
+                    object_member_separators.extend(separators);
+                    object_members.extend(members);
+                }
+                // All the other expressions will be spreaded.
+                _ => {
+                    object_members.push(AnyJsObjectMember::JsSpread(make::js_spread(
+                        make::token(JsSyntaxKind::DOT3),
+                        expression,
+                    )));
+                    object_member_separators.push(make::token(T![,]));
+                }
+            }
+        }
+        let mut mutation = ctx.root().begin();
+        // Building the final object expression.
+        // Formatter should be able to remove unnecessary trailing comma depending on configuration.
+        let result_object = make::js_object_expression(
+            make::token(T!['{']),
+            make::js_object_member_list(object_members, object_member_separators),
+            make::token(T!['}']),
+        );
+
+        mutation.replace_node(
+            AnyJsExpression::JsCallExpression(ctx.query().clone()),
+            // Wrap into parens in case we are in a statement expression or arrow function body.
+            // Formatter should be able to remove unnecessary parens.
+            AnyJsExpression::JsParenthesizedExpression(make::js_parenthesized_expression(
+                make::token(T!['(']),
+                AnyJsExpression::JsObjectExpression(result_object.clone()),
+                make::token(T![')']),
+            )),
+        );
+
+        Some(JsRuleAction::new(
+            ctx.metadata().action_category(ctx.category(), ctx.group()),
+            ctx.metadata().applicability(),
+            markup! {
                 "Replace "<Emphasis>"Object.assign({...}, <object>)"</Emphasis>
                 " with "<Emphasis>"{ ...<object> }"</Emphasis>"."
-            }),
-        )
+            }
+            .to_owned(),
+            mutation,
+        ))
     }
 }
+
+pub struct RuleState {
+    member_expr_range: TextRange,
+    args: JsCallArgumentList,
+}

crates/biome_js_analyze/src/lint/nursery/use_object_spread.rs

@@ -1,7 +1,44 @@
 Object.assign({}, foo);
+Object.assign  ({}, foo);
+Object  .assign  ({}, foo);
 Object.assign({}, {foo: 'bar'});
 Object.assign({ foo: 'bar'}, baz);
 Object.assign({}, baz, { foo: 'bar' });
+Object.assign({}, { foo: 'bar', baz: 'foo' });
 Object.assign({}, { ...baz });
 Object.assign({});
 Object.assign({ foo: bar });
+Object.assign({ foo: 'bar' }, cats, dogs, trees, birds);
+Object.assign({ foo: 'bar' }, Object.assign({ bar: 'foo' }, baz));
+({foo: 'bar', ...Object.assign({ bar: 'foo' }, baz)});
+Object.assign({ foo: 'bar' }, Object.assign({ bar: 'foo' }, Object.assign({}, { superNested: 'butwhy' })));
+Object.assign({foo: 'bar', ...bar}, baz);
+Object.assign({}, { foo, bar, baz });
+Object.assign({}, { [bar]: 'foo' });
+Object.assign({ ...bar }, { ...baz });
+Object.assign({ ...bar }, {
+    // this is a bar
+    foo: 'bar',
+    baz: "cats"
+});
+Object.assign({
+    boo: "lol",
+    // I'm a comment
+    dog: "cat"
+}, {
+    // this is a bar
+    foo: 'bar',
+    baz: "cats"
+});
+const test1 = Object.assign({ ...bar }, {
+    foo: 'bar', // inline comment
+    baz: "cats"
+});
+const test2 = Object.assign({ ...bar }, {
+    /**
+     * foo
+     */
+    foo: 'bar',
+    baz: "cats"
+});
+globalThis.Object.assign({}, foo);