Skip to content

Commit

Permalink
feat(linter): Implement no-array-index-key
Browse files Browse the repository at this point in the history
  • Loading branch information
@gaoachao
gaoachao committed Oct 28, 2024
1 parent 428a762 commit 7f63270
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 83 deletions.
82 changes: 32 additions & 50 deletions crates/oxc_linter/src/rules/react/no_array_index_key.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_span::Span;

use crate::{context::LintContext, rule::Rule, AstNode};
use crate::{ast_util::is_method_call, context::LintContext, rule::Rule, AstNode};

fn no_array_index_key_diagnostic(span: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("Usage of Array index in keys is not allowed")
.with_help("Should use the unique key to avoid unnecessary renders")
.with_help("Use a unique data-dependent key to avoid unnecessary rerenders")
.with_label(span)
}

Expand All @@ -22,10 +22,13 @@ pub struct NoArrayIndexKey;

declare_oxc_lint!(
/// ### What it does
///
/// Warn if an element uses an Array index in its key.
///
/// ### Why is this bad?
///
/// It's a bad idea to use the array index since it doesn't uniquely identify your elements.
/// In cases where the array is sorted or an element is added to the beginning of the array,
/// the index will be changed even though the element representing that index may be the same.
/// This results in unnecessary renders.
///
/// ### Examples
///
Expand Down Expand Up @@ -78,7 +81,7 @@ fn check_jsx_element<'a>(
};

if expr.name.as_str() == index_param_name {
ctx.diagnostic(no_array_index_key_diagnostic(jsx.span));
ctx.diagnostic(no_array_index_key_diagnostic(attr.span));
}
}
}
Expand All @@ -92,45 +95,28 @@ fn check_react_clone_element<'a>(
return;
};

