Skip to content

fix(useDestructuring): exclude plain assignments and non-iterable index types #9457

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

Closed
Vadaski wants to merge 4 commits into from
Closed

fix(useDestructuring): exclude plain assignments and non-iterable index types #9457

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
963b130
fix(useDestructuring): exclude plain assignments and non-iterable ind…
Vadaski Mar 12, 2026
4924937
test(useDestructuring): fix type mismatch in valid.ts test case
Vadaski Mar 12, 2026
0687f52
fix(useDestructuring): remove depth short-circuit per maintainer review
Vadaski Mar 12, 2026
54d0d9c
fix(useDestructuring): add cycle detection to prevent infinite recurs…
Vadaski Mar 12, 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
5 changes: 5 additions & 0 deletions .changeset/fix-use-destructuring-cyclic-aliases.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@biomejs/biome": patch
---

Fixed a stack overflow in `useDestructuring` when analyzing cyclic TypeScript type aliases such as `type A = B; type B = A;`.
8 changes: 8 additions & 0 deletions .changeset/fix-use-destructuring-illegal-cases.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"@biomejs/biome": patch
---

Fixed [#8480](https://github.com/biomejs/biome/issues/8480): [`useDestructuring`](https://biomejs.dev/linter/rules/use-destructuring/) no longer suggests illegal destructuring for:

1. Plain assignments to pre-declared variables (e.g. `thing = obj.thing` where `thing` is already declared) — destructuring would require `({ thing } = obj)` which may not be a valid refactor.
2. Numeric index access on types with index signatures but no iterable protocol (e.g. `const x = obj[0]` where `obj: { [key: string]: string }`) — array destructuring `[x] = obj` would be a type error.
231 changes: 173 additions & 58 deletions crates/biome_js_analyze/src/lint/nursery/use_destructuring.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
use biome_analyze::{
Ast, Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule,
};
use crate::services::semantic::Semantic;
use biome_analyze::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule};
use biome_console::markup;
use biome_js_semantic::SemanticModel;
use biome_js_syntax::{
AnyJsAssignment, AnyJsAssignmentPattern, AnyJsBinding, AnyJsBindingPattern, AnyJsExpression,
AnyJsLiteralExpression, AnyJsName, JsAssignmentExpression, JsAssignmentOperator,
JsVariableDeclaration, JsVariableDeclarator,
AnyJsBinding, AnyJsBindingPattern, AnyJsExpression, AnyJsLiteralExpression, AnyJsName,
AnyTsType, JsVariableDeclaration, JsVariableDeclarator, TsTypeAnnotation,
binding_ext::AnyJsBindingDeclaration,
};
use biome_rowan::{AstNode, declare_node_union};
use biome_rowan::{AstNode, TextRange};
use biome_rule_options::use_destructuring::UseDestructuringOptions;
use std::collections::HashSet;

