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 694997b4e983a..578fba08ff081 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 @@ -48,9 +48,18 @@ impl<'a> MayHaveSideEffects<'a> for Expression<'a> { Expression::LogicalExpression(e) => e.may_have_side_effects(ctx), Expression::ParenthesizedExpression(e) => e.expression.may_have_side_effects(ctx), Expression::ConditionalExpression(e) => { - e.test.may_have_side_effects(ctx) - || e.consequent.may_have_side_effects(ctx) - || e.alternate.may_have_side_effects(ctx) + if e.test.may_have_side_effects(ctx) { + return true; + } + // typeof x === 'undefined' ? fallback : x + if is_side_effect_free_unbound_identifier_ref(&e.alternate, &e.test, false, ctx) { + return e.consequent.may_have_side_effects(ctx); + } + // typeof x !== 'undefined' ? x : fallback + if is_side_effect_free_unbound_identifier_ref(&e.consequent, &e.test, true, ctx) { + return e.alternate.may_have_side_effects(ctx); + } + e.consequent.may_have_side_effects(ctx) || e.alternate.may_have_side_effects(ctx) } Expression::SequenceExpression(e) => { e.expressions.iter().any(|e| e.may_have_side_effects(ctx)) @@ -285,7 +294,25 @@ fn is_known_global_constructor(name: &str) -> bool { impl<'a> MayHaveSideEffects<'a> for LogicalExpression<'a> { fn may_have_side_effects(&self, ctx: &impl MayHaveSideEffectsContext<'a>) -> bool { - self.left.may_have_side_effects(ctx) || self.right.may_have_side_effects(ctx) + if self.left.may_have_side_effects(ctx) { + return true; + } + match self.operator { + LogicalOperator::And => { + // Pattern: typeof x !== 'undefined' && x + if is_side_effect_free_unbound_identifier_ref(&self.right, &self.left, true, ctx) { + return false; + } + } + LogicalOperator::Or => { + // Pattern: typeof x === 'undefined' || x + if is_side_effect_free_unbound_identifier_ref(&self.right, &self.left, false, ctx) { + return false; + } + } + LogicalOperator::Coalesce => {} + } + self.right.may_have_side_effects(ctx) } } @@ -542,3 +569,103 @@ impl<'a> MayHaveSideEffects<'a> for Argument<'a> { } } } + +/// Helper function to check if accessing an unbound identifier reference is side-effect-free based on a guard condition. +/// +/// This function analyzes patterns like: +/// - `typeof x === 'undefined' && x` (safe to access x in the right branch) +/// - `typeof x !== 'undefined' || x` (safe to access x in the right branch) +/// - `typeof x < 'u' && x` (safe to access x in the right branch) +/// +/// Ported from: +fn is_side_effect_free_unbound_identifier_ref<'a>( + value: &Expression<'a>, + guard_condition: &Expression<'a>, + mut is_yes_branch: bool, + ctx: &impl MayHaveSideEffectsContext<'a>, +) -> bool { + let Some(ident) = value.get_identifier_reference() else { + return false; + }; + if !ctx.is_global_reference(ident) { + return false; + } + + let Expression::BinaryExpression(bin_expr) = guard_condition else { + return false; + }; + match bin_expr.operator { + BinaryOperator::StrictEquality + | BinaryOperator::StrictInequality + | BinaryOperator::Equality + | BinaryOperator::Inequality => { + let (mut ty_of, mut string) = (&bin_expr.left, &bin_expr.right); + if matches!(ty_of, Expression::StringLiteral(_)) { + std::mem::swap(&mut string, &mut ty_of); + } + + let Expression::UnaryExpression(unary) = ty_of else { + return false; + }; + if !(unary.operator == UnaryOperator::Typeof + && matches!(unary.argument, Expression::Identifier(_))) + { + return false; + } + + let Expression::StringLiteral(string) = string else { + return false; + }; + + let is_undefined_check = string.value == "undefined"; + if (is_undefined_check == is_yes_branch) + == matches!( + bin_expr.operator, + BinaryOperator::Inequality | BinaryOperator::StrictInequality + ) + && unary.argument.is_specific_id(&ident.name) + { + return true; + } + } + BinaryOperator::LessThan + | BinaryOperator::LessEqualThan + | BinaryOperator::GreaterThan + | BinaryOperator::GreaterEqualThan => { + let (mut ty_of, mut string) = (&bin_expr.left, &bin_expr.right); + if matches!(ty_of, Expression::StringLiteral(_)) { + std::mem::swap(&mut string, &mut ty_of); + is_yes_branch = !is_yes_branch; + } + + let Expression::UnaryExpression(unary) = ty_of else { + return false; + }; + if !(unary.operator == UnaryOperator::Typeof + && matches!(unary.argument, Expression::Identifier(_))) + { + return false; + } + + let Expression::StringLiteral(string) = string else { + return false; + }; + if string.value != "u" { + return false; + } + + if is_yes_branch + == matches!( + bin_expr.operator, + BinaryOperator::LessThan | BinaryOperator::LessEqualThan + ) + && unary.argument.is_specific_id(&ident.name) + { + return true; + } + } + _ => {} + } + + false +} 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 d7b4aa232daef..5d760e548b592 100644 --- a/crates/oxc_minifier/tests/ecmascript/may_have_side_effects.rs +++ b/crates/oxc_minifier/tests/ecmascript/may_have_side_effects.rs @@ -832,3 +832,73 @@ fn test_object_with_to_primitive_related_properties_overridden() { test("+{ ...{ valueOf() { return Symbol() } } }", true); test("+{ ...{ [Symbol.toPrimitive]() { return Symbol() } } }", true); } + +#[test] +fn test_typeof_guard_patterns() { + test_with_global_variables("typeof x !== 'undefined' && x", vec!["x".to_string()], false); + test_with_global_variables("typeof x != 'undefined' && x", vec!["x".to_string()], false); + test_with_global_variables("'undefined' !== typeof x && x", vec!["x".to_string()], false); + test_with_global_variables("'undefined' != typeof x && x", vec!["x".to_string()], false); + test_with_global_variables("typeof x === 'undefined' || x", vec!["x".to_string()], false); + test_with_global_variables("typeof x == 'undefined' || x", vec!["x".to_string()], false); + test_with_global_variables("'undefined' === typeof x || x", vec!["x".to_string()], false); + test_with_global_variables("'undefined' == typeof x || x", vec!["x".to_string()], false); + test_with_global_variables("typeof x < 'u' && x", vec!["x".to_string()], false); + test_with_global_variables("typeof x <= 'u' && x", vec!["x".to_string()], false); + test_with_global_variables("'u' > typeof x && x", vec!["x".to_string()], false); + test_with_global_variables("'u' >= typeof x && x", vec!["x".to_string()], false); + test_with_global_variables("typeof x > 'u' || x", vec!["x".to_string()], false); + test_with_global_variables("typeof x >= 'u' || x", vec!["x".to_string()], false); + test_with_global_variables("'u' < typeof x || x", vec!["x".to_string()], false); + test_with_global_variables("'u' <= typeof x || x", vec!["x".to_string()], false); + + test_with_global_variables("typeof x === 'undefined' ? 0 : x", vec!["x".to_string()], false); + test_with_global_variables("typeof x == 'undefined' ? 0 : x", vec!["x".to_string()], false); + test_with_global_variables("'undefined' === typeof x ? 0 : x", vec!["x".to_string()], false); + test_with_global_variables("'undefined' == typeof x ? 0 : x", vec!["x".to_string()], false); + test_with_global_variables("typeof x !== 'undefined' ? x : 0", vec!["x".to_string()], false); + test_with_global_variables("typeof x != 'undefined' ? x : 0", vec!["x".to_string()], false); + test_with_global_variables("'undefined' !== typeof x ? x : 0", vec!["x".to_string()], false); + test_with_global_variables("'undefined' != typeof x ? x : 0", vec!["x".to_string()], false); + + test_with_global_variables( + "typeof x !== 'undefined' && (x + foo())", + vec!["x".to_string()], + true, + ); + test_with_global_variables( + "typeof x === 'undefined' || (x + foo())", + vec!["x".to_string()], + true, + ); + test_with_global_variables("typeof x === 'undefined' ? foo() : x", vec!["x".to_string()], true); + test_with_global_variables("typeof x !== 'undefined' ? x : foo()", vec!["x".to_string()], true); + test_with_global_variables("typeof foo() !== 'undefined' && x", vec!["x".to_string()], true); + test_with_global_variables("typeof foo() === 'undefined' || x", vec!["x".to_string()], true); + test_with_global_variables("typeof foo() === 'undefined' ? 0 : x", vec!["x".to_string()], true); + test_with_global_variables( + "typeof y !== 'undefined' && x", + vec!["x".to_string(), "y".to_string()], + true, + ); + test_with_global_variables( + "typeof y === 'undefined' || x", + vec!["x".to_string(), "y".to_string()], + true, + ); + test_with_global_variables( + "typeof y === 'undefined' ? 0 : x", + vec!["x".to_string(), "y".to_string()], + true, + ); + + test("typeof localVar !== 'undefined' && localVar", false); + test("typeof localVar === 'undefined' || localVar", false); + test("typeof localVar === 'undefined' ? 0 : localVar", false); + + test_with_global_variables( + "typeof x !== 'undefined' && typeof y !== 'undefined' && x && y", + vec!["x".to_string(), "y".to_string()], + true, // This can be improved + ); +}