From ea3de06327e430dfd3d272aa1529aa777e5a7c33 Mon Sep 17 00:00:00 2001 From: sapphi-red <49056869+sapphi-red@users.noreply.github.com> Date: Sun, 23 Mar 2025 09:17:42 +0000 Subject: [PATCH] feat(mangler): support `keep_names` option (#9898) Same with #9873, but built on top of #9897. close #9873 close #9711 --- crates/oxc_mangler/src/keep_names.rs | 145 ++++++++++++------ crates/oxc_mangler/src/lib.rs | 44 +++++- crates/oxc_minifier/examples/mangler.rs | 18 ++- crates/oxc_minifier/src/lib.rs | 2 +- crates/oxc_minifier/tests/mangler/mod.rs | 31 +++- .../tests/mangler/snapshots/mangler.snap | 39 +++++ napi/minify/index.d.ts | 21 +++ napi/minify/src/options.rs | 30 ++++ 8 files changed, 263 insertions(+), 67 deletions(-) diff --git a/crates/oxc_mangler/src/keep_names.rs b/crates/oxc_mangler/src/keep_names.rs index eb5969031b4d4..93d871c9e2874 100644 --- a/crates/oxc_mangler/src/keep_names.rs +++ b/crates/oxc_mangler/src/keep_names.rs @@ -3,24 +3,65 @@ use oxc_ast::{AstKind, ast::*}; use oxc_semantic::{AstNode, AstNodes, ReferenceId, Scoping, SymbolId}; use rustc_hash::FxHashSet; -#[cfg_attr(not(test), expect(dead_code))] -pub fn collect_name_symbols(scoping: &Scoping, ast_nodes: &AstNodes) -> FxHashSet { - let collector = NameSymbolCollector::new(scoping, ast_nodes); +#[derive(Debug, Clone, Copy, Default)] +pub struct MangleOptionsKeepNames { + /// Preserve `name` property for functions. + /// + /// Default `false` + pub function: bool, + + /// Preserve `name` property for classes. + /// + /// Default `false` + pub class: bool, +} + +impl MangleOptionsKeepNames { + pub fn all_false() -> Self { + Self { function: false, class: false } + } + + pub fn all_true() -> Self { + Self { function: true, class: true } + } +} + +impl From for MangleOptionsKeepNames { + fn from(keep_names: bool) -> Self { + if keep_names { Self::all_true() } else { Self::all_false() } + } +} + +pub fn collect_name_symbols( + options: MangleOptionsKeepNames, + scoping: &Scoping, + ast_nodes: &AstNodes, +) -> FxHashSet { + let collector = NameSymbolCollector::new(options, scoping, ast_nodes); collector.collect() } /// Collects symbols that are used to set `name` properties of functions and classes. struct NameSymbolCollector<'a, 'b> { + options: MangleOptionsKeepNames, scoping: &'b Scoping, ast_nodes: &'b AstNodes<'a>, } impl<'a, 'b: 'a> NameSymbolCollector<'a, 'b> { - fn new(scoping: &'b Scoping, ast_nodes: &'b AstNodes<'a>) -> Self { - Self { scoping, ast_nodes } + fn new( + options: MangleOptionsKeepNames, + scoping: &'b Scoping, + ast_nodes: &'b AstNodes<'a>, + ) -> Self { + Self { options, scoping, ast_nodes } } fn collect(self) -> FxHashSet { + if !self.options.function && !self.options.class { + return FxHashSet::default(); + } + self.scoping .symbol_ids() .filter(|symbol_id| { @@ -42,9 +83,12 @@ impl<'a, 'b: 'a> NameSymbolCollector<'a, 'b> { fn is_name_set_declare_node(&self, node: &'a AstNode, symbol_id: SymbolId) -> bool { match node.kind() { AstKind::Function(function) => { - function.id.as_ref().is_some_and(|id| id.symbol_id() == symbol_id) + self.options.function + && function.id.as_ref().is_some_and(|id| id.symbol_id() == symbol_id) + } + AstKind::Class(cls) => { + self.options.class && cls.id.as_ref().is_some_and(|id| id.symbol_id() == symbol_id) } - AstKind::Class(cls) => cls.id.as_ref().is_some_and(|id| id.symbol_id() == symbol_id), AstKind::VariableDeclarator(decl) => { if let BindingPatternKind::BindingIdentifier(id) = &decl.id.kind { if id.symbol_id() == symbol_id { @@ -176,9 +220,18 @@ impl<'a, 'b: 'a> NameSymbolCollector<'a, 'b> { } } - #[expect(clippy::unused_self)] fn is_expression_whose_name_needs_to_be_kept(&self, expr: &Expression) -> bool { - expr.is_anonymous_function_definition() + let is_anonymous = expr.is_anonymous_function_definition(); + if !is_anonymous { + return false; + } + + if self.options.class && self.options.function { + return true; + } + + let is_class = matches!(expr, Expression::ClassExpression(_)); + (self.options.class && is_class) || (self.options.function && !is_class) } } @@ -189,11 +242,10 @@ mod test { use oxc_semantic::SemanticBuilder; use oxc_span::SourceType; use rustc_hash::FxHashSet; - use std::iter::once; - use super::collect_name_symbols; + use super::{MangleOptionsKeepNames, collect_name_symbols}; - fn collect(source_text: &str) -> FxHashSet { + fn collect(opts: MangleOptionsKeepNames, source_text: &str) -> FxHashSet { let allocator = Allocator::default(); let ret = Parser::new(&allocator, source_text, SourceType::mjs()).parse(); assert!(!ret.panicked, "{source_text}"); @@ -201,69 +253,72 @@ mod test { let ret = SemanticBuilder::new().build(&ret.program); assert!(ret.errors.is_empty(), "{source_text}"); let semantic = ret.semantic; - let symbols = collect_name_symbols(semantic.scoping(), semantic.nodes()); + let symbols = collect_name_symbols(opts, semantic.scoping(), semantic.nodes()); symbols .into_iter() .map(|symbol_id| semantic.scoping().symbol_name(symbol_id).to_string()) .collect() } + fn data(s: &str) -> FxHashSet { + FxHashSet::from_iter([s.to_string()]) + } + + fn function_only() -> MangleOptionsKeepNames { + MangleOptionsKeepNames { function: true, class: false } + } + + fn class_only() -> MangleOptionsKeepNames { + MangleOptionsKeepNames { function: false, class: true } + } + #[test] fn test_declarations() { - assert_eq!(collect("function foo() {}"), once("foo".to_string()).collect()); - assert_eq!(collect("class Foo {}"), once("Foo".to_string()).collect()); + assert_eq!(collect(function_only(), "function foo() {}"), data("foo")); + assert_eq!(collect(class_only(), "class Foo {}"), data("Foo")); } #[test] fn test_simple_declare_init() { - assert_eq!(collect("var foo = function() {}"), once("foo".to_string()).collect()); - assert_eq!(collect("var foo = () => {}"), once("foo".to_string()).collect()); - assert_eq!(collect("var Foo = class {}"), once("Foo".to_string()).collect()); + assert_eq!(collect(function_only(), "var foo = function() {}"), data("foo")); + assert_eq!(collect(function_only(), "var foo = () => {}"), data("foo")); + assert_eq!(collect(class_only(), "var Foo = class {}"), data("Foo")); } #[test] fn test_simple_assign() { - assert_eq!(collect("var foo; foo = function() {}"), once("foo".to_string()).collect()); - assert_eq!(collect("var foo; foo = () => {}"), once("foo".to_string()).collect()); - assert_eq!(collect("var Foo; Foo = class {}"), once("Foo".to_string()).collect()); + assert_eq!(collect(function_only(), "var foo; foo = function() {}"), data("foo")); + assert_eq!(collect(function_only(), "var foo; foo = () => {}"), data("foo")); + assert_eq!(collect(class_only(), "var Foo; Foo = class {}"), data("Foo")); - assert_eq!(collect("var foo; foo ||= function() {}"), once("foo".to_string()).collect()); - assert_eq!( - collect("var foo = 1; foo &&= function() {}"), - once("foo".to_string()).collect() - ); - assert_eq!(collect("var foo; foo ??= function() {}"), once("foo".to_string()).collect()); + assert_eq!(collect(function_only(), "var foo; foo ||= function() {}"), data("foo")); + assert_eq!(collect(function_only(), "var foo = 1; foo &&= function() {}"), data("foo")); + assert_eq!(collect(function_only(), "var foo; foo ??= function() {}"), data("foo")); } #[test] fn test_default_declarations() { - assert_eq!(collect("var [foo = function() {}] = []"), once("foo".to_string()).collect()); - assert_eq!(collect("var [foo = () => {}] = []"), once("foo".to_string()).collect()); - assert_eq!(collect("var [Foo = class {}] = []"), once("Foo".to_string()).collect()); - assert_eq!(collect("var { foo = function() {} } = {}"), once("foo".to_string()).collect()); + assert_eq!(collect(function_only(), "var [foo = function() {}] = []"), data("foo")); + assert_eq!(collect(function_only(), "var [foo = () => {}] = []"), data("foo")); + assert_eq!(collect(class_only(), "var [Foo = class {}] = []"), data("Foo")); + assert_eq!(collect(function_only(), "var { foo = function() {} } = {}"), data("foo")); } #[test] fn test_default_assign() { + assert_eq!(collect(function_only(), "var foo; [foo = function() {}] = []"), data("foo")); + assert_eq!(collect(function_only(), "var foo; [foo = () => {}] = []"), data("foo")); + assert_eq!(collect(class_only(), "var Foo; [Foo = class {}] = []"), data("Foo")); assert_eq!( - collect("var foo; [foo = function() {}] = []"), - once("foo".to_string()).collect() - ); - assert_eq!(collect("var foo; [foo = () => {}] = []"), once("foo".to_string()).collect()); - assert_eq!(collect("var Foo; [Foo = class {}] = []"), once("Foo".to_string()).collect()); - assert_eq!( - collect("var foo; ({ foo = function() {} } = {})"), - once("foo".to_string()).collect() + collect(function_only(), "var foo; ({ foo = function() {} } = {})"), + data("foo") ); } #[test] fn test_for_in_declaration() { - assert_eq!( - collect("for (var foo = function() {} in []) {}"), - once("foo".to_string()).collect() - ); - assert_eq!(collect("for (var foo = () => {} in []) {}"), once("foo".to_string()).collect()); - assert_eq!(collect("for (var Foo = class {} in []) {}"), once("Foo".to_string()).collect()); + assert_eq!(collect(function_only(), "for (var foo = function() {} in []) {}"), data("foo")); + assert_eq!(collect(function_only(), "for (var foo = () => {} in []) {}"), data("foo")); + assert_eq!(collect(class_only(), "for (var Foo = class {} in []) {}"), data("Foo")); } } diff --git a/crates/oxc_mangler/src/lib.rs b/crates/oxc_mangler/src/lib.rs index 3c9080df55e2c..59bda2ff7d0b0 100644 --- a/crates/oxc_mangler/src/lib.rs +++ b/crates/oxc_mangler/src/lib.rs @@ -2,6 +2,7 @@ use std::iter::{self, repeat_with}; use fixedbitset::FixedBitSet; use itertools::Itertools; +use keep_names::collect_name_symbols; use rustc_hash::FxHashSet; use base54::base54; @@ -9,12 +10,14 @@ use oxc_allocator::{Allocator, Vec}; use oxc_ast::ast::{Declaration, Program, Statement}; use oxc_data_structures::inline_string::InlineString; use oxc_index::Idx; -use oxc_semantic::{Scoping, Semantic, SemanticBuilder, SymbolId}; +use oxc_semantic::{AstNodes, Scoping, Semantic, SemanticBuilder, SymbolId}; use oxc_span::Atom; pub(crate) mod base54; mod keep_names; +pub use keep_names::MangleOptionsKeepNames; + #[derive(Default, Debug, Clone, Copy)] pub struct MangleOptions { /// Pass true to mangle names declared in the top level scope. @@ -22,6 +25,9 @@ pub struct MangleOptions { /// Default: `false` pub top_level: bool, + /// Keep function / class names + pub keep_names: MangleOptionsKeepNames, + /// Use more readable mangled names /// (e.g. `slot_0`, `slot_1`, `slot_2`, ...) for debugging. /// @@ -207,6 +213,8 @@ impl Mangler { } else { Default::default() }; + let (keep_name_names, keep_name_symbols) = + Mangler::collect_keep_name_symbols(self.options.keep_names, &scoping, &ast_nodes); let allocator = Allocator::default(); @@ -226,6 +234,16 @@ impl Mangler { continue; } + // Sort `bindings` in declaration order. + tmp_bindings.clear(); + tmp_bindings.extend( + bindings.values().copied().filter(|binding| !keep_name_symbols.contains(binding)), + ); + tmp_bindings.sort_unstable(); + if tmp_bindings.is_empty() { + continue; + } + let mut slot = slot_liveness.len(); reusable_slots.clear(); @@ -236,11 +254,11 @@ impl Mangler { .enumerate() .filter(|(_, slot_liveness)| !slot_liveness.contains(scope_id.index())) .map(|(slot, _)| slot) - .take(bindings.len()), + .take(tmp_bindings.len()), ); // The number of new slots that needs to be allocated. - let remaining_count = bindings.len() - reusable_slots.len(); + let remaining_count = tmp_bindings.len() - reusable_slots.len(); reusable_slots.extend(slot..slot + remaining_count); slot += remaining_count; @@ -249,10 +267,6 @@ impl Mangler { .resize_with(slot, || FixedBitSet::with_capacity(scoping.scopes_len())); } - // Sort `bindings` in declaration order. - tmp_bindings.clear(); - tmp_bindings.extend(bindings.values().copied()); - tmp_bindings.sort_unstable(); for (&symbol_id, assigned_slot) in tmp_bindings.iter().zip(reusable_slots.iter().copied()) { @@ -283,6 +297,7 @@ impl Mangler { let frequencies = self.tally_slot_frequencies( &scoping, &exported_symbols, + &keep_name_symbols, total_number_of_slots, &slots, &allocator, @@ -305,6 +320,8 @@ impl Mangler { && !root_unresolved_references.contains_key(n) && !(root_bindings.contains_key(n) && (!self.options.top_level || exported_names.contains(n))) + // TODO: only skip the names that are kept in the current scope + && !keep_name_names.contains(n) { break name; } @@ -369,6 +386,7 @@ impl Mangler { &'a self, scoping: &Scoping, exported_symbols: &FxHashSet, + keep_name_symbols: &FxHashSet, total_number_of_slots: usize, slots: &[Slot], allocator: &'a Allocator, @@ -389,6 +407,9 @@ impl Mangler { if is_special_name(scoping.symbol_name(symbol_id)) { continue; } + if keep_name_symbols.contains(&symbol_id) { + continue; + } let index = slot; frequencies[index].slot = slot; frequencies[index].frequency += scoping.get_resolved_reference_ids(symbol_id).len(); @@ -422,6 +443,15 @@ impl Mangler { .map(|id| (id.name, id.symbol_id())) .collect() } + + fn collect_keep_name_symbols<'a>( + keep_names: MangleOptionsKeepNames, + scoping: &'a Scoping, + nodes: &AstNodes, + ) -> (FxHashSet<&'a str>, FxHashSet) { + let ids = collect_name_symbols(keep_names, scoping, nodes); + (ids.iter().map(|id| scoping.symbol_name(*id)).collect(), ids) + } } fn is_special_name(name: &str) -> bool { diff --git a/crates/oxc_minifier/examples/mangler.rs b/crates/oxc_minifier/examples/mangler.rs index d8b6fd764490f..232dbbed471d2 100644 --- a/crates/oxc_minifier/examples/mangler.rs +++ b/crates/oxc_minifier/examples/mangler.rs @@ -3,7 +3,7 @@ use std::path::Path; use oxc_allocator::Allocator; use oxc_codegen::CodeGenerator; -use oxc_mangler::{MangleOptions, Mangler}; +use oxc_mangler::{MangleOptions, MangleOptionsKeepNames, Mangler}; use oxc_parser::Parser; use oxc_span::SourceType; use pico_args::Arguments; @@ -15,6 +15,7 @@ use pico_args::Arguments; fn main() -> std::io::Result<()> { let mut args = Arguments::from_env(); + let keep_names = args.contains("--keep-names"); let debug = args.contains("--debug"); let twice = args.contains("--twice"); let name = args.free_from_str().unwrap_or_else(|_| "test.js".to_string()); @@ -23,11 +24,16 @@ fn main() -> std::io::Result<()> { let source_text = std::fs::read_to_string(path)?; let source_type = SourceType::from_path(path).unwrap(); - let printed = mangler(&source_text, source_type, debug); + let options = MangleOptions { + top_level: source_type.is_module(), + keep_names: MangleOptionsKeepNames { function: keep_names, class: keep_names }, + debug, + }; + let printed = mangler(&source_text, source_type, options); println!("{printed}"); if twice { - let printed2 = mangler(&printed, source_type, debug); + let printed2 = mangler(&printed, source_type, options); println!("{printed2}"); println!("same = {}", printed == printed2); } @@ -35,11 +41,9 @@ fn main() -> std::io::Result<()> { Ok(()) } -fn mangler(source_text: &str, source_type: SourceType, debug: bool) -> String { +fn mangler(source_text: &str, source_type: SourceType, options: MangleOptions) -> String { let allocator = Allocator::default(); let ret = Parser::new(&allocator, source_text, source_type).parse(); - let symbol_table = Mangler::new() - .with_options(MangleOptions { debug, top_level: source_type.is_module() }) - .build(&ret.program); + let symbol_table = Mangler::new().with_options(options).build(&ret.program); CodeGenerator::new().with_scoping(Some(symbol_table)).build(&ret.program).code } diff --git a/crates/oxc_minifier/src/lib.rs b/crates/oxc_minifier/src/lib.rs index 0837a7d908cdf..0451ba0ab49f7 100644 --- a/crates/oxc_minifier/src/lib.rs +++ b/crates/oxc_minifier/src/lib.rs @@ -16,7 +16,7 @@ use oxc_ast::ast::Program; use oxc_mangler::Mangler; use oxc_semantic::{Scoping, SemanticBuilder, Stats}; -pub use oxc_mangler::MangleOptions; +pub use oxc_mangler::{MangleOptions, MangleOptionsKeepNames}; pub use crate::{ compressor::Compressor, options::CompressOptions, options::CompressOptionsKeepNames, diff --git a/crates/oxc_minifier/tests/mangler/mod.rs b/crates/oxc_minifier/tests/mangler/mod.rs index be3f3846aaf65..e8ef8bbbd2699 100644 --- a/crates/oxc_minifier/tests/mangler/mod.rs +++ b/crates/oxc_minifier/tests/mangler/mod.rs @@ -2,24 +2,24 @@ use std::fmt::Write; use oxc_allocator::Allocator; use oxc_codegen::CodeGenerator; -use oxc_mangler::{MangleOptions, Mangler}; +use oxc_mangler::{MangleOptions, MangleOptionsKeepNames, Mangler}; use oxc_parser::Parser; use oxc_span::SourceType; -fn mangle(source_text: &str, top_level: bool) -> String { +fn mangle(source_text: &str, options: MangleOptions) -> String { let allocator = Allocator::default(); let source_type = SourceType::mjs(); let ret = Parser::new(&allocator, source_text, source_type).parse(); let program = ret.program; - let symbol_table = - Mangler::new().with_options(MangleOptions { debug: false, top_level }).build(&program); + let symbol_table = Mangler::new().with_options(options).build(&program); CodeGenerator::new().with_scoping(Some(symbol_table)).build(&program).code } #[test] fn direct_eval() { let source_text = "function foo() { let NO_MANGLE; eval('') }"; - let mangled = mangle(source_text, false); + let options = MangleOptions::default(); + let mangled = mangle(source_text, options); assert_eq!(mangled, "function foo() {\n\tlet NO_MANGLE;\n\teval(\"\");\n}\n"); } @@ -61,14 +61,31 @@ fn mangler() { "export const foo = 1; foo", "const foo = 1; foo; export { foo }", ]; + let keep_name_cases = [ + "function _() { function foo() { var x } }", + "function _() { var foo = function() { var x } }", + "function _() { var foo = () => { var x } }", + "function _() { class Foo { foo() { var x } } }", + "function _() { var Foo = class { foo() { var x } } }", + ]; let mut snapshot = String::new(); cases.into_iter().fold(&mut snapshot, |w, case| { - write!(w, "{case}\n{}\n", mangle(case, false)).unwrap(); + let options = MangleOptions::default(); + write!(w, "{case}\n{}\n", mangle(case, options)).unwrap(); w }); top_level_cases.into_iter().fold(&mut snapshot, |w, case| { - write!(w, "{case}\n{}\n", mangle(case, true)).unwrap(); + let options = MangleOptions { top_level: true, ..MangleOptions::default() }; + write!(w, "{case}\n{}\n", mangle(case, options)).unwrap(); + w + }); + keep_name_cases.into_iter().fold(&mut snapshot, |w, case| { + let options = MangleOptions { + keep_names: MangleOptionsKeepNames::all_true(), + ..MangleOptions::default() + }; + write!(w, "{case}\n{}\n", mangle(case, options)).unwrap(); w }); diff --git a/crates/oxc_minifier/tests/mangler/snapshots/mangler.snap b/crates/oxc_minifier/tests/mangler/snapshots/mangler.snap index 85ab853cc83aa..0aebcf67ff29d 100644 --- a/crates/oxc_minifier/tests/mangler/snapshots/mangler.snap +++ b/crates/oxc_minifier/tests/mangler/snapshots/mangler.snap @@ -235,3 +235,42 @@ const foo = 1; foo; export { foo } const e = 1; e; export { e as foo }; + +function _() { function foo() { var x } } +function _() { + function foo() { + var e; + } +} + +function _() { var foo = function() { var x } } +function _() { + var foo = function() { + var e; + }; +} + +function _() { var foo = () => { var x } } +function _() { + var foo = () => { + var e; + }; +} + +function _() { class Foo { foo() { var x } } } +function _() { + class Foo { + foo() { + var e; + } + } +} + +function _() { var Foo = class { foo() { var x } } } +function _() { + var Foo = class { + foo() { + var e; + } + }; +} diff --git a/napi/minify/index.d.ts b/napi/minify/index.d.ts index 8daf831a49737..2baf2e7af57fe 100644 --- a/napi/minify/index.d.ts +++ b/napi/minify/index.d.ts @@ -65,10 +65,31 @@ export interface MangleOptions { * @default false */ toplevel?: boolean + /** + * Preserve `name` property for functions and classes. + * + * @default false + */ + keepNames?: boolean | MangleOptionsKeepNames /** Debug mangled names. */ debug?: boolean } +export interface MangleOptionsKeepNames { + /** + * Preserve `name` property for functions. + * + * @default false + */ + function: boolean + /** + * Preserve `name` property for classes. + * + * @default false + */ + class: boolean +} + /** Minify synchronously. */ export declare function minify(filename: string, sourceText: string, options?: MinifyOptions | undefined | null): MinifyResult diff --git a/napi/minify/src/options.rs b/napi/minify/src/options.rs index ff2ddb3fe753f..9d2dfcd624701 100644 --- a/napi/minify/src/options.rs +++ b/napi/minify/src/options.rs @@ -92,6 +92,11 @@ pub struct MangleOptions { /// @default false pub toplevel: Option, + /// Preserve `name` property for functions and classes. + /// + /// @default false + pub keep_names: Option>, + /// Debug mangled names. pub debug: Option, } @@ -101,11 +106,36 @@ impl From<&MangleOptions> for oxc_minifier::MangleOptions { let default = oxc_minifier::MangleOptions::default(); Self { top_level: o.toplevel.unwrap_or(default.top_level), + keep_names: match &o.keep_names { + Some(Either::A(false)) => oxc_minifier::MangleOptionsKeepNames::all_false(), + Some(Either::A(true)) => oxc_minifier::MangleOptionsKeepNames::all_true(), + Some(Either::B(o)) => oxc_minifier::MangleOptionsKeepNames::from(o), + None => default.keep_names, + }, debug: o.debug.unwrap_or(default.debug), } } } +#[napi(object)] +pub struct MangleOptionsKeepNames { + /// Preserve `name` property for functions. + /// + /// @default false + pub function: bool, + + /// Preserve `name` property for classes. + /// + /// @default false + pub class: bool, +} + +impl From<&MangleOptionsKeepNames> for oxc_minifier::MangleOptionsKeepNames { + fn from(o: &MangleOptionsKeepNames) -> Self { + oxc_minifier::MangleOptionsKeepNames { function: o.function, class: o.class } + } +} + #[napi(object)] pub struct CodegenOptions { /// Remove whitespace.