From c418bf53ce7d1bd40f3c5c075839e8c634f4c682 Mon Sep 17 00:00:00 2001
From: Dunqing <29533304+Dunqing@users.noreply.github.com>
Date: Mon, 15 Jul 2024 03:02:18 +0000
Subject: [PATCH] refactor(semantic): directly record `current_node_id` when
 adding a scope (#4265)

close: #4234
---
 crates/oxc_semantic/src/builder.rs         | 29 ++++++++--------------
 crates/oxc_semantic/src/scope.rs           | 13 +++++-----
 crates/oxc_traverse/src/context/scoping.rs |  2 +-
 3 files changed, 19 insertions(+), 25 deletions(-)

diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs
index ef2d5d9afd102..47108fd474c0c 100644
--- a/crates/oxc_semantic/src/builder.rs
+++ b/crates/oxc_semantic/src/builder.rs
@@ -189,7 +189,7 @@ impl<'a> SemanticBuilder<'a> {
     /// # Panics
     pub fn build(mut self, program: &Program<'a>) -> SemanticBuilderReturn<'a> {
         if self.source_type.is_typescript_definition() {
-            let scope_id = self.scope.add_scope(None, ScopeFlags::Top);
+            let scope_id = self.scope.add_scope(None, AstNodeId::dummy(), ScopeFlags::Top);
             program.scope_id.set(Some(scope_id));
         } else {
             self.visit_program(program);
@@ -449,7 +449,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
             flags = self.scope.get_new_scope_flags(flags, parent_scope_id);
         }
 
-        self.current_scope_id = self.scope.add_scope(parent_scope_id, flags);
+        self.current_scope_id = self.scope.add_scope(parent_scope_id, self.current_node_id, flags);
         scope_id.set(Some(self.current_scope_id));
 
         if let Some(parent_scope_id) = parent_scope_id {
@@ -471,8 +471,6 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
         if !flags.is_top() {
             self.bind_function_or_class_expression();
         }
-
-        self.add_current_node_id_to_current_scope();
     }
 
     fn leave_scope(&mut self) {
@@ -501,6 +499,15 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
 
     fn visit_program(&mut self, program: &Program<'a>) {
         let kind = AstKind::Program(self.alloc(program));
+        /* cfg */
+        let error_harness = control_flow!(|self, cfg| {
+            let error_harness = cfg.attach_error_harness(ErrorEdgeKind::Implicit);
+            let _program_basic_block = cfg.new_basic_block_normal();
+            error_harness
+        });
+        /* cfg - must be above directives as directives are in cfg */
+
+        self.enter_node(kind);
         self.enter_scope(
             {
                 let mut flags = ScopeFlags::Top;
@@ -512,16 +519,6 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
             &program.scope_id,
         );
 
-        /* cfg */
-        let error_harness = control_flow!(|self, cfg| {
-            let error_harness = cfg.attach_error_harness(ErrorEdgeKind::Implicit);
-            let _program_basic_block = cfg.new_basic_block_normal();
-            error_harness
-        });
-        /* cfg - must be above directives as directives are in cfg */
-
-        self.enter_node(kind);
-
         for directive in &program.directives {
             self.visit_directive(directive);
         }
@@ -1778,10 +1775,6 @@ impl<'a> SemanticBuilder<'a> {
         }
     }
 
-    fn add_current_node_id_to_current_scope(&mut self) {
-        self.scope.set_node_id(self.current_scope_id, self.current_node_id);
-    }
-
     fn make_all_namespaces_valuelike(&mut self) {
         for symbol_id in &self.namespace_stack {
             // Ambient modules cannot be value modules
diff --git a/crates/oxc_semantic/src/scope.rs b/crates/oxc_semantic/src/scope.rs
index e3437d5499281..d1ac3d4c8ae96 100644
--- a/crates/oxc_semantic/src/scope.rs
+++ b/crates/oxc_semantic/src/scope.rs
@@ -179,12 +179,17 @@ impl ScopeTree {
         &mut self.bindings[scope_id]
     }
 
-    pub fn add_scope(&mut self, parent_id: Option<ScopeId>, flags: ScopeFlags) -> ScopeId {
+    pub fn add_scope(
+        &mut self,
+        parent_id: Option<ScopeId>,
+        node_id: AstNodeId,
+        flags: ScopeFlags,
+    ) -> ScopeId {
         let scope_id = self.parent_ids.push(parent_id);
         _ = self.child_ids.push(vec![]);
         _ = self.flags.push(flags);
         _ = self.bindings.push(Bindings::default());
-        _ = self.node_ids.push(AstNodeId::dummy());
+        _ = self.node_ids.push(node_id);
 
         // Set this scope as child of parent scope.
         if let Some(parent_id) = parent_id {
@@ -194,10 +199,6 @@ impl ScopeTree {
         scope_id
     }
 
-    pub(crate) fn set_node_id(&mut self, scope_id: ScopeId, node_id: AstNodeId) {
-        self.node_ids[scope_id] = node_id;
-    }
-
     pub fn add_binding(&mut self, scope_id: ScopeId, name: CompactStr, symbol_id: SymbolId) {
         self.bindings[scope_id].insert(name, symbol_id);
     }
diff --git a/crates/oxc_traverse/src/context/scoping.rs b/crates/oxc_traverse/src/context/scoping.rs
index 3610cce8b5767..28642b34f235b 100644
--- a/crates/oxc_traverse/src/context/scoping.rs
+++ b/crates/oxc_traverse/src/context/scoping.rs
@@ -121,7 +121,7 @@ impl TraverseScoping {
     /// `flags` provided are amended to inherit from parent scope's flags.
     pub fn create_scope_child_of_current(&mut self, flags: ScopeFlags) -> ScopeId {
         let flags = self.scopes.get_new_scope_flags(flags, self.current_scope_id);
-        self.scopes.add_scope(Some(self.current_scope_id), flags)
+        self.scopes.add_scope(Some(self.current_scope_id), AstNodeId::dummy(), flags)
     }
 
     /// Insert a scope into scope tree below a statement.