fix(es/minifier): avoid preserved name collisions

crates/swc_ecma_minifier/src/pass/mangle_names/preserver.rs

@@ -22,6 +22,7 @@ where
         preserved: Default::default(),
         should_preserve: false,
         in_top_level: false,
+        preserve_top_level_vars: false,
 
         idents: Vec::new(),
         unresolved_ctx: SyntaxContext::empty().apply_mark(marks.unresolved_mark),
@@ -64,6 +65,7 @@ pub(crate) struct Preserver<'a> {
 
     should_preserve: bool,
     in_top_level: bool,
+    preserve_top_level_vars: bool,
 
     idents: Vec<Id>,
     unresolved_ctx: SyntaxContext,
@@ -92,6 +94,30 @@ impl Preserver<'_> {
         }
         self.in_top_level = old_top_level;
     }
+
+    fn visit_non_top_level<N>(&mut self, node: &N)
+    where
+        N: VisitWith<Self>,
+    {
+        let old_top_level = self.in_top_level;
+        self.in_top_level = false;
+        node.visit_with(self);
+        self.in_top_level = old_top_level;
+    }
+
+    /// Visits a loop body while preserving `var` declarations hoisted to the
+    /// surrounding top-level scope.
+    fn visit_loop_body(&mut self, body: &Stmt) {
+        let old_top_level = self.in_top_level;
+        let old_preserve_top_level_vars = self.preserve_top_level_vars;
+
+        self.in_top_level = false;
+        self.preserve_top_level_vars |= old_top_level;
+        body.visit_with(self);
+
+        self.preserve_top_level_vars = old_preserve_top_level_vars;
+        self.in_top_level = old_top_level;
+    }
 }
 
 impl Visit for Preserver<'_> {
@@ -104,7 +130,10 @@ impl Visit for Preserver<'_> {
     }
 
     fn visit_function_body(&mut self, n: &FunctionBody) {
+        let old_preserve_top_level_vars = self.preserve_top_level_vars;
+        self.preserve_top_level_vars = false;
         self.visit_non_top_level_stmts(&n.stmts);
+        self.preserve_top_level_vars = old_preserve_top_level_vars;
     }
 
     fn visit_catch_clause(&mut self, n: &CatchClause) {
@@ -189,6 +218,36 @@ impl Visit for Preserver<'_> {
         }
     }
 
+    fn visit_for_in_stmt(&mut self, n: &ForInStmt) {
+        match &n.left {
+            ForHead::VarDecl(var) if var.kind == VarDeclKind::Var => var.visit_with(self),
+            left => self.visit_non_top_level(left),
+        }
+        n.right.visit_with(self);
+        self.visit_loop_body(&n.body);
+    }
+
+    fn visit_for_of_stmt(&mut self, n: &ForOfStmt) {
+        match &n.left {
+            ForHead::VarDecl(var) if var.kind == VarDeclKind::Var => var.visit_with(self),
+            left => self.visit_non_top_level(left),
+        }
+        n.right.visit_with(self);
+        self.visit_loop_body(&n.body);
+    }
+
+    fn visit_for_stmt(&mut self, n: &ForStmt) {
+        match &n.init {
+            Some(VarDeclOrExpr::VarDecl(var)) if var.kind == VarDeclKind::Var => {
+                var.visit_with(self)
+            }
+            init => self.visit_non_top_level(init),
+        }
+        n.test.visit_with(self);
+        n.update.visit_with(self);
+        self.visit_loop_body(&n.body);
+    }
+
     fn visit_ident(&mut self, i: &Ident) {
         self.append_ident(i);
     }
@@ -242,4 +301,16 @@ impl Visit for Preserver<'_> {
             }
         }
     }
+
+    fn visit_var_decl(&mut self, n: &VarDecl) {
+        if n.kind == VarDeclKind::Var && self.preserve_top_level_vars {
+            let old_top_level = self.in_top_level;
+            self.in_top_level = true;
+            n.visit_children_with(self);
+            self.in_top_level = old_top_level;
+            return;
+        }
+
+        n.visit_children_with(self);
+    }
 }

crates/swc_ecma_minifier/tests/fixture/issues/12155/config.json

@@ -0,0 +1,4 @@
+{
+    "defaults": true,
+    "toplevel": false
+}

crates/swc_ecma_minifier/tests/fixture/issues/12155/expected.stdout

@@ -0,0 +1 @@
+undefined

crates/swc_ecma_minifier/tests/fixture/issues/12155/input.js

@@ -0,0 +1,3 @@
+let a;
+console.log(a);
+for (const a of [1]);

crates/swc_ecma_minifier/tests/fixture/issues/12155/mangle.json

@@ -0,0 +1,3 @@
+{
+    "toplevel": false
+}

crates/swc_ecma_minifier/tests/fixture/issues/12155/output.js

@@ -0,0 +1,4 @@
+let a;
+for (const o of (console.log(a), [
+    1
+]));

crates/swc_ecma_minifier/tests/fixture/issues/12155/unbraced-loop-body/config.json

@@ -0,0 +1,4 @@
+{
+    "defaults": false,
+    "toplevel": false
+}

crates/swc_ecma_minifier/tests/fixture/issues/12155/unbraced-loop-body/input.js

@@ -0,0 +1,6 @@
+for (key in {}) var forInBody;
+for (value of []) var forOfBody;
+for (; false;) var classicForBody;
+for (; false;) if (flag) var conditionalBody;
+for (; false;) label: var labeledBody;
+for (; false;) for (; false;) var nestedLoopBody;

