bundler: add every top-level symbol before renaming nested scopes

src/bundler/linker_context/renameSymbolsInChunk.rs

@@ -320,6 +320,11 @@ pub(crate) unsafe fn rename_symbols_in_chunk(
 
     let mut sorted: Vec<u32> = Vec::new();
 
+    // Renamed in a second pass, once every top-level symbol in the chunk is
+    // in the root scope. Interleaving the passes let a nested local shadow a
+    // later part's top-level symbol (#41054).
+    let mut nested_scopes: Vec<(u32, *const bun_ast::Scope)> = Vec::new();
+
     for &source_index in files_in_order {
         let wrap = all_flags[source_index as usize].wrap;
         // Need `&mut [Part]` for `add_top_level_declared_symbols`.
@@ -398,16 +403,10 @@ pub(crate) unsafe fn rename_symbols_in_chunk(
                         }
                     }
                 }
-                // Reshaped for borrowck — `&mut r.root` while `r` is the
-                // `&mut self` receiver. Take a raw pointer; `assign_names_*` does
-                // not touch `self.root` through `self`.
-                let root: *mut renamer::NumberScope = core::ptr::addr_of_mut!(r.root);
-                r.assign_names_recursive_with_number_scope(
-                    root,
-                    &all_module_scopes[source_index as usize],
+                nested_scopes.push((
                     source_index,
-                    &mut sorted,
-                );
+                    &raw const all_module_scopes[source_index as usize],
+                ));
                 continue;
             }
 
@@ -441,16 +440,8 @@ pub(crate) unsafe fn rename_symbols_in_chunk(
             r.add_top_level_declared_symbols(&mut part.declared_symbols);
             // `Part.scopes: StoreSlice<*mut Scope>` — safe `Deref` to `&[*mut Scope]`.
             for scope in part.scopes.iter() {
-                let root: *mut renamer::NumberScope = core::ptr::addr_of_mut!(r.root);
-                // SAFETY: each `*mut Scope` is a valid arena-allocated scope.
-                r.assign_names_recursive_with_number_scope(
-                    root,
-                    unsafe { &**scope },
-                    source_index,
-                    &mut sorted,
-                );
+                nested_scopes.push((source_index, (*scope).cast_const()));
             }
-            r.number_scope_pool.hive.used = bun_collections::hive_array::HiveBitSet::init_empty();
         }
     }
 
@@ -460,5 +451,20 @@ pub(crate) unsafe fn rename_symbols_in_chunk(
         r.add_top_level_symbol(copy);
     }
 
+    for &(source_index, scope) in &nested_scopes {
+        // Raw pointer for borrowck: `assign_names_*` takes `&mut r` plus
+        // `r.root`, and never reaches `self.root` through `self`.
+        let root: *mut renamer::NumberScope = core::ptr::addr_of_mut!(r.root);
+        // SAFETY: each `scope` is a live arena-allocated scope collected
+        // above; nothing mutates those scopes in between.
+        r.assign_names_recursive_with_number_scope(
+            root,
+            unsafe { &*scope },
+            source_index,
+            &mut sorted,
+        );
+        r.number_scope_pool.hive.used = bun_collections::hive_array::HiveBitSet::init_empty();
+    }
+
     Ok(ChunkRenamer::Number(r))
 }

test/bundler/bundler_bytecode_portable.test.ts

