From 660c314f4ee449518bbc6ff23c8e515f8c5a820f Mon Sep 17 00:00:00 2001 From: sapphi-red <49056869+sapphi-red@users.noreply.github.com> Date: Mon, 10 Feb 2025 05:36:12 +0000 Subject: [PATCH] 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. --- .../src/side_effects/may_have_side_effects.rs | 53 +++++++++++++---- .../tests/ecmascript/may_have_side_effects.rs | 59 +++++++++++++------ 2 files changed, 83 insertions(+), 29 deletions(-) diff --git a/crates/oxc_ecmascript/src/side_effects/may_have_side_effects.rs b/crates/oxc_ecmascript/src/side_effects/may_have_side_effects.rs index fd76a004f95c5..a26c1db78a198 100644 --- a/crates/oxc_ecmascript/src/side_effects/may_have_side_effects.rs +++ b/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 { diff --git a/crates/oxc_minifier/tests/ecmascript/may_have_side_effects.rs b/crates/oxc_minifier/tests/ecmascript/may_have_side_effects.rs index d668cbdd97e71..895cd346d75d7 100644 --- a/crates/oxc_minifier/tests/ecmascript/may_have_side_effects.rs +++ b/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