From c100826b42fd6d6e1623eb529ee4c3816c853049 Mon Sep 17 00:00:00 2001
From: overlookmotel <557937+overlookmotel@users.noreply.github.com>
Date: Fri, 23 Aug 2024 09:28:13 +0000
Subject: [PATCH] refactor(semantic)!: always create a scope for `for`
statements (#5110)
Part of #5008. Make scopes for `ForStatement`, `ForInStatement` and `ForOfStatement` unconditional. i.e. always create a scope, even if there is no lexical binding (e.g. `for (i of a) {}`).
---
crates/oxc_ast/src/ast/js.rs | 6 +-
crates/oxc_ast/src/generated/visit.rs | 28 ++-----
crates/oxc_ast/src/generated/visit_mut.rs | 28 ++-----
crates/oxc_semantic/src/builder.rs | 28 ++-----
crates/oxc_traverse/src/context/scoping.rs | 18 +----
crates/oxc_traverse/src/walk.rs | 57 ++++++--------
tasks/coverage/semantic_typescript.snap | 92 +++++++++++-----------
7 files changed, 94 insertions(+), 163 deletions(-)
diff --git a/crates/oxc_ast/src/ast/js.rs b/crates/oxc_ast/src/ast/js.rs
index ae8d6f9d265e8..5811da2d9c72b 100644
--- a/crates/oxc_ast/src/ast/js.rs
+++ b/crates/oxc_ast/src/ast/js.rs
@@ -1345,7 +1345,7 @@ pub struct WhileStatement<'a> {
/// For Statement
#[ast(visit)]
-#[scope(if(self.init.as_ref().is_some_and(ForStatementInit::is_lexical_declaration)))]
+#[scope]
#[derive(Debug)]
#[generate_derive(CloneIn, GetSpan, GetSpanMut)]
#[cfg_attr(feature = "serialize", derive(Serialize, Tsify))]
@@ -1383,7 +1383,7 @@ pub enum ForStatementInit<'a> {
/// For-In Statement
#[ast(visit)]
-#[scope(if(self.left.is_lexical_declaration()))]
+#[scope]
#[derive(Debug)]
#[generate_derive(CloneIn, GetSpan, GetSpanMut)]
#[cfg_attr(feature = "serialize", derive(Serialize, Tsify))]
@@ -1419,7 +1419,7 @@ pub enum ForStatementLeft<'a> {
}
/// For-Of Statement
#[ast(visit)]
-#[scope(if(self.left.is_lexical_declaration()))]
+#[scope]
#[derive(Debug)]
#[generate_derive(CloneIn, GetSpan, GetSpanMut)]
#[cfg_attr(feature = "serialize", derive(Serialize, Tsify))]
diff --git a/crates/oxc_ast/src/generated/visit.rs b/crates/oxc_ast/src/generated/visit.rs
index fd4fa802fe1de..9c0268a716677 100644
--- a/crates/oxc_ast/src/generated/visit.rs
+++ b/crates/oxc_ast/src/generated/visit.rs
@@ -3504,16 +3504,11 @@ pub mod walk {
pub fn walk_for_in_statement<'a, V: Visit<'a>>(visitor: &mut V, it: &ForInStatement<'a>) {
let kind = AstKind::ForInStatement(visitor.alloc(it));
visitor.enter_node(kind);
- let scope_events_cond = it.left.is_lexical_declaration();
- if scope_events_cond {
- visitor.enter_scope(ScopeFlags::empty(), &it.scope_id);
- }
+ visitor.enter_scope(ScopeFlags::empty(), &it.scope_id);
visitor.visit_for_statement_left(&it.left);
visitor.visit_expression(&it.right);
visitor.visit_statement(&it.body);
- if scope_events_cond {
- visitor.leave_scope();
- }
+ visitor.leave_scope();
visitor.leave_node(kind);
}
@@ -3575,16 +3570,11 @@ pub mod walk {
pub fn walk_for_of_statement<'a, V: Visit<'a>>(visitor: &mut V, it: &ForOfStatement<'a>) {
let kind = AstKind::ForOfStatement(visitor.alloc(it));
visitor.enter_node(kind);
- let scope_events_cond = it.left.is_lexical_declaration();
- if scope_events_cond {
- visitor.enter_scope(ScopeFlags::empty(), &it.scope_id);
- }
+ visitor.enter_scope(ScopeFlags::empty(), &it.scope_id);
visitor.visit_for_statement_left(&it.left);
visitor.visit_expression(&it.right);
visitor.visit_statement(&it.body);
- if scope_events_cond {
- visitor.leave_scope();
- }
+ visitor.leave_scope();
visitor.leave_node(kind);
}
@@ -3592,11 +3582,7 @@ pub mod walk {
pub fn walk_for_statement<'a, V: Visit<'a>>(visitor: &mut V, it: &ForStatement<'a>) {
let kind = AstKind::ForStatement(visitor.alloc(it));
visitor.enter_node(kind);
- let scope_events_cond =
- it.init.as_ref().is_some_and(ForStatementInit::is_lexical_declaration);
- if scope_events_cond {
- visitor.enter_scope(ScopeFlags::empty(), &it.scope_id);
- }
+ visitor.enter_scope(ScopeFlags::empty(), &it.scope_id);
if let Some(init) = &it.init {
visitor.visit_for_statement_init(init);
}
@@ -3607,9 +3593,7 @@ pub mod walk {
visitor.visit_expression(update);
}
visitor.visit_statement(&it.body);
- if scope_events_cond {
- visitor.leave_scope();
- }
+ visitor.leave_scope();
visitor.leave_node(kind);
}
diff --git a/crates/oxc_ast/src/generated/visit_mut.rs b/crates/oxc_ast/src/generated/visit_mut.rs
index 8426af9f655aa..6dd5f3d985b7f 100644
--- a/crates/oxc_ast/src/generated/visit_mut.rs
+++ b/crates/oxc_ast/src/generated/visit_mut.rs
@@ -3692,16 +3692,11 @@ pub mod walk_mut {
) {
let kind = AstType::ForInStatement;
visitor.enter_node(kind);
- let scope_events_cond = it.left.is_lexical_declaration();
- if scope_events_cond {
- visitor.enter_scope(ScopeFlags::empty(), &it.scope_id);
- }
+ visitor.enter_scope(ScopeFlags::empty(), &it.scope_id);
visitor.visit_for_statement_left(&mut it.left);
visitor.visit_expression(&mut it.right);
visitor.visit_statement(&mut it.body);
- if scope_events_cond {
- visitor.leave_scope();
- }
+ visitor.leave_scope();
visitor.leave_node(kind);
}
@@ -3772,16 +3767,11 @@ pub mod walk_mut {
) {
let kind = AstType::ForOfStatement;
visitor.enter_node(kind);
- let scope_events_cond = it.left.is_lexical_declaration();
- if scope_events_cond {
- visitor.enter_scope(ScopeFlags::empty(), &it.scope_id);
- }
+ visitor.enter_scope(ScopeFlags::empty(), &it.scope_id);
visitor.visit_for_statement_left(&mut it.left);
visitor.visit_expression(&mut it.right);
visitor.visit_statement(&mut it.body);
- if scope_events_cond {
- visitor.leave_scope();
- }
+ visitor.leave_scope();
visitor.leave_node(kind);
}
@@ -3789,11 +3779,7 @@ pub mod walk_mut {
pub fn walk_for_statement<'a, V: VisitMut<'a>>(visitor: &mut V, it: &mut ForStatement<'a>) {
let kind = AstType::ForStatement;
visitor.enter_node(kind);
- let scope_events_cond =
- it.init.as_ref().is_some_and(ForStatementInit::is_lexical_declaration);
- if scope_events_cond {
- visitor.enter_scope(ScopeFlags::empty(), &it.scope_id);
- }
+ visitor.enter_scope(ScopeFlags::empty(), &it.scope_id);
if let Some(init) = &mut it.init {
visitor.visit_for_statement_init(init);
}
@@ -3804,9 +3790,7 @@ pub mod walk_mut {
visitor.visit_expression(update);
}
visitor.visit_statement(&mut it.body);
- if scope_events_cond {
- visitor.leave_scope();
- }
+ visitor.leave_scope();
visitor.leave_node(kind);
}
diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs
index 0b118da5a11ef..4977834c96c42 100644
--- a/crates/oxc_semantic/src/builder.rs
+++ b/crates/oxc_semantic/src/builder.rs
@@ -945,11 +945,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
fn visit_for_statement(&mut self, stmt: &ForStatement<'a>) {
let kind = AstKind::ForStatement(self.alloc(stmt));
self.enter_node(kind);
- let is_lexical_declaration =
- stmt.init.as_ref().is_some_and(ForStatementInit::is_lexical_declaration);
- if is_lexical_declaration {
- self.enter_scope(ScopeFlags::empty(), &stmt.scope_id);
- }
+ self.enter_scope(ScopeFlags::empty(), &stmt.scope_id);
if let Some(init) = &stmt.init {
self.visit_for_statement_init(init);
}
@@ -1007,19 +1003,14 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
});
/* cfg */
- if is_lexical_declaration {
- self.leave_scope();
- }
+ self.leave_scope();
self.leave_node(kind);
}
fn visit_for_in_statement(&mut self, stmt: &ForInStatement<'a>) {
let kind = AstKind::ForInStatement(self.alloc(stmt));
self.enter_node(kind);
- let is_lexical_declaration = stmt.left.is_lexical_declaration();
- if is_lexical_declaration {
- self.enter_scope(ScopeFlags::empty(), &stmt.scope_id);
- }
+ self.enter_scope(ScopeFlags::empty(), &stmt.scope_id);
self.visit_for_statement_left(&stmt.left);
@@ -1071,19 +1062,14 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
});
/* cfg */
- if is_lexical_declaration {
- self.leave_scope();
- }
+ self.leave_scope();
self.leave_node(kind);
}
fn visit_for_of_statement(&mut self, stmt: &ForOfStatement<'a>) {
let kind = AstKind::ForOfStatement(self.alloc(stmt));
self.enter_node(kind);
- let is_lexical_declaration = stmt.left.is_lexical_declaration();
- if is_lexical_declaration {
- self.enter_scope(ScopeFlags::empty(), &stmt.scope_id);
- }
+ self.enter_scope(ScopeFlags::empty(), &stmt.scope_id);
self.visit_for_statement_left(&stmt.left);
@@ -1134,9 +1120,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
});
/* cfg */
- if is_lexical_declaration {
- self.leave_scope();
- }
+ self.leave_scope();
self.leave_node(kind);
}
diff --git a/crates/oxc_traverse/src/context/scoping.rs b/crates/oxc_traverse/src/context/scoping.rs
index e53d4078aaa7c..41772f9ea5eaa 100644
--- a/crates/oxc_traverse/src/context/scoping.rs
+++ b/crates/oxc_traverse/src/context/scoping.rs
@@ -539,27 +539,15 @@ impl<'a> Visit<'a> for ChildScopeCollector {
}
fn visit_for_statement(&mut self, stmt: &ForStatement<'a>) {
- if let Some(scope_id) = stmt.scope_id.get() {
- self.scope_ids.push(scope_id);
- } else {
- walk::walk_for_statement(self, stmt);
- }
+ self.scope_ids.push(stmt.scope_id.get().unwrap());
}
fn visit_for_in_statement(&mut self, stmt: &ForInStatement<'a>) {
- if let Some(scope_id) = stmt.scope_id.get() {
- self.scope_ids.push(scope_id);
- } else {
- walk::walk_for_in_statement(self, stmt);
- }
+ self.scope_ids.push(stmt.scope_id.get().unwrap());
}
fn visit_for_of_statement(&mut self, stmt: &ForOfStatement<'a>) {
- if let Some(scope_id) = stmt.scope_id.get() {
- self.scope_ids.push(scope_id);
- } else {
- walk::walk_for_of_statement(self, stmt);
- }
+ self.scope_ids.push(stmt.scope_id.get().unwrap());
}
fn visit_switch_statement(&mut self, stmt: &SwitchStatement<'a>) {
diff --git a/crates/oxc_traverse/src/walk.rs b/crates/oxc_traverse/src/walk.rs
index d3ded3cc5d957..66fc3e442800a 100644
--- a/crates/oxc_traverse/src/walk.rs
+++ b/crates/oxc_traverse/src/walk.rs
@@ -1617,14 +1617,13 @@ pub(crate) unsafe fn walk_for_statement<'a, Tr: Traverse<'a>>(
ctx: &mut TraverseCtx<'a>,
) {
traverser.enter_for_statement(&mut *node, ctx);
- let mut previous_scope_id = None;
- if let Some(scope_id) = (*((node as *mut u8).add(ancestor::OFFSET_FOR_STATEMENT_SCOPE_ID)
- as *mut Cell