Skip to content

fix(lint): detect array index in any position of template string #8947

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
murataslan1 wants to merge 2 commits into
base: main
Choose a base branch
from
+39 −7
Open

fix(lint): detect array index in any position of template string #8947

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ee638d9
fix(lint): detect array index in any position of template string
murataslan1 Feb 2, 2026
9f3a509
chore: add changeset and additional test case
murataslan1 Feb 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions crates/biome_js_analyze/src/lint/suspicious/no_array_index_key.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,13 +150,17 @@ impl Rule for NoArrayIndexKey {
let template_elements = template_expression.elements();
for element in template_elements {
if let AnyJsTemplateElement::JsTemplateElement(template_element) = element {
let cap_index_value = template_element
.expression()
.ok()?
.as_js_identifier_expression()?
.name()
.ok();
capture_array_index = cap_index_value;
// Check if this element is an identifier expression (potential array index)
if let Some(identifier_expr) =
template_element.expression().ok()?.as_js_identifier_expression()
{
if let Ok(name) = identifier_expr.name() {
// Found a potential array index reference, use it and stop searching
// This fixes the issue where index position in template doesn't matter
Copy link
Contributor

@dyc3 dyc3 Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit

Suggested change
// This fixes the issue where index position in template doesn't matter

capture_array_index = Some(name);
break;
}
Comment on lines +153 to +162
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Potential regression: template literal now ignores later identifiers

Lines 153-162: breaking on the first identifier means ${item}-${index} will capture item and miss index, so order dependence creeps back in. Please consider collecting all identifier expressions in the template and selecting the one that resolves to the array index parameter (or iterating candidates until one passes is_array_method_index).

🤖 Prompt for AI Agents 
In `@crates/biome_js_analyze/src/lint/suspicious/no_array_index_key.rs` around
lines 153 - 162, The current logic breaks early on the first identifier
expression in a template (the block using
template_element.expression().ok()?.as_js_identifier_expression() and setting
capture_array_index), which causes templates like `${item}-${index}` to capture
the wrong identifier; instead, collect all identifier expressions from the
template (or continue scanning instead of breaking) and then choose the
identifier that satisfies is_array_method_index, or iterate candidates calling
is_array_method_index until one returns true before assigning
capture_array_index; update the loop around template_element/identifier_expr to
accumulate candidates and only set capture_array_index when
is_array_method_index confirms the identifier is the array index.

}
}
}
}
Expand Down
10 changes: 10 additions & 0 deletions crates/biome_js_analyze/tests/specs/suspicious/noArrayIndexKey/invalid.jsx
View file
Open in desktop
Copy link
Contributor

@dyc3 dyc3 Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

snapshots need to be updated

Original file line number Diff line number Diff line change
Expand Up @@ -131,4 +131,14 @@ function Component10() {
{({ things }) => things.map((_, index) => <Component key={"test" + index + "test"} />)}
</HoC>
);
}

// Issue #8812: template string with index NOT at the end should still trigger
function Component11() {
return things.map((item, index) => <div key={`${index}-${item}`}>{item}</div>);
}

// Issue #8812: object property access after index should still trigger
function Component12() {
return things.map((item, index) => <div key={`${index}-${item.title}`}>{item.title}</div>);
}
Loading