mirror of
https://github.com/danbulant/oxc
synced 2026-05-21 21:29:01 +00:00
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:
parent
ee7ac8b0b7
commit
ddf83ff1b9
2 changed files with 132 additions and 1 deletions
|
|
@ -1,5 +1,5 @@
|
||||||
use oxc_ast::{
|
use oxc_ast::{
|
||||||
ast::{JSXAttributeItem, JSXAttributeName, JSXElement, JSXFragment, Statement},
|
ast::{Expression, JSXAttributeItem, JSXAttributeName, JSXElement, JSXFragment, Statement},
|
||||||
AstKind,
|
AstKind,
|
||||||
};
|
};
|
||||||
use oxc_diagnostics::OxcDiagnostic;
|
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 {
|
enum InsideArrayOrIterator {
|
||||||
Array,
|
Array,
|
||||||
Iterator(Span),
|
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>) {
|
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 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| {
|
if !jsx_elem.opening_element.attributes.iter().any(|attr| {
|
||||||
let JSXAttributeItem::Attribute(attr) = attr else {
|
let JSXAttributeItem::Attribute(attr) = attr else {
|
||||||
return false;
|
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![
|
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();
|
Tester::new(JsxKey::NAME, pass, fail).test_and_snapshot();
|
||||||
|
|
|
||||||
|
|
@ -315,3 +315,34 @@ source: crates/oxc_linter/src/tester.rs
|
||||||
8 │ <Text foo bar baz qux onClick={() => onClickHandler()} onPointerDown={() => onPointerDownHandler()} onMouseDown={() => onMouseDownHandler()} />
|
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).
|
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).
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue