Skip to content

feat(ecmascript): handle typeof guarded global access as side effect free #12981

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Boshen merged 7 commits into from
Aug 12, 2025
Merged

feat(ecmascript): handle typeof guarded global access as side effect free #12981

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
662cff2
Initial plan
Copilot Aug 11, 2025
a79e43e
Port is_side_effect_free_unbound_identifier_ref from Rolldown to Oxc
Copilot Aug 11, 2025
95480a2
Integrate side effect analysis into MayHaveSideEffects trait implemen…
Copilot Aug 11, 2025
4fac2a2
Add comprehensive tests for typeof guard patterns in side effect anal…
Copilot Aug 11, 2025
a2295ca
Address review feedback: change return type to bool, extract variable…
Copilot Aug 11, 2025
f3cea22
update
sapphi-red Aug 11, 2025
a9e817c
update
sapphi-red Aug 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
135 changes: 131 additions & 4 deletions crates/oxc_ecmascript/src/side_effects/may_have_side_effects.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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: <https://github.com/evanw/esbuild/blob/d34e79e2a998c21bb71d57b92b0017ca11756912/internal/js_ast/js_ast_helpers.go#L2594-L2639>
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
}
70 changes: 70 additions & 0 deletions crates/oxc_minifier/tests/ecmascript/may_have_side_effects.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
}
Loading