refactor(minifier): change AST in-place instead of returning Option<Expression>

Author: Boshen

crates/oxc_minifier/src/peephole/fold_constants.rs

@@ -1,7 +1,7 @@
 use oxc_allocator::TakeIn;
 use oxc_ast::ast::*;
 use oxc_ecmascript::{
-    ToJsString,
+    GlobalContext, ToJsString,
     constant_evaluation::{ConstantEvaluation, ConstantValue, DetermineValueType, ValueType},
     side_effects::MayHaveSideEffects,
 };
@@ -16,112 +16,107 @@ impl<'a> PeepholeOptimizations {
     /// Constant Folding
     ///
     /// <https://github.com/google/closure-compiler/blob/v20240609/src/com/google/javascript/jscomp/PeepholeFoldConstants.java>
-    pub fn fold_constants_exit_expression(&self, expr: &mut Expression<'a>, ctx: &mut Ctx<'a, '_>) {
-        match expr {
+    pub fn fold_constants_exit_expression(&self, e: &mut Expression<'a>, ctx: &mut Ctx<'a, '_>) {
+        match e {
             Expression::TemplateLiteral(t) => {
                 self.try_inline_values_in_template_literal(t, ctx);
             }
             Expression::ObjectExpression(e) => self.fold_object_spread(e, ctx),
+            Expression::BinaryExpression(_) => {
+                Self::try_fold_binary_expr(e, ctx);
+                Self::try_fold_binary_typeof_comparison(e, ctx);
+            }
+            Expression::UnaryExpression(_) => Self::try_fold_unary_expr(e, ctx),
+            Expression::StaticMemberExpression(_) => Self::try_fold_static_member_expr(e, ctx),
+            Expression::ComputedMemberExpression(_) => Self::try_fold_computed_member_expr(e, ctx),
+            Expression::LogicalExpression(_) => Self::try_fold_logical_expr(e, ctx),
+            Expression::ChainExpression(_) => Self::try_fold_optional_chain(e, ctx),
+            Expression::CallExpression(_) => Self::try_fold_number_constructor(e, ctx),
             _ => {}
         }
-
-        if let Some(folded_expr) = match expr {
-            Expression::BinaryExpression(e) => Self::try_fold_binary_expr(e, ctx)
-                .or_else(|| Self::try_fold_binary_typeof_comparison(e, ctx)),
-            Expression::UnaryExpression(e) => Self::try_fold_unary_expr(e, ctx),
-            Expression::StaticMemberExpression(e) => Self::try_fold_static_member_expr(e, ctx),
-            Expression::ComputedMemberExpression(e) => Self::try_fold_computed_member_expr(e, ctx),
-            Expression::LogicalExpression(e) => Self::try_fold_logical_expr(e, ctx),
-            Expression::ChainExpression(e) => Self::try_fold_optional_chain(e, ctx),
-            Expression::CallExpression(e) => Self::try_fold_number_constructor(e, ctx),
-            _ => None,
-        } {
-            *expr = folded_expr;
-            ctx.state.changed = true;
-        }
     }
 
     #[expect(clippy::float_cmp)]
-    fn try_fold_unary_expr(
-        e: &UnaryExpression<'a>,
-        ctx: &mut Ctx<'a, '_>,
-    ) -> Option<Expression<'a>> {
+    fn try_fold_unary_expr(expr: &mut Expression<'a>, ctx: &mut Ctx<'a, '_>) {
+        let Expression::UnaryExpression(e) = expr else { return };
         match e.operator {
             // Do not fold `void 0` back to `undefined`.
-            UnaryOperator::Void if e.argument.is_number_0() => None,
+            UnaryOperator::Void if e.argument.is_number_0() => {}
             // Do not fold `true` and `false` back to `!0` and `!1`
-            UnaryOperator::LogicalNot if matches!(&e.argument, Expression::NumericLiteral(lit) if lit.value == 0.0 || lit.value == 1.0) => {
-                None
-            }
+            UnaryOperator::LogicalNot if matches!(&e.argument, Expression::NumericLiteral(lit) if lit.value == 0.0 || lit.value == 1.0) =>
+                {}
             // Do not fold big int.
-            UnaryOperator::UnaryNegation if e.argument.is_big_int_literal() => None,
+            UnaryOperator::UnaryNegation if e.argument.is_big_int_literal() => {}
+            _ if e.may_have_side_effects(ctx) => {}
             _ => {
-                if e.may_have_side_effects(ctx) {
-                    None
-                } else {
-                    e.evaluate_value(ctx).map(|v| ctx.value_to_expr(e.span, v))
+                if let Some(changed) = e.evaluate_value(ctx).map(|v| ctx.value_to_expr(e.span, v)) {
+                    *expr = changed;
+                    ctx.state.changed = true;
                 }
             }
         }
     }
 
-    fn try_fold_static_member_expr(
-        e: &StaticMemberExpression<'a>,
-        ctx: &mut Ctx<'a, '_>,
-    ) -> Option<Expression<'a>> {
+    fn try_fold_static_member_expr(expr: &mut Expression<'a>, ctx: &mut Ctx<'a, '_>) {
+        let Expression::StaticMemberExpression(e) = expr else { return };
         // TODO: tryFoldObjectPropAccess(n, left, name)
         if e.object.may_have_side_effects(ctx) {
-            None
-        } else {
-            e.evaluate_value(ctx).map(|value| ctx.value_to_expr(e.span, value))
+            return;
+        }
+        if let Some(changed) = e.evaluate_value(ctx).map(|value| ctx.value_to_expr(e.span, value)) {
+            *expr = changed;
+            ctx.state.changed = true;
         }
     }
 
-    fn try_fold_computed_member_expr(
-        e: &ComputedMemberExpression<'a>,
-        ctx: &mut Ctx<'a, '_>,
-    ) -> Option<Expression<'a>> {
+    fn try_fold_computed_member_expr(expr: &mut Expression<'a>, ctx: &mut Ctx<'a, '_>) {
+        let Expression::ComputedMemberExpression(e) = expr else { return };
         // TODO: tryFoldObjectPropAccess(n, left, name)
         if e.object.may_have_side_effects(ctx) || e.expression.may_have_side_effects(ctx) {
-            None
-        } else {
-            e.evaluate_value(ctx).map(|value| ctx.value_to_expr(e.span, value))
+            return;
+        }
+        if let Some(changed) = e.evaluate_value(ctx).map(|value| ctx.value_to_expr(e.span, value)) {
+            *expr = changed;
+            ctx.state.changed = true;
         }
     }
 
-    fn try_fold_logical_expr(
-        logical_expr: &mut LogicalExpression<'a>,
-        ctx: &mut Ctx<'a, '_>,
-    ) -> Option<Expression<'a>> {
-        match logical_expr.operator {
-            LogicalOperator::And | LogicalOperator::Or => Self::try_fold_and_or(logical_expr, ctx),
-            LogicalOperator::Coalesce => Self::try_fold_coalesce(logical_expr, ctx),
+    fn try_fold_logical_expr(expr: &mut Expression<'a>, ctx: &mut Ctx<'a, '_>) {
+        let Expression::LogicalExpression(e) = expr else { return };
+        if let Some(changed) = match e.operator {
+            LogicalOperator::And | LogicalOperator::Or => Self::try_fold_and_or(e, ctx),
+            LogicalOperator::Coalesce => Self::try_fold_coalesce(e, ctx),
+        } {
+            *expr = changed;
+            ctx.state.changed = true;
         }
     }
 
-    fn try_fold_optional_chain(
-        chain_expr: &ChainExpression<'a>,
-        ctx: &mut Ctx<'a, '_>,
-    ) -> Option<Expression<'a>> {
-        let left_expr = match &chain_expr.expression {
+    fn try_fold_optional_chain(expr: &mut Expression<'a>, ctx: &mut Ctx<'a, '_>) {
+        let Expression::ChainExpression(e) = expr else { return };
+        let left_expr = match &e.expression {
             match_member_expression!(ChainElement) => {
-                let member_expr = chain_expr.expression.to_member_expression();
+                let member_expr = e.expression.to_member_expression();
                 if !member_expr.optional() {
-                    return None;
+                    return;
                 }
                 member_expr.object()
             }
             ChainElement::CallExpression(call_expr) => {
                 if !call_expr.optional {
-                    return None;
+                    return;
                 }
                 &call_expr.callee
             }
-            ChainElement::TSNonNullExpression(_) => return None,
+            ChainElement::TSNonNullExpression(_) => return,
         };
         let ty = left_expr.value_type(ctx);
-        (ty.is_null() || ty.is_undefined())
-            .then(|| ctx.value_to_expr(chain_expr.span, ConstantValue::Undefined))
+        if let Some(changed) = (ty.is_null() || ty.is_undefined())
+            .then(|| ctx.value_to_expr(e.span, ConstantValue::Undefined))
+        {
+            *expr = changed;
+            ctx.state.changed = true;
+        }
     }
 
     /// Try to fold a AND / OR node.
@@ -278,16 +273,14 @@ impl<'a> PeepholeOptimizations {
     }
 
     #[expect(clippy::cast_possible_truncation, clippy::cast_sign_loss)]
-    fn try_fold_binary_expr(
-        e: &mut BinaryExpression<'a>,
-        ctx: &mut Ctx<'a, '_>,
-    ) -> Option<Expression<'a>> {
+    fn try_fold_binary_expr(expr: &mut Expression<'a>, ctx: &mut Ctx<'a, '_>) {
+        let Expression::BinaryExpression(e) = expr else { return };
         // TODO: tryReduceOperandsForOp
 
         // https://github.com/evanw/esbuild/blob/v0.24.2/internal/js_ast/js_ast_helpers.go#L1136
         // https://github.com/evanw/esbuild/blob/v0.24.2/internal/js_ast/js_ast_helpers.go#L1222
         let span = e.span;
-        match e.operator {
+        let changed = match e.operator {
             BinaryOperator::Equality
             | BinaryOperator::Inequality
             | BinaryOperator::StrictEquality
@@ -339,30 +332,30 @@ impl<'a> PeepholeOptimizations {
                 .filter(|(_, right)| *right == 0.0 || right.is_nan() || right.is_infinite())
                 .and_then(|_| ctx.eval_binary(e)),
             BinaryOperator::ShiftLeft => {
-                if let Some((left, right)) = Self::extract_numeric_values(e) {
+                Self::extract_numeric_values(e).and_then(|(left, right)| {
                     let result = e.evaluate_value(ctx)?.into_number()?;
                     let left_len = Self::approximate_printed_int_char_count(left);
                     let right_len = Self::approximate_printed_int_char_count(right);
                     let result_len = Self::approximate_printed_int_char_count(result);
-                    if result_len <= left_len + 2 + right_len {
-                        return Some(ctx.value_to_expr(span, ConstantValue::Number(result)));
-                    }
-                }
-                None
+                    (result_len <= left_len + 2 + right_len)
+                        .then(|| ctx.value_to_expr(span, ConstantValue::Number(result)))
+                })
             }
             BinaryOperator::ShiftRightZeroFill => {
-                if let Some((left, right)) = Self::extract_numeric_values(e) {
+                Self::extract_numeric_values(e).and_then(|(left, right)| {
                     let result = e.evaluate_value(ctx)?.into_number()?;
                     let left_len = Self::approximate_printed_int_char_count(left);
                     let right_len = Self::approximate_printed_int_char_count(right);
                     let result_len = Self::approximate_printed_int_char_count(result);
-                    if result_len <= left_len + 3 + right_len {
-                        return Some(ctx.value_to_expr(span, ConstantValue::Number(result)));
-                    }
-                }
-                None
+                    (result_len <= left_len + 3 + right_len)
+                        .then(|| ctx.value_to_expr(span, ConstantValue::Number(result)))
+                })
             }
             BinaryOperator::In => None,
+        };
+        if let Some(changed) = changed {
+            *expr = changed;
+            ctx.state.changed = true;
         }
     }
 
@@ -544,21 +537,15 @@ impl<'a> PeepholeOptimizations {
         ))
     }
 
-    fn try_fold_number_constructor(
-        e: &CallExpression<'a>,
-        ctx: &mut Ctx<'a, '_>,
-    ) -> Option<Expression<'a>> {
-        let Expression::Identifier(ident) = &e.callee else { return None };
-        if ident.name != "Number" {
-            return None;
-        }
-        if !ctx.is_global_reference(ident) {
-            return None;
+    fn try_fold_number_constructor(expr: &mut Expression<'a>, ctx: &mut Ctx<'a, '_>) {
+        let Expression::CallExpression(e) = expr else { return };
+        if !ctx.is_global_expr("Number", &e.callee) {
+            return;
         }
         if e.arguments.len() != 1 {
-            return None;
+            return;
         }
-        let arg = e.arguments[0].as_expression()?;
+        let Some(arg) = e.arguments[0].as_expression() else { return };
         let value = ConstantValue::Number(match arg {
             // `Number(undefined)` -> `NaN`
             Expression::Identifier(ident) if ctx.is_identifier_undefined(ident) => f64::NAN,
@@ -574,27 +561,28 @@ impl<'a> PeepholeOptimizations {
                 if let Some(n) = arg.evaluate_value_to_number(ctx) {
                     n
                 } else {
-                    return Some(ctx.ast.expression_unary(
+                    *expr = ctx.ast.expression_unary(
                         e.span,
                         UnaryOperator::UnaryPlus,
                         ctx.ast.expression_string_literal(n.span, n.value, n.raw),
-                    ));
+                    );
+                    ctx.state.changed = true;
+                    return;
                 }
             }
             e if e.is_void_0() => f64::NAN,
-            _ => return None,
+            _ => return,
         });
-        Some(ctx.value_to_expr(e.span, value))
+        *expr = ctx.value_to_expr(e.span, value);
+        ctx.state.changed = true;
     }
 
-    fn try_fold_binary_typeof_comparison(
-        bin_expr: &BinaryExpression<'a>,
-        ctx: &mut Ctx<'a, '_>,
-    ) -> Option<Expression<'a>> {
+    fn try_fold_binary_typeof_comparison(expr: &mut Expression<'a>, ctx: &mut Ctx<'a, '_>) {
+        let Expression::BinaryExpression(e) = expr else { return };
         // `typeof a == typeof a` -> `true`, `typeof a != typeof a` -> `false`
-        if bin_expr.operator.is_equality() {
+        if e.operator.is_equality() {
             if let (Expression::UnaryExpression(left), Expression::UnaryExpression(right)) =
-                (&bin_expr.left, &bin_expr.right)
+                (&e.left, &e.right)
             {
                 if left.operator.is_typeof() && right.operator.is_typeof() {
                     if let (
@@ -603,13 +591,13 @@ impl<'a> PeepholeOptimizations {
                     ) = (&left.argument, &right.argument)
                     {
                         if left_ident.name == right_ident.name {
-                            return Some(ctx.ast.expression_boolean_literal(
-                                bin_expr.span,
-                                matches!(
-                                    bin_expr.operator,
-                                    BinaryOperator::StrictEquality | BinaryOperator::Equality
-                                ),
-                            ));
+                            let b = matches!(
+                                e.operator,
+                                BinaryOperator::StrictEquality | BinaryOperator::Equality
+                            );
+                            *expr = ctx.ast.expression_boolean_literal(e.span, b);
+                            ctx.state.changed = true;
+                            return;
                         }
                     }
                 }
@@ -618,18 +606,20 @@ impl<'a> PeepholeOptimizations {
 
         // `typeof a === 'asd` -> `false``
         // `typeof a !== 'b'` -> `true``
-        if let Expression::UnaryExpression(left) = &bin_expr.left {
-            if left.operator.is_typeof() && bin_expr.operator.is_equality() {
-                let right_ty = bin_expr.right.value_type(ctx);
+        if let Expression::UnaryExpression(left) = &e.left {
+            if left.operator.is_typeof() && e.operator.is_equality() {
+                let right_ty = e.right.value_type(ctx);
 
                 if !right_ty.is_undetermined() && right_ty != ValueType::String {
-                    return Some(ctx.ast.expression_boolean_literal(
-                        bin_expr.span,
-                        bin_expr.operator == BinaryOperator::Inequality
-                            || bin_expr.operator == BinaryOperator::StrictInequality,
-                    ));
+                    *expr = ctx.ast.expression_boolean_literal(
+                        e.span,
+                        e.operator == BinaryOperator::Inequality
+                            || e.operator == BinaryOperator::StrictInequality,
+                    );
+                    ctx.state.changed = true;
+                    return;
                 }
-                if let Expression::StringLiteral(string_lit) = &bin_expr.right {
+                if let Expression::StringLiteral(string_lit) = &e.right {
                     if !matches!(
                         string_lit.value.as_str(),
                         "string"
@@ -642,17 +632,16 @@ impl<'a> PeepholeOptimizations {
                             | "function"
                             | "unknown" // IE
                     ) {
-                        return Some(ctx.ast.expression_boolean_literal(
-                            bin_expr.span,
-                            bin_expr.operator == BinaryOperator::Inequality
-                                || bin_expr.operator == BinaryOperator::StrictInequality,
-                        ));
+                        *expr = ctx.ast.expression_boolean_literal(
+                            e.span,
+                            e.operator == BinaryOperator::Inequality
+                                || e.operator == BinaryOperator::StrictInequality,
+                        );
+                        ctx.state.changed = true;
                     }
                 }
             }
         }
-
-        None
     }
 
     fn fold_object_spread(&self, e: &mut ObjectExpression<'a>, ctx: &mut Ctx<'a, '_>) {