fix: keep the context of function call while deconflict symbols

rolldown · Feb 2, 2024 · 7401a05 · 7401a05
1 parent f6cfe5c
commit 7401a05
Show file tree

Hide file tree

Showing 6 changed files with 98 additions and 37 deletions.
diff --git a/crates/rolldown/src/bundler/utils/finalizer.rs b/crates/rolldown/src/bundler/utils/finalizer.rs
@@ -80,6 +80,57 @@ where
     }
     None
   }
+
+  fn finalize_identifier_reference(&self, expr: &mut ast::Expression<'ast>, is_callee: bool) {
+    let ast::Expression::Identifier(id_ref) = expr else {
+      return;
+    };
+    let Some(reference_id) = id_ref.reference_id.get() else {
+      // Some `IdentifierReference`s constructed by bundler don't have `ReferenceId` and we just ignore them.
+      return;
+    };
+
+    let Some(symbol_id) = self.scope.symbol_id_for(reference_id) else {
+      // we will hit this branch if the reference is for a global variable
+      return;
+    };
+
+    let symbol_ref: SymbolRef = (self.ctx.id, symbol_id).into();
+    let canonical_ref = self.ctx.symbols.par_canonical_ref_for(symbol_ref);
+    let symbol = self.ctx.symbols.get(canonical_ref);
+
+    if let Some(ns_alias) = &symbol.namespace_alias {
+      let canonical_ns_name = self
+        .canonical_name_for(ns_alias.namespace_ref)
+        .expect("namespace alias should have a canonical name");
+      let prop_name = &ns_alias.property_name;
+      if is_callee {
+        let callee = ast::Expression::MemberExpression(
+          self
+            .snippet
+            .identifier_member_expression(canonical_ns_name.clone(), prop_name.clone())
+            .into_in(self.alloc),
+        );
+        let wrapped_callee = self.snippet.seq_expr2(self.snippet.number_expr(0.0), callee);
+        *expr = wrapped_callee;
+      } else {
+        *expr = ast::Expression::MemberExpression(
+          self
+            .snippet
+            .identifier_member_expression(canonical_ns_name.clone(), prop_name.clone())
+            .into_in(self.alloc),
+        );
+      }
+    } else {
+      if let Some(canonical_name) = self.canonical_name_for(canonical_ref) {
+        if id_ref.name != canonical_name {
+          id_ref.name = canonical_name.clone();
+        }
+      } else {
+        // FIXME: all bindings should have a canonical name
+      }
+    }
+  }
 }
 // visit
 
