fix(noUselessFragments): check Fragment when it under JSX Element or …

…Condition Expr (#4882)

.changeset/fix_fragament_4751.md

@@ -0,0 +1,26 @@
+---
+biome_analyze: patch
+---
+
+# Fix Fragment when it under JSXElement or JS Condition expr
+
+Fix [#4751](https://github.com/biomejs/biome/issues/4751) by checking fragments inside `JSXElement` and conditional expressions. For example:
+
+The Case:
+
+```jsx
+<section>
+  <>
+    <div />
+    <div />
+  </>
+</section>;
+```
+
+And:
+
+```jsx
+showFullName ? <>{fullName}</> : <>{firstName}</>;
+```
+
+It will report.

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

@@ -75,6 +75,7 @@ declare_lint_rule! {
 pub enum NoUselessFragmentsState {
     Empty,
     Child(AnyJsxChild),
+    Children(JsxChildList),
 }
 
 declare_node_union! {
@@ -128,6 +129,7 @@ impl Rule for NoUselessFragments {
         let mut in_jsx_attr_expr = false;
         let mut in_js_logical_expr = false;
         let mut in_jsx_expr = false;
+        let mut in_jsx_list = false;
         match node {
             NoUselessFragmentsQuery::JsxFragment(fragment) => {
                 let parents_where_fragments_must_be_preserved = node.syntax().parent().map_or(
@@ -158,14 +160,20 @@ impl Rule for NoUselessFragments {
                                     parent.kind(),
                                     JsSyntaxKind::JS_RETURN_STATEMENT
                                         | JsSyntaxKind::JS_INITIALIZER_CLAUSE
-                                        | JsSyntaxKind::JS_CONDITIONAL_EXPRESSION
                                         | JsSyntaxKind::JS_ARROW_FUNCTION_EXPRESSION
                                         | JsSyntaxKind::JS_FUNCTION_EXPRESSION
                                         | JsSyntaxKind::JS_FUNCTION_DECLARATION
                                         | JsSyntaxKind::JS_PROPERTY_OBJECT_MEMBER
                                 )
                             }),
-                        Err(_) => JsxAttributeInitializerClause::try_cast(parent.clone()).is_ok(),
+                        Err(_) => {
+                            if JsxChildList::try_cast(parent.clone()).is_ok() {
+                                in_jsx_list = true;
+                                false
+                            } else {
+                                JsxAttributeInitializerClause::try_cast(parent.clone()).is_ok()
+                            }
+                        }
                     },
                 );
 
@@ -240,7 +248,7 @@ impl Rule for NoUselessFragments {
                                 None
                             }
                         }
-                        _ => None,
+                        _ => in_jsx_list.then_some(NoUselessFragmentsState::Children(child_list)),
                     }
                 } else {
                     None
@@ -308,15 +316,22 @@ impl Rule for NoUselessFragments {
             .parent()
             .map_or(false, |parent| JsxChildList::can_cast(parent.kind()));
         if is_in_list {
-            let new_child = match state {
-                NoUselessFragmentsState::Empty => None,
-                NoUselessFragmentsState::Child(child) => Some(child.clone()),
-            };
-
-            if let Some(new_child) = new_child {
-                node.replace_node(&mut mutation, new_child);
-            } else {
-                node.remove_node_from_list(&mut mutation);
+            match state {
+                NoUselessFragmentsState::Child(child) => {
+                    node.replace_node(&mut mutation, child.clone());
+                }
+                NoUselessFragmentsState::Children(children) => {
+                    if let Some(old_children) = node
+                        .syntax()
+                        .parent()
+                        .and_then(|parent| JsxChildList::cast(parent.clone()))
+                    {
+                        mutation.replace_node(old_children, children.clone());
+                    }
+                }
+                _ => {
+                    node.remove_node_from_list(&mut mutation);
+                }
             }
         } else if let Some(parent) = node.parent::<JsxTagExpression>() {
             let parent = match parent.parent::<JsxExpressionAttributeValue>() {

crates/biome_js_analyze/tests/quick_test.rs

@@ -15,7 +15,7 @@ use std::{fs::read_to_string, slice};
 #[ignore]
 #[test]
 fn quick_test() {
-    let input_file = Utf8Path::new("tests/specs/complexity/noUselessFragments/issue_4553.jsx");
+    let input_file = Utf8Path::new("tests/specs/complexity/noUselessFragments/issue_4751.jsx");
     let file_name = input_file.file_name().unwrap();
 
     let (group, rule) = parse_test_path(input_file);

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

@@ -0,0 +1,8 @@
+<section>
+  <>
+    <div />
+    <div />
+  </>
+</section>;
+
+showFullName ? <>{fullName}</> : <>{firstName}</>;

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

@@ -0,0 +1,87 @@
+---
+source: crates/biome_js_analyze/tests/spec_tests.rs
+expression: issue_4751.jsx
+snapshot_kind: text
+---
+# Input
+```jsx
+<section>
+  <>
+    <div />
+    <div />
+  </>
+</section>;
+
+showFullName ? <>{fullName}</> : <>{firstName}</>;
+```
+
+# Diagnostics
+```
+issue_4751.jsx:2:3 lint/complexity/noUselessFragments  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Avoid using unnecessary Fragment.
+  
+    1 │ <section>
+  > 2 │   <>
+      │   ^^
+  > 3 │     <div />
+  > 4 │     <div />
+  > 5 │   </>
+      │   ^^^
+    6 │ </section>;
+    7 │ 
+  
+  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 │   <section>
+    2   │ - ··<>
+    3 2 │       <div />
+    4 3 │       <div />
+    5   │ - ··</>
+    6   │ - </section>;
+      4 │ + ··</section>;
+    7 5 │   
+    8 6 │   showFullName ? <>{fullName}</> : <>{firstName}</>;
+  
+
+```
+
+```
+issue_4751.jsx:8:16 lint/complexity/noUselessFragments  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Avoid using unnecessary Fragment.
+  
+    6 │ </section>;
+    7 │ 
+  > 8 │ showFullName ? <>{fullName}</> : <>{firstName}</>;
+      │                ^^^^^^^^^^^^^^^
+  
+  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
+  
+    8 │ showFullName·?·<>{fullName}</>·:·<>{firstName}</>;
+      │                ---        ----                    
+
+```
+
+```
+issue_4751.jsx:8:34 lint/complexity/noUselessFragments  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Avoid using unnecessary Fragment.
+  
+    6 │ </section>;
+    7 │ 
+  > 8 │ showFullName ? <>{fullName}</> : <>{firstName}</>;
+      │                                  ^^^^^^^^^^^^^^^^
+  
+  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
+  
+    8 │ showFullName·?·<>{fullName}</>·:·<>{firstName}</>;
+      │                                  ---         ---- 
+
+```