diff --git a/crates/oxc_ast/src/ast_impl/literal.rs b/crates/oxc_ast/src/ast_impl/literal.rs index 3f52a6c115b89..38fa73d26d715 100644 --- a/crates/oxc_ast/src/ast_impl/literal.rs +++ b/crates/oxc_ast/src/ast_impl/literal.rs @@ -122,6 +122,11 @@ impl BigIntLiteral<'_> { pub fn is_zero(&self) -> bool { self.raw == "0n" } + + /// Is this BigInt literal negative? (e.g. `-1n`). + pub fn is_negative(&self) -> bool { + self.raw.starts_with('-') + } } impl fmt::Display for BigIntLiteral<'_> { 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 82b6a89071576..30e934e754f44 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 @@ -4,11 +4,12 @@ use oxc_ast::ast::*; /// /// This trait assumes the following: /// - `.toString()`, `.valueOf()`, and `[Symbol.toPrimitive]()` are side-effect free. +/// - This is mainly to assume `ToPrimitive` is side-effect free. /// - Errors thrown when creating a String or an Array that exceeds the maximum length does not happen. /// - TDZ errors does not happen. /// /// Ported from [closure-compiler](https://github.com/google/closure-compiler/blob/f3ce5ed8b630428e311fe9aa2e20d36560d975e2/src/com/google/javascript/jscomp/AstAnalyzer.java#L94) -pub trait MayHaveSideEffects { +pub trait MayHaveSideEffects: Sized { fn is_global_reference(&self, ident: &IdentifierReference<'_>) -> bool; fn expression_may_have_side_effects(&self, e: &Expression<'_>) -> bool { @@ -73,58 +74,99 @@ pub trait MayHaveSideEffects { } } 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)) - } - Expression::ArrayExpression(arr) => { - self.array_expression_may_have_side_effects(arr) - } - // unless `Symbol.toPrimitive`, `valueOf`, `toString` is overridden, - // ToPrimitive for an object returns `"[object Object]"` - Expression::ObjectExpression(obj) => !obj.properties.is_empty(), - // ToNumber throws an error when the argument is Symbol / BigInt / an object that - // returns Symbol or BigInt from ToPrimitive - _ => true, - } + // ToNumber throws an error when the argument is Symbol / BigInt / an object that + // returns Symbol or BigInt from ToPrimitive + maybe_symbol_or_bigint_or_to_primitive_may_return_symbol_or_bigint( + self, + &e.argument, + ) || self.expression_may_have_side_effects(&e.argument) } 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)) - } - Expression::ArrayExpression(arr) => { - self.array_expression_may_have_side_effects(arr) - } - // unless `Symbol.toPrimitive`, `valueOf`, `toString` is overridden, - // ToPrimitive for an object returns `"[object Object]"` - Expression::ObjectExpression(obj) => !obj.properties.is_empty(), - // ToNumber throws an error when the argument is Symbol an object that - // returns Symbol from ToPrimitive - _ => true, - } + // ToNumeric throws an error when the argument is Symbol / an object that + // returns Symbol from ToPrimitive + maybe_symbol_or_to_primitive_may_return_symbol(self, &e.argument) + || self.expression_may_have_side_effects(&e.argument) } } } fn binary_expression_may_have_side_effects(&self, e: &BinaryExpression<'_>) -> bool { - // `instanceof` and `in` can throw `TypeError` - if matches!(e.operator, BinaryOperator::In | BinaryOperator::Instanceof) { - return true; + match e.operator { + BinaryOperator::Equality + | BinaryOperator::Inequality + | BinaryOperator::StrictEquality + | BinaryOperator::StrictInequality + | BinaryOperator::LessThan + | BinaryOperator::LessEqualThan + | BinaryOperator::GreaterThan + | BinaryOperator::GreaterEqualThan => { + self.expression_may_have_side_effects(&e.left) + || self.expression_may_have_side_effects(&e.right) + } + BinaryOperator::In | BinaryOperator::Instanceof => { + // instanceof and in can throw `TypeError` + true + } + BinaryOperator::Addition => { + if is_string_or_to_primitive_returns_string(&e.left) + || is_string_or_to_primitive_returns_string(&e.right) + { + let other_side = if is_string_or_to_primitive_returns_string(&e.left) { + &e.right + } else { + &e.left + }; + maybe_symbol_or_to_primitive_may_return_symbol(self, other_side) + || self.expression_may_have_side_effects(&e.left) + || self.expression_may_have_side_effects(&e.right) + } else if e.left.is_number() || e.right.is_number() { + let other_side = if e.left.is_number() { &e.right } else { &e.left }; + !matches!( + other_side, + Expression::NullLiteral(_) + | Expression::NumericLiteral(_) + | Expression::BooleanLiteral(_) + ) + } else { + !(e.left.is_big_int_literal() && e.right.is_big_int_literal()) + } + } + BinaryOperator::Subtraction + | BinaryOperator::Multiplication + | BinaryOperator::Division + | BinaryOperator::Remainder + | BinaryOperator::ShiftLeft + | BinaryOperator::BitwiseOR + | BinaryOperator::ShiftRight + | BinaryOperator::BitwiseXOR + | BinaryOperator::BitwiseAnd + | BinaryOperator::Exponential + | BinaryOperator::ShiftRightZeroFill => { + if e.left.is_big_int_literal() || e.right.is_big_int_literal() { + if let (Expression::BigIntLiteral(_), Expression::BigIntLiteral(right)) = + (&e.left, &e.right) + { + match e.operator { + BinaryOperator::Exponential => right.is_negative(), + BinaryOperator::Division | BinaryOperator::Remainder => right.is_zero(), + BinaryOperator::ShiftRightZeroFill => true, + _ => false, + } + } else { + true + } + } else if !(maybe_symbol_or_bigint_or_to_primitive_may_return_symbol_or_bigint( + self, &e.left, + ) || maybe_symbol_or_bigint_or_to_primitive_may_return_symbol_or_bigint( + self, &e.right, + )) { + self.expression_may_have_side_effects(&e.left) + || self.expression_may_have_side_effects(&e.right) + } else { + true + } + } } - self.expression_may_have_side_effects(&e.left) - || self.expression_may_have_side_effects(&e.right) } fn logical_expression_may_have_side_effects(&self, e: &LogicalExpression<'_>) -> bool { @@ -215,3 +257,65 @@ pub trait MayHaveSideEffects { } } } + +fn is_string_or_to_primitive_returns_string(expr: &Expression<'_>) -> bool { + match expr { + Expression::StringLiteral(_) + | Expression::TemplateLiteral(_) + | Expression::RegExpLiteral(_) + | Expression::ArrayExpression(_) => true, + // unless `Symbol.toPrimitive`, `valueOf`, `toString` is overridden, + // ToPrimitive for an object returns `"[object Object]"` + Expression::ObjectExpression(obj) => obj.properties.is_empty(), + _ => false, + } +} + +/// Whether the given expression may be a `Symbol` or converted to a `Symbol` when passed to `toPrimitive`. +fn maybe_symbol_or_to_primitive_may_return_symbol( + m: &impl MayHaveSideEffects, + expr: &Expression<'_>, +) -> bool { + match expr { + Expression::Identifier(ident) => { + !(matches!(ident.name.as_str(), "Infinity" | "NaN" | "undefined") + && m.is_global_reference(ident)) + } + Expression::StringLiteral(_) + | Expression::TemplateLiteral(_) + | Expression::NullLiteral(_) + | Expression::NumericLiteral(_) + | Expression::BigIntLiteral(_) + | Expression::BooleanLiteral(_) + | Expression::RegExpLiteral(_) + | Expression::ArrayExpression(_) => false, + // unless `Symbol.toPrimitive`, `valueOf`, `toString` is overridden, + // ToPrimitive for an object returns `"[object Object]"` + Expression::ObjectExpression(obj) => !obj.properties.is_empty(), + _ => true, + } +} + +/// Whether the given expression may be a `Symbol`/`BigInt` or converted to a `Symbol`/`BigInt` when passed to `toPrimitive`. +fn maybe_symbol_or_bigint_or_to_primitive_may_return_symbol_or_bigint( + m: &impl MayHaveSideEffects, + expr: &Expression<'_>, +) -> bool { + match expr { + Expression::Identifier(ident) => { + !(matches!(ident.name.as_str(), "Infinity" | "NaN" | "undefined") + && m.is_global_reference(ident)) + } + Expression::StringLiteral(_) + | Expression::TemplateLiteral(_) + | Expression::NullLiteral(_) + | Expression::NumericLiteral(_) + | Expression::BooleanLiteral(_) + | Expression::RegExpLiteral(_) + | Expression::ArrayExpression(_) => false, + // unless `Symbol.toPrimitive`, `valueOf`, `toString` is overridden, + // ToPrimitive for an object returns `"[object Object]"` + Expression::ObjectExpression(obj) => !obj.properties.is_empty(), + _ => true, + } +} 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 2e22929a55c3c..b0db43cae7d2a 100644 --- a/crates/oxc_minifier/tests/ecmascript/may_have_side_effects.rs +++ b/crates/oxc_minifier/tests/ecmascript/may_have_side_effects.rs @@ -84,10 +84,10 @@ fn closure_compiler_tests() { test("/abc/gi", false); test("('a')", false); // wrapped with parentheses to avoid treated as a directive test("0", false); - test("a + c", false); + test("a + c", true); test("'c' + a[0]", true); test("a[0][1]", true); - test("'a' + c", false); + test("'a' + c", true); test("'a' + a.name", true); test("1, 2, 3", false); test("a, b, 3", false); @@ -361,6 +361,8 @@ fn test_unary_expressions() { test("+null", false); // 0 test("+true", false); // 1 test("+'foo'", false); // NaN + test("+`foo`", false); // NaN + 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 @@ -377,6 +379,8 @@ fn test_unary_expressions() { test("-null", false); // -0 test("-true", false); // -1 test("-'foo'", false); // -NaN + test("-`foo`", false); // NaN + 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 @@ -419,12 +423,108 @@ fn test_other_expressions() { #[test] fn test_binary_expressions() { + test("a === b", false); + test("a() === b", true); + test("a !== b", false); + test("a() !== b", true); + test("a == b", false); test("a() == b", true); + // These actually have a side effect, but this treated as side-effect free. + test("'' == { toString() { console.log('sideeffect') } }", false); + test("'' == { valueOf() { console.log('sideeffect') } }", false); + test("'' == { [s]() { console.log('sideeffect') } }", false); // assuming s is Symbol.toPrimitive + test("a != b", false); + test("a() != b", true); + test("a < b", false); test("a() < b", true); - test("a + b", false); - test("a() + b", true); + // These actually have a side effect, but this treated as side-effect free. + test("'' < { toString() { console.log('sideeffect') } }", false); + test("'' < { valueOf() { console.log('sideeffect') } }", false); + test("'' < { [s]() { console.log('sideeffect') } }", false); // assuming s is Symbol.toPrimitive + test("a > b", false); + test("a() > b", true); + test("a >= b", false); + test("a() >= b", true); + test("a <= b", false); + test("a() <= b", true); + + test("'' + ''", false); + test("'' + ``", false); + test("'' + `${foo()}`", true); + test("'' + null", false); + test("'' + 0", false); + test("'' + 0n", false); + test("'' + true", false); + test("'' + /a/", false); + test("'' + []", false); + test("'' + [foo()]", true); + test("'' + Symbol()", true); + 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); + test("'' + s", true); // assuming s is Symbol + test("Symbol() + ''", true); + test("'' + {}", false); + test("'' + { toString() { return Symbol() } }", true); + test("'' + { valueOf() { return Symbol() } }", true); + test("'' + { [s]() { return Symbol() } }", true); // assuming s is Symbol.toPrimitive + test("/a/ + 1", false); // /a/1 + test("[] + 1", false); // 1 + test("({} + 1)", false); // [object Object]1 + test("0 + 1", false); + test("0 + null", false); // 0 + test("0 + true", false); // 1 + test("0 + a", true); // a can be BigInt + test("0n + 1n", false); + test("0n + a", true); // a can be Number + test("a + b", true); + + test("0n - 1n", false); + test("0n - 0", true); + test("0n - a", true); // a can be Number + test("a - 0n", true); // a can be Number + test("0n - a()", true); + test("0 - 1", false); + test("0 - a", true); // a can be BigInt + test("0 - ''", false); // 0 + test("0 - ``", false); // 0 + test("0 - true", false); // -1 + test("0 - /a/", false); // NaN + test("0 - []", false); // 0 + test("0 - [foo()]", true); + test_with_global_variables("0 - Infinity", vec!["Infinity".to_string()], false); // -Infinity + test_with_global_variables("0 - NaN", vec!["NaN".to_string()], false); // NaN + test_with_global_variables("0 - undefined", vec!["undefined".to_string()], false); // NaN + test_with_global_variables("null - Infinity", vec!["Infinity".to_string()], false); // -Infinity + test("0 - {}", false); // NaN + test("'' - { toString() { return Symbol() } }", true); + test("'' - { valueOf() { return Symbol() } }", true); + test("'' - { [s]() { return Symbol() } }", true); // assuming s is Symbol.toPrimitive + test("a - b", true); + test("0 * 1", false); + test("0 * a", true); + test("0 / 1", false); + test("0 / a", true); + test("0 % 1", false); + test("0 % a", true); + test("0 << 1", false); + test("0 << a", true); + test("0 | 1", false); + test("0 | a", true); + test("0 >> 1", false); + test("0 >> a", true); + test("0 ^ 1", false); + test("0 ^ a", true); + test("0 & 1", false); + test("0 & a", true); + test("0 ** 1", false); + test("0 ** a", true); + test("1n ** (-1n)", true); // `**` throws an error when the right operand is negative + test("1n / 0n", true); // `/` throws an error when the right operand is zero + test("1n % 0n", true); // `%` throws an error when the right operand is zero + test("0n >>> 1n", true); // `>>>` throws an error even when both operands are bigint // b maybe not a object // b maybe a proxy that has a side effectful "has" trap @@ -502,22 +602,3 @@ fn test_side_effectful_expressions() { test("a[0]", true); test("a?.b", true); } - -#[test] -fn tests() { - // This actually have a side effect, but this treated as side-effect free. - test("'' + { toString() { console.log('sideeffect') } }", false); - 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 - // same for -, *, /, %, **, <<, >> - test("'' + s", false); // assuming s is Symbol - test("0 + b", false); // assuming b is a BitInt - - // FIXME: actually these throws an error, but it's ignored now - test("1n ** (-1n)", false); - test("1n / 0n", false); - test("1n % 0n", false); - test("0n >>> 1n", false); // >>> throws an error even when both operands are bigint -}