fix: noUselessFragments code actions (#1988)

CHANGELOG.md

@@ -436,6 +436,27 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b
 
 #### Enhancements
 
+- [noUselessFragments](https://biomejs.dev/linter/rules/no-useless-fragments/) now rule not triggered for jsx attributes when
+   the fragment child is simple text.
+
+  ```js
+  export function SomeComponent() {
+    return <div x-some-prop={<>Foo</>} />;
+  }
+  ```
+
+   Also fixes code action when the fragment child is of type `JsxExpressionChild`.
+
+  ```js
+  <>
+    <Hello leftIcon={<>{provider?.icon}</>} />
+    {<>{provider?.icon}</>}
+    <>{provider?.icon}</>
+  </>
+  ```
+
+  Contributed by @vasucp1207
+
 - [noUselessTernary](https://biomejs.dev/linter/rules/no-useless-ternary) now provides unsafe code fixes. Contributed by
   @vasucp1207
 

crates/biome_js_analyze/src/lib.rs

@@ -257,7 +257,7 @@ mod tests {
             String::from_utf8(buffer).unwrap()
         }
 
-        const SOURCE: &str = r#"<a href="class/html-css1/navigation/links#" onclick="window.location.href=index.html"> Home </a>
+        const SOURCE: &str = r#"<>{provider}</>
         "#;
 
         let parsed = parse(SOURCE, JsFileSource::tsx(), JsParserOptions::default());
@@ -270,7 +270,7 @@ mod tests {
             dependencies_index: Some(1),
             stable_result: StableHookResult::None,
         };
-        let rule_filter = RuleFilter::Rule("a11y", "useValidAnchor");
+        let rule_filter = RuleFilter::Rule("complexity", "noUselessFragments");
 
         options.configuration.rules.push_rule(
             RuleKey::new("nursery", "useHookAtTopLevel"),

crates/biome_js_analyze/src/semantic_analyzers/complexity/no_useless_fragments.rs

@@ -6,11 +6,13 @@ use biome_analyze::{declare_rule, ActionCategory, FixKind, Rule, RuleDiagnostic,
 use biome_console::markup;
 use biome_diagnostics::Applicability;
 use biome_js_factory::make::{
-    ident, js_expression_statement, js_string_literal_expression, jsx_string, jsx_tag_expression,
+    ident, js_expression_statement, js_string_literal_expression, jsx_expression_child, jsx_string,
+    jsx_tag_expression, token, JsxExpressionChildBuilder,
 };
 use biome_js_syntax::{
     AnyJsxChild, AnyJsxElementName, AnyJsxTag, JsLanguage, JsParenthesizedExpression, JsSyntaxKind,
-    JsxChildList, JsxElement, JsxExpressionAttributeValue, JsxFragment, JsxTagExpression,
+    JsxChildList, JsxElement, JsxExpressionAttributeValue, JsxFragment, JsxTagExpression, JsxText,
+    T,
 };
 use biome_rowan::{declare_node_union, AstNode, AstNodeList, BatchMutation, BatchMutationExt};
 
@@ -120,6 +122,7 @@ impl Rule for NoUselessFragments {
     fn run(ctx: &RuleContext<Self>) -> Self::Signals {
         let node = ctx.query();
         let model = ctx.model();
+        let mut in_jsx_attr_expr = false;
         match node {
             NoUselessFragmentsQuery::JsxFragment(fragment) => {
                 let parents_where_fragments_must_be_preserved = node
@@ -130,6 +133,9 @@ impl Rule for NoUselessFragments {
                             .syntax()
                             .parent()
                             .and_then(|parent| {
+                                if JsxExpressionAttributeValue::can_cast(parent.kind()) {
+                                    in_jsx_attr_expr = true;
+                                }
                                 if let Some(parenthesized_expression) =
                                     JsParenthesizedExpression::cast_ref(&parent)
                                 {
@@ -160,6 +166,9 @@ impl Rule for NoUselessFragments {
                 if !parents_where_fragments_must_be_preserved {
                     match child_list.first() {
                         Some(first) if child_list.len() == 1 => {
+                            if JsxText::can_cast(first.syntax().kind()) && in_jsx_attr_expr {
+                                return None;
+                            }
                             Some(NoUselessFragmentsState::Child(first))
                         }
                         None => Some(NoUselessFragmentsState::Empty),
@@ -222,10 +231,15 @@ impl Rule for NoUselessFragments {
         let node = ctx.query();
         let mut mutation = ctx.root().begin();
 
+        let in_jsx_attr = node.syntax().grand_parent().map_or(false, |parent| {
+            JsxExpressionAttributeValue::can_cast(parent.kind())
+        });
+
         let is_in_list = node
             .syntax()
             .parent()
             .map_or(false, |parent| JsxChildList::can_cast(parent.kind()));
+
         if is_in_list {
             let new_child = match state {
                 NoUselessFragmentsState::Empty => None,
@@ -238,7 +252,6 @@ impl Rule for NoUselessFragments {
                 node.remove_node_from_list(&mut mutation);
             }
         } else if let Some(parent) = node.parent::<JsxTagExpression>() {
-            // We need to remove {} if the fragment is inside an attribute value: <div x-some-prop={<>Foo</>} />
             let parent = match parent.parent::<JsxExpressionAttributeValue>() {
                 Some(grand_parent) => grand_parent.into_syntax(),
                 None => parent.into_syntax(),
@@ -266,9 +279,24 @@ impl Rule for NoUselessFragments {
                         }
                     }
                     AnyJsxChild::JsxExpressionChild(child) => {
-                        child.expression().map(|expression| {
-                            js_expression_statement(expression).build().into_syntax()
-                        })
+                        if in_jsx_attr
+                            || !JsxTagExpression::can_cast(node.syntax().parent()?.kind())
+                        {
+                            child.expression().map(|expression| {
+                                let jsx_expr_child =
+                                    jsx_expression_child(token(T!['{']), token(T!['}']));
+                                JsxExpressionChildBuilder::with_expression(
+                                    jsx_expr_child,
+                                    expression,
+                                )
+                                .build()
+                                .into_syntax()
+                            })
+                        } else {
+                            child.expression().map(|expression| {
+                                js_expression_statement(expression).build().into_syntax()
+                            })
+                        }
                     }
 
                     // can't apply a code action because it will create invalid syntax

crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/issue_1926.jsx

@@ -0,0 +1,5 @@
+<>
+    <Hello leftIcon={<>{provider?.icon}</>} />
+    {<>{provider?.icon}</>}
+    <>{provider?.icon}</>
+</>

crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/issue_1926.jsx.snap

@@ -0,0 +1,76 @@
+---
+source: crates/biome_js_analyze/tests/spec_tests.rs
+expression: issue_1926.jsx
+---
+# Input
+```jsx
+<>
+    <Hello leftIcon={<>{provider?.icon}</>} />
+    {<>{provider?.icon}</>}
+    <>{provider?.icon}</>
+</>
+
+```
+
+# Diagnostics
+```
+issue_1926.jsx:2:22 lint/complexity/noUselessFragments  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Avoid using unnecessary Fragment.
+
+    1 │ <>
+  > 2 │     <Hello leftIcon={<>{provider?.icon}</>} />
+      │                      ^^^^^^^^^^^^^^^^^^^^^
+    3 │     {<>{provider?.icon}</>}
+    4 │     <>{provider?.icon}</>
+
+  i A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
+
+  i Unsafe fix: Remove the Fragment
+
+    2 │ ····<Hello·leftIcon={<>{provider?.icon}</>}·/>
+      │                      ---              ----    
+
+```
+
+```
+issue_1926.jsx:3:6 lint/complexity/noUselessFragments  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Avoid using unnecessary Fragment.
+
+    1 │ <>
+    2 │     <Hello leftIcon={<>{provider?.icon}</>} />
+  > 3 │     {<>{provider?.icon}</>}
+      │      ^^^^^^^^^^^^^^^^^^^^^
+    4 │     <>{provider?.icon}</>
+    5 │ </>
+
+  i A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
+
+  i Unsafe fix: Remove the Fragment
+
+    3 │ ····{<>{provider?.icon}</>}
+      │      ---              ---- 
+
+```
+
+```
+issue_1926.jsx:4:5 lint/complexity/noUselessFragments  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Avoid using unnecessary Fragment.
+
+    2 │     <Hello leftIcon={<>{provider?.icon}</>} />
+    3 │     {<>{provider?.icon}</>}
+  > 4 │     <>{provider?.icon}</>
+      │     ^^^^^^^^^^^^^^^^^^^^^
+    5 │ </>
+    6 │ 
+
+  i A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
+
+  i Unsafe fix: Remove the Fragment
+
+    4 │ ····<>{provider?.icon}</>
+      │     --                ---
+
+```

crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/issue_4639.jsx.snap

@@ -9,30 +9,3 @@ export function SomeComponent() {
 }
 
 ```
-
-# Diagnostics
-```
-issue_4639.jsx:2:28 lint/complexity/noUselessFragments  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
-
-  ! Avoid using unnecessary Fragment.
-  
-    1 │ export function SomeComponent() {
-  > 2 │   return <div x-some-prop={<>Foo</>} />;
-      │                            ^^^^^^^^
-    3 │ }
-    4 │ 
-  
-  i A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
-  
-  i Unsafe fix: Remove the Fragment
-  
-    1 1 │   export function SomeComponent() {
-    2   │ - ··return·<div·x-some-prop={<>Foo</>}·/>;
-      2 │ + ··return·<div·x-some-prop="Foo"·/>;
-    3 3 │   }
-    4 4 │   
-  
-
-```
-
-

crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/withJsxExpressionChildValid.jsx

@@ -0,0 +1,3 @@
+function jsxExpressionChild() {
+    return <>{foo}</>
+}

crates/biome_js_analyze/tests/specs/complexity/noUselessFragments/withJsxExpressionChildValid.jsx.snap

@@ -0,0 +1,11 @@
+---
+source: crates/biome_js_analyze/tests/spec_tests.rs
+expression: withJsxExpressionChildValid.jsx
+---
+# Input
+```jsx
+function jsxExpressionChild() {
+    return <>{foo}</>
+}
+
+```

website/src/content/docs/internals/changelog.md

@@ -442,6 +442,27 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b
 
 #### Enhancements
 
+- [noUselessFragments](https://biomejs.dev/linter/rules/no-useless-fragments/) now rule not triggered for jsx attributes when
+   the fragment child is simple text.
+
+  ```js
+  export function SomeComponent() {
+    return <div x-some-prop={<>Foo</>} />;
+  }
+  ```
+
+   Also fixes code action when the fragment child is of type `JsxExpressionChild`.
+
+  ```js
+  <>
+    <Hello leftIcon={<>{provider?.icon}</>} />
+    {<>{provider?.icon}</>}
+    <>{provider?.icon}</>
+  </>
+  ```
+
+  Contributed by @vasucp1207
+
 - [noUselessTernary](https://biomejs.dev/linter/rules/no-useless-ternary) now provides unsafe code fixes. Contributed by
   @vasucp1207