refactor(ecmascript): extract to_numeric (#9111)

Extracted to_numeric from all places so that the conditions are more clear. Also this adds some advanced cases as to_numeric uses to_primitive internally.

crates/oxc_ecmascript/src/constant_evaluation/mod.rs

@@ -7,8 +7,8 @@ use equality_comparison::{abstract_equality_comparison, strict_equality_comparis
 use oxc_ast::{ast::*, AstBuilder};
 
 use crate::{
-    is_global_reference::IsGlobalReference, side_effects::MayHaveSideEffects, ToBigInt, ToBoolean,
-    ToInt32, ToJsString, ToNumber,
+    is_global_reference::IsGlobalReference, side_effects::MayHaveSideEffects,
+    to_numeric::ToNumeric, ToBigInt, ToBoolean, ToInt32, ToJsString, ToNumber,
 };
 
 mod equality_comparison;
@@ -244,25 +244,35 @@ pub trait ConstantEvaluation<'a>: IsGlobalReference {
     ) -> Option<ConstantValue<'a>> {
         match operator {
             BinaryOperator::Addition => {
+                use crate::to_primitive::ToPrimitive;
                 if left.may_have_side_effects(self) || right.may_have_side_effects(self) {
                     return None;
                 }
-                let left_type = left.value_type(self);
-                let right_type = right.value_type(self);
-                if left_type.is_string() || right_type.is_string() {
+                let left_to_primitive = left.to_primitive(self);
+                let right_to_primitive = right.to_primitive(self);
+                if left_to_primitive.is_string() == Some(true)
+                    || right_to_primitive.is_string() == Some(true)
+                {
                     let lval = self.eval_expression(left)?;
                     let rval = self.eval_expression(right)?;
                     let lstr = lval.to_js_string(self)?;
                     let rstr = rval.to_js_string(self)?;
                     return Some(ConstantValue::String(lstr + rstr));
                 }
-                if left_type.is_number() || right_type.is_number() {
+                let left_to_numeric_type = left_to_primitive.to_numeric(self);
+                let right_to_numeric_type = right_to_primitive.to_numeric(self);
+                if left_to_numeric_type.is_number() || right_to_numeric_type.is_number() {
                     let lval = self.eval_expression(left)?;
                     let rval = self.eval_expression(right)?;
                     let lnum = lval.to_number(self)?;
                     let rnum = rval.to_number(self)?;
                     return Some(ConstantValue::Number(lnum + rnum));
                 }
+                if left_to_numeric_type.is_bigint() && right_to_numeric_type.is_bigint() {
+                    let lval = self.eval_to_big_int(left)?;
+                    let rval = self.eval_to_big_int(right)?;
+                    return Some(ConstantValue::BigInt(lval + rval));
+                }
                 None
             }
             BinaryOperator::Subtraction

crates/oxc_ecmascript/src/constant_evaluation/value_type.rs

@@ -4,7 +4,9 @@ use oxc_ast::ast::{
 };
 use oxc_syntax::operator::{BinaryOperator, UnaryOperator};
 
-use crate::is_global_reference::IsGlobalReference;
+use crate::{
+    is_global_reference::IsGlobalReference, to_numeric::ToNumeric, to_primitive::ToPrimitive,
+};
 
 /// JavaScript Language Type
 ///
@@ -135,21 +137,19 @@ impl DetermineValueType for BinaryExpression<'_> {
     fn value_type(&self, is_global_reference: &impl IsGlobalReference) -> ValueType {
         match self.operator {
             BinaryOperator::Addition => {
-                let left = self.left.value_type(is_global_reference);
-                let right = self.right.value_type(is_global_reference);
-                if left == ValueType::Boolean
-                    && matches!(right, ValueType::Undefined | ValueType::Null | ValueType::Number)
-                {
-                    return ValueType::Number;
-                }
-                if left == ValueType::String || right == ValueType::String {
+                let left = self.left.to_primitive(is_global_reference);
+                let right = self.right.to_primitive(is_global_reference);
+                if left.is_string() == Some(true) || right.is_string() == Some(true) {
                     return ValueType::String;
                 }
-                // There are some pretty weird cases for object types:
-                //   {} + [] === "0"
-                //   [] + {} === "[object Object]"
-                if left == ValueType::Object || right == ValueType::Object {
-                    return ValueType::Undetermined;
+                let left_to_numeric_type = left.to_numeric(is_global_reference);
+                let right_to_numeric_type = right.to_numeric(is_global_reference);
+                // we need to check both operands because the other operand might be undetermined and maybe a string
+                if left_to_numeric_type.is_number() && right_to_numeric_type.is_number() {
+                    return ValueType::Number;
+                }
+                if left_to_numeric_type.is_bigint() && right_to_numeric_type.is_bigint() {
+                    return ValueType::BigInt;
                 }
                 ValueType::Undetermined
             }
@@ -163,17 +163,14 @@ impl DetermineValueType for BinaryExpression<'_> {
             | BinaryOperator::BitwiseXOR
             | BinaryOperator::BitwiseAnd
             | BinaryOperator::Exponential => {
-                let left = self.left.value_type(is_global_reference);
-                let right = self.right.value_type(is_global_reference);
-                if left.is_bigint() || right.is_bigint() {
-                    ValueType::BigInt
-                } else if !(left.is_object() || left.is_undetermined())
-                    || !(right.is_object() || right.is_undetermined())
-                {
-                    // non-object values other than BigInt are converted to number by `ToNumber`
-                    // if either operand is a number, the result is always a number
-                    // because if the other operand is a bigint, an error is thrown
+                let left_to_numeric_type = self.left.to_numeric(is_global_reference);
+                let right_to_numeric_type = self.right.to_numeric(is_global_reference);
+                // if either operand is a number, the result is always a number
+                // because if the other operand is a bigint, an error is thrown
+                if left_to_numeric_type.is_number() || right_to_numeric_type.is_number() {
                     ValueType::Number
+                } else if left_to_numeric_type.is_bigint() || right_to_numeric_type.is_bigint() {
+                    ValueType::BigInt
                 } else {
                     ValueType::Undetermined
                 }

crates/oxc_ecmascript/src/lib.rs

@@ -20,6 +20,7 @@ mod to_boolean;
 mod to_int_32;
 mod to_integer_or_infinity;
 mod to_number;
+mod to_numeric;
 mod to_primitive;
 mod to_string;
 

crates/oxc_ecmascript/src/side_effects/may_have_side_effects.rs

@@ -1,6 +1,8 @@
 use oxc_ast::ast::*;
 
-use crate::{is_global_reference::IsGlobalReference, to_primitive::ToPrimitive};
+use crate::{
+    is_global_reference::IsGlobalReference, to_numeric::ToNumeric, to_primitive::ToPrimitive,
+};
 
 /// Returns true if subtree changes application state.
 ///
@@ -116,28 +118,26 @@ impl MayHaveSideEffects for BinaryExpression<'_> {
                 true
             }
             BinaryOperator::Addition => {
-                if self.left.to_primitive(is_global_reference).is_string() == Some(true)
-                    || self.right.to_primitive(is_global_reference).is_string() == Some(true)
-                {
-                    let other_side =
-                        if self.left.to_primitive(is_global_reference).is_string() == Some(true) {
-                            &self.right
-                        } else {
-                            &self.left
-                        };
-                    other_side.to_primitive(is_global_reference).is_symbol() != Some(false)
+                let left = self.left.to_primitive(is_global_reference);
+                let right = self.right.to_primitive(is_global_reference);
+                if left.is_string() == Some(true) || right.is_string() == Some(true) {
+                    // If either side is a string, ToString is called for both sides.
+                    let other_side = if left.is_string() == Some(true) { right } else { left };
+                    // ToString() for Symbols throws an error.
+                    return other_side.is_symbol() != Some(false)
                         || self.left.may_have_side_effects(is_global_reference)
+                        || self.right.may_have_side_effects(is_global_reference);
+                }
+
+                let left_to_numeric_type = left.to_numeric(is_global_reference);
+                let right_to_numeric_type = right.to_numeric(is_global_reference);
+                if (left_to_numeric_type.is_number() && right_to_numeric_type.is_number())
+                    || (left_to_numeric_type.is_bigint() && right_to_numeric_type.is_bigint())
+                {
+                    self.left.may_have_side_effects(is_global_reference)
                         || self.right.may_have_side_effects(is_global_reference)
-                } else if self.left.is_number() || self.right.is_number() {
-                    let other_side = if self.left.is_number() { &self.right } else { &self.left };
-                    !matches!(
-                        other_side,
-                        Expression::NullLiteral(_)
-                            | Expression::NumericLiteral(_)
-                            | Expression::BooleanLiteral(_)
-                    )
                 } else {
-                    !(self.left.is_big_int_literal() && self.right.is_big_int_literal())
+                    true
                 }
             }
             BinaryOperator::Subtraction
@@ -151,24 +151,37 @@ impl MayHaveSideEffects for BinaryExpression<'_> {
             | BinaryOperator::BitwiseAnd
             | BinaryOperator::Exponential
             | BinaryOperator::ShiftRightZeroFill => {
-                if self.left.is_big_int_literal() || self.right.is_big_int_literal() {
-                    if let (Expression::BigIntLiteral(_), Expression::BigIntLiteral(right)) =
-                        (&self.left, &self.right)
-                    {
-                        match self.operator {
-                            BinaryOperator::Exponential => right.is_negative(),
-                            BinaryOperator::Division | BinaryOperator::Remainder => right.is_zero(),
-                            BinaryOperator::ShiftRightZeroFill => true,
-                            _ => false,
+                let left_to_numeric_type = self.left.to_numeric(is_global_reference);
+                let right_to_numeric_type = self.right.to_numeric(is_global_reference);
+                if left_to_numeric_type.is_bigint() && right_to_numeric_type.is_bigint() {
+                    if self.operator == BinaryOperator::ShiftRightZeroFill {
+                        true
+                    } else if matches!(
+                        self.operator,
+                        BinaryOperator::Exponential
+                            | BinaryOperator::Division
+                            | BinaryOperator::Remainder
+                    ) {
+                        if let Expression::BigIntLiteral(right) = &self.right {
+                            match self.operator {
+                                BinaryOperator::Exponential => {
+                                    right.is_negative()
+                                        || self.left.may_have_side_effects(is_global_reference)
+                                }
+                                BinaryOperator::Division | BinaryOperator::Remainder => {
+                                    right.is_zero()
+                                        || self.left.may_have_side_effects(is_global_reference)
+                                }
+                                _ => unreachable!(),
+                            }
+                        } else {
+                            true
                         }
                     } else {
-                        true
+                        self.left.may_have_side_effects(is_global_reference)
+                            || self.right.may_have_side_effects(is_global_reference)
                     }
-                } else if self.left.to_primitive(is_global_reference).is_symbol_or_bigint()
-                    == Some(false)
-                    && self.right.to_primitive(is_global_reference).is_symbol_or_bigint()
-                        == Some(false)
-                {
+                } else if left_to_numeric_type.is_number() && right_to_numeric_type.is_number() {
                     self.left.may_have_side_effects(is_global_reference)
                         || self.right.may_have_side_effects(is_global_reference)
                 } else {

crates/oxc_ecmascript/src/to_numeric.rs

@@ -0,0 +1,53 @@
+use oxc_ast::ast::Expression;
+
+use crate::{
+    is_global_reference::IsGlobalReference,
+    to_primitive::{ToPrimitive, ToPrimitiveResult},
+};
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum ToNumericResult {
+    Number,
+    BigInt,
+    Undetermined,
+}
+
+impl ToNumericResult {
+    pub fn is_number(self) -> bool {
+        matches!(self, Self::Number)
+    }
+
+    pub fn is_bigint(self) -> bool {
+        matches!(self, Self::BigInt)
+    }
+}
+
+/// `ToNumeric`
+///
+/// <https://tc39.es/ecma262/multipage/abstract-operations.html#sec-tonumeric>
+pub trait ToNumeric<'a> {
+    fn to_numeric(&self, is_global_reference: &impl IsGlobalReference) -> ToNumericResult;
+}
+
+impl ToNumeric<'_> for Expression<'_> {
+    fn to_numeric(&self, is_global_reference: &impl IsGlobalReference) -> ToNumericResult {
+        self.to_primitive(is_global_reference).to_numeric(is_global_reference)
+    }
+}
+
+impl ToNumeric<'_> for ToPrimitiveResult {
+    fn to_numeric(&self, _is_global_reference: &impl IsGlobalReference) -> ToNumericResult {
+        match self {
+            // Symbol throws an error when passed to ToNumber in step 3
+            ToPrimitiveResult::Symbol | ToPrimitiveResult::Undetermined => {
+                ToNumericResult::Undetermined
+            }
+            ToPrimitiveResult::BigInt => ToNumericResult::BigInt,
+            ToPrimitiveResult::Boolean
+            | ToPrimitiveResult::Null
+            | ToPrimitiveResult::Number
+            | ToPrimitiveResult::String
+            | ToPrimitiveResult::Undefined => ToNumericResult::Number,
+        }
+    }
+}

crates/oxc_minifier/src/peephole/minimize_conditions.rs

@@ -1310,7 +1310,7 @@ mod test {
 
         test("(x = 1) === 1", "(x = 1) == 1");
         test("(x = 1) !== 1", "(x = 1) != 1");
-        test("!0 + null !== 1", "!0 + null != 1");
+        test("v = !0 + null !== 1", "v = !1");
     }
 
     #[test]

crates/oxc_minifier/tests/ecmascript/value_type.rs

@@ -120,16 +120,16 @@ fn binary_tests() {
     test("true + undefined", ValueType::Number);
     test("true + null", ValueType::Number);
     test("true + 0", ValueType::Number);
-    // test("undefined + true", ValueType::Number);
-    // test("null + true", ValueType::Number);
-    // test("0 + true", ValueType::Number);
+    test("undefined + true", ValueType::Number);
+    test("null + true", ValueType::Number);
+    test("0 + true", ValueType::Number);
     test("true + 0n", ValueType::Undetermined); // throws an error
-    test("({} + [])", ValueType::Undetermined);
-    test("[] + {}", ValueType::Undetermined);
+    test("({} + [])", ValueType::String); // [object Object]
+    test("[] + {}", ValueType::String); // [object Object]
 
     test("1 - 0", ValueType::Number);
     test("1n - 0n", ValueType::BigInt);
-    test("1 - 0n", ValueType::BigInt); // throws an error
+    test("1 - 0n", ValueType::Number); // throws an error
     test("foo - 1", ValueType::Number);
     test("foo - 1n", ValueType::BigInt);
     test("foo - undefined", ValueType::Number);

crates/oxc_minifier/tests/peephole/esbuild.rs

@@ -230,7 +230,7 @@ fn js_parser_test() {
     test("while (x) { if (y) continue; var y }", "for (; x; ) if (!y) var y; ");
     test("console.log(undefined)", "console.log(void 0);");
     test("console.log(+undefined)", "console.log(NaN);");
-    test("console.log(undefined + undefined)", "console.log(void 0 + void 0);");
+    test("console.log(undefined + undefined)", "console.log(NaN);");
     test("const x = undefined", "const x = void 0;");
     test("let x = undefined", "let x;");
     test("var x = undefined", "var x = void 0;");
@@ -339,9 +339,9 @@ fn js_parser_test() {
     test("a = '' + 1n", "a = '1';");
     test("a = '' + 123n", "a = '123';");
     test("a = '' + 1_2_3n", "a = '123';");
-    // test("a = '' + 0b0n", "a = '' + 0b0n;");
-    // test("a = '' + 0o0n", "a = '' + 0o0n;");
-    // test("a = '' + 0x0n", "a = '' + 0x0n;");
+    test("a = '' + 0b0n", "a = '0';");
+    test("a = '' + 0o0n", "a = '0';");
+    test("a = '' + 0x0n", "a = '0';");
     // test("a = '' + /a\\b/ig", "a = '/a\\\\b/ig';");
     // test("a = /a\\b/ig + ''", "a = '/a\\\\b/ig';");
     // test("a = '' + ''.constructor", "a = 'function String() { [native code] }';");