refactor(ecmascript): don't check side effects in constant_evaluation (#9122)

crates/oxc_ecmascript/src/constant_evaluation/equality_comparison.rs

@@ -24,7 +24,7 @@ pub(super) fn abstract_equality_comparison<'a>(
         if matches!((left, right), (ValueType::Number, ValueType::String))
             || matches!(right, ValueType::Boolean)
         {
-            if let Some(num) = right_expr.get_side_free_number_value(ctx) {
+            if let Some(num) = right_expr.evaluate_value_to_number(ctx) {
                 let number_literal_expr = ctx.ast().expression_numeric_literal(
                     oxc_span::SPAN,
                     num,
@@ -39,7 +39,7 @@ pub(super) fn abstract_equality_comparison<'a>(
         if matches!((left, right), (ValueType::String, ValueType::Number))
             || matches!(left, ValueType::Boolean)
         {
-            if let Some(num) = left_expr.get_side_free_number_value(ctx) {
+            if let Some(num) = left_expr.evaluate_value_to_number(ctx) {
                 let number_literal_expr = ctx.ast().expression_numeric_literal(
                     oxc_span::SPAN,
                     num,
@@ -52,8 +52,8 @@ pub(super) fn abstract_equality_comparison<'a>(
         }
 
         if matches!(left, ValueType::BigInt) || matches!(right, ValueType::BigInt) {
-            let left_bigint = left_expr.get_side_free_bigint_value(ctx);
-            let right_bigint = right_expr.get_side_free_bigint_value(ctx);
+            let left_bigint = left_expr.evaluate_value_to_bigint(ctx);
+            let right_bigint = right_expr.evaluate_value_to_bigint(ctx);
             if let (Some(l_big), Some(r_big)) = (left_bigint, right_bigint) {
                 return Some(l_big.eq(&r_big));
             }

crates/oxc_ecmascript/src/constant_evaluation/mod.rs

@@ -188,9 +188,6 @@ fn binary_operation_evaluate_value_to<'a>(
     match operator {
         BinaryOperator::Addition => {
             use crate::to_primitive::ToPrimitive;
-            if left.may_have_side_effects(ctx) || right.may_have_side_effects(ctx) {
-                return None;
-            }
             let left_to_primitive = left.to_primitive(ctx);
             let right_to_primitive = right.to_primitive(ctx);
             if left_to_primitive.is_string() == Some(true)
@@ -249,8 +246,8 @@ fn binary_operation_evaluate_value_to<'a>(
         BinaryOperator::ShiftLeft
         | BinaryOperator::ShiftRight
         | BinaryOperator::ShiftRightZeroFill => {
-            let left = left.get_side_free_number_value(ctx)?;
-            let right = right.get_side_free_number_value(ctx)?;
+            let left = left.evaluate_value_to_number(ctx)?;
+            let right = right.evaluate_value_to_number(ctx)?;
             let left = left.to_int_32();
             let right = (right.to_int_32() as u32) & 31;
             Some(ConstantValue::Number(match operator {
@@ -286,8 +283,8 @@ fn binary_operation_evaluate_value_to<'a>(
         }
         BinaryOperator::BitwiseAnd | BinaryOperator::BitwiseOR | BinaryOperator::BitwiseXOR => {
             if left.value_type(ctx).is_bigint() && right.value_type(ctx).is_bigint() {
-                let left_val = left.get_side_free_bigint_value(ctx)?;
-                let right_val = right.get_side_free_bigint_value(ctx)?;
+                let left_val = left.evaluate_value_to_bigint(ctx)?;
+                let right_val = right.evaluate_value_to_bigint(ctx)?;
                 let result_val: BigInt = match operator {
                     BinaryOperator::BitwiseAnd => left_val & right_val,
                     BinaryOperator::BitwiseOR => left_val | right_val,
@@ -296,8 +293,8 @@ fn binary_operation_evaluate_value_to<'a>(
                 };
                 return Some(ConstantValue::BigInt(result_val));
             }
-            let left_num = left.get_side_free_number_value(ctx);
-            let right_num = right.get_side_free_number_value(ctx);
+            let left_num = left.evaluate_value_to_number(ctx);
+            let right_num = right.evaluate_value_to_number(ctx);
             if let (Some(left_val), Some(right_val)) = (left_num, right_num) {
                 let left_val_int = left_val.to_int_32();
                 let right_val_int = right_val.to_int_32();
@@ -313,9 +310,6 @@ fn binary_operation_evaluate_value_to<'a>(
             None
         }
         BinaryOperator::Instanceof => {
-            if left.may_have_side_effects(ctx) {
-                return None;
-            }
             if let Expression::Identifier(right_ident) = right {
                 let name = right_ident.name.as_str();
                 if matches!(name, "Object" | "Number" | "Boolean" | "String")
@@ -336,9 +330,6 @@ fn binary_operation_evaluate_value_to<'a>(
         | BinaryOperator::StrictInequality
         | BinaryOperator::Equality
         | BinaryOperator::Inequality => {
-            if left.may_have_side_effects(ctx) || right.may_have_side_effects(ctx) {
-                return None;
-            }
             let value = match operator {
                 BinaryOperator::StrictEquality | BinaryOperator::StrictInequality => {
                     strict_equality_comparison(ctx, left, right)?
@@ -406,9 +397,6 @@ impl<'a> ConstantEvaluation<'a> for UnaryExpression<'a> {
     ) -> Option<ConstantValue<'a>> {
         match self.operator {
             UnaryOperator::Typeof => {
-                if self.argument.may_have_side_effects(ctx) {
-                    return None;
-                }
                 let arg_ty = self.argument.value_type(ctx);
                 let s = match arg_ty {
                     ValueType::BigInt => "bigint",
@@ -429,26 +417,22 @@ impl<'a> ConstantEvaluation<'a> for UnaryExpression<'a> {
                 };
                 Some(ConstantValue::String(Cow::Borrowed(s)))
             }
-            UnaryOperator::Void => {
-                (!self.argument.may_have_side_effects(ctx)).then_some(ConstantValue::Undefined)
+            UnaryOperator::Void => Some(ConstantValue::Undefined),
+            UnaryOperator::LogicalNot => {
+                self.argument.evaluate_value_to_boolean(ctx).map(|b| !b).map(ConstantValue::Boolean)
             }
-            UnaryOperator::LogicalNot => self
-                .argument
-                .get_side_free_boolean_value(ctx)
-                .map(|b| !b)
-                .map(ConstantValue::Boolean),
             UnaryOperator::UnaryPlus => {
-                self.argument.get_side_free_number_value(ctx).map(ConstantValue::Number)
+                self.argument.evaluate_value_to_number(ctx).map(ConstantValue::Number)
             }
             UnaryOperator::UnaryNegation => match self.argument.value_type(ctx) {
                 ValueType::BigInt => self
                     .argument
-                    .get_side_free_bigint_value(ctx)
+                    .evaluate_value_to_bigint(ctx)
                     .map(|v| -v)
                     .map(ConstantValue::BigInt),
                 ValueType::Number => self
                     .argument
-                    .get_side_free_number_value(ctx)
+                    .evaluate_value_to_number(ctx)
                     .map(|v| if v.is_nan() { v } else { -v })
                     .map(ConstantValue::Number),
                 ValueType::Undefined => Some(ConstantValue::Number(f64::NAN)),
@@ -458,13 +442,13 @@ impl<'a> ConstantEvaluation<'a> for UnaryExpression<'a> {
             UnaryOperator::BitwiseNot => match self.argument.value_type(ctx) {
                 ValueType::BigInt => self
                     .argument
-                    .get_side_free_bigint_value(ctx)
+                    .evaluate_value_to_bigint(ctx)
                     .map(|v| !v)
                     .map(ConstantValue::BigInt),
                 #[expect(clippy::cast_lossless)]
                 _ => self
                     .argument
-                    .get_side_free_number_value(ctx)
+                    .evaluate_value_to_number(ctx)
                     .map(|v| (!v.to_int_32()) as f64)
                     .map(ConstantValue::Number),
             },
@@ -483,15 +467,10 @@ impl<'a> ConstantEvaluation<'a> for StaticMemberExpression<'a> {
             "length" => {
                 if let Some(ConstantValue::String(s)) = self.object.evaluate_value(ctx) {
                     Some(ConstantValue::Number(s.encode_utf16().count().to_f64().unwrap()))
+                } else if let Expression::ArrayExpression(arr) = &self.object {
+                    Some(ConstantValue::Number(arr.elements.len().to_f64().unwrap()))
                 } else {
-                    if self.object.may_have_side_effects(ctx) {
-                        return None;
-                    }
-                    if let Expression::ArrayExpression(arr) = &self.object {
-                        Some(ConstantValue::Number(arr.elements.len().to_f64().unwrap()))
-                    } else {
-                        None
-                    }
+                    None
                 }
             }
             _ => None,
@@ -509,15 +488,10 @@ impl<'a> ConstantEvaluation<'a> for ComputedMemberExpression<'a> {
             Expression::StringLiteral(s) if s.value == "length" => {
                 if let Some(ConstantValue::String(s)) = self.object.evaluate_value(ctx) {
                     Some(ConstantValue::Number(s.encode_utf16().count().to_f64().unwrap()))
+                } else if let Expression::ArrayExpression(arr) = &self.object {
+                    Some(ConstantValue::Number(arr.elements.len().to_f64().unwrap()))
                 } else {
-                    if self.object.may_have_side_effects(ctx) {
-                        return None;
-                    }
-                    if let Expression::ArrayExpression(arr) = &self.object {
-                        Some(ConstantValue::Number(arr.elements.len().to_f64().unwrap()))
-                    } else {
-                        None
-                    }
+                    None
                 }
             }
             _ => None,
@@ -531,10 +505,6 @@ fn is_less_than<'a>(
     x: &Expression<'a>,
     y: &Expression<'a>,
 ) -> Option<ConstantValue<'a>> {
-    if x.may_have_side_effects(ctx) || y.may_have_side_effects(ctx) {
-        return None;
-    }
-
     // a. Let px be ? ToPrimitive(x, NUMBER).
     // b. Let py be ? ToPrimitive(y, NUMBER).
     let px = x.value_type(ctx);

crates/oxc_minifier/src/ctx.rs

@@ -4,6 +4,7 @@ use oxc_ast::{AstBuilder, ast::*};
 use oxc_ecmascript::constant_evaluation::{
     ConstantEvaluation, ConstantEvaluationCtx, ConstantValue, binary_operation_evaluate_value,
 };
+use oxc_ecmascript::side_effects::MayHaveSideEffects;
 use oxc_semantic::{IsGlobalReference, SymbolTable};
 use oxc_traverse::TraverseCtx;
 
@@ -44,7 +45,11 @@ impl<'a> Ctx<'a, '_> {
     }
 
     pub fn eval_binary(self, e: &BinaryExpression<'a>) -> Option<Expression<'a>> {
-        e.evaluate_value(&self).map(|v| self.value_to_expr(e.span, v))
+        if e.may_have_side_effects(&self) {
+            None
+        } else {
+            e.evaluate_value(&self).map(|v| self.value_to_expr(e.span, v))
+        }
     }
 
     pub fn eval_binary_operation(

crates/oxc_minifier/src/peephole/fold_constants.rs

@@ -49,7 +49,13 @@ impl<'a> PeepholeOptimizations {
             }
             // Do not fold big int.
             UnaryOperator::UnaryNegation if e.argument.is_big_int_literal() => None,
-            _ => e.evaluate_value(&ctx).map(|v| ctx.value_to_expr(e.span, v)),
+            _ => {
+                if e.may_have_side_effects(&ctx) {
+                    None
+                } else {
+                    e.evaluate_value(&ctx).map(|v| ctx.value_to_expr(e.span, v))
+                }
+            }
         }
     }
 
@@ -58,7 +64,11 @@ impl<'a> PeepholeOptimizations {
         ctx: Ctx<'a, '_>,
     ) -> Option<Expression<'a>> {
         // TODO: tryFoldObjectPropAccess(n, left, name)
-        e.evaluate_value(&ctx).map(|value| ctx.value_to_expr(e.span, value))
+        if e.object.may_have_side_effects(&ctx) {
+            None
+        } else {
+            e.evaluate_value(&ctx).map(|value| ctx.value_to_expr(e.span, value))
+        }
     }
 
     fn try_fold_computed_member_expr(
@@ -66,7 +76,11 @@ impl<'a> PeepholeOptimizations {
         ctx: Ctx<'a, '_>,
     ) -> Option<Expression<'a>> {
         // TODO: tryFoldObjectPropAccess(n, left, name)
-        e.evaluate_value(&ctx).map(|value| ctx.value_to_expr(e.span, value))
+        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))
+        }
     }
 
     fn try_fold_logical_expr(
@@ -304,8 +318,10 @@ impl<'a> PeepholeOptimizations {
 
     // Simplified version of `tryFoldAdd` from closure compiler.
     fn try_fold_add(e: &mut BinaryExpression<'a>, ctx: Ctx<'a, '_>) -> Option<Expression<'a>> {
-        if let Some(v) = e.evaluate_value(&ctx) {
-            return Some(ctx.value_to_expr(e.span, v));
+        if !e.may_have_side_effects(&ctx) {
+            if let Some(v) = e.evaluate_value(&ctx) {
+                return Some(ctx.value_to_expr(e.span, v));
+            }
         }
         debug_assert_eq!(e.operator, BinaryOperator::Addition);