Skip to content

Commit

Permalink
refactor(semantic): infer ReferenceFlags from Visit methods
Browse files Browse the repository at this point in the history
  • Loading branch information
Dunqing committed Nov 21, 2024
1 parent be152c0 commit 419b321
Show file tree
Hide file tree
Showing 20 changed files with 22,267 additions and 112 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ fn test() {
let pass = vec![
(
"var foo = 5;
label: while (true) {
console.log(foo);
break label;
Expand All @@ -43,7 +43,7 @@ fn test() {
),
(
"var foo = 5;
while (true) {
console.log(foo);
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,6 @@ fn test_vars_reassignment() {
}
",
"let a = 0; let b = a++; f(b);",
"let a = 0, b = 1; let c = b = a = 1; f(c+b);",
// implicit returns
"
let i = 0;
Expand Down Expand Up @@ -332,6 +331,7 @@ fn test_vars_reassignment() {
"let a = 0; let b = (0, (a++, 0)); f(b);",
"let a = 0; let b = ((0, a++), 0); f(b);",
"let a = 0; let b = (a, 0) + 1; f(b);",
"let a = 0, b = 1; let c = b = a = 1; f(c+b);",
];

Tester::new(NoUnusedVars::NAME, pass, fail)
Expand Down
10 changes: 9 additions & 1 deletion crates/oxc_linter/src/snapshots/[email protected]
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
---
source: crates/oxc_linter/src/tester.rs
snapshot_kind: text
---
eslint(no-unused-vars): Function 'foox' is declared but never used.
╭─[no_unused_vars.tsx:1:10]
Expand Down Expand Up @@ -261,6 +260,15 @@ snapshot_kind: text
╰────
help: Consider removing this declaration.

eslint(no-unused-vars): Variable 'a' is assigned a value but never used.
╭─[no_unused_vars.tsx:1:20]
1function f() { var a = 1; return function(){ f(a = 2); }; }
· ┬ ┬
· │ ╰── it was last assigned here
· ╰── 'a' is declared here
╰────
help: Did you mean to use this variable?

eslint(no-unused-vars): Identifier 'x' is imported but never used.
╭─[no_unused_vars.tsx:1:8]
1import x from "y";
Expand Down
10 changes: 9 additions & 1 deletion crates/oxc_linter/src/snapshots/[email protected]
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
---
source: crates/oxc_linter/src/tester.rs
snapshot_kind: text
---
eslint(no-unused-vars): Variable 'a' is assigned a value but never used. Unused variables should start with a '_'.
╭─[no_unused_vars.tsx:1:5]
Expand Down Expand Up @@ -99,3 +98,12 @@ snapshot_kind: text
· ╰── 'a' is declared here
╰────
help: Consider removing this declaration.

eslint(no-unused-vars): Variable 'a' is assigned a value but never used. Unused variables should start with a '_'.
╭─[no_unused_vars.tsx:1:5]
1let a = 0, b = 1; let c = b = a = 1; f(c+b);
· ┬ ┬
· │ ╰── it was last assigned here
· ╰── 'a' is declared here
╰────
help: Did you mean to use this variable?
148 changes: 85 additions & 63 deletions crates/oxc_semantic/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use oxc_cfg::{
};
use oxc_diagnostics::OxcDiagnostic;
use oxc_span::{Atom, CompactStr, SourceType, Span};
use oxc_syntax::{module_record::ModuleRecord, operator::AssignmentOperator};
use oxc_syntax::module_record::ModuleRecord;

use crate::{
binder::Binder,
Expand Down Expand Up @@ -890,6 +890,13 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {

let kind = AstKind::AssignmentExpression(self.alloc(expr));
self.enter_node(kind);

// Only the operator is not an `=` operator, the left-hand side is both read and write.
// <https://tc39.es/ecma262/#sec-assignment-operators-runtime-semantics-evaluation>
if !expr.operator.is_assign() {
self.current_reference_flags = ReferenceFlags::Read;
}

self.visit_assignment_target(&expr.left);

/* cfg */
Expand Down Expand Up @@ -1740,6 +1747,81 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
self.leave_node(kind);
self.leave_scope();
}

fn visit_update_expression(&mut self, it: &UpdateExpression<'a>) {
let kind = AstKind::UpdateExpression(self.alloc(it));
self.enter_node(kind);
// `++a` or `a--`
// ^ ^ We always treat `a` as Read and Write reference,
// the Write flags will add in `visit_simple_assignment_target`
self.current_reference_flags = ReferenceFlags::Read;
self.visit_simple_assignment_target(&it.argument);
self.leave_node(kind);
}

fn visit_member_expression(&mut self, it: &MemberExpression<'a>) {
let kind = AstKind::MemberExpression(self.alloc(it));
self.enter_node(kind);

// A.B = 1;
// ^^^ Can't treat A as a Write reference since it's A's property(B) that changes.
self.current_reference_flags -= ReferenceFlags::Write;

match it {
MemberExpression::ComputedMemberExpression(it) => {
self.visit_computed_member_expression(it);
}
MemberExpression::StaticMemberExpression(it) => self.visit_static_member_expression(it),
MemberExpression::PrivateFieldExpression(it) => self.visit_private_field_expression(it),
}
self.leave_node(kind);
}

fn visit_simple_assignment_target(&mut self, it: &SimpleAssignmentTarget<'a>) {
let kind = AstKind::SimpleAssignmentTarget(self.alloc(it));
self.enter_node(kind);
let prev_reference_flags = self.current_reference_flags;
self.current_reference_flags |= ReferenceFlags::Write;
match it {
SimpleAssignmentTarget::AssignmentTargetIdentifier(it) => {
self.visit_identifier_reference(it);
}
SimpleAssignmentTarget::TSAsExpression(it) => {
self.visit_ts_as_expression(it);
}
SimpleAssignmentTarget::TSSatisfiesExpression(it) => {
self.visit_ts_satisfies_expression(it);
}
SimpleAssignmentTarget::TSNonNullExpression(it) => {
self.visit_ts_non_null_expression(it);
}
SimpleAssignmentTarget::TSTypeAssertion(it) => {
self.visit_ts_type_assertion(it);
}
SimpleAssignmentTarget::TSInstantiationExpression(it) => {
self.visit_ts_instantiation_expression(it);
}
match_member_expression!(SimpleAssignmentTarget) => {
self.visit_member_expression(it.to_member_expression());
}
}
self.current_reference_flags = prev_reference_flags;
self.leave_node(kind);
}

fn visit_assignment_target_property_identifier(
&mut self,
it: &AssignmentTargetPropertyIdentifier<'a>,
) {
// NOTE: AstKind doesn't exists!
let prev_reference_flags = self.current_reference_flags;
self.current_reference_flags |= ReferenceFlags::Write;
self.visit_identifier_reference(&it.binding);
self.current_reference_flags = prev_reference_flags;
if let Some(init) = &it.init {
self.visit_expression(init);
}
}
}

impl<'a> SemanticBuilder<'a> {
Expand Down Expand Up @@ -1920,29 +2002,6 @@ impl<'a> SemanticBuilder<'a> {
AstKind::IdentifierReference(ident) => {
self.reference_identifier(ident);
}
AstKind::UpdateExpression(_) => {
if !self.current_reference_flags.is_type()
&& self.is_not_expression_statement_parent()
{
self.current_reference_flags |= ReferenceFlags::Read;
}
self.current_reference_flags |= ReferenceFlags::Write;
}
AstKind::AssignmentExpression(expr) => {
if expr.operator != AssignmentOperator::Assign
|| self.is_not_expression_statement_parent()
{
self.current_reference_flags |= ReferenceFlags::Read;
}
}
AstKind::MemberExpression(_) => {
// A.B = 1;
// ^^^ we can't treat A as Write reference, because it's the property(B) of A that change
self.current_reference_flags -= ReferenceFlags::Write;
}
AstKind::AssignmentTarget(_) => {
self.current_reference_flags |= ReferenceFlags::Write;
}
AstKind::LabeledStatement(stmt) => {
self.unused_labels.add(stmt.label.name.as_str());
}
Expand Down Expand Up @@ -1998,27 +2057,12 @@ impl<'a> SemanticBuilder<'a> {
AstKind::TSTypeName(_) => {
self.current_reference_flags -= ReferenceFlags::Type;
}
AstKind::UpdateExpression(_) => {
if self.is_not_expression_statement_parent() {
self.current_reference_flags -= ReferenceFlags::Read;
}
self.current_reference_flags -= ReferenceFlags::Write;
}
AstKind::AssignmentExpression(_) | AstKind::ExportNamedDeclaration(_)
AstKind::ExportNamedDeclaration(_)
| AstKind::TSTypeQuery(_)
// Clear the reference flags that are set in AstKind::PropertySignature
| AstKind::PropertyKey(_) => {
self.current_reference_flags = ReferenceFlags::empty();
}
AstKind::AssignmentTarget(_) =>{
// Handle nested assignment targets like `({a: b} = obj)`
if !matches!(
self.nodes.parent_kind(self.current_node_id),
Some(AstKind::ObjectAssignmentTarget(_) | AstKind::ArrayAssignmentTarget(_))
) {
self.current_reference_flags -= ReferenceFlags::Write;
}
},
AstKind::LabeledStatement(_) => self.unused_labels.mark_unused(self.current_node_id),
_ => {}
}
Expand Down Expand Up @@ -2046,34 +2090,12 @@ impl<'a> SemanticBuilder<'a> {
}

/// Resolve reference flags for the current ast node.
#[inline]
fn resolve_reference_usages(&self) -> ReferenceFlags {
if self.current_reference_flags.is_empty() {
ReferenceFlags::Read
} else {
self.current_reference_flags
}
}

fn is_not_expression_statement_parent(&self) -> bool {
for node in self.nodes.ancestors(self.current_node_id).skip(1) {
return match node.kind() {
AstKind::ParenthesizedExpression(_) => continue,
AstKind::ExpressionStatement(_) => {
if self.current_scope_flags().is_arrow() {
if let Some(node) = self.nodes.ancestors(node.id()).nth(2) {
// (x) => x++
// ^^^ implicit return, we need to treat `x` as a read reference
if matches!(node.kind(), AstKind::ArrowFunctionExpression(arrow) if arrow.expression)
{
return true;
}
}
}
false
}
_ => true,
};
}
false
}
}
16 changes: 8 additions & 8 deletions crates/oxc_semantic/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,9 +365,8 @@ mod tests {
// assignment expressions count as read-write
(SourceType::default(), "let a = 1, b; b = a += 5", ReferenceFlags::read_write()),
(SourceType::default(), "let a = 1; a += 5", ReferenceFlags::read_write()),
// note: we consider a to be written, and the read of `1` propagates upwards
(SourceType::default(), "let a, b; b = a = 1", ReferenceFlags::read_write()),
(SourceType::default(), "let a, b; b = (a = 1)", ReferenceFlags::read_write()),
(SourceType::default(), "let a, b; b = a = 1", ReferenceFlags::write()),
(SourceType::default(), "let a, b; b = (a = 1)", ReferenceFlags::write()),
(SourceType::default(), "let a, b, c; b = c = a", ReferenceFlags::read()),
// sequences return last read_write in sequence
(SourceType::default(), "let a, b; b = (0, a++)", ReferenceFlags::read_write()),
Expand Down Expand Up @@ -404,27 +403,28 @@ mod tests {
// least, or now)
(SourceType::default(), "let a, b; b = (a, 0)", ReferenceFlags::read()),
(SourceType::default(), "let a, b; b = (--a, 0)", ReferenceFlags::read_write()),
// other reads after a is written
// a = 1 writes, but the CallExpression reads the rhs (1) so a isn't read
(
SourceType::default(),
"let a; function foo(a) { return a }; foo(a = 1)",
ReferenceFlags::read_write(),
// ^ write
ReferenceFlags::write(),
),
// member expression
(SourceType::default(), "let a; a.b = 1", ReferenceFlags::read()),
(SourceType::default(), "let a; let b; b[a += 1] = 1", ReferenceFlags::read_write()),
(
SourceType::default(),
"let a; let b; let c; b[c[a = c['a']] = 'c'] = 'b'",
ReferenceFlags::read_write(),
// ^ write
ReferenceFlags::write(),
),
(
SourceType::default(),
"let a; let b; let c; a[c[b = c['a']] = 'c'] = 'b'",
ReferenceFlags::read(),
),
(SourceType::default(), "console.log;let a=0;a++", ReferenceFlags::write()),
(SourceType::default(), "console.log;let a=0;a++", ReferenceFlags::read_write()),
// ^^^ UpdateExpression is always a read | write
// typescript
(typescript, "let a: number = 1; (a as any) = true", ReferenceFlags::write()),
(typescript, "let a: number = 1; a = true as any", ReferenceFlags::write()),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
---
source: crates/oxc_semantic/tests/main.rs
input_file: crates/oxc_semantic/tests/fixtures/oxc/assignment/nested-assignment.ts
snapshot_kind: text
---
[
{
Expand Down Expand Up @@ -32,7 +31,13 @@ snapshot_kind: text
"flags": "ReferenceFlags(Write)",
"id": 2,
"name": "y",
"node_id": 40
"node_id": 41
},
{
"flags": "ReferenceFlags(Read)",
"id": 4,
"name": "y",
"node_id": 51
}
]
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
let y;
({ y } = {});
({ 0: {}, y } = {0: {}});
([[], y] = [[]])
([[], y] = [[]])
({
unknown = y,
} = unknown);
Loading

0 comments on commit 419b321

Please sign in to comment.