fix(lint/useJsxKeyInIterable): flag jsx in variable declarations (#2803)

biomejs · May 11, 2024 · 50c270e · 50c270e
1 parent 93bb41c
commit 50c270e
Show file tree

Hide file tree

Showing 5 changed files with 130 additions and 25 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
@@ -6,8 +6,8 @@ use biome_console::markup;
 use biome_js_semantic::SemanticModel;
 use biome_js_syntax::{
     AnyJsExpression, AnyJsFunctionBody, AnyJsMemberExpression, AnyJsObjectMember, AnyJsxAttribute,
-    AnyJsxChild, JsArrayExpression, JsCallExpression, JsObjectExpression, JsxAttributeList,
-    JsxExpressionChild, JsxTagExpression,
+    AnyJsxChild, JsArrayExpression, JsCallExpression, JsFunctionBody, JsObjectExpression,
+    JsxAttributeList, JsxExpressionChild, JsxTagExpression,
 };
 use biome_rowan::{declare_node_union, AstNode, AstNodeList, AstSeparatedList, TextRange};
 
@@ -160,18 +160,7 @@ fn handle_iterators(node: &JsCallExpression, model: &SemanticModel) -> Option<Ve
     match callback_argument {
         AnyJsExpression::JsFunctionExpression(callback) => {
             let body = callback.body().ok()?;
-            let res = body
-                .statements()
-                .into_iter()
-                .filter_map(|statement| {
-                    let statement = statement.as_js_return_statement()?;
-                    let returned_value = statement.argument()?;
-                    handle_potential_react_component(returned_value, model, is_inside_jsx)
-                })
-                .flatten()
-                .collect::<Vec<_>>();
-
-            Some(res)
+            Some(handle_function_body(&body, model, is_inside_jsx))
         }
         AnyJsExpression::JsArrowFunctionExpression(callback) => {
             let body = callback.body().ok()?;
@@ -180,24 +169,48 @@ fn handle_iterators(node: &JsCallExpression, model: &SemanticModel) -> Option<Ve
                     handle_potential_react_component(expr, model, is_inside_jsx)
                 }
                 AnyJsFunctionBody::JsFunctionBody(body) => {
-                    let res = body
-                        .statements()
-                        .into_iter()
-                        .filter_map(|statement| {
-                            let statement = statement.as_js_return_statement()?;
-                            let returned_value = statement.argument()?;
-                            handle_potential_react_component(returned_value, model, is_inside_jsx)
-                        })
-                        .flatten()
-                        .collect::<Vec<_>>();
-                    Some(res)
+                    Some(handle_function_body(&body, model, is_inside_jsx))
                 }
             }
         }
         _ => None,
     }
 }
 
+/// Inspects each statement for variable declarations and return statements to find potential React components.
+fn handle_function_body(
+    node: &JsFunctionBody,
+    model: &SemanticModel,
+    is_inside_jsx: bool,
+) -> Vec<TextRange> {
+    node.statements()
+        .iter()
+        .filter_map(|statement| {
+            if let Some(statement) = statement.as_js_variable_statement() {
+                let declaration = statement.declaration().ok()?;
+                Some(
+                    declaration
+                        .declarators()
+                        .iter()
+                        .filter_map(|declarator| {
+                            let decl = declarator.ok()?;
+                            let init = decl.initializer()?.expression().ok()?;
+                            handle_potential_react_component(init, model, is_inside_jsx)
+                        })
+                        .flatten()
+                        .collect(),
+                )
+            } else if let Some(statement) = statement.as_js_return_statement() {
+                let returned_value = statement.argument()?;
+                handle_potential_react_component(returned_value, model, is_inside_jsx)
+            } else {
+                None
+            }
+        })
+        .flatten()
+        .collect()
+}
+
 fn handle_potential_react_component(
     node: AnyJsExpression,
     model: &SemanticModel,

diff --git a/crates/biome_js_analyze/tests/specs/correctness/useJsxKeyInIterable/invalid.jsx b/crates/biome_js_analyze/tests/specs/correctness/useJsxKeyInIterable/invalid.jsx
@@ -51,3 +51,15 @@ React.Children.map(c => React.cloneElement(c));
 [].map((item) => {
 	return <>{item.condition ? <div /> : <div>foo</div>}</>;
 });
+
+[].map((item) => {
+	const x = 5;
+	const div = <div>{x}</div>;
+	return div;
+});
+
+[].map(function(item) {
+	const x = 5;
+	const div = <div>{x}</div>;
+	return div;
+});
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
@@ -58,6 +58,18 @@ React.Children.map(c => React.cloneElement(c));
 	return <>{item.condition ? <div /> : <div>foo</div>}</>;
 });
 
