fix(linter/react): Fixed false positive with missing key inside React.Children.toArray() (#4945)

Closes: #4421 

Added three new functions in order to check if an element is inside of
React.Children.toArray.
Tests added for this fix are exactly the same as those used by
eslint-plugin-react.
This commit is contained in:
Earl Chase 2024-08-21 19:05:14 -05:00 committed by GitHub
parent ee7ac8b0b7
commit ddf83ff1b9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 132 additions and 1 deletions

View file

@ -1,5 +1,5 @@
use oxc_ast::{
ast::{JSXAttributeItem, JSXAttributeName, JSXElement, JSXFragment, Statement},
ast::{Expression, JSXAttributeItem, JSXAttributeName, JSXElement, JSXFragment, Statement},
AstKind,
};
use oxc_diagnostics::OxcDiagnostic;
@ -69,6 +69,76 @@ impl Rule for JsxKey {
}
}
pub fn is_to_array(node: &AstNode<'_>) -> bool {
const TOARRAY: &str = "toArray";
let AstKind::CallExpression(call) = node.kind() else { return false };
let Some(subject) = call.callee_name() else { return false };
if subject != TOARRAY {
return false;
}
true
}
pub fn import_matcher<'a>(
ctx: &LintContext<'a>,
actual_local_name: &'a str,
expected_module_name: &'a str,
) -> bool {
ctx.semantic().module_record().import_entries.iter().any(|import| {
import.module_request.name().as_str() == expected_module_name.to_lowercase()
&& import.local_name.name().as_str() == actual_local_name
})
}
pub fn is_import<'a>(
ctx: &LintContext<'a>,
actual_local_name: &'a str,
expected_local_name: &'a str,
expected_module_name: &'a str,
) -> bool {
let total_imports = ctx.semantic().module_record().requested_modules.len();
let total_variables = ctx.scopes().get_bindings(ctx.scopes().root_scope_id()).len();
if total_variables == 0 && total_imports == 0 {
return actual_local_name == expected_local_name;
}
import_matcher(ctx, actual_local_name, expected_module_name)
}
pub fn is_children<'a, 'b>(node: &'b AstNode<'a>, ctx: &'b LintContext<'a>) -> bool {
const REACT: &str = "React";
const CHILDREN: &str = "Children";
let AstKind::CallExpression(call) = node.kind() else { return false };
let Some(member) = call.callee.as_member_expression() else { return false };
if let Expression::Identifier(ident) = member.object() {
return is_import(ctx, ident.name.as_str(), CHILDREN, REACT);
}
let Some(inner_member) = member.object().get_inner_expression().as_member_expression() else {
return false;
};
let Some(ident) = inner_member.object().get_identifier_reference() else { return false };
let Some(local_name) = inner_member.static_property_name() else { return false };
return is_import(ctx, ident.name.as_str(), REACT, REACT) && local_name == CHILDREN;
}
fn is_within_children_to_array<'a, 'b>(node: &'b AstNode<'a>, ctx: &'b LintContext<'a>) -> bool {
let parents_iter = ctx.nodes().iter_parents(node.id()).skip(2);
parents_iter
.filter(|parent_node| matches!(parent_node.kind(), AstKind::CallExpression(_)))
.any(|parent_node| is_children(parent_node, ctx) && is_to_array(parent_node))
}
enum InsideArrayOrIterator {
Array,
Iterator(Span),
@ -151,6 +221,9 @@ fn is_in_array_or_iter<'a, 'b>(
fn check_jsx_element<'a>(node: &AstNode<'a>, jsx_elem: &JSXElement<'a>, ctx: &LintContext<'a>) {
if let Some(outer) = is_in_array_or_iter(node, ctx) {
if is_within_children_to_array(node, ctx) {
return;
}
if !jsx_elem.opening_element.attributes.iter().any(|attr| {
let JSXAttributeItem::Attribute(attr) = attr else {
return false;
@ -400,6 +473,20 @@ fn test() {
}
];
",
r"{React.Children.toArray(items.map((item) => {
return (
<div>
{item}
</div>
);}))}
",
r#"import { Children } from "react";
Children.toArray([1, 2 ,3].map(x => <App />));
"#,
r#"import React from "react";
React.Children.toArray([1, 2 ,3].map(x => <App />));
"#,
r"React.Children.toArray([1, 2 ,3].map(x => <App />));",
];
let fail = vec![
@ -500,6 +587,19 @@ fn test() {
);
};
",
r"foo.Children.toArray([1, 2 ,3].map(x => <App />));",
r"
import Act from 'react';
import { Children as ReactChildren } from 'react';
const { Children } = Act;
const { toArray } = Children;
Act.Children.toArray([1, 2 ,3].map(x => <App />));
Act.Children.toArray(Array.from([1, 2 ,3], x => <App />));
Children.toArray([1, 2 ,3].map(x => <App />));
Children.toArray(Array.from([1, 2 ,3], x => <App />));
",
];
Tester::new(JsxKey::NAME, pass, fail).test_and_snapshot();

View file

@ -315,3 +315,34 @@ source: crates/oxc_linter/src/tester.rs
8 │ <Text foo bar baz qux onClick={() => onClickHandler()} onPointerDown={() => onPointerDownHandler()} onMouseDown={() => onMouseDownHandler()} />
╰────
help: Add a "key" prop to the element in the iterator (https://react.dev/learn/rendering-lists#keeping-list-items-in-order-with-key).
⚠ eslint-plugin-react(jsx-key): Missing "key" prop for element in iterator.
╭─[jsx_key.tsx:1:32]
1 │ foo.Children.toArray([1, 2 ,3].map(x => <App />));
· ─┬─ ─┬─
· │ ╰── Element generated here.
· ╰── Iterator starts here.
╰────
help: Add a "key" prop to the element in the iterator (https://react.dev/learn/rendering-lists#keeping-list-items-in-order-with-key).
⚠ eslint-plugin-react(jsx-key): Missing "key" prop for element in iterator.
╭─[jsx_key.tsx:10:36]
9 │ Act.Children.toArray(Array.from([1, 2 ,3], x => <App />));
10 │ Children.toArray([1, 2 ,3].map(x => <App />));
· ─┬─ ─┬─
· │ ╰── Element generated here.
· ╰── Iterator starts here.
11 │ Children.toArray(Array.from([1, 2 ,3], x => <App />));
╰────
help: Add a "key" prop to the element in the iterator (https://react.dev/learn/rendering-lists#keeping-list-items-in-order-with-key).
⚠ eslint-plugin-react(jsx-key): Missing "key" prop for element in iterator.
╭─[jsx_key.tsx:11:32]
10 │ Children.toArray([1, 2 ,3].map(x => <App />));
11 │ Children.toArray(Array.from([1, 2 ,3], x => <App />));
· ──┬─ ─┬─
· │ ╰── Element generated here.
· ╰── Iterator starts here.
12 │
╰────
help: Add a "key" prop to the element in the iterator (https://react.dev/learn/rendering-lists#keeping-list-items-in-order-with-key).