fix(es/minifier): Fix handling of eval (#4273)

crates/swc_ecma_minifier/README.md

@@ -6,6 +6,11 @@ EcmaScript minifier for the SWC project. This is basically a port of terser.
 
 Currently name mangler is very simple. To focus on creating a MVP, I (kdy1) will use simple logic for name mangler and implement the content-aware name mangler after publishing first non-beta version.
 
+## Debugging tips
+
+If the output contains variables named `e`, `t`, `n` it typically means the original library is published in a minified form and the input contains `eval`.
+The current swc name mangler does not do anything if `eval` is used.
+
 ## Testing
 
 ### Real-library tests

crates/swc_ecma_minifier/scripts/next/evaluate.js

@@ -8,9 +8,12 @@ const rl = readline.createInterface({
     terminal: false
 });
 
+function looseJsonParse(obj) {
+    return Function('"use strict";return (' + obj + ')')();
+}
 rl.on('line', async (data) => {
     try {
-        const { name, source } = eval(`(${data})`)
+        const { name, source } = looseJsonParse(`(${data})`)
         const targetPath = path.join(__dirname, '..', '..', 'tests', 'compress', 'fixture', 'next', 'raw', name.replace('.js', ''), 'input.js');
 
 

crates/swc_ecma_minifier/src/analyzer/mod.rs

@@ -235,9 +235,10 @@ where
         {
             let child_scope = child.data.scope(child_ctxt);
 
-            child_scope.merge(child.scope, false);
+            child_scope.merge(child.scope.clone(), false);
         }
 
+        self.scope.merge(child.scope, true);
         self.data.merge(kind, child.data);
 
         ret

crates/swc_ecma_minifier/src/compress/optimize/mod.rs

@@ -2841,107 +2841,111 @@ where
             true
         });
 
-        for v in vars.iter_mut() {
+        let uses_eval = self.data.scopes.get(&self.ctx.scope).unwrap().has_eval_call;
-            if v.init
-                .as_deref()
-                .map(|e| !e.is_ident() && !e.may_have_side_effects())
-                .unwrap_or(true)
-            {
-                self.drop_unused_var_declarator(v, &mut None);
-            }
-        }
 
-        let mut can_prepend = true;
+        if !uses_eval {
-        let mut side_effects = vec![];
+            for v in vars.iter_mut() {
-
+                if v.init
-        for v in vars.iter_mut() {
+                    .as_deref()
-            let mut storage = None;
+                    .map(|e| !e.is_ident() && !e.may_have_side_effects())
-            self.drop_unused_var_declarator(v, &mut storage);
+                    .unwrap_or(true)
-            side_effects.extend(storage);
+                {
-
+                    self.drop_unused_var_declarator(v, &mut None);
-            // Dropped. Side effects of the initializer is stored in `side_effects`.
+                }
-            if v.name.is_invalid() {
-                continue;
             }
 
-            // If initializer is none, we can check next item without thinking about side
+            let mut can_prepend = true;
-            // effects.
+            let mut side_effects = vec![];
-            if v.init.is_none() {
+
-                continue;
+            for v in vars.iter_mut() {
+                let mut storage = None;
+                self.drop_unused_var_declarator(v, &mut storage);
+                side_effects.extend(storage);
+
+                // Dropped. Side effects of the initializer is stored in `side_effects`.
+                if v.name.is_invalid() {
+                    continue;
+                }
+
+                // If initializer is none, we can check next item without thinking about side
+                // effects.
+                if v.init.is_none() {
+                    continue;
+                }
+
+                // We can drop the next variable, as we don't have to worry about the side
+                // effect.
+                if side_effects.is_empty() {
+                    can_prepend = false;
+                    continue;
+                }
+
+                // We now have to handle side effects.
+
+                if can_prepend {
+                    can_prepend = false;
+
+                    self.prepend_stmts.push(Stmt::Expr(ExprStmt {
+                        span: DUMMY_SP,
+                        expr: if side_effects.len() == 1 {
+                            side_effects.remove(0)
+                        } else {
+                            Box::new(Expr::Seq(SeqExpr {
+                                span: DUMMY_SP,
+                                exprs: side_effects.take(),
+                            }))
+                        },
+                    }));
+                } else {
+                    // We prepend side effects to the initializer.
+
+                    let seq = v.init.as_mut().unwrap().force_seq();
+                    seq.exprs = side_effects
+                        .drain(..)
+                        .into_iter()
+                        .chain(seq.exprs.take())
+                        .filter(|e| !e.is_invalid())
+                        .collect();
+                }
             }
 
-            // We can drop the next variable, as we don't have to worry about the side
+            // We append side effects.
-            // effect.
+            if !side_effects.is_empty() {
-            if side_effects.is_empty() {
+                self.append_stmts.push(Stmt::Expr(ExprStmt {
-                can_prepend = false;
-                continue;
-            }
-
-            // We now have to handle side effects.
-
-            if can_prepend {
-                can_prepend = false;
-
-                self.prepend_stmts.push(Stmt::Expr(ExprStmt {
                     span: DUMMY_SP,
                     expr: if side_effects.len() == 1 {
                         side_effects.remove(0)
                     } else {
                         Box::new(Expr::Seq(SeqExpr {
                             span: DUMMY_SP,
-                            exprs: side_effects.take(),
+                            exprs: side_effects,
                         }))
                     },
                 }));
-            } else {
-                // We prepend side effects to the initializer.
-
-                let seq = v.init.as_mut().unwrap().force_seq();
-                seq.exprs = side_effects
-                    .drain(..)
-                    .into_iter()
-                    .chain(seq.exprs.take())
-                    .filter(|e| !e.is_invalid())
-                    .collect();
-            }
-        }
-
-        // We append side effects.
-        if !side_effects.is_empty() {
-            self.append_stmts.push(Stmt::Expr(ExprStmt {
-                span: DUMMY_SP,
-                expr: if side_effects.len() == 1 {
-                    side_effects.remove(0)
-                } else {
-                    Box::new(Expr::Seq(SeqExpr {
-                        span: DUMMY_SP,
-                        exprs: side_effects,
-                    }))
-                },
-            }));
-        }
-
-        vars.retain_mut(|var| {
-            if var.name.is_invalid() {
-                self.changed = true;
-                return false;
             }
 
-            if let Some(Expr::Invalid(..)) = var.init.as_deref() {
+            vars.retain_mut(|var| {
-                if let Pat::Ident(i) = &var.name {
+                if var.name.is_invalid() {
-                    if let Some(usage) = self.data.vars.get(&i.id.to_id()) {
+                    self.changed = true;
-                        if usage.declared_as_catch_param {
+                    return false;
-                            var.init = None;
-                            return true;
-                        }
-                    }
                 }
 
-                return false;
+                if let Some(Expr::Invalid(..)) = var.init.as_deref() {
-            }
+                    if let Pat::Ident(i) = &var.name {
+                        if let Some(usage) = self.data.vars.get(&i.id.to_id()) {
+                            if usage.declared_as_catch_param {
+                                var.init = None;
+                                return true;
+                            }
+                        }
+                    }
 
-            true
+                    return false;
-        });
+                }
+
+                true
+            });
+        }
     }
 
     fn visit_mut_while_stmt(&mut self, n: &mut WhileStmt) {

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

@@ -1,8 +1,12 @@
-use swc_common::{SyntaxContext, DUMMY_SP};
+use rustc_hash::FxHashSet;
+use swc_atoms::{js_word, JsWord};
+use swc_common::{collections::AHashMap, SyntaxContext};
 use swc_ecma_ast::*;
 use swc_ecma_transforms_base::hygiene::rename;
-use swc_ecma_utils::UsageFinder;
+use swc_ecma_visit::{
-use swc_ecma_visit::{noop_visit_mut_type, VisitMut, VisitMutWith, VisitWith};
+    noop_visit_mut_type, noop_visit_type, visit_obj_and_computed, Visit, VisitMut, VisitMutWith,
+    VisitWith,
+};
 
 use super::{analyzer::Analyzer, preserver::idents_to_preserve};
 use crate::{marks::Marks, option::MangleOptions};
@@ -10,52 +14,67 @@ use crate::{marks::Marks, option::MangleOptions};
 pub(crate) fn name_mangler(
     options: MangleOptions,
     _marks: Marks,
-    top_level_ctxt: SyntaxContext,
+    _top_level_ctxt: SyntaxContext,
 ) -> impl VisitMut {
     Mangler {
         options,
-        top_level_ctxt,
+        preserved: Default::default(),
     }
 }
 
 struct Mangler {
     options: MangleOptions,
 
-    /// Used to check `eval`.
+    preserved: FxHashSet<Id>,
-    top_level_ctxt: SyntaxContext,
 }
 
 impl Mangler {
     fn contains_eval<N>(&self, node: &N) -> bool
     where
-        N: for<'aa> VisitWith<UsageFinder<'aa>>,
+        N: VisitWith<EvalFinder>,
     {
-        UsageFinder::find(
+        let mut v = EvalFinder { found: false };
-            &Ident::new("eval".into(), DUMMY_SP.with_ctxt(self.top_level_ctxt)),
+        node.visit_with(&mut v);
-            node,
+        v.found
-        )
+    }
+
+    fn get_map<N>(&self, node: &N) -> AHashMap<Id, JsWord>
+    where
+        N: VisitWith<Analyzer>,
+    {
+        let mut analyzer = Analyzer {
+            scope: Default::default(),
+            is_pat_decl: Default::default(),
+        };
+        node.visit_with(&mut analyzer);
+
+        analyzer.into_rename_map(&self.preserved)
     }
 }
 
 impl VisitMut for Mangler {
     noop_visit_mut_type!();
 
+    /// Only called if `eval` exists
+    fn visit_mut_fn_expr(&mut self, n: &mut FnExpr) {
+        if self.contains_eval(n) {
+            n.visit_mut_children_with(self);
+        } else {
+            let map = self.get_map(n);
+
+            n.visit_mut_with(&mut rename(&map));
+        }
+    }
+
     fn visit_mut_module(&mut self, m: &mut Module) {
+        self.preserved = idents_to_preserve(self.options.clone(), &*m);
+
         if self.contains_eval(m) {
+            m.visit_mut_children_with(self);
             return;
         }
 
-        let preserved = idents_to_preserve(self.options.clone(), &*m);
+        let map = self.get_map(m);
-
-        let map = {
-            let mut analyzer = Analyzer {
-                scope: Default::default(),
-                is_pat_decl: Default::default(),
-            };
-            m.visit_with(&mut analyzer);
-
-            analyzer.into_rename_map(&preserved)
-        };
 
         m.visit_mut_with(&mut rename(&map));
     }
@@ -65,18 +84,31 @@ impl VisitMut for Mangler {
             return;
         }
 
-        let preserved = idents_to_preserve(self.options.clone(), &*s);
+        self.preserved = idents_to_preserve(self.options.clone(), &*s);
 
-        let map = {
+        if self.contains_eval(s) {
-            let mut analyzer = Analyzer {
+            s.visit_mut_children_with(self);
-                scope: Default::default(),
+            return;
-                is_pat_decl: Default::default(),
+        }
-            };
-            s.visit_with(&mut analyzer);
 
-            analyzer.into_rename_map(&preserved)
+        let map = self.get_map(s);
-        };
 
         s.visit_mut_with(&mut rename(&map));
     }
 }
+
+struct EvalFinder {
+    found: bool,
+}
+
+impl Visit for EvalFinder {
+    noop_visit_type!();
+
+    visit_obj_and_computed!();
+
+    fn visit_ident(&mut self, i: &Ident) {
+        if i.sym == js_word!("eval") {
+            self.found = true;
+        }
+    }
+}

crates/swc_ecma_minifier/tests/exec.rs

@@ -9613,3 +9613,71 @@ fn internal_1() {
 
     run_exec_test(src, config, false);
 }
+
+#[test]
+fn direct_eval_1() {
+    let src = r###"
+    const obj = {
+        1: function () {
+            const foo = 1;
+            return {
+                test: function (s) {
+                    return eval(s)
+                }
+            }
+        },
+        2: function foo(mod1) {
+            console.log(mod1.test('foo'))
+        }
+    };
+    
+    
+    obj[2](obj[1]());
+    "###;
+
+    let config = r###"{
+        "defaults": true,
+        "toplevel": true
+    }"###;
+
+    run_exec_test(src, config, false);
+}
+
+#[test]
+fn indirect_eval_1() {
+    let src = r###"
+    const obj = {
+        1: function () {
+            const foo = 1;
+            return {
+                test: function (s) {
+                    const e = eval;
+                    return e(s)
+                }
+            }
+        },
+        2: function foo(mod1) {
+            let success = false;
+            try {
+                mod1.test('foo')
+            } catch (e) {
+                success = true;
+                console.log('PASS');
+            }
+            if (!success) {
+                throw new Error('indirect eval should not be direct eval');
+            }
+        }
+    };
+    
+    
+    obj[2](obj[1]());
+    "###;
+
+    let config = r###"{
+        "defaults": true,
+        "toplevel": true
+    }"###;
+
+    run_exec_test(src, config, false);
+}