+[].map((item) => {
+	const x = 5;
+	const div = <div>{x}</div>;
+	return div;
+});
+
+[].map(function(item) {
+	const x = 5;
+	const div = <div>{x}</div>;
+	return div;
+});
+
 ```
 
 # Diagnostics
@@ -756,3 +768,41 @@ invalid.jsx:52:39 lint/correctness/useJsxKeyInIterable ━━━━━━━━
   
 
 ```
+
+```
+invalid.jsx:57:14 lint/correctness/useJsxKeyInIterable ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Missing key property for this element in iterable.
+  
+    55 │ [].map((item) => {
+    56 │ 	const x = 5;
+  > 57 │ 	const div = <div>{x}</div>;
+       │ 	            ^^^^^
+    58 │ 	return div;
+    59 │ });
+  
+  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:63:14 lint/correctness/useJsxKeyInIterable ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Missing key property for this element in iterable.
+  
+    61 │ [].map(function(item) {
+    62 │ 	const x = 5;
+  > 63 │ 	const div = <div>{x}</div>;
+       │ 	            ^^^^^
+    64 │ 	return div;
+    65 │ });
+  
+  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. 
+  
+
+```
diff --git a/crates/biome_js_analyze/tests/specs/correctness/useJsxKeyInIterable/valid.jsx b/crates/biome_js_analyze/tests/specs/correctness/useJsxKeyInIterable/valid.jsx
@@ -68,6 +68,10 @@ React.Children.map(c => React.cloneElement(c, {key: c}));
 	return <><div key={item.id}>foo</div></>;
 });
 
+[].map((item) => {
+	return item.condition ? <div key={item.id} /> : <div key={item.id}>foo</div>;
+});
+
 [].map((item) => {
 	return <>{item.condition ? <div key={item.id} /> : <div key={item.id}>foo</div>}</>;
 });
@@ -76,3 +80,14 @@ React.Children.map(c => React.cloneElement(c, {key: c}));
 	const div = <div key={item.id} />;
 	return <>{item.condition ? div : <div key={item.id}>{div}</div>}</>;
 });
+
+[].map(function(item) {
+	const x = 5;
+	return <div key={item.id}>{x}</div>;
+});
+
+[].map(function(item) {
+	const x = 5;
+	const div = <div key={item.id}>{x}</div>;
+	return div;
+});
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
@@ -74,6 +74,10 @@ React.Children.map(c => React.cloneElement(c, {key: c}));
 	return <><div key={item.id}>foo</div></>;
 });
 
+[].map((item) => {
+	return item.condition ? <div key={item.id} /> : <div key={item.id}>foo</div>;
+});
+
 [].map((item) => {
 	return <>{item.condition ? <div key={item.id} /> : <div key={item.id}>foo</div>}</>;
 });
@@ -83,4 +87,15 @@ React.Children.map(c => React.cloneElement(c, {key: c}));
 	return <>{item.condition ? div : <div key={item.id}>{div}</div>}</>;
 });
 
+[].map(function(item) {
+	const x = 5;
+	return <div key={item.id}>{x}</div>;
+});
+
+[].map(function(item) {
+	const x = 5;
+	const div = <div key={item.id}>{x}</div>;
+	return div;
+});
+
 ```