feat(minifier): port undefined_comparison1 (#458)

crates/oxc_hir/src/hir_util.rs

@@ -101,19 +101,39 @@ pub trait CheckForStateChange<'a, 'b> {
 }
 
 impl<'a, 'b> CheckForStateChange<'a, 'b> for Expression<'a> {
-    fn check_for_state_change(&self, _check_for_new_objects: bool) -> bool {
+    fn check_for_state_change(&self, check_for_new_objects: bool) -> bool {
         match self {
             Self::NumberLiteral(_)
             | Self::BooleanLiteral(_)
             | Self::StringLiteral(_)
             | Self::BigintLiteral(_)
             | Self::NullLiteral(_)
-            | Self::RegExpLiteral(_) => false,
+            | Self::RegExpLiteral(_)
+            | Self::FunctionExpression(_) => false,
             Self::Identifier(ident) => {
                 !matches!(ident.name.as_str(), "undefined" | "Infinity" | "NaN")
             }
             Self::UnaryExpression(unary_expr) => {
-                unary_expr.check_for_state_change(_check_for_new_objects)
+                unary_expr.check_for_state_change(check_for_new_objects)
+            }
+            Self::ObjectExpression(object_expr) => {
+                if check_for_new_objects {
+                    return true;
+                }
+
+                object_expr
+                    .properties
+                    .iter()
+                    .any(|property| property.check_for_state_change(check_for_new_objects))
+            }
+            Self::ArrayExpression(array_expr) => {
+                if check_for_new_objects {
+                    return true;
+                }
+                array_expr
+                    .elements
+                    .iter()
+                    .any(|element| element.check_for_state_change(check_for_new_objects))
             }
             _ => true,
         }
@@ -121,14 +141,60 @@ impl<'a, 'b> CheckForStateChange<'a, 'b> for Expression<'a> {
 }
 
 impl<'a, 'b> CheckForStateChange<'a, 'b> for UnaryExpression<'a> {
-    fn check_for_state_change(&self, _check_for_new_objects: bool) -> bool {
+    fn check_for_state_change(&self, check_for_new_objects: bool) -> bool {
         if is_simple_unary_operator(self.operator) {
-            return self.argument.check_for_state_change(_check_for_new_objects);
+            return self.argument.check_for_state_change(check_for_new_objects);
         }
         true
     }
 }
 
+impl<'a, 'b> CheckForStateChange<'a, 'b> for ArrayExpressionElement<'a> {
+    fn check_for_state_change(&self, check_for_new_objects: bool) -> bool {
+        match self {
+            Self::SpreadElement(element) => element.check_for_state_change(check_for_new_objects),
+            Self::Expression(expr) => expr.check_for_state_change(check_for_new_objects),
+            Self::Elision(_) => false,
+        }
+    }
+}
+
+impl<'a, 'b> CheckForStateChange<'a, 'b> for ObjectPropertyKind<'a> {
+    fn check_for_state_change(&self, check_for_new_objects: bool) -> bool {
+        match self {
+            Self::ObjectProperty(method) => method.check_for_state_change(check_for_new_objects),
+            Self::SpreadProperty(spread_element) => {
+                spread_element.check_for_state_change(check_for_new_objects)
+            }
+        }
+    }
+}
+
+impl<'a, 'b> CheckForStateChange<'a, 'b> for SpreadElement<'a> {
+    fn check_for_state_change(&self, _check_for_new_objects: bool) -> bool {
+        // Object-rest and object-spread may trigger a getter.
+        // TODO: Closure Compiler assumes that getters may side-free when set `assumeGettersArePure`.
+        // https://github.com/google/closure-compiler/blob/a4c880032fba961f7a6c06ef99daa3641810bfdd/src/com/google/javascript/jscomp/AstAnalyzer.java#L282
+        true
+    }
+}
+
+impl<'a, 'b> CheckForStateChange<'a, 'b> for ObjectProperty<'a> {
+    fn check_for_state_change(&self, check_for_new_objects: bool) -> bool {
+        self.key.check_for_state_change(check_for_new_objects)
+            || self.value.check_for_state_change(check_for_new_objects)
+    }
+}
+
+impl<'a, 'b> CheckForStateChange<'a, 'b> for PropertyKey<'a> {
+    fn check_for_state_change(&self, check_for_new_objects: bool) -> bool {
+        match self {
+            Self::Identifier(_) | Self::PrivateIdentifier(_) => false,
+            Self::Expression(expr) => expr.check_for_state_change(check_for_new_objects),
+        }
+    }
+}
+
 impl<'a, 'b> MayHaveSideEffects<'a, 'b> for Expression<'a> {}
 impl<'a, 'b> MayHaveSideEffects<'a, 'b> for UnaryExpression<'a> {}
 
