From e594c3988d3c5456711a1f36f318e91189587f87 Mon Sep 17 00:00:00 2001 From: Boshen <1430279+Boshen@users.noreply.github.com> Date: Wed, 25 Dec 2024 13:54:51 +0000 Subject: [PATCH] refactor(minifier): clean up `peephole_substitute_alternate_syntax.rs` (#8111) --- Cargo.lock | 2 + .../peephole_substitute_alternate_syntax.rs | 273 ++++++++---------- tasks/minsize/Cargo.toml | 2 + tasks/minsize/minsize.snap | 2 +- tasks/minsize/src/lib.rs | 9 + 5 files changed, 142 insertions(+), 146 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 886f7c39b7980..cab6130bbc748 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1793,8 +1793,10 @@ dependencies = [ "oxc_codegen", "oxc_minifier", "oxc_parser", + "oxc_semantic", "oxc_span", "oxc_tasks_common", + "oxc_transformer", "rustc-hash", ] diff --git a/crates/oxc_minifier/src/ast_passes/peephole_substitute_alternate_syntax.rs b/crates/oxc_minifier/src/ast_passes/peephole_substitute_alternate_syntax.rs index 4887eafcaed91..75f2166eb2106 100644 --- a/crates/oxc_minifier/src/ast_passes/peephole_substitute_alternate_syntax.rs +++ b/crates/oxc_minifier/src/ast_passes/peephole_substitute_alternate_syntax.rs @@ -79,76 +79,33 @@ impl<'a> Traverse<'a> for PeepholeSubstituteAlternateSyntax { fn exit_expression(&mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) { let ctx = Ctx(ctx); + + // Change syntax match expr { - Expression::AssignmentExpression(assignment_expr) => { - if let Some(new_expr) = - Self::try_compress_assignment_expression(assignment_expr, ctx) - { - *expr = new_expr; - self.changed = true; - } - } - Expression::LogicalExpression(logical_expr) => { - if let Some(new_expr) = Self::try_compress_is_null_or_undefined(logical_expr, ctx) { - *expr = new_expr; - self.changed = true; - } + Expression::ArrowFunctionExpression(arrow_expr) => { + self.try_compress_arrow_expression(arrow_expr, ctx); } + Expression::ChainExpression(e) => self.try_compress_chain_call_expression(e, ctx), _ => {} } - self.try_compress_boolean(expr, ctx); - self.try_compress_undefined(expr, ctx); - match expr { - Expression::NewExpression(new_expr) => { - if let Some(new_expr) = Self::try_fold_new_expression(new_expr, ctx) { - *expr = new_expr; - self.changed = true; - } - } - Expression::CallExpression(call_expr) => { - if let Some(new_expr) = - Self::try_fold_literal_constructor_call_expression(call_expr, ctx) - .or_else(|| Self::try_fold_simple_function_call(call_expr, ctx)) - { - *expr = new_expr; - self.changed = true; - } - } - Expression::ChainExpression(chain_expr) => { - if let ChainElement::CallExpression(call_expr) = &mut chain_expr.expression { - self.try_fold_chain_call_expression(call_expr, ctx); - } - } - Expression::TemplateLiteral(_) => { - if let Some(val) = expr.to_js_string() { - *expr = ctx.ast.expression_string_literal(expr.span(), val, None); - self.changed = true; - } - } - // `() => { return foo })` -> `() => foo` - Expression::ArrowFunctionExpression(arrow_expr) => { - if !arrow_expr.expression - && arrow_expr.body.directives.is_empty() - && arrow_expr.body.statements.len() == 1 - { - if let Some(body) = arrow_expr.body.statements.first_mut() { - if let Statement::ReturnStatement(ret_stmt) = body { - let return_stmt_arg = - ret_stmt.argument.as_mut().map(|arg| ctx.ast.move_expression(arg)); - - if let Some(return_stmt_arg) = return_stmt_arg { - *body = ctx.ast.statement_expression(SPAN, return_stmt_arg); - arrow_expr.expression = true; - self.changed = true; - } - } - } - } - } - Expression::BinaryExpression(expr) => { - self.compress_typeof_undefined(expr, ctx); + + // Fold + if let Some(folded_expr) = match expr { + Expression::Identifier(ident) => self.try_compress_undefined(ident, ctx), + Expression::BooleanLiteral(_) => self.try_compress_boolean(expr, ctx), + Expression::AssignmentExpression(e) => Self::try_compress_assignment_expression(e, ctx), + Expression::LogicalExpression(e) => Self::try_compress_is_null_or_undefined(e, ctx), + Expression::NewExpression(e) => Self::try_fold_new_expression(e, ctx), + Expression::CallExpression(e) => { + Self::try_fold_literal_constructor_call_expression(e, ctx) + .or_else(|| Self::try_fold_simple_function_call(e, ctx)) } - _ => {} + Expression::TemplateLiteral(t) => Self::try_fold_template_literal(t, ctx), + Expression::BinaryExpression(e) => Self::try_compress_typeof_undefined(e, ctx), + _ => None, + } { + *expr = folded_expr; + self.changed = true; } } } @@ -161,14 +118,18 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax { /* Utilities */ /// Transforms `undefined` => `void 0` - fn try_compress_undefined(&mut self, expr: &mut Expression<'a>, ctx: Ctx<'a, 'b>) { + fn try_compress_undefined( + &self, + ident: &IdentifierReference<'a>, + ctx: Ctx<'a, 'b>, + ) -> Option> { if self.in_fixed_loop { - return; + return None; } - if ctx.is_expression_undefined(expr) { - *expr = ctx.ast.void_0(expr.span()); - self.changed = true; + if !ctx.is_identifier_undefined(ident) { + return None; } + Some(ctx.ast.void_0(ident.span)) } /// Test `Object.defineProperty(exports, ...)` @@ -207,48 +168,80 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax { /// Transforms boolean expression `true` => `!0` `false` => `!1`. /// Do not compress `true` in `Object.defineProperty(exports, 'Foo', {enumerable: true, ...})`. - fn try_compress_boolean(&mut self, expr: &mut Expression<'a>, ctx: Ctx<'a, 'b>) { + fn try_compress_boolean( + &self, + expr: &mut Expression<'a>, + ctx: Ctx<'a, 'b>, + ) -> Option> { if self.in_fixed_loop { - return; + return None; } - let Expression::BooleanLiteral(lit) = expr else { return }; - if !self.in_define_export { - let parent = ctx.ancestry.parent(); - let no_unary = { - if let Ancestor::BinaryExpressionRight(u) = parent { - !matches!( - u.operator(), - BinaryOperator::Addition // Other effect, like string concatenation. + let Expression::BooleanLiteral(lit) = expr else { return None }; + if self.in_define_export { + return None; + } + let parent = ctx.ancestry.parent(); + let no_unary = { + if let Ancestor::BinaryExpressionRight(u) = parent { + !matches!( + u.operator(), + BinaryOperator::Addition // Other effect, like string concatenation. | BinaryOperator::Instanceof // Relational operator. | BinaryOperator::In | BinaryOperator::StrictEquality // It checks type, so we should not fold. | BinaryOperator::StrictInequality - ) - } else { - false - } - }; - // XOR: We should use `!neg` when it is not in binary expression. - let num = ctx.ast.expression_numeric_literal( - SPAN, - if lit.value ^ no_unary { 0.0 } else { 1.0 }, - None, - NumberBase::Decimal, - ); - *expr = if no_unary { - num + ) } else { - ctx.ast.expression_unary(SPAN, UnaryOperator::LogicalNot, num) - }; - self.changed = true; + false + } + }; + // XOR: We should use `!neg` when it is not in binary expression. + let num = ctx.ast.expression_numeric_literal( + SPAN, + if lit.value ^ no_unary { 0.0 } else { 1.0 }, + None, + NumberBase::Decimal, + ); + Some(if no_unary { + num + } else { + ctx.ast.expression_unary(SPAN, UnaryOperator::LogicalNot, num) + }) + } + + /// `() => { return foo })` -> `() => foo` + fn try_compress_arrow_expression( + &mut self, + arrow_expr: &mut ArrowFunctionExpression<'a>, + ctx: Ctx<'a, 'b>, + ) { + if !arrow_expr.expression + && arrow_expr.body.directives.is_empty() + && arrow_expr.body.statements.len() == 1 + { + if let Some(body) = arrow_expr.body.statements.first_mut() { + if let Statement::ReturnStatement(ret_stmt) = body { + let return_stmt_arg = + ret_stmt.argument.as_mut().map(|arg| ctx.ast.move_expression(arg)); + + if let Some(return_stmt_arg) = return_stmt_arg { + *body = ctx.ast.statement_expression(SPAN, return_stmt_arg); + arrow_expr.expression = true; + self.changed = true; + } + } + } } } /// Compress `typeof foo == "undefined"` into `typeof foo > "u"` /// Enabled by `compress.typeofs` - fn compress_typeof_undefined(&mut self, expr: &mut BinaryExpression<'a>, ctx: Ctx<'a, 'b>) { + fn try_compress_typeof_undefined( + expr: &mut BinaryExpression<'a>, + ctx: Ctx<'a, 'b>, + ) -> Option> { if !matches!(expr.operator, BinaryOperator::Equality | BinaryOperator::StrictEquality) { - return; + return None; } let pair = Self::commutative_pair( (&expr.left, &expr.right), @@ -264,16 +257,11 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax { None }, ); - let Some((_void_exp, id_ref)) = pair else { - return; - }; + let (_void_exp, id_ref) = pair?; let argument = Expression::Identifier(ctx.alloc(id_ref)); let left = ctx.ast.expression_unary(SPAN, UnaryOperator::Typeof, argument); let right = ctx.ast.expression_string_literal(SPAN, "u", None); - let binary_expr = - ctx.ast.binary_expression(expr.span, left, BinaryOperator::GreaterThan, right); - *expr = binary_expr; - self.changed = true; + Some(ctx.ast.expression_binary(expr.span, left, BinaryOperator::GreaterThan, right)) } /// Compress `foo === null || foo === undefined` into `foo == null`. @@ -290,16 +278,11 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax { ctx: Ctx<'a, 'b>, ) -> Option> { let op = expr.operator; - if !matches!(op, LogicalOperator::Or | LogicalOperator::And) { - return None; - } - #[allow(clippy::match_wildcard_for_single_variants)] let target_ops = match op { LogicalOperator::Or => (BinaryOperator::StrictEquality, BinaryOperator::Equality), LogicalOperator::And => (BinaryOperator::StrictInequality, BinaryOperator::Inequality), - _ => unreachable!(), + LogicalOperator::Coalesce => return None, }; - if let Some(new_expr) = Self::try_compress_is_null_or_undefined_for_left_and_right( &expr.left, &expr.right, @@ -309,14 +292,12 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax { ) { return Some(new_expr); } - let Expression::LogicalExpression(left) = &mut expr.left else { return None; }; if left.operator != op { return None; } - Self::try_compress_is_null_or_undefined_for_left_and_right( &left.right, &expr.right, @@ -445,36 +426,32 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax { ctx: Ctx<'a, 'b>, ) -> Option> { let target = expr.left.as_simple_assignment_target_mut()?; - if matches!(expr.operator, AssignmentOperator::Subtraction) { - match &expr.right { - Expression::NumericLiteral(num) if num.value.to_int_32() == 1 => { + if !matches!(expr.operator, AssignmentOperator::Subtraction) { + return None; + } + match &expr.right { + Expression::NumericLiteral(num) if num.value.to_int_32() == 1 => { + // The `_` will not be placed to the target code. + let target = std::mem::replace( + target, + ctx.ast.simple_assignment_target_identifier_reference(SPAN, "_"), + ); + Some(ctx.ast.expression_update(SPAN, UpdateOperator::Decrement, true, target)) + } + Expression::UnaryExpression(un) + if matches!(un.operator, UnaryOperator::UnaryNegation) => + { + let Expression::NumericLiteral(num) = &un.argument else { return None }; + (num.value.to_int_32() == 1).then(|| { // The `_` will not be placed to the target code. let target = std::mem::replace( target, ctx.ast.simple_assignment_target_identifier_reference(SPAN, "_"), ); - Some(ctx.ast.expression_update(SPAN, UpdateOperator::Decrement, true, target)) - } - Expression::UnaryExpression(un) - if matches!(un.operator, UnaryOperator::UnaryNegation) => - { - if let Expression::NumericLiteral(num) = &un.argument { - (num.value.to_int_32() == 1).then(|| { - // The `_` will not be placed to the target code. - let target = std::mem::replace( - target, - ctx.ast.simple_assignment_target_identifier_reference(SPAN, "_"), - ); - ctx.ast.expression_update(SPAN, UpdateOperator::Increment, true, target) - }) - } else { - None - } - } - _ => None, + ctx.ast.expression_update(SPAN, UpdateOperator::Increment, true, target) + }) } - } else { - None + _ => None, } } @@ -645,19 +622,25 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax { } } - fn try_fold_chain_call_expression( + fn try_compress_chain_call_expression( &mut self, - call_expr: &mut CallExpression<'a>, + chain_expr: &mut ChainExpression<'a>, ctx: Ctx<'a, 'b>, ) { - // `window.Object?.()` -> `Object?.()` - if call_expr.arguments.is_empty() && Self::is_window_object(&call_expr.callee) { - call_expr.callee = - ctx.ast.expression_identifier_reference(call_expr.callee.span(), "Object"); - self.changed = true; + if let ChainElement::CallExpression(call_expr) = &mut chain_expr.expression { + // `window.Object?.()` -> `Object?.()` + if call_expr.arguments.is_empty() && Self::is_window_object(&call_expr.callee) { + call_expr.callee = + ctx.ast.expression_identifier_reference(call_expr.callee.span(), "Object"); + self.changed = true; + } } } + fn try_fold_template_literal(t: &TemplateLiteral, ctx: Ctx<'a, 'b>) -> Option> { + t.to_js_string().map(|val| ctx.ast.expression_string_literal(t.span(), val, None)) + } + /// returns an `Array()` constructor call with zero, one, or more arguments, copying from the input fn array_constructor_call( arguments: Vec<'a, Argument<'a>>, diff --git a/tasks/minsize/Cargo.toml b/tasks/minsize/Cargo.toml index bfc2bd01ed9c0..c842002da53fe 100644 --- a/tasks/minsize/Cargo.toml +++ b/tasks/minsize/Cargo.toml @@ -22,7 +22,9 @@ oxc_allocator = { workspace = true } oxc_codegen = { workspace = true } oxc_minifier = { workspace = true } oxc_parser = { workspace = true } +oxc_semantic = { workspace = true } oxc_span = { workspace = true } +oxc_transformer = { workspace = true } flate2 = { workspace = true } oxc_tasks_common = { workspace = true } diff --git a/tasks/minsize/minsize.snap b/tasks/minsize/minsize.snap index 76c69877c44b5..bc9fede0fae17 100644 --- a/tasks/minsize/minsize.snap +++ b/tasks/minsize/minsize.snap @@ -1,7 +1,7 @@ | Oxc | ESBuild | Oxc | ESBuild | Original | minified | minified | gzip | gzip | Fixture ------------------------------------------------------------------------------------- -72.14 kB | 24.04 kB | 23.70 kB | 8.61 kB | 8.54 kB | react.development.js +72.14 kB | 24.00 kB | 23.70 kB | 8.58 kB | 8.54 kB | react.development.js 173.90 kB | 61.55 kB | 59.82 kB | 19.54 kB | 19.33 kB | moment.js diff --git a/tasks/minsize/src/lib.rs b/tasks/minsize/src/lib.rs index f6f7d8eed3def..e3655b6977010 100644 --- a/tasks/minsize/src/lib.rs +++ b/tasks/minsize/src/lib.rs @@ -11,8 +11,10 @@ use oxc_allocator::Allocator; use oxc_codegen::{CodeGenerator, CodegenOptions}; use oxc_minifier::{CompressOptions, MangleOptions, Minifier, MinifierOptions}; use oxc_parser::Parser; +use oxc_semantic::SemanticBuilder; use oxc_span::SourceType; use oxc_tasks_common::{project_root, TestFile, TestFiles}; +use oxc_transformer::{ReplaceGlobalDefines, ReplaceGlobalDefinesConfig}; use rustc_hash::FxHashMap; // #[test] @@ -137,6 +139,13 @@ fn minify(source_text: &str, source_type: SourceType, options: MinifierOptions) let allocator = Allocator::default(); let ret = Parser::new(&allocator, source_text, source_type).parse(); let mut program = ret.program; + let (symbols, scopes) = + SemanticBuilder::new().build(&program).semantic.into_symbol_table_and_scope_tree(); + let _ = ReplaceGlobalDefines::new( + &allocator, + ReplaceGlobalDefinesConfig::new(&[("process.env.NODE_ENV", "'development'")]).unwrap(), + ) + .build(symbols, scopes, &mut program); let ret = Minifier::new(options).build(&allocator, &mut program); CodeGenerator::new() .with_options(CodegenOptions { minify: true, ..CodegenOptions::default() })