From cdb5c7c07e0e8b828ce3a9ce42fe7ddf0dda75b4 Mon Sep 17 00:00:00 2001 From: vladimirivanov Date: Sat, 20 Sep 2025 16:08:29 +0100 Subject: [PATCH 01/25] feat(biome-js-analyze): add is_meaningful_read to semantic class reads --- .../src/services/semantic_class.rs | 501 +++++++++++++----- 1 file changed, 365 insertions(+), 136 deletions(-) diff --git a/crates/biome_js_analyze/src/services/semantic_class.rs b/crates/biome_js_analyze/src/services/semantic_class.rs index 09caccad4ffc..0d2cbf93d9bf 100644 --- a/crates/biome_js_analyze/src/services/semantic_class.rs +++ b/crates/biome_js_analyze/src/services/semantic_class.rs @@ -1,20 +1,21 @@ -use biome_js_syntax::{ - AnyJsClassMember, AnyJsExpression, AnyJsRoot, JsArrayAssignmentPattern, - JsArrowFunctionExpression, JsAssignmentExpression, JsClassDeclaration, JsClassMemberList, - JsConstructorClassMember, JsFunctionBody, JsLanguage, JsObjectAssignmentPattern, - JsObjectBindingPattern, JsPostUpdateExpression, JsPreUpdateExpression, JsPropertyClassMember, - JsStaticMemberAssignment, JsStaticMemberExpression, JsSyntaxKind, JsSyntaxNode, - JsVariableDeclarator, TextRange, TsPropertyParameter, -}; - use biome_analyze::{ AddVisitor, FromServices, Phase, Phases, QueryKey, QueryMatch, Queryable, RuleKey, RuleMetadata, ServiceBag, ServicesDiagnostic, Visitor, VisitorContext, VisitorFinishContext, }; +use biome_js_syntax::{ + AnyJsClassMember, AnyJsExpression, AnyJsObjectBindingPatternMember, AnyJsRoot, + JsArrayAssignmentPattern, JsArrowFunctionExpression, JsAssignmentExpression, + JsClassDeclaration, JsClassMemberList, JsConstructorClassMember, JsFunctionBody, JsLanguage, + JsObjectAssignmentPattern, JsObjectBindingPattern, JsPostUpdateExpression, + JsPreUpdateExpression, JsPropertyClassMember, JsStaticMemberAssignment, + JsStaticMemberExpression, JsSyntaxKind, JsSyntaxNode, JsVariableDeclarator, TextRange, + TsPropertyParameter, +}; use biome_rowan::{ AstNode, AstNodeList, AstSeparatedList, SyntaxNode, Text, WalkEvent, declare_node_union, }; use rustc_hash::FxHashSet; +use std::option::Option; #[derive(Clone)] pub struct SemanticClassServices { @@ -129,6 +130,7 @@ where pub struct ClassMemberReference { pub name: Text, pub range: TextRange, + pub is_meaningful_read: Option, } #[derive(Debug, Clone, Eq, PartialEq, Default)] @@ -141,6 +143,14 @@ declare_node_union! { pub AnyPropertyMember = JsPropertyClassMember | TsPropertyParameter } +declare_node_union! { + pub MeaningfulReadNode = AnyJsUpdateExpression | AnyJsObjectBindingPatternMember | JsStaticMemberExpression +} + +declare_node_union! { + pub AnyJsUpdateExpression = JsPreUpdateExpression | JsPostUpdateExpression +} + /// Collects all `this` property references used within the members of a JavaScript class. /// /// This function traverses a `JsClassMemberList` and extracts property references from method bodies, @@ -336,7 +346,6 @@ impl ThisScopeReferences { .filter_map(|node| node.as_js_variable_statement().cloned()) .filter_map(|stmt| stmt.declaration().ok().map(|decl| decl.declarators())) .flat_map(|declarators| { - // .into() not working here, JsVariableDeclaratorList is not implmenting it correctly declarators.into_iter().filter_map(|declaration| { declaration.ok().map(|declarator| declarator.as_fields()) }) @@ -350,6 +359,8 @@ impl ThisScopeReferences { ClassMemberReference { name: id.to_trimmed_text().clone(), range: id.syntax().text_trimmed_range(), + // right hand side of a js variable statement is meaningful read + is_meaningful_read: Some(true), } }) }) @@ -406,6 +417,7 @@ struct ThisPatternResolver {} impl ThisPatternResolver { /// Extracts `this` references from array assignments (e.g., `[this.#value]` or `[...this.#value]`). + /// only applicable to writes fn collect_array_assignment_names( array_assignment_pattern: &JsArrayAssignmentPattern, scoped_this_references: &[FunctionThisReferences], @@ -426,6 +438,8 @@ impl ThisPatternResolver { Self::extract_this_member_reference( assignment.as_js_static_member_assignment(), scoped_this_references, + // it is a write reference + None, ) }) } @@ -441,6 +455,8 @@ impl ThisPatternResolver { Self::extract_this_member_reference( assignment.as_js_static_member_assignment(), scoped_this_references, + // it is a write reference + None, ) }) } else { @@ -451,6 +467,7 @@ impl ThisPatternResolver { } /// Collects assignment names from a JavaScript object assignment pattern, e.g. `{...this.#value}`. + /// only applicable to writes fn collect_object_assignment_names( assignment: &JsObjectAssignmentPattern, scoped_this_references: &[FunctionThisReferences], @@ -468,6 +485,8 @@ impl ThisPatternResolver { return Self::extract_this_member_reference( rest_params.target().ok()?.as_js_static_member_assignment(), scoped_this_references, + // it is a write reference + None, ); } if let Some(property) = prop @@ -483,6 +502,8 @@ impl ThisPatternResolver { .as_any_js_assignment()? .as_js_static_member_assignment(), scoped_this_references, + // it is a write reference + None, ); } None @@ -501,6 +522,7 @@ impl ThisPatternResolver { fn extract_this_member_reference( operand: Option<&JsStaticMemberAssignment>, scoped_this_references: &[FunctionThisReferences], + is_meaningful_read: Option, ) -> Option { operand.and_then(|assignment| { if let Ok(object) = assignment.object() @@ -512,6 +534,7 @@ impl ThisPatternResolver { .map(|name| ClassMemberReference { name: name.to_trimmed_text(), range: name.syntax().text_trimmed_range(), + is_meaningful_read, }) .or_else(|| { member @@ -519,6 +542,7 @@ impl ThisPatternResolver { .map(|private_name| ClassMemberReference { name: private_name.to_trimmed_text(), range: private_name.syntax().text_trimmed_range(), + is_meaningful_read, }) }) }) @@ -565,7 +589,14 @@ fn visit_references_in_body( handle_object_binding_pattern(&node, scoped_this_references, reads); handle_static_member_expression(&node, scoped_this_references, reads); handle_assignment_expression(&node, scoped_this_references, reads, writes); - handle_pre_or_post_update_expression(&node, scoped_this_references, reads, writes); + if let Some(js_update_expression) = AnyJsUpdateExpression::cast_ref(&node) { + handle_pre_or_post_update_expression( + &js_update_expression, + scoped_this_references, + reads, + writes, + ); + } } WalkEvent::Leave(_) => {} } @@ -603,8 +634,9 @@ fn handle_object_binding_pattern( && is_this_reference(&expression, scoped_this_references) { reads.insert(ClassMemberReference { - name: declarator.to_trimmed_text(), - range: declarator.syntax().text_trimmed_range(), + name: declarator.clone().to_trimmed_text(), + range: declarator.clone().syntax().text_trimmed_range(), + is_meaningful_read: is_meaningful_read(&declarator.into()), }); } } @@ -640,6 +672,7 @@ fn handle_static_member_expression( reads.insert(ClassMemberReference { name: member.to_trimmed_text(), range: static_member.syntax().text_trimmed_range(), + is_meaningful_read: is_meaningful_read(&static_member.into()), }); } } @@ -685,6 +718,8 @@ fn handle_assignment_expression( && let Some(name) = ThisPatternResolver::extract_this_member_reference( operand.as_js_static_member_assignment(), scoped_this_references, + // nodes inside assignment expressions are considered meaningful reads e.g. this.x += 1; + Some(true), ) { reads.insert(name.clone()); @@ -711,6 +746,8 @@ fn handle_assignment_expression( && let Some(name) = ThisPatternResolver::extract_this_member_reference( assignment.as_js_static_member_assignment(), scoped_this_references, + // it is a write reference + None, ) { writes.insert(name.clone()); @@ -733,23 +770,35 @@ fn handle_assignment_expression( /// } /// ``` fn handle_pre_or_post_update_expression( - node: &SyntaxNode, + js_update_expression: &AnyJsUpdateExpression, scoped_this_references: &[FunctionThisReferences], reads: &mut FxHashSet, writes: &mut FxHashSet, ) { - let operand = JsPostUpdateExpression::cast_ref(node) - .and_then(|expr| expr.operand().ok()) - .or_else(|| JsPreUpdateExpression::cast_ref(node).and_then(|expr| expr.operand().ok())); + let operand = match js_update_expression { + AnyJsUpdateExpression::JsPreUpdateExpression(expr) => expr.operand().ok(), + AnyJsUpdateExpression::JsPostUpdateExpression(expr) => expr.operand().ok(), + }; if let Some(operand) = operand && let Some(name) = ThisPatternResolver::extract_this_member_reference( operand.as_js_static_member_assignment(), scoped_this_references, + None, ) { - writes.insert(name.clone()); - reads.insert(name.clone()); + writes.insert(ClassMemberReference { + name: name.name.clone(), + range: name.range, + is_meaningful_read: None, + }); + reads.insert(ClassMemberReference { + name: name.name, + range: name.range, + is_meaningful_read: is_meaningful_read(&MeaningfulReadNode::from( + js_update_expression.clone(), + )), + }); } } @@ -790,6 +839,9 @@ fn collect_class_property_reads_from_static_member( reads.insert(ClassMemberReference { name, range: static_member.syntax().text_trimmed_range(), + is_meaningful_read: is_meaningful_read(&MeaningfulReadNode::from( + static_member.clone(), + )), }); } @@ -817,6 +869,43 @@ fn is_within_scope_without_shadowing( false } +pub fn is_meaningful_read(node: &MeaningfulReadNode) -> Option { + is_used_in_expression_context(node) +} + +fn skip_parentheses(mut node: JsSyntaxNode) -> JsSyntaxNode { + while let Some(child) = node.first_child() { + if child.kind() == JsSyntaxKind::JS_PARENTHESIZED_EXPRESSION { + node = child; // move ownership + } else { + break; + } + } + node +} + +fn is_used_in_expression_context(node: &MeaningfulReadNode) -> Option { + let mut current: JsSyntaxNode = node.syntax().clone(); + + // Limit the number of parent traversals to avoid deep recursion + for _ in 0..8 { + if let Some(parent) = current.parent() { + let parent = skip_parentheses(parent); // strip parentheses + match parent.kind() { + JsSyntaxKind::JS_RETURN_STATEMENT + | JsSyntaxKind::JS_CALL_ARGUMENTS + | JsSyntaxKind::JS_CONDITIONAL_EXPRESSION + | JsSyntaxKind::JS_LOGICAL_EXPRESSION + | JsSyntaxKind::JS_BINARY_EXPRESSION => return Some(true), + _ => current = parent, + } + } else { + break; + } + } + Some(false) +} + #[cfg(test)] mod tests { use super::*; @@ -827,8 +916,56 @@ mod tests { struct TestCase<'a> { description: &'a str, code: &'a str, - expected_reads: Vec<&'a str>, - expected_writes: Vec<&'a str>, + expected_reads: Vec<(&'a str, Option)>, // (name, is_meaningful_read) + expected_writes: Vec<(&'a str, Option)>, // (name, is_meaningful_read) + } + + fn assert_reads( + reads: &FxHashSet, + expected: &[(&str, Option)], + description: &str, + ) { + for (expected_name, expected_meaningful) in expected { + let found = reads + .iter() + .find(|r| r.name.clone().text() == *expected_name) + .unwrap_or_else(|| { + panic!( + "Case '{}' failed: expected to find read '{}'", + description, expected_name + ) + }); + + assert_eq!( + found.is_meaningful_read, *expected_meaningful, + "Case '{}' failed: read '{}' meaningful mismatch", + description, expected_name + ); + } + } + + fn assert_writes( + writes: &FxHashSet, + expected: &[(&str, Option)], + description: &str, + ) { + for (expected_name, expected_meaningful) in expected { + let found = writes + .iter() + .find(|r| r.name.clone().text() == *expected_name) + .unwrap_or_else(|| { + panic!( + "Case '{}' failed: expected to find write '{}'", + description, expected_name + ) + }); + + assert_eq!( + found.is_meaningful_read, *expected_meaningful, + "Case '{}' failed: write '{}' meaningful mismatch", + description, expected_name + ); + } } fn parse_ts(code: &str) -> Parse { @@ -857,26 +994,26 @@ mod tests { TestCase { description: "reads from this", code: r#" - class Example { - method() { - const { foo, bar } = this; - } + class Example { + method() { + const { foo, bar } = this; } - "#, - expected_reads: vec!["foo", "bar"], + } + "#, + expected_reads: vec![("foo", Some(false)), ("bar", Some(false))], expected_writes: vec![], }, TestCase { description: "reads from aliasForThis", code: r#" - class Example { - method() { - const aliasForThis = this; - const { baz, qux } = aliasForThis; - } + class Example { + method() { + const aliasForThis = this; + const { baz, qux } = aliasForThis; } - "#, - expected_reads: vec!["baz", "qux"], + } + "#, + expected_reads: vec![("baz", Some(false)), ("qux", Some(false))], expected_writes: vec![], }, ]; @@ -896,16 +1033,7 @@ mod tests { handle_object_binding_pattern(&node, &function_this_references, &mut reads); - let names: Vec<_> = reads.into_iter().map(|r| r.name.to_string()).collect(); - - for expected_reads in &case.expected_reads { - assert!( - names.contains(&(*expected_reads).to_string()), - "Case '{}' failed: expected to find '{}'", - case.description, - expected_reads - ); - } + assert_reads(&reads, &case.expected_reads, case.description); } } @@ -915,28 +1043,28 @@ mod tests { TestCase { description: "reads static members from this", code: r#" - class Example { - method() { - console.log(this.foo); - console.log(this.bar); - } + class Example { + method() { + console.log(this.foo); + console.log(this.bar); } - "#, - expected_reads: vec!["foo", "bar"], + } + "#, + expected_reads: vec![("foo", Some(true)), ("bar", Some(true))], expected_writes: vec![], }, TestCase { description: "reads static members from aliasForThis", code: r#" - class Example { - method() { - const aliasForThis = this; - aliasForThis.baz; - aliasForThis.qux; - } + class Example { + method() { + const aliasForThis = this; + aliasForThis.baz; + aliasForThis.qux; } - "#, - expected_reads: vec!["baz", "qux"], + } + "#, + expected_reads: vec![("baz", Some(true)), ("qux", Some(true))], expected_writes: vec![], }, ]; @@ -952,7 +1080,6 @@ mod tests { let function_this_references = ThisScopeReferences::new(&body).collect_function_this_references(); - // Collect all static member expressions in the syntax let mut reads = FxHashSet::default(); for member_expr in syntax @@ -966,16 +1093,7 @@ mod tests { ); } - let names: Vec<_> = reads.into_iter().map(|r| r.name.to_string()).collect(); - - for expected_reads in &case.expected_reads { - assert!( - names.contains(&(*expected_reads).to_string()), - "Case '{}' failed: expected to find '{}'", - case.description, - expected_reads - ); - } + assert_reads(&reads, &case.expected_reads, case.description); } } @@ -985,32 +1103,32 @@ mod tests { TestCase { description: "assignment reads and writes with this", code: r#" - class Example { - method() { - this.x += 1; - [this.y] = [10]; - ({ a: this.z } = obj); - } + class Example { + method() { + this.x += 1; + [this.y] = [10]; + ({ a: this.z } = obj); } - "#, - expected_reads: vec!["x"], - expected_writes: vec!["x", "y", "z"], + } + "#, + expected_reads: vec![("x", Some(true))], // x is read due to += + expected_writes: vec![("x", None), ("y", None), ("z", None)], }, TestCase { description: "assignment reads and writes with aliasForThis", code: r#" - class Example { - method() { - const aliasForThis = this; - [aliasForThis.value] = [42]; - aliasForThis.x += 1; - [aliasForThis.y] = [10]; - ({ a: aliasForThis.z } = obj); - } + class Example { + method() { + const aliasForThis = this; + [aliasForThis.value] = [42]; + aliasForThis.x += 1; + [aliasForThis.y] = [10]; + ({ a: aliasForThis.z } = obj); } - "#, - expected_reads: vec!["x"], - expected_writes: vec!["x", "y", "z"], + } + "#, + expected_reads: vec![("x", Some(true))], + expected_writes: vec![("x", None), ("y", None), ("z", None)], }, ]; @@ -1040,26 +1158,8 @@ mod tests { ); } - let read_names: Vec<_> = reads.into_iter().map(|r| r.name.to_string()).collect(); - let write_names: Vec<_> = writes.into_iter().map(|r| r.name.to_string()).collect(); - - for expected_read in &case.expected_reads { - assert!( - read_names.contains(&(*expected_read).to_string()), - "Case '{}' failed: expected to find read '{}'", - case.description, - expected_read - ); - } - - for expected_write in &case.expected_writes { - assert!( - write_names.contains(&(*expected_write).to_string()), - "Case '{}' failed: expected to find write '{}'", - case.description, - expected_write - ); - } + assert_reads(&reads, &case.expected_reads, case.description); + assert_writes(&writes, &case.expected_writes, case.description); } } @@ -1069,15 +1169,30 @@ mod tests { TestCase { description: "pre/post update expressions on this properties", code: r#" - class Example { + class AnyJsUpdateExpression { method() { this.count++; --this.total; + + if (this.inIfCondition++ > 5) { + } + + return this.inReturn++; } } "#, - expected_reads: vec!["count", "total"], - expected_writes: vec!["count", "total"], + expected_reads: vec![ + ("count", Some(false)), + ("total", Some(false)), + ("inIfCondition", Some(true)), + ("inReturn", Some(true)), + ], + expected_writes: vec![ + ("count", None), + ("total", None), + ("inIfCondition", None), + ("inReturn", None), + ], }, TestCase { description: "pre/post update expressions on aliasForThis properties", @@ -1086,13 +1201,23 @@ mod tests { method() { const aliasForThis = this; const anotherAlias = this; - aliasForThis.count++; + aliasForThis.count++; --anotherAlias.total; + + return anotherAlias.inReturnIncrement++; } } "#, - expected_reads: vec!["count", "total"], - expected_writes: vec!["count", "total"], + expected_reads: vec![ + ("count", Some(false)), + ("total", Some(false)), + ("inReturnIncrement", Some(true)), + ], + expected_writes: vec![ + ("count", None), + ("total", None), + ("inReturnIncrement", None), + ], }, ]; @@ -1111,34 +1236,138 @@ mod tests { let mut writes = FxHashSet::default(); for node in syntax.descendants() { - handle_pre_or_post_update_expression( - &node, - &function_this_references, - &mut reads, - &mut writes, - ); + if let Some(js_update_expression) = AnyJsUpdateExpression::cast_ref(&node) { + handle_pre_or_post_update_expression( + &js_update_expression, + &function_this_references, + &mut reads, + &mut writes, + ); + } } - let read_names: Vec<_> = reads.into_iter().map(|r| r.name.to_string()).collect(); - let write_names: Vec<_> = writes.into_iter().map(|r| r.name.to_string()).collect(); + assert_reads(&reads, &case.expected_reads, case.description); + assert_writes(&writes, &case.expected_writes, case.description); + } + } - for expected_name in &case.expected_reads { - assert!( - read_names.contains(&(*expected_name).to_string()), - "Case '{}' failed: expected to find read '{}'", - case.description, - expected_name - ); + mod is_meaningful_read_tests { + use super::*; + fn extract_all_meaningful_nodes(code: &str) -> Vec { + let parsed = parse_ts(code); + let root = parsed.syntax(); + + let mut nodes = vec![]; + + for descendant in root.descendants() { + // Try to cast the node itself + if let Some(node) = MeaningfulReadNode::cast_ref(&descendant) { + nodes.push(node); + } + + // If this is an assignment, also include LHS + if let Some(assign_expr) = JsAssignmentExpression::cast_ref(&descendant) { + if let Ok(lhs) = assign_expr.left() + && let Some(node) = MeaningfulReadNode::cast_ref(lhs.syntax()) + { + nodes.push(node.clone()); + } + + if let Ok(rhs) = assign_expr.right() + && let Some(node) = MeaningfulReadNode::cast_ref(rhs.syntax()) + { + nodes.push(node.clone()); + } + } } - for expected_name in &case.expected_writes { + nodes + } + + struct TestCase<'a> { + code: &'a str, + node_index: usize, + expected: Option, + } + + fn run_test_cases(cases: &[TestCase]) { + for case in cases { + let nodes = extract_all_meaningful_nodes(case.code); assert!( - write_names.contains(&(*expected_name).to_string()), - "Case '{}' failed: expected to find write '{}'", - case.description, - expected_name + nodes.len() > case.node_index, + "Not enough nodes found in code: {}", + case.code + ); + + let node = &nodes[case.node_index]; + let meaningful_node = MeaningfulReadNode::cast_ref(node.syntax()) + .expect("Failed to cast node to MeaningfulReadNode"); + + assert_eq!( + is_meaningful_read(&meaningful_node), + case.expected, + "Failed for code: {}", + case.code ); } } + + #[test] + fn test_meaningful_reads() { + let cases = [ + TestCase { + code: "class Test { method() { return this.x; } }", + node_index: 0, + expected: Some(true), + }, + TestCase { + code: "class Test { method() { if(this.x === 2) {} } }", + node_index: 0, + expected: Some(true), + }, + TestCase { + code: "class Test { method() { foo(this.x); } }", + node_index: 0, + expected: Some(true), + }, + TestCase { + code: "class Test { method() { const y = this.x + 1; } }", + node_index: 0, + expected: Some(true), + }, + TestCase { + code: "class Test { method() { bar(this.x || 0); } }", + node_index: 0, + expected: Some(true), + }, + TestCase { + code: "class Test { method() { this.x--; } }", + node_index: 0, + expected: Some(false), + }, + TestCase { + code: "class Test { method() { const unused = this.y; } }", + node_index: 0, + expected: Some(false), + }, + TestCase { + code: "class Test { method() { ++this.x; } }", + node_index: 0, + expected: Some(false), + }, + TestCase { + code: "class Test { method() { const incremented = this.x + 1; } }", + node_index: 0, + expected: Some(true), + }, + TestCase { + code: "class Test { method() { return this.x; } }", + node_index: 0, + expected: Some(true), + }, + ]; + + run_test_cases(&cases); + } } } From ef15859ee362c32b2111c42ede0bfa876a1038a2 Mon Sep 17 00:00:00 2001 From: vladimirivanov Date: Sat, 20 Sep 2025 16:46:33 +0100 Subject: [PATCH 02/25] feat(biome-js-analyze): add is_meaningful_read to semantic class reads --- crates/biome_js_analyze/src/services/semantic_class.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/biome_js_analyze/src/services/semantic_class.rs b/crates/biome_js_analyze/src/services/semantic_class.rs index 0d2cbf93d9bf..593eef12c211 100644 --- a/crates/biome_js_analyze/src/services/semantic_class.rs +++ b/crates/biome_js_analyze/src/services/semantic_class.rs @@ -636,7 +636,7 @@ fn handle_object_binding_pattern( reads.insert(ClassMemberReference { name: declarator.clone().to_trimmed_text(), range: declarator.clone().syntax().text_trimmed_range(), - is_meaningful_read: is_meaningful_read(&declarator.into()), + is_meaningful_read: is_meaningful_read(&declarator.clone().into()), }); } } @@ -1064,7 +1064,7 @@ mod tests { } } "#, - expected_reads: vec![("baz", Some(true)), ("qux", Some(true))], + expected_reads: vec![("baz", Some(false)), ("qux", Some(false))], expected_writes: vec![], }, ]; From 87d54c6f6877356abe1f7dd0ac3f901c59429678 Mon Sep 17 00:00:00 2001 From: vladimirivanov Date: Sat, 20 Sep 2025 16:52:09 +0100 Subject: [PATCH 03/25] feat(biome-js-analyze): add is_meaningful_read to semantic class reads --- crates/biome_js_analyze/src/services/semantic_class.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/biome_js_analyze/src/services/semantic_class.rs b/crates/biome_js_analyze/src/services/semantic_class.rs index 593eef12c211..6e933ea9a0e8 100644 --- a/crates/biome_js_analyze/src/services/semantic_class.rs +++ b/crates/biome_js_analyze/src/services/semantic_class.rs @@ -294,7 +294,6 @@ impl ThisScopeVisitor<'_> { } WalkEvent::Leave(node) => { - // println!("leave node in ThisScopeVisitor {:?}", node); if let Some(last) = self.skipped_ranges.last() && *last == node.text_range() { From ab01a06e583b5cc4cc82775b5139af26e0ad3137 Mon Sep 17 00:00:00 2001 From: vladimirivanov Date: Sat, 20 Sep 2025 17:16:41 +0100 Subject: [PATCH 04/25] feat(biome-js-analyze): add is_meaningful_read to semantic class reads --- .../biome_js_analyze/src/services/semantic_class.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/biome_js_analyze/src/services/semantic_class.rs b/crates/biome_js_analyze/src/services/semantic_class.rs index 6e933ea9a0e8..12624c4dc789 100644 --- a/crates/biome_js_analyze/src/services/semantic_class.rs +++ b/crates/biome_js_analyze/src/services/semantic_class.rs @@ -872,12 +872,13 @@ pub fn is_meaningful_read(node: &MeaningfulReadNode) -> Option { is_used_in_expression_context(node) } -fn skip_parentheses(mut node: JsSyntaxNode) -> JsSyntaxNode { - while let Some(child) = node.first_child() { - if child.kind() == JsSyntaxKind::JS_PARENTHESIZED_EXPRESSION { - node = child; // move ownership - } else { - break; +/// If the node is a parenthesized expression, return the inner expression, +/// otherwise return the node itself. +fn skip_parentheses(node: JsSyntaxNode) -> JsSyntaxNode { + if node.kind() == JsSyntaxKind::JS_PARENTHESIZED_EXPRESSION { + // Assume the first child is the expression inside parentheses + if let Some(inner) = node.first_child() { + return inner; } } node From 3de314f3cbfbed1c0283b47ff9ad6be1df548a67 Mon Sep 17 00:00:00 2001 From: vladimirivanov Date: Sat, 20 Sep 2025 19:09:04 +0100 Subject: [PATCH 05/25] feat(biome-js-analyze): add is_meaningful_read to semantic class reads --- crates/biome_js_analyze/src/services/semantic_class.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/biome_js_analyze/src/services/semantic_class.rs b/crates/biome_js_analyze/src/services/semantic_class.rs index 12624c4dc789..308200fe36cd 100644 --- a/crates/biome_js_analyze/src/services/semantic_class.rs +++ b/crates/biome_js_analyze/src/services/semantic_class.rs @@ -670,7 +670,7 @@ fn handle_static_member_expression( { reads.insert(ClassMemberReference { name: member.to_trimmed_text(), - range: static_member.syntax().text_trimmed_range(), + range: member.syntax().text_trimmed_range(), is_meaningful_read: is_meaningful_read(&static_member.into()), }); } From 01d870b5c36d5aace36fc34cb33a13e53bf5b61f Mon Sep 17 00:00:00 2001 From: vladimir ivanov Date: Sun, 21 Sep 2025 11:40:35 +0100 Subject: [PATCH 06/25] Update crates/biome_js_analyze/src/services/semantic_class.rs Co-authored-by: Emanuele Stoppa --- crates/biome_js_analyze/src/services/semantic_class.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/biome_js_analyze/src/services/semantic_class.rs b/crates/biome_js_analyze/src/services/semantic_class.rs index 308200fe36cd..5e8311ffc35a 100644 --- a/crates/biome_js_analyze/src/services/semantic_class.rs +++ b/crates/biome_js_analyze/src/services/semantic_class.rs @@ -466,7 +466,7 @@ impl ThisPatternResolver { } /// Collects assignment names from a JavaScript object assignment pattern, e.g. `{...this.#value}`. - /// only applicable to writes + /// Only applicable to writes. fn collect_object_assignment_names( assignment: &JsObjectAssignmentPattern, scoped_this_references: &[FunctionThisReferences], From c986c89d9b3d20517c7715ed85719ade0911e3d7 Mon Sep 17 00:00:00 2001 From: vladimir ivanov Date: Sun, 21 Sep 2025 12:04:59 +0100 Subject: [PATCH 07/25] Update crates/biome_js_analyze/src/services/semantic_class.rs Co-authored-by: Emanuele Stoppa --- crates/biome_js_analyze/src/services/semantic_class.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/biome_js_analyze/src/services/semantic_class.rs b/crates/biome_js_analyze/src/services/semantic_class.rs index 5e8311ffc35a..9c239cd18a29 100644 --- a/crates/biome_js_analyze/src/services/semantic_class.rs +++ b/crates/biome_js_analyze/src/services/semantic_class.rs @@ -416,7 +416,7 @@ struct ThisPatternResolver {} impl ThisPatternResolver { /// Extracts `this` references from array assignments (e.g., `[this.#value]` or `[...this.#value]`). - /// only applicable to writes + /// Only applicable to writes. fn collect_array_assignment_names( array_assignment_pattern: &JsArrayAssignmentPattern, scoped_this_references: &[FunctionThisReferences], From 7d751109bb92da1650a42069062c513359bc9fd1 Mon Sep 17 00:00:00 2001 From: vladimirivanov Date: Sun, 21 Sep 2025 12:11:11 +0100 Subject: [PATCH 08/25] feat(biome-js-analyze): add is_meaningful_read to semantic class reads --- .../src/services/semantic_class.rs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/crates/biome_js_analyze/src/services/semantic_class.rs b/crates/biome_js_analyze/src/services/semantic_class.rs index 9c239cd18a29..a5bf1eb15ac5 100644 --- a/crates/biome_js_analyze/src/services/semantic_class.rs +++ b/crates/biome_js_analyze/src/services/semantic_class.rs @@ -144,7 +144,7 @@ declare_node_union! { } declare_node_union! { - pub MeaningfulReadNode = AnyJsUpdateExpression | AnyJsObjectBindingPatternMember | JsStaticMemberExpression + pub AnyMeaningfulReadNode = AnyJsUpdateExpression | AnyJsObjectBindingPatternMember | JsStaticMemberExpression } declare_node_union! { @@ -794,7 +794,7 @@ fn handle_pre_or_post_update_expression( reads.insert(ClassMemberReference { name: name.name, range: name.range, - is_meaningful_read: is_meaningful_read(&MeaningfulReadNode::from( + is_meaningful_read: is_meaningful_read(&AnyMeaningfulReadNode::from( js_update_expression.clone(), )), }); @@ -838,7 +838,7 @@ fn collect_class_property_reads_from_static_member( reads.insert(ClassMemberReference { name, range: static_member.syntax().text_trimmed_range(), - is_meaningful_read: is_meaningful_read(&MeaningfulReadNode::from( + is_meaningful_read: is_meaningful_read(&AnyMeaningfulReadNode::from( static_member.clone(), )), }); @@ -868,7 +868,7 @@ fn is_within_scope_without_shadowing( false } -pub fn is_meaningful_read(node: &MeaningfulReadNode) -> Option { +pub fn is_meaningful_read(node: &AnyMeaningfulReadNode) -> Option { is_used_in_expression_context(node) } @@ -884,7 +884,7 @@ fn skip_parentheses(node: JsSyntaxNode) -> JsSyntaxNode { node } -fn is_used_in_expression_context(node: &MeaningfulReadNode) -> Option { +fn is_used_in_expression_context(node: &AnyMeaningfulReadNode) -> Option { let mut current: JsSyntaxNode = node.syntax().clone(); // Limit the number of parent traversals to avoid deep recursion @@ -1253,7 +1253,7 @@ mod tests { mod is_meaningful_read_tests { use super::*; - fn extract_all_meaningful_nodes(code: &str) -> Vec { + fn extract_all_meaningful_nodes(code: &str) -> Vec { let parsed = parse_ts(code); let root = parsed.syntax(); @@ -1261,20 +1261,20 @@ mod tests { for descendant in root.descendants() { // Try to cast the node itself - if let Some(node) = MeaningfulReadNode::cast_ref(&descendant) { + if let Some(node) = AnyMeaningfulReadNode::cast_ref(&descendant) { nodes.push(node); } // If this is an assignment, also include LHS if let Some(assign_expr) = JsAssignmentExpression::cast_ref(&descendant) { if let Ok(lhs) = assign_expr.left() - && let Some(node) = MeaningfulReadNode::cast_ref(lhs.syntax()) + && let Some(node) = AnyMeaningfulReadNode::cast_ref(lhs.syntax()) { nodes.push(node.clone()); } if let Ok(rhs) = assign_expr.right() - && let Some(node) = MeaningfulReadNode::cast_ref(rhs.syntax()) + && let Some(node) = AnyMeaningfulReadNode::cast_ref(rhs.syntax()) { nodes.push(node.clone()); } @@ -1300,7 +1300,7 @@ mod tests { ); let node = &nodes[case.node_index]; - let meaningful_node = MeaningfulReadNode::cast_ref(node.syntax()) + let meaningful_node = AnyMeaningfulReadNode::cast_ref(node.syntax()) .expect("Failed to cast node to MeaningfulReadNode"); assert_eq!( From 790145afd9b10f1d58834025712dfc3d04e285f0 Mon Sep 17 00:00:00 2001 From: vladimirivanov Date: Sun, 21 Sep 2025 12:20:43 +0100 Subject: [PATCH 09/25] feat(biome-js-analyze): add is_meaningful_read to semantic class reads --- .../src/services/semantic_class.rs | 36 ++++++++----------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/crates/biome_js_analyze/src/services/semantic_class.rs b/crates/biome_js_analyze/src/services/semantic_class.rs index a5bf1eb15ac5..7e905e6d8aed 100644 --- a/crates/biome_js_analyze/src/services/semantic_class.rs +++ b/crates/biome_js_analyze/src/services/semantic_class.rs @@ -230,7 +230,7 @@ struct ThisScopeVisitor<'a> { inherited_this_references: &'a [ClassMemberReference], current_this_scopes: Vec, } -// can not implement `Visitor` directly because it requires a new ctx that can not be created here +// Can not implement `Visitor` directly because it requires a new ctx that can not be created here impl ThisScopeVisitor<'_> { fn visit(&mut self, event: &WalkEvent>) { match event { @@ -358,7 +358,7 @@ impl ThisScopeReferences { ClassMemberReference { name: id.to_trimmed_text().clone(), range: id.syntax().text_trimmed_range(), - // right hand side of a js variable statement is meaningful read + // Right hand side of a js variable statement is meaningful read is_meaningful_read: Some(true), } }) @@ -437,7 +437,7 @@ impl ThisPatternResolver { Self::extract_this_member_reference( assignment.as_js_static_member_assignment(), scoped_this_references, - // it is a write reference + // It is a write reference, so not applicable for meaningful read None, ) }) @@ -454,7 +454,7 @@ impl ThisPatternResolver { Self::extract_this_member_reference( assignment.as_js_static_member_assignment(), scoped_this_references, - // it is a write reference + // It is a write reference, so not applicable for meaningful read None, ) }) @@ -484,7 +484,7 @@ impl ThisPatternResolver { return Self::extract_this_member_reference( rest_params.target().ok()?.as_js_static_member_assignment(), scoped_this_references, - // it is a write reference + // It is a write reference, so not applicable for meaningful read None, ); } @@ -501,7 +501,7 @@ impl ThisPatternResolver { .as_any_js_assignment()? .as_js_static_member_assignment(), scoped_this_references, - // it is a write reference + // It is a write reference, so not applicable for meaningful read None, ); } @@ -717,7 +717,7 @@ fn handle_assignment_expression( && let Some(name) = ThisPatternResolver::extract_this_member_reference( operand.as_js_static_member_assignment(), scoped_this_references, - // nodes inside assignment expressions are considered meaningful reads e.g. this.x += 1; + // Nodes inside assignment expressions are considered meaningful reads e.g. this.x += 1; Some(true), ) { @@ -745,7 +745,7 @@ fn handle_assignment_expression( && let Some(name) = ThisPatternResolver::extract_this_member_reference( assignment.as_js_static_member_assignment(), scoped_this_references, - // it is a write reference + // It is a write reference, so not applicable for meaningful read None, ) { @@ -872,25 +872,17 @@ pub fn is_meaningful_read(node: &AnyMeaningfulReadNode) -> Option { is_used_in_expression_context(node) } -/// If the node is a parenthesized expression, return the inner expression, -/// otherwise return the node itself. -fn skip_parentheses(node: JsSyntaxNode) -> JsSyntaxNode { - if node.kind() == JsSyntaxKind::JS_PARENTHESIZED_EXPRESSION { - // Assume the first child is the expression inside parentheses - if let Some(inner) = node.first_child() { - return inner; - } - } - node -} - fn is_used_in_expression_context(node: &AnyMeaningfulReadNode) -> Option { - let mut current: JsSyntaxNode = node.syntax().clone(); + let mut current = + if let Some(expression) = AnyJsExpression::cast(node.syntax().into()) { + expression.omit_parentheses().syntax().clone() + } else { + node.syntax().clone() + }; // Limit the number of parent traversals to avoid deep recursion for _ in 0..8 { if let Some(parent) = current.parent() { - let parent = skip_parentheses(parent); // strip parentheses match parent.kind() { JsSyntaxKind::JS_RETURN_STATEMENT | JsSyntaxKind::JS_CALL_ARGUMENTS From 6a7ae800b27905c30cd324a5c04293dc3bd19d71 Mon Sep 17 00:00:00 2001 From: vladimirivanov Date: Sun, 21 Sep 2025 12:22:28 +0100 Subject: [PATCH 10/25] feat(biome-js-analyze): add is_meaningful_read to semantic class reads --- .../biome_js_analyze/src/services/semantic_class.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/biome_js_analyze/src/services/semantic_class.rs b/crates/biome_js_analyze/src/services/semantic_class.rs index 7e905e6d8aed..678ccbf320a4 100644 --- a/crates/biome_js_analyze/src/services/semantic_class.rs +++ b/crates/biome_js_analyze/src/services/semantic_class.rs @@ -130,6 +130,8 @@ where pub struct ClassMemberReference { pub name: Text, pub range: TextRange, + /// Indicates if the read is meaningful (e.g., used in an expression) or not (e.g. part of a destructuring assignment). + /// `None` if not applicable (e.g. for write references). pub is_meaningful_read: Option, } @@ -873,12 +875,11 @@ pub fn is_meaningful_read(node: &AnyMeaningfulReadNode) -> Option { } fn is_used_in_expression_context(node: &AnyMeaningfulReadNode) -> Option { - let mut current = - if let Some(expression) = AnyJsExpression::cast(node.syntax().into()) { - expression.omit_parentheses().syntax().clone() - } else { - node.syntax().clone() - }; + let mut current = if let Some(expression) = AnyJsExpression::cast(node.syntax().into()) { + expression.omit_parentheses().syntax().clone() + } else { + node.syntax().clone() + }; // Limit the number of parent traversals to avoid deep recursion for _ in 0..8 { From 9272a2588130bae3d11fc5adea598cce0190ef48 Mon Sep 17 00:00:00 2001 From: vladimirivanov Date: Sun, 21 Sep 2025 13:08:46 +0100 Subject: [PATCH 11/25] feat(biome-js-analyze): add is_meaningful_read to semantic class reads --- .../src/services/semantic_class.rs | 150 ++++++++++++------ 1 file changed, 103 insertions(+), 47 deletions(-) diff --git a/crates/biome_js_analyze/src/services/semantic_class.rs b/crates/biome_js_analyze/src/services/semantic_class.rs index 678ccbf320a4..68475938bd8f 100644 --- a/crates/biome_js_analyze/src/services/semantic_class.rs +++ b/crates/biome_js_analyze/src/services/semantic_class.rs @@ -875,10 +875,10 @@ pub fn is_meaningful_read(node: &AnyMeaningfulReadNode) -> Option { } fn is_used_in_expression_context(node: &AnyMeaningfulReadNode) -> Option { - let mut current = if let Some(expression) = AnyJsExpression::cast(node.syntax().into()) { - expression.omit_parentheses().syntax().clone() + let mut current: JsSyntaxNode = if let Some(expression) = AnyJsExpression::cast(node.syntax().clone()) { + expression.syntax().clone() // get JsSyntaxNode } else { - node.syntax().clone() + node.syntax().clone() // fallback to the node itself }; // Limit the number of parent traversals to avoid deep recursion @@ -889,6 +889,18 @@ fn is_used_in_expression_context(node: &AnyMeaningfulReadNode) -> Option { | JsSyntaxKind::JS_CALL_ARGUMENTS | JsSyntaxKind::JS_CONDITIONAL_EXPRESSION | JsSyntaxKind::JS_LOGICAL_EXPRESSION + | JsSyntaxKind::JS_THROW_STATEMENT + | JsSyntaxKind::JS_AWAIT_EXPRESSION + | JsSyntaxKind::JS_YIELD_EXPRESSION + | JsSyntaxKind::JS_UNARY_EXPRESSION + | JsSyntaxKind::JS_TEMPLATE_EXPRESSION + | JsSyntaxKind::JS_CALL_EXPRESSION // (callee) + | JsSyntaxKind::JS_NEW_EXPRESSION + | JsSyntaxKind::JS_IF_STATEMENT + | JsSyntaxKind::JS_SWITCH_STATEMENT + | JsSyntaxKind::JS_FOR_STATEMENT + | JsSyntaxKind::JS_FOR_IN_STATEMENT + | JsSyntaxKind::JS_FOR_OF_STATEMENT | JsSyntaxKind::JS_BINARY_EXPRESSION => return Some(true), _ => current = parent, } @@ -1246,6 +1258,7 @@ mod tests { mod is_meaningful_read_tests { use super::*; + fn extract_all_meaningful_nodes(code: &str) -> Vec { let parsed = parse_ts(code); let root = parsed.syntax(); @@ -1278,85 +1291,128 @@ mod tests { } struct TestCase<'a> { + description: &'a str, code: &'a str, - node_index: usize, - expected: Option, + expected: Vec<(&'a str, Option)>, // (member name, is_meaningful_read) } fn run_test_cases(cases: &[TestCase]) { for case in cases { let nodes = extract_all_meaningful_nodes(case.code); assert!( - nodes.len() > case.node_index, - "Not enough nodes found in code: {}", - case.code + !nodes.is_empty(), + "No match found for test case: '{}'", + case.description ); - let node = &nodes[case.node_index]; - let meaningful_node = AnyMeaningfulReadNode::cast_ref(node.syntax()) - .expect("Failed to cast node to MeaningfulReadNode"); - + // Ensure the number of nodes matches expected assert_eq!( - is_meaningful_read(&meaningful_node), - case.expected, - "Failed for code: {}", - case.code + nodes.len(), + case.expected.len(), + "Number of nodes does not match expected for test case: '{}'", + case.description ); + + for (node, (expected_name, expected_meaningful)) in nodes.iter().zip(&case.expected) { + let meaningful_node = AnyMeaningfulReadNode::cast_ref(node.syntax()) + .expect("Failed to cast node to AnyMeaningfulReadNode"); + + // Compare node name + let node_name = meaningful_node.to_trimmed_text(); + assert_eq!( + &node_name, expected_name, + "Node name mismatch for test case: '{}'", + case.description + ); + + // Compare is_meaningful_read + let actual_meaningful = is_meaningful_read(&meaningful_node); + assert_eq!( + actual_meaningful, *expected_meaningful, + "Meaningful read mismatch for node '{}' in test case: '{}'", + expected_name, case.description + ); + } } } #[test] - fn test_meaningful_reads() { + fn test_meaningful_read_contexts() { let cases = [ TestCase { - code: "class Test { method() { return this.x; } }", - node_index: 0, - expected: Some(true), + description: "return statement", + code: r#"class Test { method() { return this.x; } }"#, + expected: vec![("this.x", Some(true))], + }, + TestCase { + description: "call arguments", + code: r#"class Test { method() { foo(this.y); } }"#, + expected: vec![("this.y", Some(true))], + }, + TestCase { + description: "conditional expression", + code: r#"class Test { method() { const a = this.z ? 1 : 2; } }"#, + expected: vec![("this.z", Some(true))], + }, + TestCase { + description: "logical expression", + code: r#"class Test { method() { const a = this.a && this.b; } }"#, + expected: vec![("this.a", Some(true)), ("this.b", Some(true))], + }, + TestCase { + description: "throw statement", + code: r#"class Test { method() { throw this.err; } }"#, + expected: vec![("this.err", Some(true))], + }, + TestCase { + description: "await expression", + code: r#"class Test { async method() { await this.promise; } }"#, + expected: vec![("this.promise", Some(true))], }, TestCase { - code: "class Test { method() { if(this.x === 2) {} } }", - node_index: 0, - expected: Some(true), + description: "yield expression", + code: r#"class Test { *method() { yield this.gen; } }"#, + expected: vec![("this.gen", Some(true))], }, TestCase { - code: "class Test { method() { foo(this.x); } }", - node_index: 0, - expected: Some(true), + description: "unary expression", + code: r#"class Test { method() { -this.num; } }"#, + expected: vec![("this.num", Some(true))], }, TestCase { - code: "class Test { method() { const y = this.x + 1; } }", - node_index: 0, - expected: Some(true), + description: "template expression", + code: r#"class Test { method() { `${this.str}`; } }"#, + expected: vec![("this.str", Some(true))], }, TestCase { - code: "class Test { method() { bar(this.x || 0); } }", - node_index: 0, - expected: Some(true), + description: "call expression callee", + code: r#"class Test { method() { this.func(); } }"#, + expected: vec![("this.func", Some(true))], }, TestCase { - code: "class Test { method() { this.x--; } }", - node_index: 0, - expected: Some(false), + description: "new expression", + code: r#"class Test { method() { new this.ClassName(); } }"#, + expected: vec![("this.ClassName", Some(true))], }, TestCase { - code: "class Test { method() { const unused = this.y; } }", - node_index: 0, - expected: Some(false), + description: "if statement", + code: r#"class Test { method() { if(this.cond) {} } }"#, + expected: vec![("this.cond", Some(true))], }, TestCase { - code: "class Test { method() { ++this.x; } }", - node_index: 0, - expected: Some(false), + description: "switch statement", + code: r#"class Test { method() { switch(this.val) {} } }"#, + expected: vec![("this.val", Some(true))], }, TestCase { - code: "class Test { method() { const incremented = this.x + 1; } }", - node_index: 0, - expected: Some(true), + description: "for statement", + code: r#"class Test { method() { for(this.i = 0; this.i < 10; this.i++) {} } }"#, // First this.i = 0 is a write, so not a match at all + expected: vec![("this.i", Some(true)), ("this.i++", Some(true))], }, TestCase { - code: "class Test { method() { return this.x; } }", - node_index: 0, - expected: Some(true), + description: "binary expression", + code: r#"class Test { method() { const sum = this.a + this.b; } }"#, + expected: vec![("this.a", Some(true)), ("this.b", Some(true))], }, ]; From 3ccc1e4c99040d40c81bc33d2c8f801fa68df429 Mon Sep 17 00:00:00 2001 From: vladimirivanov Date: Sun, 21 Sep 2025 13:18:37 +0100 Subject: [PATCH 12/25] feat(biome-js-analyze): add is_meaningful_read to semantic class reads --- .../src/services/semantic_class.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/crates/biome_js_analyze/src/services/semantic_class.rs b/crates/biome_js_analyze/src/services/semantic_class.rs index 68475938bd8f..fc6b5187f22d 100644 --- a/crates/biome_js_analyze/src/services/semantic_class.rs +++ b/crates/biome_js_analyze/src/services/semantic_class.rs @@ -875,11 +875,12 @@ pub fn is_meaningful_read(node: &AnyMeaningfulReadNode) -> Option { } fn is_used_in_expression_context(node: &AnyMeaningfulReadNode) -> Option { - let mut current: JsSyntaxNode = if let Some(expression) = AnyJsExpression::cast(node.syntax().clone()) { - expression.syntax().clone() // get JsSyntaxNode - } else { - node.syntax().clone() // fallback to the node itself - }; + let mut current: JsSyntaxNode = + if let Some(expression) = AnyJsExpression::cast(node.syntax().clone()) { + expression.syntax().clone() // get JsSyntaxNode + } else { + node.syntax().clone() // fallback to the node itself + }; // Limit the number of parent traversals to avoid deep recursion for _ in 0..8 { @@ -1313,7 +1314,8 @@ mod tests { case.description ); - for (node, (expected_name, expected_meaningful)) in nodes.iter().zip(&case.expected) { + for (node, (expected_name, expected_meaningful)) in nodes.iter().zip(&case.expected) + { let meaningful_node = AnyMeaningfulReadNode::cast_ref(node.syntax()) .expect("Failed to cast node to AnyMeaningfulReadNode"); From efba128dd5b591be5e8a3b95662e92f70c9e2ef6 Mon Sep 17 00:00:00 2001 From: vladimirivanov Date: Sun, 21 Sep 2025 15:30:36 +0100 Subject: [PATCH 13/25] feat(biome-js-analyze): add is_meaningful_read to semantic class reads --- .../src/services/semantic_class.rs | 114 ++++++++++-------- 1 file changed, 67 insertions(+), 47 deletions(-) diff --git a/crates/biome_js_analyze/src/services/semantic_class.rs b/crates/biome_js_analyze/src/services/semantic_class.rs index fc6b5187f22d..d404688e54eb 100644 --- a/crates/biome_js_analyze/src/services/semantic_class.rs +++ b/crates/biome_js_analyze/src/services/semantic_class.rs @@ -3,8 +3,8 @@ use biome_analyze::{ RuleMetadata, ServiceBag, ServicesDiagnostic, Visitor, VisitorContext, VisitorFinishContext, }; use biome_js_syntax::{ - AnyJsClassMember, AnyJsExpression, AnyJsObjectBindingPatternMember, AnyJsRoot, - JsArrayAssignmentPattern, JsArrowFunctionExpression, JsAssignmentExpression, + AnyJsBindingPattern, AnyJsClassMember, AnyJsExpression, AnyJsObjectBindingPatternMember, + AnyJsRoot, JsArrayAssignmentPattern, JsArrowFunctionExpression, JsAssignmentExpression, JsClassDeclaration, JsClassMemberList, JsConstructorClassMember, JsFunctionBody, JsLanguage, JsObjectAssignmentPattern, JsObjectBindingPattern, JsPostUpdateExpression, JsPreUpdateExpression, JsPropertyClassMember, JsStaticMemberAssignment, @@ -146,7 +146,7 @@ declare_node_union! { } declare_node_union! { - pub AnyMeaningfulReadNode = AnyJsUpdateExpression | AnyJsObjectBindingPatternMember | JsStaticMemberExpression + pub AnyCandidateForUsedInExpressionNode = AnyJsUpdateExpression | AnyJsObjectBindingPatternMember | JsStaticMemberExpression | AnyJsBindingPattern } declare_node_union! { @@ -355,13 +355,13 @@ impl ThisScopeReferences { let id = fields.id.ok()?; let expr = fields.initializer?.expression().ok()?; let unwrapped = &expr.omit_parentheses(); - (unwrapped.syntax().first_token()?.text_trimmed() == "this").then(|| { ClassMemberReference { name: id.to_trimmed_text().clone(), range: id.syntax().text_trimmed_range(), - // Right hand side of a js variable statement is meaningful read - is_meaningful_read: Some(true), + is_meaningful_read: Some(is_used_in_expression_context( + &AnyCandidateForUsedInExpressionNode::from(id), + )), } }) }) @@ -637,7 +637,9 @@ fn handle_object_binding_pattern( reads.insert(ClassMemberReference { name: declarator.clone().to_trimmed_text(), range: declarator.clone().syntax().text_trimmed_range(), - is_meaningful_read: is_meaningful_read(&declarator.clone().into()), + is_meaningful_read: Some(is_used_in_expression_context( + &declarator.clone().into(), + )), }); } } @@ -673,7 +675,7 @@ fn handle_static_member_expression( reads.insert(ClassMemberReference { name: member.to_trimmed_text(), range: member.syntax().text_trimmed_range(), - is_meaningful_read: is_meaningful_read(&static_member.into()), + is_meaningful_read: Some(is_used_in_expression_context(&static_member.into())), }); } } @@ -796,8 +798,8 @@ fn handle_pre_or_post_update_expression( reads.insert(ClassMemberReference { name: name.name, range: name.range, - is_meaningful_read: is_meaningful_read(&AnyMeaningfulReadNode::from( - js_update_expression.clone(), + is_meaningful_read: Some(is_used_in_expression_context( + &AnyCandidateForUsedInExpressionNode::from(js_update_expression.clone()), )), }); } @@ -840,8 +842,8 @@ fn collect_class_property_reads_from_static_member( reads.insert(ClassMemberReference { name, range: static_member.syntax().text_trimmed_range(), - is_meaningful_read: is_meaningful_read(&AnyMeaningfulReadNode::from( - static_member.clone(), + is_meaningful_read: Some(is_used_in_expression_context( + &AnyCandidateForUsedInExpressionNode::from(static_member.clone()), )), }); } @@ -870,11 +872,11 @@ fn is_within_scope_without_shadowing( false } -pub fn is_meaningful_read(node: &AnyMeaningfulReadNode) -> Option { - is_used_in_expression_context(node) -} - -fn is_used_in_expression_context(node: &AnyMeaningfulReadNode) -> Option { +/// Checks if the given node is used in an expression context +/// (e.g., return, call arguments, conditionals, binary expressions). +/// NOt limited to `this` references. Can be used for any node. +/// Returns `true` if the read is meaningful, `false` otherwise. +fn is_used_in_expression_context(node: &AnyCandidateForUsedInExpressionNode) -> bool { let mut current: JsSyntaxNode = if let Some(expression) = AnyJsExpression::cast(node.syntax().clone()) { expression.syntax().clone() // get JsSyntaxNode @@ -902,14 +904,14 @@ fn is_used_in_expression_context(node: &AnyMeaningfulReadNode) -> Option { | JsSyntaxKind::JS_FOR_STATEMENT | JsSyntaxKind::JS_FOR_IN_STATEMENT | JsSyntaxKind::JS_FOR_OF_STATEMENT - | JsSyntaxKind::JS_BINARY_EXPRESSION => return Some(true), + | JsSyntaxKind::JS_BINARY_EXPRESSION => return true, _ => current = parent, } } else { break; } } - Some(false) + false } #[cfg(test)] @@ -1257,31 +1259,40 @@ mod tests { } } - mod is_meaningful_read_tests { + mod is_used_in_expression_context_tests { use super::*; - - fn extract_all_meaningful_nodes(code: &str) -> Vec { + use biome_js_syntax::binding_ext::AnyJsIdentifierBinding; + // WE n + fn extract_all_nodes(code: &str) -> Vec { let parsed = parse_ts(code); let root = parsed.syntax(); let mut nodes = vec![]; for descendant in root.descendants() { + // 1) Skip the identifier that is the class name (e.g. `Test` in `class Test {}`) + if AnyJsIdentifierBinding::can_cast(descendant.kind()) + && let Some(parent) = descendant.parent() && JsClassDeclaration::can_cast(parent.kind()) { + continue; + } + // Try to cast the node itself - if let Some(node) = AnyMeaningfulReadNode::cast_ref(&descendant) { + if let Some(node) = AnyCandidateForUsedInExpressionNode::cast_ref(&descendant) { nodes.push(node); } // If this is an assignment, also include LHS if let Some(assign_expr) = JsAssignmentExpression::cast_ref(&descendant) { if let Ok(lhs) = assign_expr.left() - && let Some(node) = AnyMeaningfulReadNode::cast_ref(lhs.syntax()) + && let Some(node) = + AnyCandidateForUsedInExpressionNode::cast_ref(lhs.syntax()) { nodes.push(node.clone()); } if let Ok(rhs) = assign_expr.right() - && let Some(node) = AnyMeaningfulReadNode::cast_ref(rhs.syntax()) + && let Some(node) = + AnyCandidateForUsedInExpressionNode::cast_ref(rhs.syntax()) { nodes.push(node.clone()); } @@ -1294,18 +1305,26 @@ mod tests { struct TestCase<'a> { description: &'a str, code: &'a str, - expected: Vec<(&'a str, Option)>, // (member name, is_meaningful_read) + expected: Vec<(&'a str, bool)>, // (member name, is_meaningful_read) } fn run_test_cases(cases: &[TestCase]) { for case in cases { - let nodes = extract_all_meaningful_nodes(case.code); + let nodes = extract_all_nodes(case.code); assert!( !nodes.is_empty(), "No match found for test case: '{}'", case.description ); + println!( + "nodes found: {:?}", + nodes + .iter() + .map(|n| n.to_trimmed_text()) + .collect::>() + ); + // Ensure the number of nodes matches expected assert_eq!( nodes.len(), @@ -1316,8 +1335,9 @@ mod tests { for (node, (expected_name, expected_meaningful)) in nodes.iter().zip(&case.expected) { - let meaningful_node = AnyMeaningfulReadNode::cast_ref(node.syntax()) - .expect("Failed to cast node to AnyMeaningfulReadNode"); + let meaningful_node = + AnyCandidateForUsedInExpressionNode::cast_ref(node.syntax()) + .expect("Failed to cast node to AnyMeaningfulReadNode"); // Compare node name let node_name = meaningful_node.to_trimmed_text(); @@ -1328,7 +1348,7 @@ mod tests { ); // Compare is_meaningful_read - let actual_meaningful = is_meaningful_read(&meaningful_node); + let actual_meaningful = is_used_in_expression_context(&meaningful_node); assert_eq!( actual_meaningful, *expected_meaningful, "Meaningful read mismatch for node '{}' in test case: '{}'", @@ -1339,82 +1359,82 @@ mod tests { } #[test] - fn test_meaningful_read_contexts() { + fn test_is_used_in_expression_contexts() { let cases = [ TestCase { description: "return statement", - code: r#"class Test { method() { return this.x; } }"#, - expected: vec![("this.x", Some(true))], + code: r#"class Test {method() { return this.x; }}"#, + expected: vec![("this.x", true)], }, TestCase { description: "call arguments", code: r#"class Test { method() { foo(this.y); } }"#, - expected: vec![("this.y", Some(true))], + expected: vec![("this.y", true)], }, TestCase { description: "conditional expression", code: r#"class Test { method() { const a = this.z ? 1 : 2; } }"#, - expected: vec![("this.z", Some(true))], + expected: vec![("a", false), ("this.z", true)], }, TestCase { description: "logical expression", code: r#"class Test { method() { const a = this.a && this.b; } }"#, - expected: vec![("this.a", Some(true)), ("this.b", Some(true))], + expected: vec![("a", false), ("this.a", true), ("this.b", true)], }, TestCase { description: "throw statement", code: r#"class Test { method() { throw this.err; } }"#, - expected: vec![("this.err", Some(true))], + expected: vec![("this.err", true)], }, TestCase { description: "await expression", code: r#"class Test { async method() { await this.promise; } }"#, - expected: vec![("this.promise", Some(true))], + expected: vec![("this.promise", true)], }, TestCase { description: "yield expression", code: r#"class Test { *method() { yield this.gen; } }"#, - expected: vec![("this.gen", Some(true))], + expected: vec![("this.gen", true)], }, TestCase { description: "unary expression", code: r#"class Test { method() { -this.num; } }"#, - expected: vec![("this.num", Some(true))], + expected: vec![("this.num", true)], }, TestCase { description: "template expression", code: r#"class Test { method() { `${this.str}`; } }"#, - expected: vec![("this.str", Some(true))], + expected: vec![("this.str", true)], }, TestCase { description: "call expression callee", code: r#"class Test { method() { this.func(); } }"#, - expected: vec![("this.func", Some(true))], + expected: vec![("this.func", true)], }, TestCase { description: "new expression", code: r#"class Test { method() { new this.ClassName(); } }"#, - expected: vec![("this.ClassName", Some(true))], + expected: vec![("this.ClassName", true)], }, TestCase { description: "if statement", code: r#"class Test { method() { if(this.cond) {} } }"#, - expected: vec![("this.cond", Some(true))], + expected: vec![("this.cond", true)], }, TestCase { description: "switch statement", code: r#"class Test { method() { switch(this.val) {} } }"#, - expected: vec![("this.val", Some(true))], + expected: vec![("this.val", true)], }, TestCase { description: "for statement", code: r#"class Test { method() { for(this.i = 0; this.i < 10; this.i++) {} } }"#, // First this.i = 0 is a write, so not a match at all - expected: vec![("this.i", Some(true)), ("this.i++", Some(true))], + expected: vec![("this.i", true), ("this.i++", true)], }, TestCase { description: "binary expression", code: r#"class Test { method() { const sum = this.a + this.b; } }"#, - expected: vec![("this.a", Some(true)), ("this.b", Some(true))], + expected: vec![("sum", false), ("this.a", true), ("this.b", true)], }, ]; From 02f8e29c524b2963ad577b3be1b09ef270e62fec Mon Sep 17 00:00:00 2001 From: vladimirivanov Date: Sun, 21 Sep 2025 15:43:09 +0100 Subject: [PATCH 14/25] feat(biome-js-analyze): add is_meaningful_read to semantic class reads --- .../src/services/semantic_class.rs | 31 ++++++++++++------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/crates/biome_js_analyze/src/services/semantic_class.rs b/crates/biome_js_analyze/src/services/semantic_class.rs index d404688e54eb..467754d97bd7 100644 --- a/crates/biome_js_analyze/src/services/semantic_class.rs +++ b/crates/biome_js_analyze/src/services/semantic_class.rs @@ -874,7 +874,7 @@ fn is_within_scope_without_shadowing( /// Checks if the given node is used in an expression context /// (e.g., return, call arguments, conditionals, binary expressions). -/// NOt limited to `this` references. Can be used for any node. +/// NOt limited to `this` references. Can be used for any node, but requires more work e.g. /// Returns `true` if the read is meaningful, `false` otherwise. fn is_used_in_expression_context(node: &AnyCandidateForUsedInExpressionNode) -> bool { let mut current: JsSyntaxNode = @@ -1272,8 +1272,10 @@ mod tests { for descendant in root.descendants() { // 1) Skip the identifier that is the class name (e.g. `Test` in `class Test {}`) if AnyJsIdentifierBinding::can_cast(descendant.kind()) - && let Some(parent) = descendant.parent() && JsClassDeclaration::can_cast(parent.kind()) { - continue; + && let Some(parent) = descendant.parent() + && JsClassDeclaration::can_cast(parent.kind()) + { + continue; } // Try to cast the node itself @@ -1317,14 +1319,6 @@ mod tests { case.description ); - println!( - "nodes found: {:?}", - nodes - .iter() - .map(|n| n.to_trimmed_text()) - .collect::>() - ); - // Ensure the number of nodes matches expected assert_eq!( nodes.len(), @@ -1436,6 +1430,21 @@ mod tests { code: r#"class Test { method() { const sum = this.a + this.b; } }"#, expected: vec![("sum", false), ("this.a", true), ("this.b", true)], }, + TestCase { + description: "binary expression nested parenthesis", + code: r#"class Test { method() { const sum = (((this.a + ((this.b * 2))))); } }"#, + expected: vec![("sum", false), ("this.a", true), ("this.b", true)], + }, + TestCase { + description: "nested logical and conditional expressions", + code: r#"class Test { method() { const val = foo(this.a && (this.b ? this.c : 7)); } }"#, + expected: vec![ + ("val", false), + ("this.a", true), + ("this.b", true), + ("this.c", true), + ], + }, ]; run_test_cases(&cases); From e45fccb2517a83a2084ba7771509152aa6db97aa Mon Sep 17 00:00:00 2001 From: vladimir ivanov Date: Mon, 22 Sep 2025 09:13:46 +0100 Subject: [PATCH 15/25] Update semantic_class.rs Co-authored-by: Emanuele Stoppa --- crates/biome_js_analyze/src/services/semantic_class.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/biome_js_analyze/src/services/semantic_class.rs b/crates/biome_js_analyze/src/services/semantic_class.rs index 467754d97bd7..f03cea8f3df1 100644 --- a/crates/biome_js_analyze/src/services/semantic_class.rs +++ b/crates/biome_js_analyze/src/services/semantic_class.rs @@ -130,7 +130,7 @@ where pub struct ClassMemberReference { pub name: Text, pub range: TextRange, - /// Indicates if the read is meaningful (e.g., used in an expression) or not (e.g. part of a destructuring assignment). + /// Indicates if the read is meaningful (e.g. used in an expression) or not (e.g. part of a destructuring assignment). /// `None` if not applicable (e.g. for write references). pub is_meaningful_read: Option, } From c14b12dce994bbc762f593c009809d5a2183ece4 Mon Sep 17 00:00:00 2001 From: vladimir ivanov Date: Mon, 22 Sep 2025 09:14:47 +0100 Subject: [PATCH 16/25] Update semantic_class.rs Co-authored-by: Emanuele Stoppa --- crates/biome_js_analyze/src/services/semantic_class.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/biome_js_analyze/src/services/semantic_class.rs b/crates/biome_js_analyze/src/services/semantic_class.rs index f03cea8f3df1..c1830c050c81 100644 --- a/crates/biome_js_analyze/src/services/semantic_class.rs +++ b/crates/biome_js_analyze/src/services/semantic_class.rs @@ -874,7 +874,7 @@ fn is_within_scope_without_shadowing( /// Checks if the given node is used in an expression context /// (e.g., return, call arguments, conditionals, binary expressions). -/// NOt limited to `this` references. Can be used for any node, but requires more work e.g. +/// Not limited to `this` references. Can be used for any node, but requires more work e.g. /// Returns `true` if the read is meaningful, `false` otherwise. fn is_used_in_expression_context(node: &AnyCandidateForUsedInExpressionNode) -> bool { let mut current: JsSyntaxNode = From c7d4cec23602c177fe60a0ab36c7e0aa818055ea Mon Sep 17 00:00:00 2001 From: vladimirivanov Date: Tue, 23 Sep 2025 19:26:10 +0100 Subject: [PATCH 17/25] feat(biome-js-analyze): add is_meaningful_read to semantic class reads --- .../src/services/semantic_class.rs | 147 ++++++++++-------- 1 file changed, 79 insertions(+), 68 deletions(-) diff --git a/crates/biome_js_analyze/src/services/semantic_class.rs b/crates/biome_js_analyze/src/services/semantic_class.rs index c1830c050c81..dff9e87e5bbc 100644 --- a/crates/biome_js_analyze/src/services/semantic_class.rs +++ b/crates/biome_js_analyze/src/services/semantic_class.rs @@ -126,13 +126,36 @@ where } } +/// Represents how a class member is accessed within the code. +/// Variants: +/// +/// - `Write`: +/// The member is being assigned to or mutated. +/// Example: `this.count = 10;` +/// This indicates the member’s value/state changes at this point. +/// +/// - `MeaningfulRead`: +/// The member’s value is retrieved and used in a way that affects program logic. +/// Example: `if (this.enabled) { ... }` or `let x = this.value + 1;` +/// These reads influence control flow or computation. +/// +/// - `TrivialRead`: +/// The member is accessed, but its value is not used in a way that +/// meaningfully affects logic. +/// Example: `this.value;` as a standalone expression, or a read that is optimized away. +/// This is mostly for distinguishing "dead reads" from truly meaningful ones. +#[derive(Debug, Clone, Hash, Eq, PartialEq)] +enum AccessKind { + Write, + MeaningfulRead, + TrivialRead, +} + #[derive(Debug, Clone, Hash, Eq, PartialEq)] pub struct ClassMemberReference { pub name: Text, pub range: TextRange, - /// Indicates if the read is meaningful (e.g. used in an expression) or not (e.g. part of a destructuring assignment). - /// `None` if not applicable (e.g. for write references). - pub is_meaningful_read: Option, + pub access_kind: AccessKind, } #[derive(Debug, Clone, Eq, PartialEq, Default)] @@ -359,9 +382,9 @@ impl ThisScopeReferences { ClassMemberReference { name: id.to_trimmed_text().clone(), range: id.syntax().text_trimmed_range(), - is_meaningful_read: Some(is_used_in_expression_context( + access_kind: get_read_access_kind( &AnyCandidateForUsedInExpressionNode::from(id), - )), + ), } }) }) @@ -439,8 +462,7 @@ impl ThisPatternResolver { Self::extract_this_member_reference( assignment.as_js_static_member_assignment(), scoped_this_references, - // It is a write reference, so not applicable for meaningful read - None, + AccessKind::Write, ) }) } @@ -456,8 +478,7 @@ impl ThisPatternResolver { Self::extract_this_member_reference( assignment.as_js_static_member_assignment(), scoped_this_references, - // It is a write reference, so not applicable for meaningful read - None, + AccessKind::Write, ) }) } else { @@ -486,8 +507,7 @@ impl ThisPatternResolver { return Self::extract_this_member_reference( rest_params.target().ok()?.as_js_static_member_assignment(), scoped_this_references, - // It is a write reference, so not applicable for meaningful read - None, + AccessKind::Write, ); } if let Some(property) = prop @@ -503,8 +523,7 @@ impl ThisPatternResolver { .as_any_js_assignment()? .as_js_static_member_assignment(), scoped_this_references, - // It is a write reference, so not applicable for meaningful read - None, + AccessKind::Write, ); } None @@ -523,7 +542,7 @@ impl ThisPatternResolver { fn extract_this_member_reference( operand: Option<&JsStaticMemberAssignment>, scoped_this_references: &[FunctionThisReferences], - is_meaningful_read: Option, + access_kind: AccessKind, ) -> Option { operand.and_then(|assignment| { if let Ok(object) = assignment.object() @@ -535,7 +554,7 @@ impl ThisPatternResolver { .map(|name| ClassMemberReference { name: name.to_trimmed_text(), range: name.syntax().text_trimmed_range(), - is_meaningful_read, + access_kind: access_kind.clone(), }) .or_else(|| { member @@ -543,7 +562,7 @@ impl ThisPatternResolver { .map(|private_name| ClassMemberReference { name: private_name.to_trimmed_text(), range: private_name.syntax().text_trimmed_range(), - is_meaningful_read, + access_kind, }) }) }) @@ -637,9 +656,7 @@ fn handle_object_binding_pattern( reads.insert(ClassMemberReference { name: declarator.clone().to_trimmed_text(), range: declarator.clone().syntax().text_trimmed_range(), - is_meaningful_read: Some(is_used_in_expression_context( - &declarator.clone().into(), - )), + access_kind: get_read_access_kind(&declarator.clone().into()), }); } } @@ -675,7 +692,7 @@ fn handle_static_member_expression( reads.insert(ClassMemberReference { name: member.to_trimmed_text(), range: member.syntax().text_trimmed_range(), - is_meaningful_read: Some(is_used_in_expression_context(&static_member.into())), + access_kind: get_read_access_kind(&static_member.into()), }); } } @@ -721,8 +738,7 @@ fn handle_assignment_expression( && let Some(name) = ThisPatternResolver::extract_this_member_reference( operand.as_js_static_member_assignment(), scoped_this_references, - // Nodes inside assignment expressions are considered meaningful reads e.g. this.x += 1; - Some(true), + AccessKind::MeaningfulRead, ) { reads.insert(name.clone()); @@ -749,8 +765,7 @@ fn handle_assignment_expression( && let Some(name) = ThisPatternResolver::extract_this_member_reference( assignment.as_js_static_member_assignment(), scoped_this_references, - // It is a write reference, so not applicable for meaningful read - None, + AccessKind::Write, ) { writes.insert(name.clone()); @@ -787,19 +802,15 @@ fn handle_pre_or_post_update_expression( && let Some(name) = ThisPatternResolver::extract_this_member_reference( operand.as_js_static_member_assignment(), scoped_this_references, - None, + AccessKind::Write, ) { - writes.insert(ClassMemberReference { - name: name.name.clone(), - range: name.range, - is_meaningful_read: None, - }); + writes.insert(name.clone()); reads.insert(ClassMemberReference { name: name.name, range: name.range, - is_meaningful_read: Some(is_used_in_expression_context( - &AnyCandidateForUsedInExpressionNode::from(js_update_expression.clone()), + access_kind: get_read_access_kind(&AnyCandidateForUsedInExpressionNode::from( + js_update_expression.clone(), )), }); } @@ -842,8 +853,8 @@ fn collect_class_property_reads_from_static_member( reads.insert(ClassMemberReference { name, range: static_member.syntax().text_trimmed_range(), - is_meaningful_read: Some(is_used_in_expression_context( - &AnyCandidateForUsedInExpressionNode::from(static_member.clone()), + access_kind: get_read_access_kind(&AnyCandidateForUsedInExpressionNode::from( + static_member.clone(), )), }); } @@ -872,45 +883,45 @@ fn is_within_scope_without_shadowing( false } +/// Determines the kind of read access for a given node. +fn get_read_access_kind(node: &AnyCandidateForUsedInExpressionNode) -> AccessKind { + if is_used_in_expression_context(node) { + AccessKind::MeaningfulRead + } else { + AccessKind::TrivialRead + } +} + /// Checks if the given node is used in an expression context /// (e.g., return, call arguments, conditionals, binary expressions). /// Not limited to `this` references. Can be used for any node, but requires more work e.g. /// Returns `true` if the read is meaningful, `false` otherwise. fn is_used_in_expression_context(node: &AnyCandidateForUsedInExpressionNode) -> bool { - let mut current: JsSyntaxNode = - if let Some(expression) = AnyJsExpression::cast(node.syntax().clone()) { - expression.syntax().clone() // get JsSyntaxNode - } else { - node.syntax().clone() // fallback to the node itself - }; - - // Limit the number of parent traversals to avoid deep recursion - for _ in 0..8 { - if let Some(parent) = current.parent() { - match parent.kind() { - JsSyntaxKind::JS_RETURN_STATEMENT - | JsSyntaxKind::JS_CALL_ARGUMENTS - | JsSyntaxKind::JS_CONDITIONAL_EXPRESSION - | JsSyntaxKind::JS_LOGICAL_EXPRESSION - | JsSyntaxKind::JS_THROW_STATEMENT - | JsSyntaxKind::JS_AWAIT_EXPRESSION - | JsSyntaxKind::JS_YIELD_EXPRESSION - | JsSyntaxKind::JS_UNARY_EXPRESSION - | JsSyntaxKind::JS_TEMPLATE_EXPRESSION - | JsSyntaxKind::JS_CALL_EXPRESSION // (callee) - | JsSyntaxKind::JS_NEW_EXPRESSION - | JsSyntaxKind::JS_IF_STATEMENT - | JsSyntaxKind::JS_SWITCH_STATEMENT - | JsSyntaxKind::JS_FOR_STATEMENT - | JsSyntaxKind::JS_FOR_IN_STATEMENT - | JsSyntaxKind::JS_FOR_OF_STATEMENT - | JsSyntaxKind::JS_BINARY_EXPRESSION => return true, - _ => current = parent, - } - } else { - break; + let mut current = node.syntax().clone(); + + if let Some(parent) = current.parent() { + match parent.kind() { + JsSyntaxKind::JS_RETURN_STATEMENT + | JsSyntaxKind::JS_CALL_ARGUMENTS + | JsSyntaxKind::JS_CONDITIONAL_EXPRESSION + | JsSyntaxKind::JS_LOGICAL_EXPRESSION + | JsSyntaxKind::JS_THROW_STATEMENT + | JsSyntaxKind::JS_AWAIT_EXPRESSION + | JsSyntaxKind::JS_YIELD_EXPRESSION + | JsSyntaxKind::JS_UNARY_EXPRESSION + | JsSyntaxKind::JS_TEMPLATE_EXPRESSION + | JsSyntaxKind::JS_CALL_EXPRESSION // (callee) + | JsSyntaxKind::JS_NEW_EXPRESSION + | JsSyntaxKind::JS_IF_STATEMENT + | JsSyntaxKind::JS_SWITCH_STATEMENT + | JsSyntaxKind::JS_FOR_STATEMENT + | JsSyntaxKind::JS_FOR_IN_STATEMENT + | JsSyntaxKind::JS_FOR_OF_STATEMENT + | JsSyntaxKind::JS_BINARY_EXPRESSION => return true, + _ => current = parent, } } + false } @@ -945,7 +956,7 @@ mod tests { }); assert_eq!( - found.is_meaningful_read, *expected_meaningful, + found.access_kind, *expected_meaningful, "Case '{}' failed: read '{}' meaningful mismatch", description, expected_name ); @@ -969,7 +980,7 @@ mod tests { }); assert_eq!( - found.is_meaningful_read, *expected_meaningful, + found.access_kind, *expected_meaningful, "Case '{}' failed: write '{}' meaningful mismatch", description, expected_name ); From a47df2f29ea4021378998640f145a1bc927395d0 Mon Sep 17 00:00:00 2001 From: vladimirivanov Date: Tue, 23 Sep 2025 19:40:37 +0100 Subject: [PATCH 18/25] feat(biome-js-analyze): add is_meaningful_read to semantic class reads --- .../src/services/semantic_class.rs | 120 ++++++++++-------- 1 file changed, 68 insertions(+), 52 deletions(-) diff --git a/crates/biome_js_analyze/src/services/semantic_class.rs b/crates/biome_js_analyze/src/services/semantic_class.rs index dff9e87e5bbc..9cb8a50c5572 100644 --- a/crates/biome_js_analyze/src/services/semantic_class.rs +++ b/crates/biome_js_analyze/src/services/semantic_class.rs @@ -145,7 +145,7 @@ where /// Example: `this.value;` as a standalone expression, or a read that is optimized away. /// This is mostly for distinguishing "dead reads" from truly meaningful ones. #[derive(Debug, Clone, Hash, Eq, PartialEq)] -enum AccessKind { +pub enum AccessKind { Write, MeaningfulRead, TrivialRead, @@ -897,32 +897,28 @@ fn get_read_access_kind(node: &AnyCandidateForUsedInExpressionNode) -> AccessKin /// Not limited to `this` references. Can be used for any node, but requires more work e.g. /// Returns `true` if the read is meaningful, `false` otherwise. fn is_used_in_expression_context(node: &AnyCandidateForUsedInExpressionNode) -> bool { - let mut current = node.syntax().clone(); - - if let Some(parent) = current.parent() { - match parent.kind() { + node.syntax() + .ancestors() + .any(|ancestor| matches!( + ancestor.kind(), JsSyntaxKind::JS_RETURN_STATEMENT - | JsSyntaxKind::JS_CALL_ARGUMENTS - | JsSyntaxKind::JS_CONDITIONAL_EXPRESSION - | JsSyntaxKind::JS_LOGICAL_EXPRESSION - | JsSyntaxKind::JS_THROW_STATEMENT - | JsSyntaxKind::JS_AWAIT_EXPRESSION - | JsSyntaxKind::JS_YIELD_EXPRESSION - | JsSyntaxKind::JS_UNARY_EXPRESSION - | JsSyntaxKind::JS_TEMPLATE_EXPRESSION - | JsSyntaxKind::JS_CALL_EXPRESSION // (callee) - | JsSyntaxKind::JS_NEW_EXPRESSION - | JsSyntaxKind::JS_IF_STATEMENT - | JsSyntaxKind::JS_SWITCH_STATEMENT - | JsSyntaxKind::JS_FOR_STATEMENT - | JsSyntaxKind::JS_FOR_IN_STATEMENT - | JsSyntaxKind::JS_FOR_OF_STATEMENT - | JsSyntaxKind::JS_BINARY_EXPRESSION => return true, - _ => current = parent, - } - } - - false + | JsSyntaxKind::JS_CALL_ARGUMENTS + | JsSyntaxKind::JS_CONDITIONAL_EXPRESSION + | JsSyntaxKind::JS_LOGICAL_EXPRESSION + | JsSyntaxKind::JS_THROW_STATEMENT + | JsSyntaxKind::JS_AWAIT_EXPRESSION + | JsSyntaxKind::JS_YIELD_EXPRESSION + | JsSyntaxKind::JS_UNARY_EXPRESSION + | JsSyntaxKind::JS_TEMPLATE_EXPRESSION + | JsSyntaxKind::JS_CALL_EXPRESSION + | JsSyntaxKind::JS_NEW_EXPRESSION + | JsSyntaxKind::JS_IF_STATEMENT + | JsSyntaxKind::JS_SWITCH_STATEMENT + | JsSyntaxKind::JS_FOR_STATEMENT + | JsSyntaxKind::JS_FOR_IN_STATEMENT + | JsSyntaxKind::JS_FOR_OF_STATEMENT + | JsSyntaxKind::JS_BINARY_EXPRESSION + )) } #[cfg(test)] @@ -935,13 +931,13 @@ mod tests { struct TestCase<'a> { description: &'a str, code: &'a str, - expected_reads: Vec<(&'a str, Option)>, // (name, is_meaningful_read) - expected_writes: Vec<(&'a str, Option)>, // (name, is_meaningful_read) + expected_reads: Vec<(&'a str, AccessKind)>, // (name, is_meaningful_read) + expected_writes: Vec<(&'a str, AccessKind)>, // (name, is_meaningful_read) } fn assert_reads( reads: &FxHashSet, - expected: &[(&str, Option)], + expected: &[(&str, AccessKind)], description: &str, ) { for (expected_name, expected_meaningful) in expected { @@ -965,7 +961,7 @@ mod tests { fn assert_writes( writes: &FxHashSet, - expected: &[(&str, Option)], + expected: &[(&str, AccessKind)], description: &str, ) { for (expected_name, expected_meaningful) in expected { @@ -1019,7 +1015,10 @@ mod tests { } } "#, - expected_reads: vec![("foo", Some(false)), ("bar", Some(false))], + expected_reads: vec![ + ("foo", AccessKind::TrivialRead), + ("bar", AccessKind::TrivialRead), + ], expected_writes: vec![], }, TestCase { @@ -1032,7 +1031,10 @@ mod tests { } } "#, - expected_reads: vec![("baz", Some(false)), ("qux", Some(false))], + expected_reads: vec![ + ("baz", AccessKind::TrivialRead), + ("qux", AccessKind::TrivialRead), + ], expected_writes: vec![], }, ]; @@ -1069,7 +1071,10 @@ mod tests { } } "#, - expected_reads: vec![("foo", Some(true)), ("bar", Some(true))], + expected_reads: vec![ + ("foo", AccessKind::MeaningfulRead), + ("bar", AccessKind::MeaningfulRead), + ], expected_writes: vec![], }, TestCase { @@ -1083,7 +1088,10 @@ mod tests { } } "#, - expected_reads: vec![("baz", Some(false)), ("qux", Some(false))], + expected_reads: vec![ + ("baz", AccessKind::TrivialRead), + ("qux", AccessKind::TrivialRead), + ], expected_writes: vec![], }, ]; @@ -1130,8 +1138,12 @@ mod tests { } } "#, - expected_reads: vec![("x", Some(true))], // x is read due to += - expected_writes: vec![("x", None), ("y", None), ("z", None)], + expected_reads: vec![("x", AccessKind::MeaningfulRead)], // x is read due to += + expected_writes: vec![ + ("x", AccessKind::Write), + ("y", AccessKind::Write), + ("z", AccessKind::Write), + ], }, TestCase { description: "assignment reads and writes with aliasForThis", @@ -1146,8 +1158,12 @@ mod tests { } } "#, - expected_reads: vec![("x", Some(true))], - expected_writes: vec![("x", None), ("y", None), ("z", None)], + expected_reads: vec![("x", AccessKind::MeaningfulRead)], + expected_writes: vec![ + ("x", AccessKind::Write), + ("y", AccessKind::Write), + ("z", AccessKind::Write), + ], }, ]; @@ -1201,16 +1217,16 @@ mod tests { } "#, expected_reads: vec![ - ("count", Some(false)), - ("total", Some(false)), - ("inIfCondition", Some(true)), - ("inReturn", Some(true)), + ("count", AccessKind::TrivialRead), + ("total", AccessKind::TrivialRead), + ("inIfCondition", AccessKind::MeaningfulRead), + ("inReturn", AccessKind::MeaningfulRead), ], expected_writes: vec![ - ("count", None), - ("total", None), - ("inIfCondition", None), - ("inReturn", None), + ("count", AccessKind::Write), + ("total", AccessKind::Write), + ("inIfCondition", AccessKind::Write), + ("inReturn", AccessKind::Write), ], }, TestCase { @@ -1228,14 +1244,14 @@ mod tests { } "#, expected_reads: vec![ - ("count", Some(false)), - ("total", Some(false)), - ("inReturnIncrement", Some(true)), + ("count", AccessKind::TrivialRead), + ("total", AccessKind::TrivialRead), + ("inReturnIncrement", AccessKind::MeaningfulRead), ], expected_writes: vec![ - ("count", None), - ("total", None), - ("inReturnIncrement", None), + ("count", AccessKind::Write), + ("total", AccessKind::Write), + ("inReturnIncrement", AccessKind::Write), ], }, ]; From 44a1edc5fb13d16ca1897b83c0e0690321834300 Mon Sep 17 00:00:00 2001 From: vladimirivanov Date: Tue, 23 Sep 2025 19:45:56 +0100 Subject: [PATCH 19/25] feat(biome-js-analyze): add is_meaningful_read to semantic class reads --- .../src/services/semantic_class.rs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/crates/biome_js_analyze/src/services/semantic_class.rs b/crates/biome_js_analyze/src/services/semantic_class.rs index 9cb8a50c5572..67e9268e9e40 100644 --- a/crates/biome_js_analyze/src/services/semantic_class.rs +++ b/crates/biome_js_analyze/src/services/semantic_class.rs @@ -143,7 +143,7 @@ where /// The member is accessed, but its value is not used in a way that /// meaningfully affects logic. /// Example: `this.value;` as a standalone expression, or a read that is optimized away. -/// This is mostly for distinguishing "dead reads" from truly meaningful ones. +/// This is mostly for distinguishing "dead reads" from truly access_kind ones. #[derive(Debug, Clone, Hash, Eq, PartialEq)] pub enum AccessKind { Write, @@ -895,7 +895,7 @@ fn get_read_access_kind(node: &AnyCandidateForUsedInExpressionNode) -> AccessKin /// Checks if the given node is used in an expression context /// (e.g., return, call arguments, conditionals, binary expressions). /// Not limited to `this` references. Can be used for any node, but requires more work e.g. -/// Returns `true` if the read is meaningful, `false` otherwise. +/// Returns `true` if the read is access_kind, `false` otherwise. fn is_used_in_expression_context(node: &AnyCandidateForUsedInExpressionNode) -> bool { node.syntax() .ancestors() @@ -940,7 +940,7 @@ mod tests { expected: &[(&str, AccessKind)], description: &str, ) { - for (expected_name, expected_meaningful) in expected { + for (expected_name, expected_access_kind) in expected { let found = reads .iter() .find(|r| r.name.clone().text() == *expected_name) @@ -952,8 +952,8 @@ mod tests { }); assert_eq!( - found.access_kind, *expected_meaningful, - "Case '{}' failed: read '{}' meaningful mismatch", + found.access_kind, *expected_access_kind, + "Case '{}' failed: read '{}' access_kind mismatch", description, expected_name ); } @@ -964,7 +964,7 @@ mod tests { expected: &[(&str, AccessKind)], description: &str, ) { - for (expected_name, expected_meaningful) in expected { + for (expected_name, expected_access_kind) in expected { let found = writes .iter() .find(|r| r.name.clone().text() == *expected_name) @@ -976,8 +976,8 @@ mod tests { }); assert_eq!( - found.access_kind, *expected_meaningful, - "Case '{}' failed: write '{}' meaningful mismatch", + found.access_kind, *expected_access_kind, + "Case '{}' failed: write '{}' access_kind mismatch", description, expected_name ); } @@ -1354,7 +1354,7 @@ mod tests { case.description ); - for (node, (expected_name, expected_meaningful)) in nodes.iter().zip(&case.expected) + for (node, (expected_name, expected_access_kind)) in nodes.iter().zip(&case.expected) { let meaningful_node = AnyCandidateForUsedInExpressionNode::cast_ref(node.syntax()) @@ -1371,7 +1371,7 @@ mod tests { // Compare is_meaningful_read let actual_meaningful = is_used_in_expression_context(&meaningful_node); assert_eq!( - actual_meaningful, *expected_meaningful, + actual_meaningful, *expected_access_kind, "Meaningful read mismatch for node '{}' in test case: '{}'", expected_name, case.description ); From 7ada3c782a2d2a9ee86230f60067d88ae4a04432 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Tue, 23 Sep 2025 18:52:29 +0000 Subject: [PATCH 20/25] [autofix.ci] apply automated fixes --- .../biome_js_analyze/src/services/semantic_class.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/crates/biome_js_analyze/src/services/semantic_class.rs b/crates/biome_js_analyze/src/services/semantic_class.rs index 67e9268e9e40..3689a0f7bd86 100644 --- a/crates/biome_js_analyze/src/services/semantic_class.rs +++ b/crates/biome_js_analyze/src/services/semantic_class.rs @@ -897,9 +897,8 @@ fn get_read_access_kind(node: &AnyCandidateForUsedInExpressionNode) -> AccessKin /// Not limited to `this` references. Can be used for any node, but requires more work e.g. /// Returns `true` if the read is access_kind, `false` otherwise. fn is_used_in_expression_context(node: &AnyCandidateForUsedInExpressionNode) -> bool { - node.syntax() - .ancestors() - .any(|ancestor| matches!( + node.syntax().ancestors().any(|ancestor| { + matches!( ancestor.kind(), JsSyntaxKind::JS_RETURN_STATEMENT | JsSyntaxKind::JS_CALL_ARGUMENTS @@ -918,7 +917,8 @@ fn is_used_in_expression_context(node: &AnyCandidateForUsedInExpressionNode) -> | JsSyntaxKind::JS_FOR_IN_STATEMENT | JsSyntaxKind::JS_FOR_OF_STATEMENT | JsSyntaxKind::JS_BINARY_EXPRESSION - )) + ) + }) } #[cfg(test)] @@ -1354,7 +1354,8 @@ mod tests { case.description ); - for (node, (expected_name, expected_access_kind)) in nodes.iter().zip(&case.expected) + for (node, (expected_name, expected_access_kind)) in + nodes.iter().zip(&case.expected) { let meaningful_node = AnyCandidateForUsedInExpressionNode::cast_ref(node.syntax()) From 4bf27a8706d25c8a9f8923a327ab3aac2e2437d2 Mon Sep 17 00:00:00 2001 From: vladimir ivanov Date: Tue, 23 Sep 2025 20:50:31 +0100 Subject: [PATCH 21/25] Update crates/biome_js_analyze/src/services/semantic_class.rs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- crates/biome_js_analyze/src/services/semantic_class.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/biome_js_analyze/src/services/semantic_class.rs b/crates/biome_js_analyze/src/services/semantic_class.rs index 3689a0f7bd86..eb10dd3401d9 100644 --- a/crates/biome_js_analyze/src/services/semantic_class.rs +++ b/crates/biome_js_analyze/src/services/semantic_class.rs @@ -894,7 +894,7 @@ fn get_read_access_kind(node: &AnyCandidateForUsedInExpressionNode) -> AccessKin /// Checks if the given node is used in an expression context /// (e.g., return, call arguments, conditionals, binary expressions). -/// Not limited to `this` references. Can be used for any node, but requires more work e.g. +/// Not limited to `this` references. It can be used for any node; additional cases may require extending the context checks. /// Returns `true` if the read is access_kind, `false` otherwise. fn is_used_in_expression_context(node: &AnyCandidateForUsedInExpressionNode) -> bool { node.syntax().ancestors().any(|ancestor| { From 86e262e28e79c5320a8d57b030d8e3d522cfc176 Mon Sep 17 00:00:00 2001 From: vladimirivanov Date: Tue, 23 Sep 2025 20:51:16 +0100 Subject: [PATCH 22/25] feat(biome-js-analyze): add is_meaningful_read to semantic class reads --- crates/biome_js_analyze/src/services/semantic_class.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/biome_js_analyze/src/services/semantic_class.rs b/crates/biome_js_analyze/src/services/semantic_class.rs index eb10dd3401d9..f31f97421acf 100644 --- a/crates/biome_js_analyze/src/services/semantic_class.rs +++ b/crates/biome_js_analyze/src/services/semantic_class.rs @@ -1289,7 +1289,7 @@ mod tests { mod is_used_in_expression_context_tests { use super::*; use biome_js_syntax::binding_ext::AnyJsIdentifierBinding; - // WE n + fn extract_all_nodes(code: &str) -> Vec { let parsed = parse_ts(code); let root = parsed.syntax(); From c0d28c20cc61d262390e594c5669870e15ebc382 Mon Sep 17 00:00:00 2001 From: vladimir ivanov Date: Tue, 23 Sep 2025 20:53:19 +0100 Subject: [PATCH 23/25] Update crates/biome_js_analyze/src/services/semantic_class.rs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- crates/biome_js_analyze/src/services/semantic_class.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/biome_js_analyze/src/services/semantic_class.rs b/crates/biome_js_analyze/src/services/semantic_class.rs index f31f97421acf..d2442a9ad7f3 100644 --- a/crates/biome_js_analyze/src/services/semantic_class.rs +++ b/crates/biome_js_analyze/src/services/semantic_class.rs @@ -143,7 +143,7 @@ where /// The member is accessed, but its value is not used in a way that /// meaningfully affects logic. /// Example: `this.value;` as a standalone expression, or a read that is optimized away. -/// This is mostly for distinguishing "dead reads" from truly access_kind ones. +/// This is mostly for distinguishing dead reads from truly meaningful ones. #[derive(Debug, Clone, Hash, Eq, PartialEq)] pub enum AccessKind { Write, From 7e9cd7083a3c39c6fea62f2342a5394bd0ab86b2 Mon Sep 17 00:00:00 2001 From: vladimirivanov Date: Tue, 23 Sep 2025 20:55:19 +0100 Subject: [PATCH 24/25] feat(biome-js-analyze): add is_meaningful_read to semantic class reads --- crates/biome_js_analyze/src/services/semantic_class.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/biome_js_analyze/src/services/semantic_class.rs b/crates/biome_js_analyze/src/services/semantic_class.rs index d2442a9ad7f3..a71ef992e2ee 100644 --- a/crates/biome_js_analyze/src/services/semantic_class.rs +++ b/crates/biome_js_analyze/src/services/semantic_class.rs @@ -894,8 +894,8 @@ fn get_read_access_kind(node: &AnyCandidateForUsedInExpressionNode) -> AccessKin /// Checks if the given node is used in an expression context /// (e.g., return, call arguments, conditionals, binary expressions). -/// Not limited to `this` references. It can be used for any node; additional cases may require extending the context checks. -/// Returns `true` if the read is access_kind, `false` otherwise. +/// Not limited to `this` references. +/// It can be used for any node; additional cases may require extending the context checks. fn is_used_in_expression_context(node: &AnyCandidateForUsedInExpressionNode) -> bool { node.syntax().ancestors().any(|ancestor| { matches!( @@ -1289,7 +1289,7 @@ mod tests { mod is_used_in_expression_context_tests { use super::*; use biome_js_syntax::binding_ext::AnyJsIdentifierBinding; - + fn extract_all_nodes(code: &str) -> Vec { let parsed = parse_ts(code); let root = parsed.syntax(); From 0d6d68ce772feba5a19f614a18675799df11624c Mon Sep 17 00:00:00 2001 From: vladimirivanov Date: Fri, 26 Sep 2025 18:24:07 +0100 Subject: [PATCH 25/25] meaningful read --- crates/biome_js_analyze/src/services/semantic_class.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/biome_js_analyze/src/services/semantic_class.rs b/crates/biome_js_analyze/src/services/semantic_class.rs index a71ef992e2ee..295be13d01c2 100644 --- a/crates/biome_js_analyze/src/services/semantic_class.rs +++ b/crates/biome_js_analyze/src/services/semantic_class.rs @@ -897,7 +897,7 @@ fn get_read_access_kind(node: &AnyCandidateForUsedInExpressionNode) -> AccessKin /// Not limited to `this` references. /// It can be used for any node; additional cases may require extending the context checks. fn is_used_in_expression_context(node: &AnyCandidateForUsedInExpressionNode) -> bool { - node.syntax().ancestors().any(|ancestor| { + node.syntax().ancestors().skip(1).any(|ancestor| { matches!( ancestor.kind(), JsSyntaxKind::JS_RETURN_STATEMENT