@@ -495,17 +495,17 @@ describe("bytecode cache portability", () => {
           },
         },
         "bun build --bytecode acorn/dist/acorn.mjs": {
-          "js": "2ed858fa1b38a20673cee13a857ccdaedb9da0a325ebc47e81c4851de77eef3c",
+          "js": "9750b95678aa2ad19085761db9ff451e592cd0ce4e04dd156c64a0c42bb32461",
           "jsc": {
             "bytes": 266176,
-            "sha256": "9fec89f154cf2ae1593f52d49bb3ba6bae0634a56e4cc8fe76aabf6abb70bfe1",
+            "sha256": "d71089597ed075372781d7f66db2ce07fbbe612ae2efc22c11194d5d3fa27735",
           },
         },
         "bun build --bytecode all.js": {
-          "js": "a2f566b81700f852fef68247aef22af8b916858de30e4dc787846fd874c9e4d2",
+          "js": "c0f212b766071d77e9691261160635b50ccaf7f1c56e08ba87ccbb6d5e01fcd4",
           "jsc": {
             "bytes": 2178656,
-            "sha256": "f743f4e9cff325bfd8d344f2dead6a8ba6b9933393a09c75334429ead924c480",
+            "sha256": "41d4c0ba8cce1abdd369a4ce729e1615bce65a4c1b854db5ceda6d4c6568b3e0",
           },
         },
         "bun build --bytecode big.js": {
@@ -523,24 +523,24 @@ describe("bytecode cache portability", () => {
           },
         },
         "bun build --bytecode happy-dom/lib/index.js": {
-          "js": "148f0d3e4baf485281725f859deb3e717a6da25a4a08de9af288d5ef54b6414b",
+          "js": "239b4f0a41cb558ebbde682e021a8542a133e4f324c964b42c27f9a67c486f6c",
           "jsc": {
-            "bytes": 2528840,
-            "sha256": "d68b260282a74f545c15b5a3b812b69cb4e828972f85c5de60f8307b73c43e57",
+            "bytes": 2528848,
+            "sha256": "a15025be8413ef264c70eea105a8c515a7cf1dd8df9eec50e5d2b37da2b490cb",
           },
         },
         "bun build --bytecode immutable/dist/immutable.es.js": {
-          "js": "c9a2ba9f6b6a662e6bdfd44128bc66284276f5eac2a8875adb4578472328dc9f",
+          "js": "82ad84644b8be6b98cac3186bb04d17307d8d43fadd9b84b0f4067e6b29c9de4",
           "jsc": {
-            "bytes": 280016,
-            "sha256": "2a99c33a2516e2d41187bc322ddd4523318530b51c60c3d9a8bd566d4f2929a1",
+            "bytes": 280384,
+            "sha256": "02548420a1064d59943269c7148768cdb43ede4ae88cc86d52ae5fd3d4e8db73",
           },
         },
         "bun build --bytecode libraries.js": {
-          "js": "f9d0c270d3f41997b904689435104703b1f8b6b1c67ead48dff30e7354890cfd",
+          "js": "19ee3fe5c7baeeb09d00b53b8c7e24f62244e4987ea28ace8480d0ffffa5309a",
           "jsc": {
-            "bytes": 23806376,
-            "sha256": "1e2e31a113d8b5ca3e9cd73820d465e678756c0942023480569f785b15987dcf",
+            "bytes": 23789512,
+            "sha256": "359e88ead1203685ffe7ba5e858e9a7870919f14fb10b0039af27490a99a09ef",
           },
         },
         "bun build --bytecode lodash/lodash.js": {

test/bundler/bundler_edgecase.test.ts

@@ -2631,6 +2631,163 @@ describe("bundler", () => {
       stdout: "",
     },
   });
+  // https://github.com/oven-sh/bun/issues/14588
+  // A function parameter must not be collision-renamed into the name of a
+  // hoisted top-level function that is declared later in the same file.
+  itBundled("identifiers/NestedParamDoesNotShadowLaterHoistedFunction", {
+    files: {
+      "/dep.js": `
+        export var e = "outer_e";
+        export var e2 = "outer_e2";
+      `,
+      "/entry.js": `
+        import { e as _e, e2 as _e2 } from "./dep.js";
+
+        function ZI(t, e, n) {
+          return e3(t, e, n);
+        }
+
+        function e3(a, b, c) {
+          return "range:" + b;
+        }
+
+        console.log(ZI(1, 2, 3), _e, _e2);
+      `,
+    },
+    entryPoints: ["/entry.js"],
+    minifyIdentifiers: false,
+    run: { stdout: "range:2 outer_e outer_e2" },
+    onAfterBundle(api) {
+      expect(api.readFile("/out.js")).not.toContain("function ZI(t, e3, n)");
+    },
+  });
+  // https://github.com/oven-sh/bun/issues/14588
+  // Same bug for a module wrapped in `__esm` via dynamic `import()`: the
+  // module's own top-level declarations are hoisted outside the closure, so a
+  // parameter in that module must not be renamed into one of them.
+  itBundled("identifiers/NestedParamDoesNotShadowLaterHoistedFunctionInEsmWrap", {
+    files: {
+      "/names.js": `
+        export var e = "E";
+        export var e2 = "E2";
+      `,
+      "/lib.js": `
+        var sideEffect = Date.now();
+        export function ZI(t, e, n) {
+          return e3(t, e, n);
+        }
+        function e3(a, b, c) {
+          return "range:" + b;
+        }
+      `,
+      "/entry.js": `
+        import { e as _e, e2 as _e2 } from "./names.js";
+        const mod = await import("./lib.js");
+        console.log(mod.ZI(1, 2, 3), _e, _e2);
+      `,
+    },
+    entryPoints: ["/entry.js"],
+    target: "bun",
+    minifyIdentifiers: false,
+    run: { stdout: "range:2 E E2" },
+    onAfterBundle(api) {
+      const out = api.readFile("/out.js");
+      expect(out).toContain("__esm");
+      expect(out).not.toMatch(/function ZI\(\w+, e3,/);
+    },
+  });
+  // https://github.com/oven-sh/bun/issues/30269
+  // Same bug for a nested `let` binding instead of a function parameter.
+  itBundled("identifiers/NestedLocalDoesNotShadowLaterHoistedFunction", {
+    files: {
+      "/conflict.js": `
+        export function r() { return "top-level r"; }
+      `,
+      "/module.js": `
+        class Expression {}
+
+        export function run() {
+          const result = typecheck({ left: new Expression(), op: {}, right: {} });
+          if (result !== true) throw new Error("expected true, got " + result);
+          return result;
+        }
+
+        function typecheck(node) {
+          let r, t, c;
+          block: {
+            r = node.left;
+            t = node.op;
+            c = node.right;
+            break block;
+          }
+          return r2().bo4(r, t, c).a();
+        }
+
+        function r2() {
+          return { bo4() { return { a() { return true; } }; } };
+        }
+      `,
+      "/entry.js": `
+        import * as conflict from "./conflict.js";
+        import { run } from "./module.js";
+        conflict.r();
+        console.log("ok:" + run());
+      `,
+    },
+    entryPoints: ["/entry.js"],
+    minifyIdentifiers: false,
+    run: { stdout: "ok:true" },
+  });
+  // https://github.com/oven-sh/bun/issues/41054
+  // Same bug in a single file: the local `n` collides with the top-level `n`,
+  // so the renamer numbers it. The numbered name must not collide with `n2`,
+  // a top-level symbol declared in a later part and called from the same
+  // function. The broken output renamed the local to `n2`, which shadowed the
+  // `n2` function.
+  itBundled("identifiers/NestedLocalDoesNotShadowLaterTopLevelSymbol", {
+    files: {
+      "/entry.js": /* js */ `
+        const n = 1;
+        export function f() {
+          let n = 2;
+          return n2(n);
+        }
+        function n2(x) {
+          return x + n + 40;
+        }
+        console.log(f());
+      `,
+    },
+    target: "bun",
+    minifyIdentifiers: false,
+    run: { stdout: "43" },
+  });
+  // Same bug for a module scope deferred as one nested scope because the
+  // module is wrapped in a CommonJS closure: a local inside the closure must
+  // not be renamed into the name of another module's wrapper (`require_*`),
+  // which is registered as a top-level symbol only when its own file is
+  // reached. The circular require makes a.js's closure call entry.js's
+  // wrapper, which is registered later.
+  itBundled("identifiers/CjsClosureLocalDoesNotShadowLaterWrapper", {
+    files: {
+      "/entry.js": /* js */ `
+        module.exports.val = 2;
+        const a = require("./a.js");
+        console.log(a.f());
+      `,
+      "/a.js": /* js */ `
+        module.exports.f = function f() {
+          let require_entry = 40;
+          const e = require("./entry.js");
+          return e.val + require_entry + 1;
+        };
+      `,
+    },
+    entryPoints: ["/entry.js"],
+    target: "bun",
+    minifyIdentifiers: false,
+    run: { stdout: "43" },
+  });
   itBundled("edgecase/MacroProtoKeyIsOwnProperty", {
     files: {
       "/entry.ts": /* js */ `