diff --git a/crates/oxc_mangler/src/lib.rs b/crates/oxc_mangler/src/lib.rs index b27b26c9840c0..81df3a8d7fd27 100644 --- a/crates/oxc_mangler/src/lib.rs +++ b/crates/oxc_mangler/src/lib.rs @@ -338,11 +338,41 @@ impl<'t> Mangler<'t> { // // TODO: This is conservative, ideally, we would reserve names per-slot. let mut eval_reserved_names: FxHashSet<&str> = FxHashSet::default(); + + // Annex B.3.2.1: In sloppy mode, function declarations inside block scopes create + // an implicit `var`-like assignment in the enclosing function scope at block exit. + // If such a function gets the same mangled name as an outer `var`, the Annex B + // assignment overwrites it. To prevent this, hoist these functions to the enclosing + // var scope for slot assignment — the same way `var` is hoisted by oxc_semantic. + let mut annex_b_hoisted: FxHashMap> = FxHashMap::default(); + let mut annex_b_skip = BitSet::new_in(scoping.symbols_len(), temp_allocator); + for (scope_id, bindings) in scoping.iter_bindings() { + let flags = scoping.scope_flags(scope_id); + if flags.is_var() || flags.is_strict_mode() { + continue; + } + for &symbol_id in bindings.values() { + if !scoping.symbol_flags(symbol_id).is_function() { + continue; + } + for ancestor_id in scoping.scope_ancestors(scope_id).skip(1) { + if scoping.scope_flags(ancestor_id).is_var() { + annex_b_hoisted + .entry(ancestor_id.index()) + .or_insert_with(|| Vec::new_in(temp_allocator)) + .push(symbol_id); + annex_b_skip.set_bit(symbol_id.index()); + break; + } + } + } + } + // Walk down the scope tree and assign a slot number for each symbol. // It is possible to do this in a loop over the symbol list, // but walking down the scope tree seems to generate a better code. for (scope_id, bindings) in scoping.iter_bindings() { - if bindings.is_empty() { + if bindings.is_empty() && !annex_b_hoisted.contains_key(&scope_id.index()) { continue; } // Scopes with direct eval: collect binding names as reserved (they can be @@ -351,16 +381,34 @@ impl<'t> Mangler<'t> { for (name, _) in bindings { eval_reserved_names.insert(name.as_str()); } + if let Some(hoisted) = annex_b_hoisted.get(&scope_id.index()) { + for &symbol_id in hoisted { + eval_reserved_names.insert(scoping.symbol_name(symbol_id)); + } + } continue; } - // Sort `bindings` in declaration order. + // Collect bindings in declaration order. + // Annex B functions are skipped here — they're processed with their var scope. tmp_bindings.clear(); - tmp_bindings.extend(bindings.values().copied().filter(|binding| { - !keep_name_symbols - .as_ref() - .is_some_and(|keep_name_symbols| keep_name_symbols.has_bit(binding.index())) - })); + tmp_bindings.extend( + bindings + .values() + .copied() + .filter(|id| !annex_b_skip.has_bit(id.index())) + .chain( + annex_b_hoisted + .get(&scope_id.index()) + .into_iter() + .flat_map(|v| v.iter().copied()), + ) + .filter(|binding| { + !keep_name_symbols.as_ref().is_some_and(|keep_name_symbols| { + keep_name_symbols.has_bit(binding.index()) + }) + }), + ); if tmp_bindings.is_empty() { continue; } diff --git a/crates/oxc_minifier/tests/mangler/mod.rs b/crates/oxc_minifier/tests/mangler/mod.rs index a0f2d0acb8d9e..ae609bc40405f 100644 --- a/crates/oxc_minifier/tests/mangler/mod.rs +++ b/crates/oxc_minifier/tests/mangler/mod.rs @@ -6,9 +6,12 @@ use oxc_mangler::{MangleOptions, MangleOptionsKeepNames, Mangler}; use oxc_parser::Parser; use oxc_span::SourceType; -fn mangle(source_text: &str, options: MangleOptions) -> String { +fn mangle_with_source_type( + source_text: &str, + options: MangleOptions, + source_type: SourceType, +) -> String { let allocator = Allocator::default(); - let source_type = SourceType::mjs().with_unambiguous(true); let ret = Parser::new(&allocator, source_text, source_type).parse(); assert!(ret.errors.is_empty(), "Parser errors: {:?}", ret.errors); let program = ret.program; @@ -20,6 +23,14 @@ fn mangle(source_text: &str, options: MangleOptions) -> String { .code } +fn mangle(source_text: &str, options: MangleOptions) -> String { + mangle_with_source_type(source_text, options, SourceType::mjs().with_unambiguous(true)) +} + +fn mangle_script(source_text: &str, options: MangleOptions) -> String { + mangle_with_source_type(source_text, options, SourceType::script()) +} + fn test(source_text: &str, expected: &str, options: MangleOptions) { let mangled = mangle(source_text, options); let expected = { @@ -207,3 +218,50 @@ fn private_member_mangling() { insta::assert_snapshot!("private_member_mangling", snapshot); }); } + +/// In sloppy mode (script), function declarations inside blocks have Annex B.3.2.1 semantics: +/// they create an implicit `var`-like assignment in the enclosing function scope at block exit. +/// The mangler must not assign the same name to such a function and an outer `var` binding, +/// or the Annex B assignment would overwrite the outer variable. +#[test] +fn annex_b_block_scoped_function() { + let cases = [ + // Core bug: var + block function in if statement (vitejs/vite#22009) + "function _() { var x = 1; if (true) { function y() {} } use(x); }", + // var + block function in try block (oxc-project/oxc#14316) + "function _() { var x = 1; try { function y() {} } finally {} use(x); }", + // var + block function in plain block + "function _() { var x = 1; { function y() {} } use(x); }", + // Parameter + block function (Annex B doesn't overwrite params, but still safe) + "function _(x) { if (true) { function y() {} } use(x); }", + // Deeply nested blocks + "function _() { var x = 1; { { if (true) { function y() {} } } } use(x); }", + // Arrow callback + block function (oxc-project/oxc#20610) + "function outer(input) { let data = input; Object.keys(data).forEach((key) => { let value = data[key]; if (value) { function appendStyle() {} console.log(appendStyle); } console.log(value); }); }", + // Multiple block functions in same scope + "function _() { var x = 1; if (true) { function y() {} function z() {} } use(x); }", + // Block function referencing outer var (liveness already covers this, but verify) + "function _() { var x = 1; if (true) { function y() { return x; } } use(x); }", + // Sibling block with let reusing function's slot is safe (let is truly block-scoped) + "function _() { var x = 1; if (true) { function y() {} } { let z = 2; use(z); } use(x); }", + // Sibling Annex B functions both get fresh slots + "function _() { var x = 1; if (true) { function y() {} } if (true) { function z() {} } use(x); }", + // Catch parameter in sibling scope + "function _() { var x = 1; if (true) { function y() {} } try { throw 0; } catch (z) { use(z); } use(x); }", + // Sibling var is hoisted to function scope, so no conflict + "function _() { var x = 1; if (true) { function y() {} } { var z = 2; use(z); } use(x); }", + // Annex B function reuses name from sibling function scope (hoisting enables this) + "function _() { function foo() { var x; use(x); } function bar() { if (true) { function baz() {} use(baz); } } }", + ]; + + let mut snapshot = String::new(); + cases.into_iter().fold(&mut snapshot, |w, case| { + let options = MangleOptions::default(); + write!(w, "{case}\n{}\n", mangle_script(case, options)).unwrap(); + w + }); + + insta::with_settings!({ prepend_module_to_snapshot => false, omit_expression => true }, { + insta::assert_snapshot!("annex_b_block_scoped_function", snapshot); + }); +} diff --git a/crates/oxc_minifier/tests/mangler/snapshots/annex_b_block_scoped_function.snap b/crates/oxc_minifier/tests/mangler/snapshots/annex_b_block_scoped_function.snap new file mode 100644 index 0000000000000..b7a63a67c79f8 --- /dev/null +++ b/crates/oxc_minifier/tests/mangler/snapshots/annex_b_block_scoped_function.snap @@ -0,0 +1,150 @@ +--- +source: crates/oxc_minifier/tests/mangler/mod.rs +--- +function _() { var x = 1; if (true) { function y() {} } use(x); } +function _() { + var e = 1; + if (true) { + function t() {} + } + use(e); +} + +function _() { var x = 1; try { function y() {} } finally {} use(x); } +function _() { + var e = 1; + try { + function t() {} + } finally {} + use(e); +} + +function _() { var x = 1; { function y() {} } use(x); } +function _() { + var e = 1; + { + function t() {} + } + use(e); +} + +function _(x) { if (true) { function y() {} } use(x); } +function _(e) { + if (true) { + function t() {} + } + use(e); +} + +function _() { var x = 1; { { if (true) { function y() {} } } } use(x); } +function _() { + var e = 1; + { + { + if (true) { + function t() {} + } + } + } + use(e); +} + +function outer(input) { let data = input; Object.keys(data).forEach((key) => { let value = data[key]; if (value) { function appendStyle() {} console.log(appendStyle); } console.log(value); }); } +function outer(e) { + let t = e; + Object.keys(t).forEach((e) => { + let n = t[e]; + if (n) { + function r() {} + console.log(r); + } + console.log(n); + }); +} + +function _() { var x = 1; if (true) { function y() {} function z() {} } use(x); } +function _() { + var e = 1; + if (true) { + function t() {} + function n() {} + } + use(e); +} + +function _() { var x = 1; if (true) { function y() { return x; } } use(x); } +function _() { + var e = 1; + if (true) { + function t() { + return e; + } + } + use(e); +} + +function _() { var x = 1; if (true) { function y() {} } { let z = 2; use(z); } use(x); } +function _() { + var e = 1; + if (true) { + function t() {} + } + { + let e = 2; + use(e); + } + use(e); +} + +function _() { var x = 1; if (true) { function y() {} } if (true) { function z() {} } use(x); } +function _() { + var e = 1; + if (true) { + function t() {} + } + if (true) { + function n() {} + } + use(e); +} + +function _() { var x = 1; if (true) { function y() {} } try { throw 0; } catch (z) { use(z); } use(x); } +function _() { + var e = 1; + if (true) { + function t() {} + } + try { + throw 0; + } catch (e) { + use(e); + } + use(e); +} + +function _() { var x = 1; if (true) { function y() {} } { var z = 2; use(z); } use(x); } +function _() { + var e = 1; + if (true) { + function t() {} + } + { + var n = 2; + use(n); + } + use(e); +} + +function _() { function foo() { var x; use(x); } function bar() { if (true) { function baz() {} use(baz); } } } +function _() { + function e() { + var e; + use(e); + } + function t() { + if (true) { + function e() {} + use(e); + } + } +}