From 1b630930172a047df38b72f511de10060e14f02a Mon Sep 17 00:00:00 2001
From: Carson McManus <dyc3@users.noreply.github.com>
Date: Fri, 3 May 2024 14:31:03 -0400
Subject: [PATCH] fix(lint/useJsxKeyInIterable): don't trigger
 `useJsxKeyInIterable` when iterating on non-jsx things (#2667)

---
 .../correctness/use_jsx_key_in_iterable.rs    | 76 ++++++-------------
 .../useJsxKeyInIterable/invalid.jsx.snap      | 57 --------------
 .../correctness/useJsxKeyInIterable/valid.jsx |  8 +-
 .../useJsxKeyInIterable/valid.jsx.snap        |  7 ++
 4 files changed, 38 insertions(+), 110 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
index 9704bfa506e4..f9ba3858f9cc 100644
--- 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
@@ -52,15 +52,9 @@ declare_node_union! {
     pub ReactComponentExpression = JsxTagExpression | JsCallExpression
 }
 
-#[derive(Debug)]
-pub enum UseJsxKeyInIterableState {
-    MissingKeyProps(TextRange),
-    CantDetermineJSXProp(TextRange),
-}
-
 impl Rule for UseJsxKeyInIterable {
     type Query = Semantic<UseJsxKeyInIterableQuery>;
-    type State = UseJsxKeyInIterableState;
+    type State = TextRange;
     type Signals = Vec<Self::State>;
     type Options = ();
 
@@ -77,38 +71,19 @@ impl Rule for UseJsxKeyInIterable {
     }
 
     fn diagnostic(_: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
-        match state {
-            UseJsxKeyInIterableState::MissingKeyProps(state) => {
-                let diagnostic = RuleDiagnostic::new(
-                    rule_category!(),
-                    state,
-                    markup! {
-                        "Missing "<Emphasis>"key"</Emphasis>" property for this element in iterable."
-                    },
-                )
-                .note(markup! {
-                    "The order of the items may change, and having a key can help React identify which item was moved."
-                }).note(markup! {
-                    "Check the "<Hyperlink href="https://react.dev/learn/rendering-lists#why-does-react-need-keys">"React documentation"</Hyperlink>". "
-                });
-                Some(diagnostic)
-            }
-            UseJsxKeyInIterableState::CantDetermineJSXProp(state) => {
-                let diagnostic = RuleDiagnostic::new(
-                    rule_category!(),
-                    state,
-                    markup! {
-                        "Cannot determine whether this child has the required "<Emphasis>"key"</Emphasis>" prop."
-                    },
-                )
-                .note(markup! {
-                    "Either return a JSX expression, or suppress this instance if you determine it is safe."
-                }).note(markup! {
-                    "Check the "<Hyperlink href="https://react.dev/learn/rendering-lists#why-does-react-need-keys">"React documentation for why a key prop is required"</Hyperlink>". "
-                });
-                Some(diagnostic)
-            }
-        }
+        let diagnostic = RuleDiagnostic::new(
+            rule_category!(),
+            state,
+            markup! {
+                "Missing "<Emphasis>"key"</Emphasis>" property for this element in iterable."
+            },
+        )
+        .note(markup! {
+            "The order of the items may change, and having a key can help React identify which item was moved."
+        }).note(markup! {
+            "Check the "<Hyperlink href="https://react.dev/learn/rendering-lists#why-does-react-need-keys">"React documentation"</Hyperlink>". "
+        });
+        Some(diagnostic)
     }
 }
 
@@ -119,10 +94,7 @@ impl Rule for UseJsxKeyInIterable {
 /// ```js
 /// [<h1></h1>, <h1></h1>]
 /// ```
-fn handle_collections(
-    node: &JsArrayExpression,
-    model: &SemanticModel,
-) -> Vec<UseJsxKeyInIterableState> {
+fn handle_collections(node: &JsArrayExpression, model: &SemanticModel) -> Vec<TextRange> {
     let is_inside_jsx = node.parent::<JsxExpressionChild>().is_some();
     node.elements()
         .iter()
@@ -133,6 +105,7 @@ fn handle_collections(
             let node = AnyJsExpression::cast(node.into_syntax())?;
             handle_potential_react_component(node, model, is_inside_jsx)
         })
+        .flatten()
         .collect()
 }
 
@@ -143,10 +116,7 @@ fn handle_collections(
 /// ```js
 /// data.map(x => <h1>{x}</h1>)
 /// ```
-fn handle_iterators(
-    node: &JsCallExpression,
-    model: &SemanticModel,
-) -> Option<Vec<UseJsxKeyInIterableState>> {
+fn handle_iterators(node: &JsCallExpression, model: &SemanticModel) -> Option<Vec<TextRange>> {
     let callee = node.callee().ok()?;
     let member_expression = AnyJsMemberExpression::cast(callee.into_syntax())?;
     let arguments = node.arguments().ok()?;
@@ -198,6 +168,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 +178,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 +188,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 +202,20 @@ fn handle_potential_react_component(
     node: AnyJsExpression,
     model: &SemanticModel,
     is_inside_jsx: bool,
-) -> Option<UseJsxKeyInIterableState> {
+) -> Option<Vec<TextRange>> {
     let node = unwrap_parenthesis(node)?;
+
     if is_inside_jsx {
         if let Some(node) = ReactComponentExpression::cast_ref(node.syntax()) {
             let range = handle_react_component(node, model)?;
-            Some(UseJsxKeyInIterableState::MissingKeyProps(range))
+            Some(vec![range])
         } else {
-            Some(UseJsxKeyInIterableState::CantDetermineJSXProp(node.range()))
+            None
         }
     } else {
         let range =
             handle_react_component(ReactComponentExpression::cast_ref(node.syntax())?, model)?;
-        Some(UseJsxKeyInIterableState::MissingKeyProps(range))
+        Some(vec![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
index 03ae8d055aee..7b64fe32f6a6 100644
--- a/crates/biome_js_analyze/tests/specs/correctness/useJsxKeyInIterable/invalid.jsx.snap
+++ b/crates/biome_js_analyze/tests/specs/correctness/useJsxKeyInIterable/invalid.jsx.snap
@@ -520,44 +520,6 @@ invalid.jsx:33:8 lint/correctness/useJsxKeyInIterable ━━━━━━━━
   i Check the React documentation. 
   
 
-```
-
-```
-invalid.jsx:33:19 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>)
-  
-  i Either return a JSX expression, or suppress this instance if you determine it is safe.
-  
-  i Check the React documentation for why a key prop is required. 
-  
-
-```
-
-```
-invalid.jsx:33:24 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>)
-  
-  i Either return a JSX expression, or suppress this instance if you determine it is safe.
-  
-  i Check the React documentation for why a key prop is required. 
-  
-
 ```
 
 ```
@@ -577,25 +539,6 @@ invalid.jsx:35:21 lint/correctness/useJsxKeyInIterable ━━━━━━━━
   i Check the React documentation. 
   
 
-```
-
-```
-invalid.jsx:37:21 lint/correctness/useJsxKeyInIterable ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
-
-  ! Cannot determine whether this child has the required key prop.
-  
-    35 │ (<h1>{data.map(c => <h1></h1>)}</h1>)
-    36 │ 
-  > 37 │ (<h1>{data.map(c => xyz)}</h1>)
-       │                     ^^^
-    38 │ 
-    39 │ (<h1>{data.map(c => (<h1></h1>))}</h1>)
-  
-  i Either return a JSX expression, or suppress this instance if you determine it is safe.
-  
-  i Check the React documentation for why a key prop is required. 
-  
-
 ```
 
 ```
diff --git a/crates/biome_js_analyze/tests/specs/correctness/useJsxKeyInIterable/valid.jsx b/crates/biome_js_analyze/tests/specs/correctness/useJsxKeyInIterable/valid.jsx
index 21c82252ba7e..c9e7be44af33 100644
--- 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>)
\ No newline at end of file
+(<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
index f2611fff4426..d52efbcdd157 100644
--- 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)}</>
+
 ```