crates/swc_ecma_minifier/tests/fixture/issues/12155/unbraced-loop-body/mangle.json

@@ -0,0 +1,3 @@
+{
+    "toplevel": false
+}

crates/swc_ecma_minifier/tests/fixture/issues/12155/unbraced-loop-body/output.js

@@ -0,0 +1,6 @@
+for(key in {})var forInBody;
+for (value of [])var forOfBody;
+for(; false;)var classicForBody;
+for(; false;)if (flag) var conditionalBody;
+for(; false;)o: var labeledBody;
+for(; false;)for(; false;)var nestedLoopBody;

crates/swc_ecma_transforms_base/src/hygiene/tests.rs

@@ -357,6 +357,63 @@ fn mangling_respects_non_finite_output_symbols() {
     });
 }
 
+#[test]
+fn mangling_respects_preserved_names_in_relevant_scopes() {
+    struct PreservedNameMangler;
+
+    impl Renamer for PreservedNameMangler {
+        type Target = Atom;
+
+        const MANGLE: bool = true;
+        const RESET_N: bool = false;
+
+        fn new_name_for(&self, _orig: &Id, n: &mut usize) -> Atom {
+            let symbol = if *n == 0 { "kept" } else { "safe" };
+            *n += 1;
+            symbol.into()
+        }
+
+        fn preserve_name(&self, orig: &Id) -> bool {
+            orig.0 == "kept"
+        }
+    }
+
+    crate::tests::Tester::run(|tester| {
+        let kept_mark = Mark::fresh(Mark::root());
+        let used_mark = Mark::fresh(Mark::root());
+        let child_mark = Mark::fresh(Mark::root());
+        let unrelated_mark = Mark::fresh(Mark::root());
+        let mut program = Program::Module(
+            tester
+                .parse_module(
+                    "actual.js",
+                    "let used; let kept; console.log(used, kept); { let child; console.log(kept, \
+                     child); } { let unrelated; console.log(unrelated); }",
+                )?
+                .fold_with(&mut marker(&[
+                    ("kept", kept_mark),
+                    ("used", used_mark),
+                    ("child", child_mark),
+                    ("unrelated", unrelated_mark),
+                ])),
+        );
+
+        program.visit_mut_with(&mut renamer(Default::default(), PreservedNameMangler));
+
+        let actual = tester.print(&program);
+        let expected = tester.parse_module(
+            "expected.js",
+            "let safe; let kept; console.log(safe, kept); { let safe; console.log(kept, safe); } \
+             { let kept; console.log(kept); }",
+        )?;
+        let expected = tester.print(&Program::Module(expected));
+
+        assert_eq!(DebugUsingDisplay(&actual), DebugUsingDisplay(&expected));
+
+        Ok(())
+    });
+}
+
 #[test]
 fn block_scoping_with_usage() {
     test(

crates/swc_ecma_transforms_base/src/rename/analyzer/scope.rs

@@ -153,13 +153,14 @@ impl Scope {
         let mut latest_n = FxHashMap::default();
         let mut n = 0;
 
+        self.record_preserved_names(renamer, to, previous, reverse, &queue, preserved);
+
         for id in queue {
-            if renamer.preserve_name(&id)
-                || preserved.contains(&id)
-                || to.get(&id).is_some()
-                || previous.get(&id).is_some()
-                || id.0 == "eval"
-            {
+            if to.get(&id).is_some() || previous.get(&id).is_some() || id.0 == "eval" {
+                continue;
+            }
+
+            if renamer.preserve_name(&id) || preserved.contains(&id) {
                 continue;
             }
 
@@ -304,13 +305,14 @@ impl Scope {
     {
         let mut n = 0;
 
+        self.record_preserved_names(renamer, to, previous, reverse, &queue, preserved);
+
         for id in queue {
-            if renamer.preserve_name(&id)
-                || preserved.contains(&id)
-                || to.get(&id).is_some()
-                || previous.get(&id).is_some()
-                || id.0 == "eval"
-            {
+            if to.get(&id).is_some() || previous.get(&id).is_some() || id.0 == "eval" {
+                continue;
+            }
+
+            if renamer.preserve_name(&id) || preserved.contains(&id) {
                 continue;
             }
 
@@ -346,4 +348,32 @@ impl Scope {
         let children = &self.children;
         self.data.queue.len() + children.iter().map(|v| v.rename_cost()).sum::<usize>()
     }
+
+    /// Records the outputs of bindings which intentionally keep their names.
+    ///
+    /// These names must be recorded before assigning any names in the scope,
+    /// and child scopes inherit the resulting reverse map. Otherwise a renamed
+    /// binding can capture references to a preserved binding.
+    fn record_preserved_names<R, V>(
+        &self,
+        renamer: &R,
+        to: &FxHashMap<Id, V>,
+        previous: &FxHashMap<Id, V>,
+        reverse: &mut ReverseMap,
+        queue: &FxIndexSet<Id>,
+        preserved: &FxHashSet<Id>,
+    ) where
+        R: Renamer,
+        V: RenamedVariable,
+    {
+        for id in queue {
+            if to.get(id).is_some() || previous.get(id).is_some() || id.0 == "eval" {
+                continue;
+            }
+
+            if renamer.preserve_name(id) || preserved.contains(id) {
+                reverse.push_entry(id.0.clone(), id.clone());
+            }
+        }
+    }
 }