Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 55 additions & 7 deletions crates/oxc_mangler/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<usize, Vec<'_, SymbolId>> = 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
Expand All @@ -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;
}
Expand Down
62 changes: 60 additions & 2 deletions crates/oxc_minifier/tests/mangler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 = {
Expand Down Expand Up @@ -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);
});
}
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
Loading