declare_lint_rule! {
/// Require destructuring from arrays and/or objects
Expand Down Expand Up @@ -54,58 +55,33 @@ declare_lint_rule! {
}

impl Rule for UseDestructuring {
type Query = Ast<UseDestructuringQuery>;
type Query = Semantic<JsVariableDeclarator>;
type State = UseDestructuringState;
type Signals = Option<Self::State>;
type Options = UseDestructuringOptions;

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let query = ctx.query();

match query {
UseDestructuringQuery::JsAssignmentExpression(node) => {
let op = node.operator().ok()?;
if op != JsAssignmentOperator::Assign {
return None;
}
let left = node.left().ok()?;
let right = node.right().ok()?;

if let AnyJsAssignmentPattern::AnyJsAssignment(
AnyJsAssignment::JsIdentifierAssignment(expr),
) = left
{
let ident = expr.name_token().ok()?;
return should_suggest_destructuring(ident.text_trimmed(), &right);
}

None
}
UseDestructuringQuery::JsVariableDeclarator(node) => {
let initializer = node.initializer()?;
let declaration = JsVariableDeclaration::cast(node.syntax().parent()?.parent()?)?;
let has_await_using = declaration.await_token().is_some();
if declaration.kind().ok()?.text_trimmed() == "using" || has_await_using {
return None;
}

if node.variable_annotation().is_some() {
return None;
}
let node = ctx.query();
let initializer = node.initializer()?;
let declaration = JsVariableDeclaration::cast(node.syntax().parent()?.parent()?)?;
let has_await_using = declaration.await_token().is_some();
if declaration.kind().ok()?.text_trimmed() == "using" || has_await_using {
return None;
}

let left = node.id().ok()?;
let right = initializer.expression().ok()?;
if node.variable_annotation().is_some() {
return None;
}

if let AnyJsBindingPattern::AnyJsBinding(AnyJsBinding::JsIdentifierBinding(expr)) =
left
{
let ident = expr.name_token().ok()?;
return should_suggest_destructuring(ident.text_trimmed(), &right);
}
let left = node.id().ok()?;
let right = initializer.expression().ok()?;

None
}
if let AnyJsBindingPattern::AnyJsBinding(AnyJsBinding::JsIdentifierBinding(expr)) = left {
let ident = expr.name_token().ok()?;
return should_suggest_destructuring(ident.text_trimmed(), &right, ctx.model());
}

None
}

fn diagnostic(ctx: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
Expand Down Expand Up @@ -149,13 +125,10 @@ impl Rule for UseDestructuring {
}
}

declare_node_union! {
pub UseDestructuringQuery = JsVariableDeclarator | JsAssignmentExpression
}

fn should_suggest_destructuring(
left: &str,
right: &AnyJsExpression,
model: &SemanticModel,
) -> Option<UseDestructuringState> {
match right {
AnyJsExpression::JsComputedMemberExpression(expr) => {
Expand All @@ -164,12 +137,14 @@ fn should_suggest_destructuring(
}

let member = expr.member().ok()?;
if let AnyJsExpression::AnyJsLiteralExpression(expr) = member {
if matches!(expr, AnyJsLiteralExpression::JsNumberLiteralExpression(_)) {
return Some(UseDestructuringState::Array);
if let AnyJsExpression::AnyJsLiteralExpression(lit) = member {
if matches!(lit, AnyJsLiteralExpression::JsNumberLiteralExpression(_)) {
let object = expr.object().ok()?;
return supports_array_destructuring(&object, model)
.then_some(UseDestructuringState::Array);
}

let value = expr.value_token().ok()?;
let value = lit.value_token().ok()?;

if left == value.text_trimmed() {
return Some(UseDestructuringState::Object);
Expand Down Expand Up @@ -202,3 +177,143 @@ pub enum UseDestructuringState {
Object,
Array,
}

fn supports_array_destructuring(object: &AnyJsExpression, model: &SemanticModel) -> bool {
let mut visited = HashSet::<TextRange>::new();
!matches!(
array_destructuring_support_for_expression(object, model, &mut visited),
Some(false)
)
Comment on lines +183 to +186
Copy link
Copy Markdown
Member

@ematipico ematipico Mar 12, 2026

Choose a reason for hiding this comment

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

This function serves little to no value. Just use it with the try operator array_destructuring_support_for_expression()? and you're good

Comment on lines +181 to +186
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 12, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

None currently means “unknown”, but it gets treated as “safe”.

Lines 180-184 accept anything except Some(false), while Lines 187-199 only prove iterability for array literals and identifiers. That means cases like foo.bar[0] or getFoo()[0] still receive an array-destructuring suggestion even though their iterability was never verified. Please split “unknown” from “supported”, or only keep the permissive path for an explicit any case.

Also applies to: 187-199

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@crates/biome_js_analyze/src/lint/nursery/use_destructuring.rs` around lines
180 - 184, The current helper treats None (unknown) as supported; adjust
array_destructuring_support_for_expression to return Some(true) for
proven-iterable, Some(false) for proven-not-iterable, and None for unknown, and
change supports_array_destructuring to only return true when
array_destructuring_support_for_expression(...) is Some(true) (i.e., do not
treat None as safe). Additionally, allow a permissive true only when the
expression's static type is explicitly the `any` type by querying the
SemanticModel (use the existing SemanticModel type-check helper you have) so
unknown expressions like foo.bar[0] or getFoo()[0] don't get suggestions unless
explicitly typed as any. Ensure the same logic is applied to the checks
implemented in the code block handled in lines 187-199.

}

fn array_destructuring_support_for_expression(
Copy link
Copy Markdown
Member

@ematipico ematipico Mar 12, 2026

Choose a reason for hiding this comment

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

Please provide some docstrings that explain the business logic of the function.

object: &AnyJsExpression,
model: &SemanticModel,
visited: &mut HashSet<TextRange>,
) -> Option<bool> {
match object.clone().omit_parentheses() {
AnyJsExpression::JsArrayExpression(_) => Some(true),
AnyJsExpression::JsIdentifierExpression(expr) => {
let reference = expr.name().ok()?;
let declaration = model.binding(&reference)?.tree().declaration()?;
array_destructuring_support_for_declaration(&declaration, model, visited)
}
_ => None,
}
}

fn array_destructuring_support_for_declaration(
declaration: &AnyJsBindingDeclaration,
model: &SemanticModel,
visited: &mut HashSet<TextRange>,
) -> Option<bool> {
match declaration {
AnyJsBindingDeclaration::JsVariableDeclarator(node) => {
if let Some(annotation) = node.variable_annotation() {
if let Some(annotation) = annotation.as_ts_type_annotation() {
return array_destructuring_support_for_type_annotation(
annotation, model, visited,
);
}
}

let initializer = node.initializer()?.expression().ok()?;
array_destructuring_support_for_expression(&initializer, model, visited)
}
AnyJsBindingDeclaration::JsFormalParameter(node) => {
let annotation = node.type_annotation()?;
array_destructuring_support_for_type_annotation(&annotation, model, visited)
}
AnyJsBindingDeclaration::TsPropertyParameter(node) => {
let annotation = node
.formal_parameter()
.ok()?
.as_js_formal_parameter()?
.type_annotation()?;
array_destructuring_support_for_type_annotation(&annotation, model, visited)
}
AnyJsBindingDeclaration::TsTypeAliasDeclaration(node) => {
let range = node.range();
if !visited.insert(range) {
return Some(false);
}
let ty = node.ty().ok()?;
array_destructuring_support_for_type(&ty, model, visited)
}
AnyJsBindingDeclaration::TsInterfaceDeclaration(_) => Some(false),
_ => None,
}
}

fn array_destructuring_support_for_type_annotation(
annotation: &TsTypeAnnotation,
model: &SemanticModel,
visited: &mut HashSet<TextRange>,
) -> Option<bool> {
let ty = annotation.ty().ok()?;
array_destructuring_support_for_type(&ty, model, visited)
}

fn array_destructuring_support_for_type(
ty: &AnyTsType,
model: &SemanticModel,
visited: &mut HashSet<TextRange>,
) -> Option<bool> {
match ty {
AnyTsType::TsArrayType(_)
| AnyTsType::TsTupleType(_)
| AnyTsType::TsStringType(_)
| AnyTsType::TsStringLiteralType(_) => Some(true),
AnyTsType::TsObjectType(_)
| AnyTsType::TsBigintType(_)
| AnyTsType::TsBigintLiteralType(_)
| AnyTsType::TsBooleanType(_)
| AnyTsType::TsBooleanLiteralType(_)
| AnyTsType::TsNullLiteralType(_)
| AnyTsType::TsNumberType(_)
| AnyTsType::TsNumberLiteralType(_)
| AnyTsType::TsSymbolType(_)
| AnyTsType::TsUndefinedType(_)
| AnyTsType::TsUnknownType(_)
| AnyTsType::TsVoidType(_)
| AnyTsType::TsNeverType(_)
| AnyTsType::TsNonPrimitiveType(_) => Some(false),
AnyTsType::TsParenthesizedType(node) => {
let ty = node.ty().ok()?;
array_destructuring_support_for_type(&ty, model, visited)
}
AnyTsType::TsTypeOperatorType(node) => {
let ty = node.ty().ok()?;
array_destructuring_support_for_type(&ty, model, visited)
}
AnyTsType::TsReferenceType(node) => {
let range = node.range();
if !visited.insert(range) {
return Some(false);
}
let name = node.name().ok()?;
if let Some(reference) = name.as_js_reference_identifier() {
let token = reference.value_token().ok()?;
let name = token.text_trimmed();
if matches!(
name,
"Array"
| "ReadonlyArray"
| "Iterable"
| "IterableIterator"
| "Iterator"
| "Generator"
| "String"
) {
return Some(true);
}

let declaration = model.binding(reference)?.tree().declaration()?;
return array_destructuring_support_for_declaration(&declaration, model, visited);
}

None
}
_ => None,
}
}
9 changes: 0 additions & 9 deletions crates/biome_js_analyze/tests/specs/nursery/useDestructuring/invalid.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@
{
var foo = array[0];
}
{
foo = array[0];
}
{
var foo = object.foo;
}
Expand All @@ -29,12 +26,6 @@
{
var foo = object['foo'];
}
{
foo = object.foo;
}
{
foo = object['foo'];
}
{
class Foo extends Bar {
static foo() {
Expand Down
Loading