@@ -253,41 +304,14 @@ impl<'ast, 'me: 'ast> VisitMut<'ast> for Finalizer<'me, 'ast> {
 
   #[allow(clippy::collapsible_else_if)]
   fn visit_expression(&mut self, expr: &mut ast::Expression<'ast>) {
-    if let ast::Expression::Identifier(id_ref) = expr {
-      if let Some(reference_id) = id_ref.reference_id.get() {
-        if let Some(symbol_id) = self.scope.symbol_id_for(reference_id) {
-          let symbol_ref: SymbolRef = (self.ctx.id, symbol_id).into();
-          let canonical_ref = self.ctx.symbols.par_canonical_ref_for(symbol_ref);
-          let symbol = self.ctx.symbols.get(canonical_ref);
-
-          if let Some(ns_alias) = &symbol.namespace_alias {
-            let canonical_ns_name = self
-              .canonical_name_for(ns_alias.namespace_ref)
-              .expect("namespace alias should have a canonical name");
-            let prop_name = &ns_alias.property_name;
-            *expr = ast::Expression::MemberExpression(
-              self
-                .snippet
-                .identifier_member_expression(canonical_ns_name.clone(), prop_name.clone())
-                .into_in(self.alloc),
-            );
-          } else {
-            if let Some(canonical_name) = self.canonical_name_for(canonical_ref) {
-              if id_ref.name != canonical_name {
-                id_ref.name = canonical_name.clone();
-              }
-            } else {
-              // FIXME: all bindings should have a canonical name
-            }
-          }
-        } else {
-          // we will hit this branch if the reference is for a global variable
-        };
-      } else {
-        // Some `IdentifierReference`s constructed by bundler don't have `ReferenceId` and we just ignore them.
+    if let ast::Expression::CallExpression(call_exp) = expr {
+      if let ast::Expression::Identifier(_) = &mut call_exp.callee {
+        self.finalize_identifier_reference(&mut call_exp.callee, true);
       }
-    } else {
-      self.visit_expression_match(expr);
     }
+
+    self.finalize_identifier_reference(expr, false);
+
+    self.visit_expression_match(expr);
   }
 }
diff --git a/crates/rolldown/tests/esbuild/default/es6_from_common_js/artifacts.snap b/crates/rolldown/tests/esbuild/default/es6_from_common_js/artifacts.snap
@@ -26,6 +26,6 @@ var require_bar = __commonJS((exports, module) => {
 
 // entry.js
 var import_foo = require_foo();
-console.log(import_foo.foo(), import_bar.bar());
+console.log((0,import_foo.foo)(), (0,import_bar.bar)());
 var import_bar = require_bar();
 ```
diff --git a/crates/rolldown/tests/esbuild/default/import_missing_common_js/artifacts.snap b/crates/rolldown/tests/esbuild/default/import_missing_common_js/artifacts.snap
@@ -18,6 +18,6 @@ var require_foo = __commonJS((exports, module) => {
 // entry.js
 var import_foo = require_foo();
 if (false) {
-	console.log(import_foo.default(import_foo.x, import_foo.y));
+	console.log((0,import_foo.default)(import_foo.x, import_foo.y));
 }
 ```
diff --git a/crates/rolldown/tests/esbuild/default/nested_es6_from_common_js/artifacts.snap b/crates/rolldown/tests/esbuild/default/nested_es6_from_common_js/artifacts.snap
@@ -20,6 +20,6 @@ var require_foo = __commonJS((exports, module) => {
 // entry.js
 var import_foo = require_foo();
 (() => {
-	console.log(import_foo.fn());
+	console.log((0,import_foo.fn)());
 })();
 ```
diff --git a/crates/rolldown_oxc/src/ast_snippet.rs b/crates/rolldown_oxc/src/ast_snippet.rs
@@ -169,4 +169,35 @@ impl<'ast> AstSnippet<'ast> {
       ast::Expression::CallExpression(commonjs_call_expr.into_in(self.alloc)),
     )
   }
+
+  /// ```js
+  /// (a, b)
+  /// ```
+  pub fn seq_expr2(
+    &self,
+    a: ast::Expression<'ast>,
+    b: ast::Expression<'ast>,
+  ) -> ast::Expression<'ast> {
+    let mut expressions = allocator::Vec::new_in(self.alloc);
+    expressions.push(a);
+    expressions.push(b);
+    ast::Expression::SequenceExpression(
+      ast::SequenceExpression { expressions, ..Dummy::dummy(self.alloc) }.into_in(self.alloc),
+    )
+  }
+
+  /// ```js
+  /// 42
+  /// ```
+  pub fn number_expr(&self, value: f64) -> ast::Expression<'ast> {
+    ast::Expression::NumberLiteral(
+      ast::NumberLiteral {
+        span: Dummy::dummy(self.alloc),
+        value,
+        raw: self.alloc.alloc(value.to_string()),
+        base: oxc::syntax::NumberBase::Decimal,
+      }
+      .into_in(self.alloc),
+    )
+  }
 }
diff --git a/crates/rolldown_oxc/src/dummy/impl_for_ast_node.rs b/crates/rolldown_oxc/src/dummy/impl_for_ast_node.rs
@@ -245,3 +245,9 @@ impl<'ast> DummyIn<'ast> for ast::FormalParameter<'ast> {
     }
   }
 }
+
+impl<'ast> DummyIn<'ast> for ast::SequenceExpression<'ast> {
+  fn dummy(alloc: &'ast Allocator) -> Self {
+    Self { span: DummyIn::dummy(alloc), expressions: DummyIn::dummy(alloc) }
+  }
+}