refactor(minifier): clean up peephole_substitute_alternate_syntax.rs (#8111)

Cargo.lock

@@ -1793,8 +1793,10 @@ dependencies = [
  "oxc_codegen",
  "oxc_minifier",
  "oxc_parser",
+ "oxc_semantic",
  "oxc_span",
  "oxc_tasks_common",
+ "oxc_transformer",
  "rustc-hash",
 ]
 

crates/oxc_minifier/src/ast_passes/peephole_substitute_alternate_syntax.rs

@@ -79,77 +79,34 @@ impl<'a> Traverse<'a> for PeepholeSubstituteAlternateSyntax {
 
     fn exit_expression(&mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) {
         let ctx = Ctx(ctx);
-        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;
-                }
-            }
-            _ => {}
-        }
-        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 {
+        // Change syntax
-                                *body = ctx.ast.statement_expression(SPAN, return_stmt_arg);
+        match expr {
-                                arrow_expr.expression = true;
+            Expression::ArrowFunctionExpression(arrow_expr) => {
-                                self.changed = true;
+                self.try_compress_arrow_expression(arrow_expr, ctx);
-                            }
-                        }
-                    }
-                }
-            }
-            Expression::BinaryExpression(expr) => {
-                self.compress_typeof_undefined(expr, ctx);
             }
+            Expression::ChainExpression(e) => self.try_compress_chain_call_expression(e, 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<Expression<'a>> {
         if self.in_fixed_loop {
-            return;
+            return None;
         }
-        if ctx.is_expression_undefined(expr) {
+        if !ctx.is_identifier_undefined(ident) {
-            *expr = ctx.ast.void_0(expr.span());
+            return None;
-            self.changed = true;
         }
+        Some(ctx.ast.void_0(ident.span))
     }
 
     /// Test `Object.defineProperty(exports, ...)`
@@ -207,12 +168,18 @@ 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<Expression<'a>> {
         if self.in_fixed_loop {
-            return;
+            return None;
+        }
+        let Expression::BooleanLiteral(lit) = expr else { return None };
+        if self.in_define_export {
+            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 {
@@ -235,20 +202,46 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax {
             None,
             NumberBase::Decimal,
         );
-            *expr = if no_unary {
+        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<Expression<'a>> {
         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 {
+        let (_void_exp, id_ref) = pair?;
-            return;
-        };
         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 =
+        Some(ctx.ast.expression_binary(expr.span, left, BinaryOperator::GreaterThan, right))
-            ctx.ast.binary_expression(expr.span, left, BinaryOperator::GreaterThan, right);
-        *expr = binary_expr;
-        self.changed = true;
     }
 
     /// Compress `foo === null || foo === undefined` into `foo == null`.
@@ -290,16 +278,11 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax {
         ctx: Ctx<'a, 'b>,
     ) -> Option<Expression<'a>> {
         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,7 +426,9 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax {
         ctx: Ctx<'a, 'b>,
     ) -> Option<Expression<'a>> {
         let target = expr.left.as_simple_assignment_target_mut()?;
-        if matches!(expr.operator, AssignmentOperator::Subtraction) {
+        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.
@@ -458,7 +441,7 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax {
             Expression::UnaryExpression(un)
                 if matches!(un.operator, UnaryOperator::UnaryNegation) =>
             {
-                    if let Expression::NumericLiteral(num) = &un.argument {
+                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(
@@ -467,15 +450,9 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax {
                     );
                     ctx.ast.expression_update(SPAN, UpdateOperator::Increment, true, target)
                 })
-                    } else {
-                        None
-                    }
             }
             _ => None,
         }
-        } else {
-            None
-        }
     }
 
     fn is_window_object(expr: &Expression) -> bool {
@@ -645,11 +622,12 @@ 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>,
     ) {
+        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 =
@@ -657,6 +635,11 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax {
                 self.changed = true;
             }
         }
+    }
+
+    fn try_fold_template_literal(t: &TemplateLiteral, ctx: Ctx<'a, 'b>) -> Option<Expression<'a>> {
+        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(

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 }

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 
 

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() })