match &call_expr.callee {
// React.cloneElement
Expression::StaticMemberExpression(static_mem_expr) => {
let Expression::Identifier(react_ident) = &static_mem_expr.object else {
return;
if is_method_call(call_expr, Some(&["React"]), Some(&["cloneElement"]), Some(2), Some(3)) {
let Some(Argument::ObjectExpression(obj_expr)) = call_expr.arguments.get(1) else {
return;
};

for prop_kind in &obj_expr.properties {
let ObjectPropertyKind::ObjectProperty(prop) = prop_kind else {
continue;
};

let PropertyKey::StaticIdentifier(key_ident) = &prop.key else {
continue;
};

if react_ident.name.as_str() == "React"
&& static_mem_expr.property.name.as_str() == "cloneElement"
{
let Some(Argument::ObjectExpression(obj_expr)) = call_expr.arguments.get(1) else {
return;
};

for prop_kind in &obj_expr.properties {
let ObjectPropertyKind::ObjectProperty(prop) = prop_kind else {
continue;
};

let PropertyKey::StaticIdentifier(key_ident) = &prop.key else {
continue;
};

let Expression::Identifier(value_ident) = &prop.value else {
continue;
};

if key_ident.name.as_str() == "key"
&& value_ident.name.as_str() == index_param_name
{
ctx.diagnostic(no_array_index_key_diagnostic(call_expr.span));
}
}
let Expression::Identifier(value_ident) = &prop.value else {
continue;
};

if key_ident.name.as_str() == "key" && value_ident.name.as_str() == index_param_name {
ctx.diagnostic(no_array_index_key_diagnostic(obj_expr.span));
}
}

// TODO: cloneElement
Expression::Identifier(indent) => if indent.name.as_str() == "cloneElement" {},
_ => (),
}
}

Expand Down Expand Up @@ -160,19 +146,15 @@ fn find_index_param_name_by_position<'a>(
) -> Option<&'a str> {
match &call_expr.arguments[0] {
Argument::ArrowFunctionExpression(arrow_fn_expr) => {
if let Some(index_param) = arrow_fn_expr.params.items.get(position) {
if let Some(index_param_name) = index_param.pattern.get_identifier() {
return Some(index_param_name.as_str());
}
}
return Some(
arrow_fn_expr.params.items.get(position)?.pattern.get_identifier()?.as_str(),
);
}

Argument::FunctionExpression(regular_fn_expr) => {
if let Some(index_param) = regular_fn_expr.params.items.get(position) {
if let Some(index_param_name) = index_param.pattern.get_identifier() {
return Some(index_param_name.as_str());
}
}
return Some(
regular_fn_expr.params.items.get(position)?.pattern.get_identifier()?.as_str(),
);
}

_ => (),
Expand Down
66 changes: 33 additions & 33 deletions crates/oxc_linter/src/snapshots/no_array_index_key.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,100 +2,100 @@
source: crates/oxc_linter/src/tester.rs
---
⚠ eslint-plugin-react(no-array-index-key): Usage of Array index in keys is not allowed
╭─[no_array_index_key.tsx:2:13]
╭─[no_array_index_key.tsx:2:20]
1 │ things.map((thing, index) => (
2 │ <Hello key={index} />
· ─────────────────────
· ───────────
3 │ ));
╰────
help: Should use the unique key to avoid unnecessary renders
help: Use a unique data-dependent key to avoid unnecessary rerenders

⚠ eslint-plugin-react(no-array-index-key): Usage of Array index in keys is not allowed
╭─[no_array_index_key.tsx:2:13]
╭─[no_array_index_key.tsx:2:39]
1 │ things.map((thing, index) => (
2 │ React.cloneElement(thing, { key: index })
· ─────────────────────────────────────────
· ──────────────
3 │ ));
╰────
help: Should use the unique key to avoid unnecessary renders
help: Use a unique data-dependent key to avoid unnecessary rerenders

⚠ eslint-plugin-react(no-array-index-key): Usage of Array index in keys is not allowed
╭─[no_array_index_key.tsx:2:30]
╭─[no_array_index_key.tsx:2:37]
1 │ things.forEach((thing, index) => {
2 │ otherThings.push(<Hello key={index} />);
· ─────────────────────
· ───────────
3 │ });
╰────
help: Should use the unique key to avoid unnecessary renders
help: Use a unique data-dependent key to avoid unnecessary rerenders

⚠ eslint-plugin-react(no-array-index-key): Usage of Array index in keys is not allowed
╭─[no_array_index_key.tsx:2:30]
╭─[no_array_index_key.tsx:2:37]
1 │ things.filter((thing, index) => {
2 │ otherThings.push(<Hello key={index} />);
· ─────────────────────
· ───────────
3 │ });
╰────
help: Should use the unique key to avoid unnecessary renders
help: Use a unique data-dependent key to avoid unnecessary rerenders

⚠ eslint-plugin-react(no-array-index-key): Usage of Array index in keys is not allowed
╭─[no_array_index_key.tsx:2:30]
╭─[no_array_index_key.tsx:2:37]
1 │ things.some((thing, index) => {
2 │ otherThings.push(<Hello key={index} />);
· ─────────────────────
· ───────────
3 │ });
╰────
help: Should use the unique key to avoid unnecessary renders
help: Use a unique data-dependent key to avoid unnecessary rerenders

⚠ eslint-plugin-react(no-array-index-key): Usage of Array index in keys is not allowed
╭─[no_array_index_key.tsx:2:30]
╭─[no_array_index_key.tsx:2:37]
1 │ things.every((thing, index) => {
2 │ otherThings.push(<Hello key={index} />);
· ─────────────────────
· ───────────
3 │ });
╰────
help: Should use the unique key to avoid unnecessary renders
help: Use a unique data-dependent key to avoid unnecessary rerenders

⚠ eslint-plugin-react(no-array-index-key): Usage of Array index in keys is not allowed
╭─[no_array_index_key.tsx:2:30]
╭─[no_array_index_key.tsx:2:37]
1 │ things.find((thing, index) => {
2 │ otherThings.push(<Hello key={index} />);
· ─────────────────────
· ───────────
3 │ });
╰────
help: Should use the unique key to avoid unnecessary renders
help: Use a unique data-dependent key to avoid unnecessary rerenders

⚠ eslint-plugin-react(no-array-index-key): Usage of Array index in keys is not allowed
╭─[no_array_index_key.tsx:2:30]
╭─[no_array_index_key.tsx:2:37]
1 │ things.findIndex((thing, index) => {
2 │ otherThings.push(<Hello key={index} />);
· ─────────────────────
· ───────────
3 │ });
╰────
help: Should use the unique key to avoid unnecessary renders
help: Use a unique data-dependent key to avoid unnecessary rerenders

⚠ eslint-plugin-react(no-array-index-key): Usage of Array index in keys is not allowed
╭─[no_array_index_key.tsx:2:13]
╭─[no_array_index_key.tsx:2:20]
1 │ things.flatMap((thing, index) => (
2 │ <Hello key={index} />
· ─────────────────────
· ───────────
3 │ ));
╰────
help: Should use the unique key to avoid unnecessary renders
help: Use a unique data-dependent key to avoid unnecessary rerenders

⚠ eslint-plugin-react(no-array-index-key): Usage of Array index in keys is not allowed
╭─[no_array_index_key.tsx:2:31]
╭─[no_array_index_key.tsx:2:38]
1 │ things.reduce((collection, thing, index) => (
2 │ collection.concat(<Hello key={index} />)
· ─────────────────────
· ───────────
3 │ ), []);
╰────
help: Should use the unique key to avoid unnecessary renders
help: Use a unique data-dependent key to avoid unnecessary rerenders

⚠ eslint-plugin-react(no-array-index-key): Usage of Array index in keys is not allowed
╭─[no_array_index_key.tsx:2:31]
╭─[no_array_index_key.tsx:2:38]
1 │ things.reduceRight((collection, thing, index) => (
2 │ collection.concat(<Hello key={index} />)
· ─────────────────────
· ───────────
3 │ ), []);
╰────
help: Should use the unique key to avoid unnecessary renders
help: Use a unique data-dependent key to avoid unnecessary rerenders

0 comments on commit 7f63270

Please sign in to comment.