js_parser: keep a dead for-in/for-of head with two or more names as written, and print a dead require() as null

src/js_parser/p.rs

@@ -1630,10 +1630,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
                 // We don't want to spend time scanning the required files if they will
                 // never be used.
                 if self.is_control_flow_dead {
-                    return Expr {
-                        data: null_expr_data(),
-                        loc: arg.loc,
-                    };
+                    return self.new_expr(E::Null {}, arg.loc);
                 }
 
                 str_.resolve_rope_if_needed(self.arena);

src/js_parser/scan/scan_side_effects.rs

@@ -570,6 +570,33 @@ impl SideEffects {
         }
     }
 
+    /// True at the second name that the pattern declares.
+    fn declares_second_name(binding: Binding, has_first: &mut bool) -> bool {
+        match binding.data {
+            bun_ast::binding::Data::BIdentifier(_) => core::mem::replace(has_first, true),
+            bun_ast::binding::Data::BArray(array) => (array.items.slice().iter())
+                .any(|item| Self::declares_second_name(item.binding, has_first)),
+            bun_ast::binding::Data::BObject(obj) => (obj.properties.slice().iter())
+                .any(|item| Self::declares_second_name(item.value, has_first)),
+            _ => false,
+        }
+    }
+
+    /// A for-in/for-of head takes one declaration: the `var a, b` of the `SLocal` arm is a
+    /// SyntaxError there, so a head that declares two or more names stays as it is written.
+    fn should_keep_for_in_or_of_head(init: Stmt, bump: &Bump) -> bool {
+        if let StmtData::SLocal(local) = init.data
+            && local.kind == bun_ast::S::Kind::KVar
+        {
+            let mut has_first = false;
+            let mut decls = local.decls.slice().iter();
+            if decls.any(|decl| Self::declares_second_name(decl.binding, &mut has_first)) {
+                return true;
+            }
+        }
+        Self::should_keep_stmt_in_dead_control_flow(init, bump)
+    }
+
     fn should_keep_stmts_in_dead_control_flow(stmts: bun_ast::StmtNodeList, bump: &Bump) -> bool {
         for child in stmts.slice() {
             if Self::should_keep_stmt_in_dead_control_flow(*child, bump) {
@@ -693,12 +720,12 @@ impl SideEffects {
             }
 
             StmtData::SForIn(for_) => {
-                Self::should_keep_stmt_in_dead_control_flow(for_.init, bump)
+                Self::should_keep_for_in_or_of_head(for_.init, bump)
                     || Self::should_keep_stmt_in_dead_control_flow(for_.body, bump)
             }
 
             StmtData::SForOf(for_) => {
-                Self::should_keep_stmt_in_dead_control_flow(for_.init, bump)
+                Self::should_keep_for_in_or_of_head(for_.init, bump)
                     || Self::should_keep_stmt_in_dead_control_flow(for_.body, bump)
             }
 

src/jsc/RuntimeTranspilerCache.rs

@@ -85,7 +85,10 @@ bun_core::declare_scope!(cache, visible);
 /// Version 50: The body of a class with lowered class decorators names the class by the
 /// binding that holds what the decorators return.
 /// Version 51: A `bun:` specifier that names no builtin is printed as it is written.
-const EXPECTED_VERSION: u32 = 51;
+/// Version 52: A `for-in` or `for-of` loop that never runs keeps a head that declares two or more
+/// names as it is written, and a `require("x")` that never runs is `null`. Older entries can hold
+/// a SyntaxError for both.
+const EXPECTED_VERSION: u32 = 52;
 
 /// Source files smaller than this are not written to / read from the on-disk
 /// transpiler cache. Originally 50 KiB, which excluded almost every file in a

test/bundler/esbuild/dce.test.ts

@@ -1390,6 +1390,37 @@ describe("bundler", () => {
     dce: true,
     minifySyntax: true,
   });
+  // Not from esbuild. With --minify-syntax esbuild 0.28.2 prints these heads as `for (var a, b of x)`,
+  // a SyntaxError: a loop head takes one declaration. Without the flag it does not trim dead code.
+  for (const minifySyntax of [false, true]) {
+    itBundled(`dce/DeadForInOrForOfHead${minifySyntax ? "MinifySyntax" : ""}`, {
+      files: {
+        "/entry.js": /* js */ `
+          function afterJump(x) {
+            return (a = 1) + (b = 2);
+            for (var [a, b] of x);
+          }
+          function inDeadBranch(x) {
+            if (false) {
+              for (var { a, b = require("./missing.js") } in require("./missing.js"));
+            }
+            return (a = 1) + (b = 2);
+          }
+          // Each function assigns its own "var" names, so they do not become globals.
+          console.log(afterJump([]), inDeadBranch({}), "a" in globalThis, "b" in globalThis);
+        `,
+      },
+      minifySyntax,
+      onAfterBundle(api) {
+        const file = api.readFile("/out.js");
+        expect(file).toContain("for (var [a, b] of x)");
+        expect(file).toContain("for (var { a, b = null } in null)");
+      },
+      run: {
+        stdout: "3 3 false false",
+      },
+    });
+  }
   itBundled("dce/RemoveTrailingReturn", {
     todo: true,
     files: {

test/bundler/transpiler/transpiler.test.js

@@ -3969,6 +3969,186 @@ console.log(resolve.length)
           "if (undefined) {\n  let y = Math.random();\n}",
         );
       });
+
+      // A dead loop stays when its head declares a `var` name. The head takes one declaration,
+      // so `for (var a, b of x)` is a SyntaxError.
+      // The default options trim an `if (false)`. `minify.syntax` also trims after a jump.
+      const expectDeadLoop = (loop, printed) => {
+        expectPrinted_(
+          `async function f(x) { if (false) { ${loop} ; } }`,
+          `async function f(x) {\n  if (false) {\n    ${printed}\n      ;\n  }\n}`,
+        );
+        expectPrintedMin_(
+          `async function f(x) { return; ${loop} ; }`,
+          `async function f(x) {\n  return;\n  ${printed}\n    ;\n}`,
+        );
+      };
+
+      it("keeps a dead for-in or for-of head that declares two or more names as it is written", () => {
+        for (const head of [
+          "var [a, b]",
+          "var { a, b }",
+          "var [, a, , b]",
+          "var [a, [b]]",
+          "var [[a], b]",
+          "var [[a, b]]",
+          "var { x: [a, b] }",
+          "var { x: { a }, y: [b] }",
+          "var [{ a }, {}, { b }]",
+          "var [a = 1, b = g()]",
+          "var { [k]: a, b }",
+          "var [a, ...b]",
+          "var { a, ...b }",
+          "var [a, a]",
+          "var [a, { b, c: [d] }]",
+        ]) {
+          expectDeadLoop(`for (${head} in x)`, `for (${head} in x)`);
+          expectDeadLoop(`for (${head} of x)`, `for (${head} of x)`);
+          expectDeadLoop(`for await (${head} of x)`, `for await (${head} of x)`);
+        }
+      });
+
+      it("keeps only the name of a dead for-in or for-of head that declares one name", () => {
+        for (const head of [
+          "var a",
+          "var [a]",
+          "var { a }",
+          "var [a = g()]",
+          "var { [k]: a }",
+          "var [, a]",
+          "var [a, {}]",
+          "var { x: [], a }",
+          "var { a, x: [] }",
+          "var [[a]]",
+          "var { x: { a } }",
+          "var [[], [{ a }]]",
+          "var [...a]",
+          "var { ...a }",
+        ]) {
+          expectDeadLoop(`for (${head} in x)`, "for (var a in x)");
+          expectDeadLoop(`for (${head} of x)`, "for (var a of x)");
+          expectDeadLoop(`for await (${head} of x)`, "for await (var a of x)");
+        }
+      });
+
+      it("keeps a head with two names where another dead statement holds the loop", () => {
+        const onOneLine = (code, transpiler_) => parsed(code, true, false, transpiler_).replaceAll(/\s+/g, " ");
+        for (const stmt of [
+          "label: for (var [a, b] of x) ;",
+          "while (x) for (var [a, b] of x) ;",
+          "do for (var [a, b] of x) ; while (x);",
+          "for (;; ) for (var [a, b] of x) ;",
+          "for (let k in x) for (var [a, b] of x) ;",
+          "for (let v of x) for (var [a, b] of x) ;",
+          "try { for (var [a, b] of x) ; } catch {}",
+          "try {} catch { for (var [a, b] of x) ; }",
+          "try {} finally { for (var [a, b] of x) ; }",
+          "if (x) for (var [a, b] of x) ;",
+          "if (x) ; else for (var [a, b] of x) ;",
+          "switch (x) { case 1: for (var [a, b] of x) ; }",
+        ]) {
+          const inDeadBranch = `function f(x) { if (false) { ${stmt} } }`;
+          expect(onOneLine(inDeadBranch, transpiler)).toBe(inDeadBranch);
+          for (const jump of ["return;", "throw x;"]) {
+            const afterJump = `function f(x) { ${jump} ${stmt} }`;
+            expect(onOneLine(afterJump, transpilerMinifySyntax)).toBe(afterJump);
+          }
+        }
+        for (const jump of ["break;", "continue;"]) {
+          const afterJump = `function f(x) { while (x) { ${jump} for (var [a, b] of x) ; } }`;
+          expect(onOneLine(afterJump, transpilerMinifySyntax)).toBe(afterJump);
+        }
+      });
+
+      it("removes a dead for-in or for-of loop that declares no `var` name", () => {
+        for (const head of [
+          "var []",
+          "var [, {}]",
+          "var { x: [] }",
+          "var [[], [{}]]",
+          "let [a, b]",
+          "const { a, b }",
+          "a",
+          "[a, b]",
+        ]) {
+          for (const loop of [`for (${head} in x)`, `for (${head} of x)`]) {
+            expectPrintedMin_(`function f(x) { return; ${loop} ; }`, "function f(x) {\n  return;\n}");
+            // The body keeps the loop, and the head stays as it is written.
+            expectPrintedMin_(
+              `function f(x) { return; ${loop} { var y = 1; } }`,
+              `function f(x) {\n  return;\n  ${loop}\n    var y;\n}`,
+            );
+          }
+        }
+        // The head of a `for (;;)` takes a list, so it still gets the names alone.
+        expectPrintedMin_(
+          "function f(x) { return; for (var [a, b] = x; a < b; a++) ; }",
+          "function f(x) {\n  return;\n  for (var a, b;a < b; a++)\n    ;\n}",
+        );
+      });
+
+      it.concurrent.each([
+        [
+          "a dead for-of loop after a return",
+          `function f(x) {
+            return (a = 1) + (b = 2);
+            for (var [a, b] of x);
+          }`,
+        ],
+        [
+          "a dead for-in loop in an if (false)",
+          `function f(x) {
+            if (false) {
+              for (var { a, b } in x);
+            }
+            return (a = 1) + (b = 2);
+          }`,
+        ],
+        [
+          "a dead require() call in the head of a dead loop",
+          `function f(x) {
+            if (false) {
+              for (var [a = require("./missing.js"), b] of require("./missing.js"));
+            }
+            return (a = 1) + (b = 2);
+          }`,
+        ],
+        [
+          "a dead require() call in another dead statement that stays",
+          `function f(x) {
+            if (false) {
+              while (require("./missing.js")) var a;
+              switch (require("./missing.js")) {
+                case require("./missing.js").y:
+                  var b;
+              }
+            }
+            return (a = 1) + (b = 2);
+          }`,
+        ],
+        [
+          // Only the name of the head stays, so the file loads as a module too.
+          "sloppy-only code in a dead head that declares one name",
+          `function f(x) {
+            if (false) {
+              for (var [a = delete q] of x);
+              for (var { [delete q]: b } in x);
+            }
+            return (a = 1) + (b = 2);
+          }`,
+        ],
+      ])("runs a file with %s", async (_, source) => {
+        using dir = tempDir("dead-for-in-of-head", {
+          // `f` assigns its own `var` names, so they do not become globals.
+          "entry.js": source + "\nconsole.log(f([]), typeof a, typeof b);",
+        });
+        expect(await bunRun(join(String(dir), "entry.js"))).toEqual({
+          stdout: "3 undefined undefined",
+          stderr: "",
+          exitCode: 0,
+          signalCode: null,
+        });
+      });
     });
   });