@@ -137,33 +203,90 @@ fn is_simple_unary_operator(operator: UnaryOperator) -> bool {
     operator != UnaryOperator::Delete
 }
 
+#[derive(PartialEq)]
+pub enum NumberValue {
+    Number(f64),
+    PositiveInfinity,
+    NegativeInfinity,
+    NaN,
+}
+
+impl NumberValue {
+    #[must_use]
+    pub fn not(&self) -> Self {
+        match self {
+            Self::Number(num) => Self::Number(-num),
+            Self::PositiveInfinity => Self::NegativeInfinity,
+            Self::NegativeInfinity => Self::PositiveInfinity,
+            Self::NaN => Self::NaN,
+        }
+    }
+}
+
 /// port from [closure compiler](https://github.com/google/closure-compiler/blob/a4c880032fba961f7a6c06ef99daa3641810bfdd/src/com/google/javascript/jscomp/NodeUtil.java#L348)
 /// Gets the value of a node as a Number, or None if it cannot be converted.
-pub fn get_number_value(expr: &Expression) -> Option<f64> {
+/// This method does not consider whether `expr` may have side effects.
+pub fn get_number_value(expr: &Expression) -> Option<NumberValue> {
     match expr {
-        Expression::NumberLiteral(number_literal) => Some(number_literal.value),
+        Expression::NumberLiteral(number_literal) => {
+            Some(NumberValue::Number(number_literal.value))
+        }
         Expression::UnaryExpression(unary_expr) => match unary_expr.operator {
             UnaryOperator::UnaryPlus => get_number_value(&unary_expr.argument),
-            UnaryOperator::UnaryNegation => get_number_value(&unary_expr.argument).map(|v| -v),
-            UnaryOperator::BitwiseNot => get_number_value(&unary_expr.argument)
-                .map(|value| f64::from(!NumberLiteral::ecmascript_to_int32(value))),
-            UnaryOperator::LogicalNot => {
-                get_boolean_value(expr).map(|boolean| if boolean { 1.0 } else { 0.0 })
-            }
+            UnaryOperator::UnaryNegation => get_number_value(&unary_expr.argument).map(|v| v.not()),
+            UnaryOperator::BitwiseNot => get_number_value(&unary_expr.argument).map(|value| {
+                match value {
+                    NumberValue::Number(num) => {
+                        NumberValue::Number(f64::from(!NumberLiteral::ecmascript_to_int32(num)))
+                    }
+                    // ~Infinity -> -1
+                    // ~-Infinity -> -1
+                    // ~NaN -> -1
+                    _ => NumberValue::Number(-1_f64),
+                }
+            }),
+            UnaryOperator::LogicalNot => get_boolean_value(expr)
+                .map(|boolean| if boolean { 1_f64 } else { 0_f64 })
+                .map(NumberValue::Number),
+            UnaryOperator::Void => Some(NumberValue::NaN),
             _ => None,
         },
         Expression::BooleanLiteral(bool_literal) => {
             if bool_literal.value {
-                Some(1.0)
+                Some(NumberValue::Number(1.0))
             } else {
-                Some(0.0)
+                Some(NumberValue::Number(0.0))
             }
         }
-        Expression::NullLiteral(_) => Some(0.0),
+        Expression::NullLiteral(_) => Some(NumberValue::Number(0.0)),
+        Expression::Identifier(ident) => match ident.name.as_str() {
+            "Infinity" => Some(NumberValue::PositiveInfinity),
+            "NaN" | "undefined" => Some(NumberValue::NaN),
+            _ => None,
+        },
+        // TODO: will be implemented in next PR, just for test pass now.
+        Expression::StringLiteral(string_literal) => string_literal
+            .value
+            .parse::<f64>()
+            .map_or(Some(NumberValue::NaN), |num| Some(NumberValue::Number(num))),
         _ => None,
     }
 }
 
+/// port from [closure compiler](https://github.com/google/closure-compiler/blob/a4c880032fba961f7a6c06ef99daa3641810bfdd/src/com/google/javascript/jscomp/AbstractPeepholeOptimization.java#L104-L114)
+/// Returns the number value of the node if it has one and it cannot have side effects.
+pub fn get_side_free_number_value(expr: &Expression) -> Option<NumberValue> {
+    let value = get_number_value(expr);
+    // Calculating the number value, if any, is likely to be faster than calculating side effects,
+    // and there are only a very few cases where we can compute a number value, but there could
+    // also be side effects. e.g. `void doSomething()` has value NaN, regardless of the behavior
+    // of `doSomething()`
+    if value.is_some() && !expr.may_have_side_effects() {
+        return value;
+    }
+    None
+}
+
 /// port from [closure compiler](https://github.com/google/closure-compiler/blob/a4c880032fba961f7a6c06ef99daa3641810bfdd/src/com/google/javascript/jscomp/NodeUtil.java#L109)
 /// Gets the boolean value of a node that represents an expression, or `None` if no
 /// such value can be determined by static analysis.
@@ -239,7 +362,7 @@ pub fn get_boolean_value(expr: &Expression) -> Option<bool> {
                 // +1 -> true
                 // +0 -> false
                 // -0 -> false
-                get_number_value(expr).map(|value| value != 0.0)
+                get_number_value(expr).map(|value| value != NumberValue::Number(0_f64))
             } else if unary_expr.operator == UnaryOperator::LogicalNot {
                 // !true -> false
                 get_boolean_value(&unary_expr.argument).map(|boolean| !boolean)

crates/oxc_minifier/src/compressor/fold.rs

@@ -4,7 +4,10 @@
 
 #[allow(clippy::wildcard_imports)]
 use oxc_hir::hir::*;
-use oxc_hir::hir_util::{get_boolean_value, get_number_value, IsLiteralValue, MayHaveSideEffects};
+use oxc_hir::hir_util::{
+    get_boolean_value, get_number_value, get_side_free_number_value, IsLiteralValue,
+    MayHaveSideEffects, NumberValue,
+};
 use oxc_span::{Atom, Span};
 use oxc_syntax::{
     operator::{BinaryOperator, UnaryOperator},
@@ -21,6 +24,20 @@ enum Tri {
     Unknown,
 }
 
+impl Tri {
+    pub fn not(self) -> Self {
+        match self {
+            Self::True => Self::False,
+            Self::False => Self::True,
+            Self::Unknown => Self::Unknown,
+        }
+    }
+
+    pub fn for_boolean(boolean: bool) -> Self {
+        if boolean { Self::True } else { Self::False }
+    }
+}
+
 /// JavaScript Language Type
 ///
 /// <https://tc39.es/ecma262/#sec-ecmascript-language-types>
@@ -44,10 +61,26 @@ impl<'a> From<&Expression<'a>> for Ty {
             Expression::BooleanLiteral(_) => Self::Boolean,
             Expression::NullLiteral(_) => Self::Null,
             Expression::NumberLiteral(_) => Self::Number,
-            Expression::ObjectExpression(_) => Self::Object,
             Expression::StringLiteral(_) => Self::Str,
+            Expression::ObjectExpression(_)
+            | Expression::ArrayExpression(_)
+            | Expression::RegExpLiteral(_)
+            | Expression::FunctionExpression(_) => Self::Object,
             Expression::Identifier(ident) => match ident.name.as_str() {
                 "undefined" => Self::Void,
+                "NaN" | "Infinity" => Self::Number,
+                _ => Self::Undetermined,
+            },
+            Expression::UnaryExpression(unary_expr) => match unary_expr.operator {
+                UnaryOperator::Void => Self::Void,
+                UnaryOperator::UnaryNegation => {
+                    let argument_ty = Self::from(&unary_expr.argument);
+                    if argument_ty == Self::BigInt {
+                        return Self::BigInt;
+                    }
+                    Self::Number
+                }
+                UnaryOperator::LogicalNot => Self::Boolean,
                 _ => Self::Undetermined,
             },
             _ => Self::Undetermined,
@@ -59,7 +92,14 @@ impl<'a> Compressor<'a> {
     pub(crate) fn fold_expression<'b>(&mut self, expr: &'b mut Expression<'a>) {
         let folded_expr = match expr {
             Expression::BinaryExpression(binary_expr) => match binary_expr.operator {
-                BinaryOperator::Equality => self.try_fold_comparison(
+                BinaryOperator::Equality
+                | BinaryOperator::Inequality
+                | BinaryOperator::StrictEquality
+                | BinaryOperator::StrictInequality
+                | BinaryOperator::LessThan
+                | BinaryOperator::LessEqualThan
+                | BinaryOperator::GreaterThan
+                | BinaryOperator::GreaterEqualThan => self.try_fold_comparison(
                     binary_expr.span,
                     binary_expr.operator,
                     &binary_expr.left,
@@ -69,8 +109,6 @@ impl<'a> Compressor<'a> {
             },
             Expression::UnaryExpression(unary_expr) => match unary_expr.operator {
                 UnaryOperator::Typeof => {
-                    // typeof +-~! 0 -> typeof 2
-                    self.fold_expression(&mut unary_expr.argument);
                     self.try_fold_typeof(unary_expr.span, &unary_expr.argument)
                 }
                 UnaryOperator::UnaryPlus
@@ -112,8 +150,27 @@ impl<'a> Compressor<'a> {
         left: &'b Expression<'a>,
         right: &'b Expression<'a>,
     ) -> Tri {
+        if left.may_have_side_effects() || right.may_have_side_effects() {
+            return Tri::Unknown;
+        }
+
         match op {
             BinaryOperator::Equality => self.try_abstract_equality_comparison(left, right),
+            BinaryOperator::Inequality => self.try_abstract_equality_comparison(left, right).not(),
+            BinaryOperator::StrictEquality => self.try_strict_equality_comparison(left, right),
+            BinaryOperator::StrictInequality => {
+                self.try_strict_equality_comparison(left, right).not()
+            }
+            BinaryOperator::LessThan => self.try_abstract_relational_comparison(left, right, false),
+            BinaryOperator::GreaterThan => {
+                self.try_abstract_relational_comparison(right, left, false)
+            }
+            BinaryOperator::LessEqualThan => {
+                self.try_abstract_relational_comparison(right, left, true).not()
+            }
+            BinaryOperator::GreaterEqualThan => {
+                self.try_abstract_relational_comparison(left, right, true).not()
+            }
             _ => Tri::Unknown,
         }
     }
@@ -133,10 +190,33 @@ impl<'a> Compressor<'a> {
             if matches!((left, right), (Ty::Null, Ty::Void) | (Ty::Void, Ty::Null)) {
                 return Tri::True;
             }
+
+            return Tri::False;
         }
         Tri::Unknown
     }
 
+    /// <https://tc39.es/ecma262/#sec-abstract-relational-comparison>
+    fn try_abstract_relational_comparison<'b>(
+        &self,
+        left: &'b Expression<'a>,
+        right: &'b Expression<'a>,
+        will_negative: bool,
+    ) -> Tri {
+        // try comparing as Numbers.
+        let left_num = get_side_free_number_value(left);
+        let right_num = get_side_free_number_value(right);
+        if let Some(left_num) = left_num && let Some(right_num) = right_num {
+            match (left_num, right_num) {
+                (NumberValue::NaN, _) | (_, NumberValue::NaN) => return Tri::for_boolean(will_negative),
+                (NumberValue::Number(left_num), NumberValue::Number(right_num)) => return Tri::for_boolean(left_num < right_num),
+                _ => {}
+            }
+        }
+
+        Tri::Unknown
+    }
+
     /// <https://tc39.es/ecma262/#sec-strict-equality-comparison>
     fn try_strict_equality_comparison<'b>(
         &self,
@@ -177,10 +257,13 @@ impl<'a> Compressor<'a> {
                 | Expression::ObjectExpression(_)
                 | Expression::ArrayExpression(_) => Some("object"),
                 Expression::Identifier(_) if argument.is_undefined() => Some("undefined"),
-                Expression::UnaryExpression(unary_expr)
-                    if unary_expr.operator == UnaryOperator::Void =>
-                {
-                    Some("undefined")
+                Expression::UnaryExpression(unary_expr) => {
+                    match unary_expr.operator {
+                        UnaryOperator::Void => Some("undefined"),
+                        // `unary_expr.argument` is literal value, so it's safe to fold
+                        UnaryOperator::LogicalNot => Some("boolean"),
+                        _ => None,
+                    }
                 }
                 _ => None,
             };
@@ -198,9 +281,6 @@ impl<'a> Compressor<'a> {
         &mut self,
         unary_expr: &'b mut UnaryExpression<'a>,
     ) -> Option<Expression<'a>> {
-        // fold its children first, so that we can fold - -4.
-        self.fold_expression(&mut unary_expr.argument);
-
         if let Some(boolean) = get_boolean_value(&unary_expr.argument) {
             match unary_expr.operator {
                 // !100 -> false
@@ -238,7 +318,8 @@ impl<'a> Compressor<'a> {
                         // +true -> 1
                         // +false -> 0
                         // +null -> 0
-                        if let Some(value) = get_number_value(&unary_expr.argument) {
+                        if let Some(value) = get_number_value(&unary_expr.argument)
+                            && let NumberValue::Number(value) = value {
                             let raw = self.hir.new_str(value.to_string().as_str());
                             let number_literal = self.hir.number_literal(
                                 unary_expr.span,

crates/oxc_minifier/src/compressor/mod.rs

@@ -279,11 +279,11 @@ impl<'a, 'b> VisitMut<'a, 'b> for Compressor<'a> {
     }
 
     fn visit_expression(&mut self, expr: &'b mut Expression<'a>) {
-        self.fold_expression(expr);
-        if self.compress_undefined(expr) || self.compress_boolean(expr) {
-            return;
-        }
         self.visit_expression_match(expr);
+        self.fold_expression(expr);
+        if !self.compress_undefined(expr) {
+            self.compress_boolean(expr);
+        }
     }
 
     fn visit_binary_expression(&mut self, expr: &'b mut BinaryExpression<'a>) {

crates/oxc_minifier/tests/closure/fold_constants.rs

@@ -5,6 +5,101 @@ use crate::{test, test_same};
 #[test]
 fn undefined_comparison1() {
     test("undefined == undefined", "!0");
+    test("undefined == null", "!0");
+    test("undefined == void 0", "!0");
+
+    test("undefined == 0", "!1");
+    test("undefined == 1", "!1");
+    test("undefined == 'hi'", "!1");
+    test("undefined == true", "!1");
+    test("undefined == false", "!1");
+
+    test("undefined === undefined", "!0");
+    test("undefined === null", "!1");
+    test("undefined === void 0", "!0");
+
+    // origin was `test_same("undefined == this");`
+    test("undefined == this", "void 0==this");
+    // origin was `test_same("undefined == x");`
+    test("undefined == x", "void 0==x");
+
+    test("undefined != undefined", "!1");
+    test("undefined != null", "!1");
+    test("undefined != void 0", "!1");
+
+    test("undefined != 0", "!0");
+    test("undefined != 1", "!0");
+    test("undefined != 'hi'", "!0");
+    test("undefined != true", "!0");
+    test("undefined != false", "!0");
+
+    test("undefined !== undefined", "!1");
+    test("undefined !== void 0", "!1");
+    test("undefined !== null", "!0");
+
+    // origin was `test_same("undefined != this");`
+    test("undefined != this", "void 0!=this");
+    // origin was `test_same("undefined != x");`
+    test("undefined != x", "void 0!=x");
+
+    test("undefined < undefined", "!1");
+    test("undefined > undefined", "!1");
+    test("undefined >= undefined", "!1");
+    test("undefined <= undefined", "!1");
+
+    test("0 < undefined", "!1");
+    test("true > undefined", "!1");
+    test("'hi' >= undefined", "!1");
+    test("null <= undefined", "!1");
+
+    test("undefined < 0", "!1");
+    test("undefined > true", "!1");
+    test("undefined >= 'hi'", "!1");
+    test("undefined <= null", "!1");
+
+    test("null == undefined", "!0");
+    test("0 == undefined", "!1");
+    test("1 == undefined", "!1");
+    test("'hi' == undefined", "!1");
+    test("true == undefined", "!1");
+    test("false == undefined", "!1");
+    test("null === undefined", "!1");
+    test("void 0 === undefined", "!0");
+
+    test("undefined == NaN", "!1");
+    test("NaN == undefined", "!1");
+    test("undefined == Infinity", "!1");
+    test("Infinity == undefined", "!1");
+    test("undefined == -Infinity", "!1");
+    test("-Infinity == undefined", "!1");
+    test("({}) == undefined", "!1");
+    test("undefined == ({})", "!1");
+    test("([]) == undefined", "!1");
+    test("undefined == ([])", "!1");
+    test("(/a/g) == undefined", "!1");
+    test("undefined == (/a/g)", "!1");
+    test("(function(){}) == undefined", "!1");
+    test("undefined == (function(){})", "!1");
+
+    test("undefined != NaN", "!0");
+    test("NaN != undefined", "!0");
+    test("undefined != Infinity", "!0");
+    test("Infinity != undefined", "!0");
+    test("undefined != -Infinity", "!0");
+    test("-Infinity != undefined", "!0");
+    test("({}) != undefined", "!0");
+    test("undefined != ({})", "!0");
+    test("([]) != undefined", "!0");
+    test("undefined != ([])", "!0");
+    test("(/a/g) != undefined", "!0");
+    test("undefined != (/a/g)", "!0");
+    test("(function(){}) != undefined", "!0");
+    test("undefined != (function(){})", "!0");
+
+    // origin was `test_same("this == undefined");`
+    test("this == undefined", "this==void 0");
+    // origin was `test_same("x == undefined");`
+    test("x == undefined", "x==void 0");
 }
 
 #[test]