Skip to content

Commit 54d1750

Browse files
CopilotCopilot
authored
feat(ecmascript): handle typeof guarded global access as side effect free (#12981)
1 parent 64326c1 commit 54d1750

File tree

2 files changed

+201
-4
lines changed

2 files changed

+201
-4
lines changed

‎crates/oxc_ecmascript/src/side_effects/may_have_side_effects.rs‎

Lines changed: 131 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,18 @@ impl<'a> MayHaveSideEffects<'a> for Expression<'a> {
4848
Expression::LogicalExpression(e) => e.may_have_side_effects(ctx),
4949
Expression::ParenthesizedExpression(e) => e.expression.may_have_side_effects(ctx),
5050
Expression::ConditionalExpression(e) => {
51-
e.test.may_have_side_effects(ctx)
52-
|| e.consequent.may_have_side_effects(ctx)
53-
|| e.alternate.may_have_side_effects(ctx)
51+
if e.test.may_have_side_effects(ctx) {
52+
return true;
53+
}
54+
// typeof x === 'undefined' ? fallback : x
55+
if is_side_effect_free_unbound_identifier_ref(&e.alternate, &e.test, false, ctx) {
56+
return e.consequent.may_have_side_effects(ctx);
57+
}
58+
// typeof x !== 'undefined' ? x : fallback
59+
if is_side_effect_free_unbound_identifier_ref(&e.consequent, &e.test, true, ctx) {
60+
return e.alternate.may_have_side_effects(ctx);
61+
}
62+
e.consequent.may_have_side_effects(ctx) || e.alternate.may_have_side_effects(ctx)
5463
}
5564
Expression::SequenceExpression(e) => {
5665
e.expressions.iter().any(|e| e.may_have_side_effects(ctx))
@@ -285,7 +294,25 @@ fn is_known_global_constructor(name: &str) -> bool {
285294

286295
impl<'a> MayHaveSideEffects<'a> for LogicalExpression<'a> {
287296
fn may_have_side_effects(&self, ctx: &impl MayHaveSideEffectsContext<'a>) -> bool {
288-
self.left.may_have_side_effects(ctx) || self.right.may_have_side_effects(ctx)
297+
if self.left.may_have_side_effects(ctx) {
298+
return true;
299+
}
300+
match self.operator {
301+
LogicalOperator::And => {
302+
// Pattern: typeof x !== 'undefined' && x
303+
if is_side_effect_free_unbound_identifier_ref(&self.right, &self.left, true, ctx) {
304+
return false;
305+
}
306+
}
307+
LogicalOperator::Or => {
308+
// Pattern: typeof x === 'undefined' || x
309+
if is_side_effect_free_unbound_identifier_ref(&self.right, &self.left, false, ctx) {
310+
return false;
311+
}
312+
}
313+
LogicalOperator::Coalesce => {}
314+
}
315+
self.right.may_have_side_effects(ctx)
289316
}
290317
}
291318

@@ -542,3 +569,103 @@ impl<'a> MayHaveSideEffects<'a> for Argument<'a> {
542569
}
543570
}
544571
}
572+
573+
/// Helper function to check if accessing an unbound identifier reference is side-effect-free based on a guard condition.
574+
///
575+
/// This function analyzes patterns like:
576+
/// - `typeof x === 'undefined' && x` (safe to access x in the right branch)
577+
/// - `typeof x !== 'undefined' || x` (safe to access x in the right branch)
578+
/// - `typeof x < 'u' && x` (safe to access x in the right branch)
579+
///
580+
/// Ported from: <https://github.com/evanw/esbuild/blob/d34e79e2a998c21bb71d57b92b0017ca11756912/internal/js_ast/js_ast_helpers.go#L2594-L2639>
581+
fn is_side_effect_free_unbound_identifier_ref<'a>(
582+
value: &Expression<'a>,
583+
guard_condition: &Expression<'a>,
584+
mut is_yes_branch: bool,
585+
ctx: &impl MayHaveSideEffectsContext<'a>,
586+
) -> bool {
587+
let Some(ident) = value.get_identifier_reference() else {
588+
return false;
589+
};
590+
if !ctx.is_global_reference(ident) {
591+
return false;
592+
}
593+
594+
let Expression::BinaryExpression(bin_expr) = guard_condition else {
595+
return false;
596+
};
597+
match bin_expr.operator {
598+
BinaryOperator::StrictEquality
599+
| BinaryOperator::StrictInequality
600+
| BinaryOperator::Equality
601+
| BinaryOperator::Inequality => {
602+
let (mut ty_of, mut string) = (&bin_expr.left, &bin_expr.right);
603+
if matches!(ty_of, Expression::StringLiteral(_)) {
604+
std::mem::swap(&mut string, &mut ty_of);
605+
}
606+
607+
let Expression::UnaryExpression(unary) = ty_of else {
608+
return false;
609+
};
610+
if !(unary.operator == UnaryOperator::Typeof
611+
&& matches!(unary.argument, Expression::Identifier(_)))
612+
{
613+
return false;
614+
}
615+
616+
let Expression::StringLiteral(string) = string else {
617+
return false;
618+
};
619+
620+
let is_undefined_check = string.value == "undefined";
621+
if (is_undefined_check == is_yes_branch)
622+
== matches!(
623+
bin_expr.operator,
624+
BinaryOperator::Inequality | BinaryOperator::StrictInequality
625+
)
626+
&& unary.argument.is_specific_id(&ident.name)
627+
{
628+
return true;
629+
}
630+
}
631+
BinaryOperator::LessThan
632+
| BinaryOperator::LessEqualThan
633+
| BinaryOperator::GreaterThan
634+
| BinaryOperator::GreaterEqualThan => {
635+
let (mut ty_of, mut string) = (&bin_expr.left, &bin_expr.right);
636+
if matches!(ty_of, Expression::StringLiteral(_)) {
637+
std::mem::swap(&mut string, &mut ty_of);
638+
is_yes_branch = !is_yes_branch;
639+
}
640+
641+
let Expression::UnaryExpression(unary) = ty_of else {
642+
return false;
643+
};
644+
if !(unary.operator == UnaryOperator::Typeof
645+
&& matches!(unary.argument, Expression::Identifier(_)))
646+
{
647+
return false;
648+
}
649+
650+
let Expression::StringLiteral(string) = string else {
651+
return false;
652+
};
653+
if string.value != "u" {
654+
return false;
655+
}
656+
657+
if is_yes_branch
658+
== matches!(
659+
bin_expr.operator,
660+
BinaryOperator::LessThan | BinaryOperator::LessEqualThan
661+
)
662+
&& unary.argument.is_specific_id(&ident.name)
663+
{
664+
return true;
665+
}
666+
}
667+
_ => {}
668+
}
669+
670+
false
671+
}

