fix(ecmascript): fix may_have_side_effects for unary expressions (#8989)

Fixes the FIXME test cases related to unary expressions in may_have_side_effects.

Author: sapphi-red

crates/oxc_ecmascript/src/side_effects/may_have_side_effects.rs

@@ -60,17 +60,50 @@ pub trait MayHaveSideEffects {
     }
 
     fn unary_expression_may_have_side_effects(&self, e: &UnaryExpression<'_>) -> bool {
-        /// A "simple" operator is one whose children are expressions, has no direct side-effects.
-        fn is_simple_unary_operator(operator: UnaryOperator) -> bool {
-            operator != UnaryOperator::Delete
-        }
-        if e.operator == UnaryOperator::Typeof && matches!(&e.argument, Expression::Identifier(_)) {
-            return false;
-        }
-        if is_simple_unary_operator(e.operator) {
-            return self.expression_may_have_side_effects(&e.argument);
+        match e.operator {
+            UnaryOperator::Delete => true,
+            UnaryOperator::Void | UnaryOperator::LogicalNot => {
+                self.expression_may_have_side_effects(&e.argument)
+            }
+            UnaryOperator::Typeof => {
+                if matches!(&e.argument, Expression::Identifier(_)) {
+                    false
+                } else {
+                    self.expression_may_have_side_effects(&e.argument)
+                }
+            }
+            UnaryOperator::UnaryPlus => {
+                match &e.argument {
+                    Expression::NumericLiteral(_)
+                    | Expression::NullLiteral(_)
+                    | Expression::BooleanLiteral(_)
+                    | Expression::StringLiteral(_) => false,
+                    Expression::Identifier(ident) => {
+                        !(matches!(ident.name.as_str(), "Infinity" | "NaN" | "undefined")
+                            && self.is_global_reference(ident))
+                    }
+                    // ToNumber throws an error when the argument is Symbol / BigInt / an object that
+                    // returns Symbol or BigInt from ToPrimitive
+                    _ => true,
+                }
+            }
+            UnaryOperator::UnaryNegation | UnaryOperator::BitwiseNot => {
+                match &e.argument {
+                    Expression::BigIntLiteral(_)
+                    | Expression::NumericLiteral(_)
+                    | Expression::NullLiteral(_)
+                    | Expression::BooleanLiteral(_)
+                    | Expression::StringLiteral(_) => false,
+                    Expression::Identifier(ident) => {
+                        !(matches!(ident.name.as_str(), "Infinity" | "NaN" | "undefined")
+                            && self.is_global_reference(ident))
+                    }
+                    // ToNumber throws an error when the argument is Symbol an object that
+                    // returns Symbol from ToPrimitive
+                    _ => true,
+                }
+            }
         }
-        true
     }
 
     fn binary_expression_may_have_side_effects(&self, e: &BinaryExpression<'_>) -> bool {

crates/oxc_minifier/tests/ecmascript/may_have_side_effects.rs

@@ -120,9 +120,9 @@ fn closure_compiler_tests() {
     test("undefined", false);
     test("void 0", false);
     test("void foo()", true);
-    test("-Infinity", false);
-    test("Infinity", false);
-    test("NaN", false);
+    test_with_global_variables("-Infinity", vec!["Infinity".to_string()], false);
+    test_with_global_variables("Infinity", vec!["Infinity".to_string()], false);
+    test_with_global_variables("NaN", vec!["NaN".to_string()], false);
     // test("({}||[]).foo = 2;", false);
     // test("(true ? {} : []).foo = 2;", false);
     // test("({},[]).foo = 2;", false);
@@ -344,21 +344,48 @@ fn test_simple_expressions() {
 
 #[test]
 fn test_unary_expressions() {
-    test("+'foo'", false);
-    test("+foo()", true);
-    test("-'foo'", false);
-    test("-foo()", true);
+    test("delete 'foo'", true);
+    test("delete foo()", true);
+
+    test("void 'foo'", false);
+    test("void foo()", true);
     test("!'foo'", false);
     test("!foo()", true);
-    test("~'foo'", false);
-    test("~foo()", true);
+
     test("typeof 'foo'", false);
     test_with_global_variables("typeof a", vec!["a".to_string()], false);
     test("typeof foo()", true);
-    test("void 'foo'", false);
-    test("void foo()", true);
-    test("delete 'foo'", true);
-    test("delete foo()", true);
+
+    test("+0", false);
+    test("+0n", true);
+    test("+null", false); // 0
+    test("+true", false); // 1
+    test("+'foo'", false); // NaN
+    test_with_global_variables("+Infinity", vec!["Infinity".to_string()], false);
+    test_with_global_variables("+NaN", vec!["NaN".to_string()], false);
+    test_with_global_variables("+undefined", vec!["undefined".to_string()], false); // NaN
+    test("+foo()", true);
+    test("+foo", true); // foo can be Symbol or BigInt
+    test("+Symbol()", true);
+    test("+{ valueOf() { return Symbol() } }", true);
+
+    test("-0", false);
+    test("-0n", false);
+    test("-null", false); // -0
+    test("-true", false); // -1
+    test("-'foo'", false); // -NaN
+    test_with_global_variables("-Infinity", vec!["Infinity".to_string()], false);
+    test_with_global_variables("-NaN", vec!["NaN".to_string()], false);
+    test_with_global_variables("-undefined", vec!["undefined".to_string()], false); // NaN
+    test("-foo()", true);
+    test("-foo", true); // foo can be Symbol
+    test("-Symbol()", true);
+    test("-{ valueOf() { return Symbol() } }", true);
+
+    test("~0", false);
+    test("~'foo'", false);
+    test("~foo()", true);
+    test("~foo", true);
 }
 
 #[test]
@@ -477,12 +504,6 @@ fn tests() {
     test("'' + { valueOf() { console.log('sideeffect') } }", false);
     test("'' + { [s]() { console.log('sideeffect') } }", false); // assuming s is Symbol.toPrimitive
 
-    // FIXME: actually these have a side effect, but it's ignored now
-    test("+s", false); // assuming s is a Symbol
-    test("+b", false); // assuming b is a BitInt
-    test("+{ valueOf() { return Symbol() } }", false);
-    test("~s", false); // assuming s is a Symbol
-
     // FIXME: actually these have a side effect, but it's ignored now
     // same for -, *, /, %, **, <<, >>
     test("'' + s", false); // assuming s is Symbol