Turbopack: don't scope hoist modules using direct eval

@@ -48,6 +48,11 @@ pub struct VarGraph<'a> {     /// Present when the module is a statically-analyzable CommonJS module (no     /// dynamic exports); carries its named exports for scope hoisting.     pub cjs_static_exports: Option<CjsStaticExports>,++    /// `true` when the module contains a direct `eval(...)` call (a call to the unshadowed+    /// global `eval`). A direct eval can access bindings in its lexical scope by name, so the+    /// module must not be merged with others (merging may rename those bindings).+    pub has_direct_eval: bool, }  impl<'a> VarGraph<'a> {@@ -82,6 +87,7 @@ pub fn create_graph<'a>(             ast_paths: Default::default(),             require_usage: Default::default(),             cjs_static_exports: Default::default(),+            has_direct_eval: false,         },         eval_context,         state: Default::default(),

@@ -1717,6 +1717,16 @@ impl VisitAstPath for Analyzer<'_, '_> {         n: &'ast CallExpr,         ast_path: &mut AstNodePath<AstParentNodeRef<'r>>,     ) {+        // A direct eval (`eval(…)` or `(eval)(…)` calling the global `eval`) can access+        // bindings in its lexical scope by name. Only parentheses are stripped: `(0, eval)(…)`+        // is an indirect eval.+        if let Callee::Expr(callee) = &n.callee+            && let Expr::Ident(ident) = callee.unwrap_parens()+            && is_global(ident, "eval", self.eval_context.unresolved_mark)+        {+            self.data.has_direct_eval = true;+        }+         // `Object.defineProperty(exports, …)` is a CommonJS export write; recognize         // it so unused entries drop (its `exports` argument is guarded below).         let is_cjs_define_property = if self.cjs_exports_enabled()

@@ -268,7 +268,8 @@ mod tests {     use rustc_hash::FxHashMap;     use swc_core::{         common::{-            FilePathMapping, GLOBALS, Globals, Mark, SourceMap, comments::SingleThreadedComments,+            FileName, FilePathMapping, GLOBALS, Globals, Mark, SourceMap,+            comments::SingleThreadedComments,         },         ecma::{             ast::{EsVersion, Id},@@ -778,6 +779,76 @@ mod tests {         Ok(())     } +    /// Builds the var graph for `src` (parsed as a sloppy script unless it uses module syntax)+    /// and returns whether a direct `eval` was detected.+    fn detect_direct_eval(src: &str) -> bool {+        let cm: Arc<SourceMap> = Arc::new(SourceMap::new(FilePathMapping::empty()));+        let globals = Globals::new();+        GLOBALS.set(&globals, || {+            let fm = cm.new_source_file(FileName::Anon.into(), src.to_string());+            let mut m = parse_file_as_program(+                &fm,+                Default::default(),+                EsVersion::latest(),+                None,+                &mut vec![],+            )+            .unwrap();+            let unresolved_mark = Mark::new();+            let top_level_mark = Mark::new();+            m.visit_mut_with(&mut resolver(unresolved_mark, top_level_mark, false));+            let eval_context = EvalContext::new(+                Some(&m),+                unresolved_mark,+                top_level_mark,+                Default::default(),+                None,+            );+            let arena = Bump::new();+            create_graph(+                &arena,+                &m,+                &eval_context,+                AnalyzeMode::code_generation(),+                true,+                SpecifiedModuleType::Automatic,+                false,+                false,+            )+            .has_direct_eval+        })+    }++    #[test]+    fn direct_eval_detection() {+        for src in [+            "eval('x')",+            "(eval)('x')",+            "((eval))('x')",+            "function f() { return eval('x') }",+            "(() => { eval('x') })()",+            "class C { m() { eval('x') } }",+            "if (cond) { eval('x') }",+            "export const v = eval('x')",+        ] {+            assert!(detect_direct_eval(src), "expected direct eval: {src}");+        }++        for src in [+            "(0, eval)('x')",+            "globalThis.eval('x')",+            "window.eval('x')",+            "eval?.('x')",+            "const e = eval; e('x')",+            "new eval('x')",+            "function f(eval) { return eval('x') }",+            "var eval = (x) => x; eval('x')",+            "export const v = 1",+        ] {+            assert!(!detect_direct_eval(src), "expected no direct eval: {src}");+        }+    }+     async fn resolve<'a>(         arena: &'a ThreadLocal<Bump>,         var_graph: &VarGraph<'a>,

@@ -998,14 +998,16 @@ impl EcmascriptChunkPlaceable for EcmascriptModuleAsset { impl MergeableModule for EcmascriptModuleAsset {     #[turbo_tasks::function]     async fn is_mergeable(self: ResolvedVc<Self>) -> Result<Vc<bool>> {-        if matches!(+        if !matches!(             &*self.get_exports().await?,             EcmascriptExports::EsmExports(_)         ) {-            return Ok(Vc::cell(true));+            return Ok(Vc::cell(false));         } -        Ok(Vc::cell(false))+        // A direct `eval` can access module bindings by name, which breaks when merging+        // renames them.+        Ok(Vc::cell(!self.analyze().await?.has_direct_eval))     }      #[turbo_tasks::function]

@@ -172,6 +172,9 @@ pub struct AnalyzeEcmascriptModuleResult {     /// Present when the module is a statically-analyzable CommonJS module;     /// carries its named exports for scope hoisting.     pub cjs_static_exports: Option<CjsStaticExports>,+    /// `true` when the module contains a direct `eval(...)` call, which can access bindings in+    /// its lexical scope by name. Such a module must not be merged with others.+    pub has_direct_eval: bool,      pub env_var_info: ResolvedVc<EnvVarInfo>, }@@ -238,6 +241,7 @@ struct AnalyzeEcmascriptModuleResultBuilder {     successful: bool,     source_map: Option<ResolvedVc<Box<dyn GenerateSourceMap>>>,     cjs_static_exports: Option<CjsStaticExports>,+    has_direct_eval: bool,      env_var_info_runtime: FxIndexMap<RcStr, EnvVarAccessMode>, @@ -262,6 +266,7 @@ impl AnalyzeEcmascriptModuleResultBuilder {             successful: false,             source_map: None,             cjs_static_exports: None,+            has_direct_eval: false,             env_var_info_runtime: Default::default(),             #[cfg(debug_assertions)]             ident: Default::default(),@@ -500,6 +505,7 @@ impl AnalyzeEcmascriptModuleResultBuilder {                 successful: self.successful,                 source_map: self.source_map,                 cjs_static_exports: self.cjs_static_exports,+                has_direct_eval: self.has_direct_eval,                 env_var_info: EnvVarInfo {                     runtime: self.env_var_info_runtime,                 }@@ -901,6 +907,7 @@ async fn analyze_ecmascript_module_internal(         let require_binding_usage = take(&mut var_graph.require_usage);         // The module's static CommonJS exports, if any, for scope hoisting.         analysis.cjs_static_exports = take(&mut var_graph.cjs_static_exports);+        analysis.has_direct_eval = var_graph.has_direct_eval;         let compile_time_info_ref = compile_time_info.await?;          let mut analysis_state = AnalysisState {

@@ -231,6 +231,13 @@ impl ChunkableModule for EcmascriptModuleLocalsModule {  #[turbo_tasks::value_impl] impl MergeableModule for EcmascriptModuleLocalsModule {+    /// The locals module contains the code of the original module, so it is mergeable exactly+    /// when the original module is (e.g. not when it contains a direct `eval`).+    #[turbo_tasks::function]+    fn is_mergeable(&self) -> Vc<bool> {+        self.module.is_mergeable()+    }+     #[turbo_tasks::function]     async fn merge(         &self,

@@ -0,0 +1,12 @@+// `local` collides with bindings in other modules, so merging would rename it,+// breaking the direct evals below that look it up by name.+const local = 'direct'++export function readLocal() {+  return eval('local')+}++export function readNested() {+  // prettier-ignore+  return (() => (eval)('local'))()+}

@@ -0,0 +1,13 @@+// `other.js` is imported (and executed) first so that, if `direct.js` were merged,+// its colliding `local` would be the one that gets renamed.+import { otherLocal } from './other.js'+import { readLocal, readNested } from './direct.js'++const local = 'index'++it('should not merge modules using direct eval', () => {+  expect(readLocal()).toBe('direct')+  expect(readNested()).toBe('direct')+  expect(otherLocal).toBe('other')+  expect(local).toBe('index')+})

@@ -0,0 +1,3 @@+const local = 'other'++export const otherLocal = local

@@ -0,0 +1,10 @@+const local = 'direct'++function nested() {+  // A direct eval can reference module bindings by name, so this module must+  // not be merged (merging may rename `local`).+  // prettier-ignore+  return (eval)('local')+}++export const direct = eval('local') + nested()

@@ -0,0 +1,6 @@+import { direct } from './direct.js'+import { indirect } from './indirect.js'++const local = 'index'++console.log(direct, indirect, local)

@@ -0,0 +1,9 @@+const local = 'indirect'++// None of these are direct evals, so this module can still be merged.+export const indirect = [+  (0, eval)('1'),+  globalThis.eval('2'),+  eval?.('3'),+  local,+]

@@ -0,0 +1,3 @@+{+  "scopeHoisting": true+}

@@ -0,0 +1,41 @@+(()=>{"use strict";(globalThis["TURBOPACK"] || (globalThis["TURBOPACK"] = [])).push(["output/0_9x_turbopack-tests_tests_snapshot_scope-hoisting_direct-eval_input_1oxos4-8yb8iv._.js",+"[project]/turbopack/crates/turbopack-tests/tests/snapshot/scope-hoisting/direct-eval/input/direct.js [test] (ecmascript)", ((__turbopack_context__) => {+"use strict";++__turbopack_context__.s([+    "direct",+    ()=>direct+]);+const local = 'direct';+function nested() {+    // A direct eval can reference module bindings by name, so this module must+    // not be merged (merging may rename `local`).+    // prettier-ignore+    return eval('local');+}+const direct = eval('local') + nested();+}),+"[project]/turbopack/crates/turbopack-tests/tests/snapshot/scope-hoisting/direct-eval/input/index.js [test] (ecmascript)", ((__turbopack_context__) => {+"use strict";++__turbopack_context__.s([], "[project]/turbopack/crates/turbopack-tests/tests/snapshot/scope-hoisting/direct-eval/input/index.js [test] (ecmascript)");+// MERGED MODULE: [project]/turbopack/crates/turbopack-tests/tests/snapshot/scope-hoisting/direct-eval/input/index.js [test] (ecmascript)+;+var __TURBOPACK__imported__module__$5b$project$5d2f$turbopack$2f$crates$2f$turbopack$2d$tests$2f$tests$2f$snapshot$2f$scope$2d$hoisting$2f$direct$2d$eval$2f$input$2f$direct$2e$js__$5b$test$5d$__$28$ecmascript$29$__ = __turbopack_context__.i("[project]/turbopack/crates/turbopack-tests/tests/snapshot/scope-hoisting/direct-eval/input/direct.js [test] (ecmascript)");+// MERGED MODULE: [project]/turbopack/crates/turbopack-tests/tests/snapshot/scope-hoisting/direct-eval/input/indirect.js [test] (ecmascript)+;+const local = 'indirect';+const indirect = [+    (0, eval)('1'),+    globalThis.eval('2'),+    eval?.('3'),+    local+];+;+;+const local1 = 'index';+console.log(__TURBOPACK__imported__module__$5b$project$5d2f$turbopack$2f$crates$2f$turbopack$2d$tests$2f$tests$2f$snapshot$2f$scope$2d$hoisting$2f$direct$2d$eval$2f$input$2f$direct$2e$js__$5b$test$5d$__$28$ecmascript$29$__["direct"], indirect, local1);+}),+]);})()++//# sourceMappingURL=0_9x_turbopack-tests_tests_snapshot_scope-hoisting_direct-eval_input_1oxos4-8yb8iv._.js.map\ No newline at end of file

@@ -0,0 +1,7 @@+{+  "version": 3,+  "sources": [],+  "sections": [+    {"offset": {"line": 4, "column": 0}, "map": {"version":3,"sources":["turbopack:///[project]/turbopack/crates/turbopack-tests/tests/snapshot/scope-hoisting/direct-eval/input/direct.js"],"sourcesContent":["const local = 'direct'\n\nfunction nested() {\n  // A direct eval can reference module bindings by name, so this module must\n  // not be merged (merging may rename `local`).\n  // prettier-ignore\n  return (eval)('local')\n}\n\nexport const direct = eval('local') + nested()\n"],"names":["local","nested","eval","direct"],"mappings":";;;;AAAA,MAAMA,QAAQ;AAEd,SAASC;IACP,2EAA2E;IAC3E,8CAA8C;IAC9C,kBAAkB;IAClB,OAAO,AAACC,KAAM;AAChB;AAEO,MAAMC,SAASD,KAAK,WAAWD"}},+    {"offset": {"line": 20, "column": 0}, "map": {"version":3,"sources":["turbopack:///[project]/turbopack/crates/turbopack-tests/tests/snapshot/scope-hoisting/direct-eval/input/index.js","turbopack:///[project]/turbopack/crates/turbopack-tests/tests/snapshot/scope-hoisting/direct-eval/input/indirect.js"],"sourcesContent":["import { direct } from './direct.js'\nimport { indirect } from './indirect.js'\n\nconst local = 'index'\n\nconsole.log(direct, indirect, local)\n","const local = 'indirect'\n\n// None of these are direct evals, so this module can still be merged.\nexport const indirect = [\n  (0, eval)('1'),\n  globalThis.eval('2'),\n  eval?.('3'),\n  local,\n]\n"],"names":["local","indirect","eval","globalThis"],"mappings":";;;AAAA;;;ACAA,MAAMA,QAAQ;AAGP,MAAMC,WAAW;IACtB,CAAC,GAAGC,IAAI,EAAE;IACVC,WAAWD,IAAI,CAAC;IAChBA,OAAO;IACPF;CACD;;;ADLD,MAAM,SAAQ;AAEd,QAAQC,GAAG,CAAC,4NAAM,EAAE,UAAU"}}]+}\ No newline at end of file

@@ -0,0 +1,5 @@+(globalThis["TURBOPACK"] || (globalThis["TURBOPACK"] = [])).push([+    "output/1ece_tests_snapshot_scope-hoisting_direct-eval_input_index_0zwjp61t_1bte.js",+    {"otherChunks":["output/0_9x_turbopack-tests_tests_snapshot_scope-hoisting_direct-eval_input_1oxos4-8yb8iv._.js"],"runtimeModuleIds":["[project]/turbopack/crates/turbopack-tests/tests/snapshot/scope-hoisting/direct-eval/input/index.js [test] (ecmascript)"]}+]);+// Dummy runtime\ No newline at end of file

@@ -0,0 +1,5 @@+{+  "version": 3,+  "sources": [],+  "sections": []+}\ No newline at end of file