‎crates/oxc_minifier/tests/ecmascript/may_have_side_effects.rs‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -832,3 +832,73 @@ fn test_object_with_to_primitive_related_properties_overridden() {
832832
test("+{ ...{ valueOf() { return Symbol() } } }", true);
833833
test("+{ ...{ [Symbol.toPrimitive]() { return Symbol() } } }", true);
834834
}
835+
836+
#[test]
837+
fn test_typeof_guard_patterns() {
838+
test_with_global_variables("typeof x !== 'undefined' && x", vec!["x".to_string()], false);
839+
test_with_global_variables("typeof x != 'undefined' && x", vec!["x".to_string()], false);
840+
test_with_global_variables("'undefined' !== typeof x && x", vec!["x".to_string()], false);
841+
test_with_global_variables("'undefined' != typeof x && x", vec!["x".to_string()], false);
842+
test_with_global_variables("typeof x === 'undefined' || x", vec!["x".to_string()], false);
843+
test_with_global_variables("typeof x == 'undefined' || x", vec!["x".to_string()], false);
844+
test_with_global_variables("'undefined' === typeof x || x", vec!["x".to_string()], false);
845+
test_with_global_variables("'undefined' == typeof x || x", vec!["x".to_string()], false);
846+
test_with_global_variables("typeof x < 'u' && x", vec!["x".to_string()], false);
847+
test_with_global_variables("typeof x <= 'u' && x", vec!["x".to_string()], false);
848+
test_with_global_variables("'u' > typeof x && x", vec!["x".to_string()], false);
849+
test_with_global_variables("'u' >= typeof x && x", vec!["x".to_string()], false);
850+
test_with_global_variables("typeof x > 'u' || x", vec!["x".to_string()], false);
851+
test_with_global_variables("typeof x >= 'u' || x", vec!["x".to_string()], false);
852+
test_with_global_variables("'u' < typeof x || x", vec!["x".to_string()], false);
853+
test_with_global_variables("'u' <= typeof x || x", vec!["x".to_string()], false);
854+
855+
test_with_global_variables("typeof x === 'undefined' ? 0 : x", vec!["x".to_string()], false);
856+
test_with_global_variables("typeof x == 'undefined' ? 0 : x", vec!["x".to_string()], false);
857+
test_with_global_variables("'undefined' === typeof x ? 0 : x", vec!["x".to_string()], false);
858+
test_with_global_variables("'undefined' == typeof x ? 0 : x", vec!["x".to_string()], false);
859+
test_with_global_variables("typeof x !== 'undefined' ? x : 0", vec!["x".to_string()], false);
860+
test_with_global_variables("typeof x != 'undefined' ? x : 0", vec!["x".to_string()], false);
861+
test_with_global_variables("'undefined' !== typeof x ? x : 0", vec!["x".to_string()], false);
862+
test_with_global_variables("'undefined' != typeof x ? x : 0", vec!["x".to_string()], false);
863+
864+
test_with_global_variables(
865+
"typeof x !== 'undefined' && (x + foo())",
866+
vec!["x".to_string()],
867+
true,
868+
);
869+
test_with_global_variables(
870+
"typeof x === 'undefined' || (x + foo())",
871+
vec!["x".to_string()],
872+
true,
873+
);
874+
test_with_global_variables("typeof x === 'undefined' ? foo() : x", vec!["x".to_string()], true);
875+
test_with_global_variables("typeof x !== 'undefined' ? x : foo()", vec!["x".to_string()], true);
876+
test_with_global_variables("typeof foo() !== 'undefined' && x", vec!["x".to_string()], true);
877+
test_with_global_variables("typeof foo() === 'undefined' || x", vec!["x".to_string()], true);
878+
test_with_global_variables("typeof foo() === 'undefined' ? 0 : x", vec!["x".to_string()], true);
879+
test_with_global_variables(
880+
"typeof y !== 'undefined' && x",
881+
vec!["x".to_string(), "y".to_string()],
882+
true,
883+
);
884+
test_with_global_variables(
885+
"typeof y === 'undefined' || x",
886+
vec!["x".to_string(), "y".to_string()],
887+
true,
888+
);
889+
test_with_global_variables(
890+
"typeof y === 'undefined' ? 0 : x",
891+
vec!["x".to_string(), "y".to_string()],
892+
true,
893+
);
894+
895+
test("typeof localVar !== 'undefined' && localVar", false);
896+
test("typeof localVar === 'undefined' || localVar", false);
897+
test("typeof localVar === 'undefined' ? 0 : localVar", false);
898+
899+
test_with_global_variables(
900+
"typeof x !== 'undefined' && typeof y !== 'undefined' && x && y",
901+
vec!["x".to_string(), "y".to_string()],
902+
true, // This can be improved
903+
);
904+
}

0 commit comments

Comments
 (0)