diff --git a/crates/oxc_ecmascript/src/side_effects/context.rs b/crates/oxc_ecmascript/src/side_effects/context.rs index fa471272d075b..616516c268223 100644 --- a/crates/oxc_ecmascript/src/side_effects/context.rs +++ b/crates/oxc_ecmascript/src/side_effects/context.rs @@ -33,6 +33,13 @@ pub trait MayHaveSideEffectsContext<'a>: GlobalContext<'a> { /// fn property_read_side_effects(&self) -> PropertyReadSideEffects; + /// Whether property write accesses have side effects. + /// + /// + fn property_write_side_effects(&self) -> bool { + true + } + /// Whether accessing a global variable has side effects. /// /// Accessing a non-existing global variable will throw an error. diff --git a/crates/oxc_ecmascript/src/side_effects/expressions.rs b/crates/oxc_ecmascript/src/side_effects/expressions.rs index 714c7437d335b..ea69d4bfe3549 100644 --- a/crates/oxc_ecmascript/src/side_effects/expressions.rs +++ b/crates/oxc_ecmascript/src/side_effects/expressions.rs @@ -65,6 +65,8 @@ impl<'a> MayHaveSideEffects<'a> for Expression<'a> { Expression::CallExpression(e) => e.may_have_side_effects(ctx), Expression::NewExpression(e) => e.may_have_side_effects(ctx), Expression::TaggedTemplateExpression(e) => e.may_have_side_effects(ctx), + Expression::AssignmentExpression(e) => e.may_have_side_effects(ctx), + Expression::UpdateExpression(e) => e.may_have_side_effects(ctx), _ => true, } } @@ -874,3 +876,59 @@ fn is_side_effect_free_unbound_identifier_ref<'a>( false } + +impl<'a> MayHaveSideEffects<'a> for AssignmentExpression<'a> { + fn may_have_side_effects(&self, ctx: &impl MayHaveSideEffectsContext<'a>) -> bool { + if ctx.property_write_side_effects() { + return true; + } + // Only simple assignments (`=`) benefit from property_write_side_effects: false. + // Compound assignments (`+=`, `-=`, etc.) always have side effects because: + // 1. They perform an implicit property read (GetValue) which can invoke getters/proxies + // 2. The operation itself performs ToPrimitive/ToNumeric coercion which can invoke + // user code (valueOf/toString) or throw (e.g. Symbol) + if self.operator != AssignmentOperator::Assign { + return true; + } + // When property_write_side_effects is false, member expression writes are considered free. + // Other writes (to variables, destructuring targets) still have side effects. + match &self.left { + AssignmentTarget::StaticMemberExpression(e) => { + e.object.may_have_side_effects(ctx) || self.right.may_have_side_effects(ctx) + } + AssignmentTarget::ComputedMemberExpression(e) => { + e.object.may_have_side_effects(ctx) + || e.expression.may_have_side_effects(ctx) + || self.right.may_have_side_effects(ctx) + } + AssignmentTarget::PrivateFieldExpression(e) => { + e.object.may_have_side_effects(ctx) || self.right.may_have_side_effects(ctx) + } + _ => true, + } + } +} + +impl<'a> MayHaveSideEffects<'a> for UpdateExpression<'a> { + fn may_have_side_effects(&self, ctx: &impl MayHaveSideEffectsContext<'a>) -> bool { + if ctx.property_write_side_effects() { + return true; + } + // When property_write_side_effects is false, member expression updates + // (e.g. obj.prop++, obj[key]--) are treated like property writes. + // The update operation (ToNumeric + PutValue) is considered side-effect-free, + // but the object/key evaluation may still have side effects. + match &self.argument { + SimpleAssignmentTarget::StaticMemberExpression(e) => { + e.object.may_have_side_effects(ctx) + } + SimpleAssignmentTarget::ComputedMemberExpression(e) => { + e.object.may_have_side_effects(ctx) || e.expression.may_have_side_effects(ctx) + } + SimpleAssignmentTarget::PrivateFieldExpression(e) => { + e.object.may_have_side_effects(ctx) + } + _ => 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 06fdffaf82215..e11e7fa5a7d41 100644 --- a/crates/oxc_minifier/tests/ecmascript/may_have_side_effects.rs +++ b/crates/oxc_minifier/tests/ecmascript/may_have_side_effects.rs @@ -18,6 +18,7 @@ struct Ctx { annotation: bool, pure_function_names: Vec, property_read_side_effects: PropertyReadSideEffects, + property_write_side_effects: bool, unknown_global_side_effects: bool, } @@ -32,6 +33,7 @@ impl Default for Ctx { annotation: true, pure_function_names: vec![], property_read_side_effects: PropertyReadSideEffects::All, + property_write_side_effects: true, unknown_global_side_effects: true, } } @@ -56,6 +58,10 @@ impl MayHaveSideEffectsContext<'_> for Ctx { self.property_read_side_effects } + fn property_write_side_effects(&self) -> bool { + self.property_write_side_effects + } + fn unknown_global_side_effects(&self) -> bool { self.unknown_global_side_effects } @@ -1166,3 +1172,55 @@ fn test_assignment_targets() { test_assign_target_with_global_variables("a.#b = 1", &["a"], true); // `a` might not be declared and cause ReferenceError in strict mode test_assign_target("(foo(), a).#b = 1", true); // `foo()` may have sideeffect } + +#[test] +fn test_property_write_side_effects_support() { + // With property_write_side_effects: true (default), all writes have side effects + let write_ctx = Ctx { property_write_side_effects: true, ..Default::default() }; + test_with_ctx("a.b = 1", &write_ctx, true); + test_with_ctx("a.b += 1", &write_ctx, true); + test_with_ctx("a.b++", &write_ctx, true); + + // With property_write_side_effects: false, simple assignment is side-effect-free + let no_write_ctx = Ctx { + property_write_side_effects: false, + property_read_side_effects: PropertyReadSideEffects::All, + ..Default::default() + }; + test_with_ctx("a.b = 1", &no_write_ctx, false); // simple assign, no read + test_with_ctx("a['b'] = 1", &no_write_ctx, false); + test_with_ctx("a.#b = 1", &no_write_ctx, false); + + // Compound assignments have an implicit read — still side-effectful when reads have side effects + test_with_ctx("a.b += 1", &no_write_ctx, true); + test_with_ctx("a.b -= 1", &no_write_ctx, true); + test_with_ctx("a.b &&= 1", &no_write_ctx, true); + test_with_ctx("a['b'] += 1", &no_write_ctx, true); + test_with_ctx("a.#b += 1", &no_write_ctx, true); + + // Update expressions have an implicit read + test_with_ctx("a.b++", &no_write_ctx, false); + test_with_ctx("a.b--", &no_write_ctx, false); + test_with_ctx("++a.b", &no_write_ctx, false); + test_with_ctx("a['b']++", &no_write_ctx, false); + test_with_ctx("a.#b++", &no_write_ctx, false); + + // Compound assignments and updates always have side effects due to implicit coercions + // (ToPrimitive/ToNumeric), even with both write and read side effects off + let no_side_effects_ctx = Ctx { + property_write_side_effects: false, + property_read_side_effects: PropertyReadSideEffects::None, + ..Default::default() + }; + test_with_ctx("a.b = 1", &no_side_effects_ctx, false); // simple assign is free + test_with_ctx("a.b += 1", &no_side_effects_ctx, true); // compound: ToNumeric coercion + test_with_ctx("a.b++", &no_side_effects_ctx, false); // update: ToNumeric coercion + test_with_ctx("a['b'] += 1", &no_side_effects_ctx, true); + test_with_ctx("a['b']++", &no_side_effects_ctx, false); + test_with_ctx("a.#b += 1", &no_side_effects_ctx, true); + test_with_ctx("a.#b++", &no_side_effects_ctx, false); + + // Sub-expression side effects still propagate + test_with_ctx("(foo()).b = 1", &no_side_effects_ctx, true); + test_with_ctx("a[foo()] = 1", &no_side_effects_ctx, true); +}