fix(lint): don't trigger useJsxKeyInIterable when iterating on non-…

…jsx things
biomejs · May 1, 2024 · 0f86ef9 · 0f86ef9
1 parent 5fda633
commit 0f86ef9
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 8 deletions.
diff --git a/crates/biome_js_analyze/src/lint/correctness/use_jsx_key_in_iterable.rs b/crates/biome_js_analyze/src/lint/correctness/use_jsx_key_in_iterable.rs
@@ -133,6 +133,7 @@ fn handle_collections(
             let node = AnyJsExpression::cast(node.into_syntax())?;
             handle_potential_react_component(node, model, is_inside_jsx)
         })
+        .flatten()
         .collect()
 }
 
@@ -198,6 +199,7 @@ fn handle_iterators(
                     let returned_value = statement.argument()?;
                     handle_potential_react_component(returned_value, model, is_inside_jsx)
                 })
+                .flatten()
                 .collect::<Vec<_>>();
 
             Some(res)
@@ -207,7 +209,6 @@ fn handle_iterators(
             match body {
                 AnyJsFunctionBody::AnyJsExpression(expr) => {
                     handle_potential_react_component(expr, model, is_inside_jsx)
-                        .map(|state| vec![state])
                 }
                 AnyJsFunctionBody::JsFunctionBody(body) => {
                     let res = body
@@ -218,6 +219,7 @@ fn handle_iterators(
                             let returned_value = statement.argument()?;
                             handle_potential_react_component(returned_value, model, is_inside_jsx)
                         })
+                        .flatten()
                         .collect::<Vec<_>>();
                     Some(res)
                 }
@@ -231,19 +233,42 @@ fn handle_potential_react_component(
     node: AnyJsExpression,
     model: &SemanticModel,
     is_inside_jsx: bool,
-) -> Option<UseJsxKeyInIterableState> {
+) -> Option<Vec<UseJsxKeyInIterableState>> {
     let node = unwrap_parenthesis(node)?;
+    dbg!(&node, is_inside_jsx);
+
     if is_inside_jsx {
+        match node {
+            AnyJsExpression::JsBinaryExpression(_) => {
+                return None;
+            }
+            AnyJsExpression::JsConditionalExpression(node) => {
+                let results = [node.consequent().ok()?, node.alternate().ok()?]
+                    .into_iter()
+                    .flat_map(|node| handle_potential_react_component(node, model, is_inside_jsx))
+                    .flatten()
+                    .collect::<Vec<_>>();
+
+                return if !results.is_empty() {
+                    Some(results)
+                } else {
+                    None
+                };
+            }
+            _ => {}
+        }
         if let Some(node) = ReactComponentExpression::cast_ref(node.syntax()) {
             let range = handle_react_component(node, model)?;
-            Some(UseJsxKeyInIterableState::MissingKeyProps(range))
+            Some(vec![UseJsxKeyInIterableState::MissingKeyProps(range)])
         } else {
-            Some(UseJsxKeyInIterableState::CantDetermineJSXProp(node.range()))
+            Some(vec![UseJsxKeyInIterableState::CantDetermineJSXProp(
+                node.range(),
+            )])
         }
     } else {
         let range =
             handle_react_component(ReactComponentExpression::cast_ref(node.syntax())?, model)?;
-        Some(UseJsxKeyInIterableState::MissingKeyProps(range))
+        Some(vec![UseJsxKeyInIterableState::MissingKeyProps(range)])
     }
 }
 

diff --git a/crates/biome_js_analyze/tests/specs/correctness/useJsxKeyInIterable/invalid.jsx.snap b/crates/biome_js_analyze/tests/specs/correctness/useJsxKeyInIterable/invalid.jsx.snap
@@ -542,14 +542,33 @@ invalid.jsx:33:19 lint/correctness/useJsxKeyInIterable ━━━━━━━━
 ```
 
 ```
-invalid.jsx:33:24 lint/correctness/useJsxKeyInIterable ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+invalid.jsx:33:29 lint/correctness/useJsxKeyInIterable ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Missing key property for this element in iterable.
+  
+    31 │ (<h1>{[<h1></h1>, <h1></h1>, <h1></h1>]}</h1>)
+    32 │ 
+  > 33 │ (<h1>{[<h1></h1>, xyz, abc? <h2></h2>: bcd]}</h1>)
+       │                             ^^^^
+    34 │ 
+    35 │ (<h1>{data.map(c => <h1></h1>)}</h1>)
+  
+  i The order of the items may change, and having a key can help React identify which item was moved.
+  
+  i Check the React documentation. 
+  
+
+```
+
+```
+invalid.jsx:33:40 lint/correctness/useJsxKeyInIterable ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
   ! Cannot determine whether this child has the required key prop.
   
     31 │ (<h1>{[<h1></h1>, <h1></h1>, <h1></h1>]}</h1>)
     32 │ 
   > 33 │ (<h1>{[<h1></h1>, xyz, abc? <h2></h2>: bcd]}</h1>)
-       │                        ^^^^^^^^^^^^^^^^^^^
+       │                                        ^^^
     34 │ 
     35 │ (<h1>{data.map(c => <h1></h1>)}</h1>)
   

diff --git a/crates/biome_js_analyze/tests/specs/correctness/useJsxKeyInIterable/valid.jsx b/crates/biome_js_analyze/tests/specs/correctness/useJsxKeyInIterable/valid.jsx
@@ -45,4 +45,10 @@ React.Children.map(c => React.cloneElement(c, {key: c}));
 
 (<h1>{data.map(c => (<h1 key={c}></h1>))}</h1>)
 
-(<h1>{data.map(c => {return (<h1 key={c}></h1>)})}</h1>)
+(<h1>{data.map(c => {return (<h1 key={c}></h1>)})}</h1>)
+
+<>{data.reduce((total, next) => total + next, 0)}</>
+
+<>{data.reduce((a, b) => Math.max(a, b), 0)}</>
+
+<>{data.reduce((a, b) => a > b ? a : b, 0)}</>
diff --git a/crates/biome_js_analyze/tests/specs/correctness/useJsxKeyInIterable/valid.jsx.snap b/crates/biome_js_analyze/tests/specs/correctness/useJsxKeyInIterable/valid.jsx.snap
@@ -52,4 +52,11 @@ React.Children.map(c => React.cloneElement(c, {key: c}));
 (<h1>{data.map(c => (<h1 key={c}></h1>))}</h1>)
 
 (<h1>{data.map(c => {return (<h1 key={c}></h1>)})}</h1>)
+
+<>{data.reduce((total, next) => total + next, 0)}</>
+
+<>{data.reduce((a, b) => Math.max(a, b), 0)}</>
+
+<>{data.reduce((a, b) => a > b ? a : b, 0)}</>
+
 ```