fix(es/minifier): Infect mutation when assigning a property (#7503)

crates/swc_ecma_minifier/src/compress/optimize/inline.rs

@@ -712,85 +712,87 @@ impl Optimizer<'_> {
 
     /// Actually inlines variables.
     pub(super) fn inline(&mut self, e: &mut Expr) {
-        if let Expr::Member(me) = e {
-            if let MemberProp::Computed(ref mut prop) = me.prop {
-                if let Expr::Lit(Lit::Num(..)) = &*prop.expr {
-                    if let Expr::Ident(obj) = &*me.obj {
-                        let new = self.vars.lits_for_array_access.get(&obj.to_id());
-
-                        if let Some(new) = new {
-                            report_change!("inline: Inlined array access");
-                            self.changed = true;
-
-                            me.obj = new.clone();
-                            // TODO(kdy1): Optimize performance by skipping visiting of children
-                            // nodes.
-                            e.visit_mut_with(&mut expr_simplifier(
-                                self.marks.unresolved_mark,
-                                Default::default(),
-                            ));
+        match e {
+            Expr::Member(me) => {
+                if let MemberProp::Computed(prop) = &mut me.prop {
+                    if let Expr::Lit(Lit::Num(..)) = &*prop.expr {
+                        if let Expr::Ident(obj) = &*me.obj {
+                            let new = self.vars.lits_for_array_access.get(&obj.to_id());
+
+                            if let Some(new) = new {
+                                report_change!("inline: Inlined array access");
+                                self.changed = true;
+
+                                me.obj = new.clone();
+                                // TODO(kdy1): Optimize performance by skipping visiting of children
+                                // nodes.
+                                e.visit_mut_with(&mut expr_simplifier(
+                                    self.marks.unresolved_mark,
+                                    Default::default(),
+                                ));
+                            }
                         }
-                        return;
                     }
                 }
             }
-        }
-
-        if let Expr::Ident(i) = e {
-            let id = i.to_id();
-            if let Some(value) = self
-                .vars
-                .lits
-                .get(&id)
-                .or_else(|| {
-                    if self.ctx.is_callee {
-                        self.vars.simple_functions.get(&i.to_id())
-                    } else {
-                        None
-                    }
-                })
-                .cloned()
-            {
-                if !matches!(&*value, Expr::Ident(..) | Expr::Member(..)) && self.ctx.is_update_arg
+            Expr::Ident(i) => {
+                let id = i.to_id();
+                if let Some(value) = self
+                    .vars
+                    .lits
+                    .get(&id)
+                    .or_else(|| {
+                        if self.ctx.is_callee {
+                            self.vars.simple_functions.get(&i.to_id())
+                        } else {
+                            None
+                        }
+                    })
+                    .cloned()
                 {
-                    return;
-                }
+                    if !matches!(&*value, Expr::Ident(..) | Expr::Member(..))
+                        && self.ctx.is_update_arg
+                    {
+                        return;
+                    }
 
-                self.changed = true;
-                report_change!("inline: Replacing a variable `{}` with cheap expression", i);
+                    self.changed = true;
+                    report_change!("inline: Replacing a variable `{}` with cheap expression", i);
 
-                if let Expr::Ident(i) = &*value {
-                    if let Some(usage) = self.data.vars.get_mut(&i.to_id()) {
-                        usage.ref_count += 1;
-                        usage.usage_count += 1;
+                    if let Expr::Ident(i) = &*value {
+                        if let Some(usage) = self.data.vars.get_mut(&i.to_id()) {
+                            usage.ref_count += 1;
+                            usage.usage_count += 1;
+                        }
                     }
-                }
 
-                *e = *value;
-                return;
-            }
-
-            // Check without cloning
-            if let Some(value) = self.vars.vars_for_inlining.get(&i.to_id()) {
-                if self.ctx.is_exact_lhs_of_assign && !is_valid_for_lhs(value) {
+                    *e = *value;
                     return;
                 }
 
-                if let Expr::Member(..) = &**value {
-                    if self.ctx.executed_multiple_time {
+                // Check without cloning
+                if let Some(value) = self.vars.vars_for_inlining.get(&i.to_id()) {
+                    if self.ctx.is_exact_lhs_of_assign && !is_valid_for_lhs(value) {
                         return;
                     }
+
+                    if let Expr::Member(..) = &**value {
+                        if self.ctx.executed_multiple_time {
+                            return;
+                        }
+                    }
                 }
-            }
 
-            if let Some(value) = self.vars.vars_for_inlining.remove(&i.to_id()) {
-                self.changed = true;
-                report_change!("inline: Replacing '{}' with an expression", i);
+                if let Some(value) = self.vars.vars_for_inlining.remove(&i.to_id()) {
+                    self.changed = true;
+                    report_change!("inline: Replacing '{}' with an expression", i);
 
-                *e = *value;
+                    *e = *value;
 
-                log_abort!("inline: [Change] {}", crate::debug::dump(&*e, false))
+                    log_abort!("inline: [Change] {}", crate::debug::dump(&*e, false))
+                }
             }
+            _ => (),
         }
     }
 }

crates/swc_ecma_minifier/src/program_data.rs

@@ -402,6 +402,31 @@ impl Storage for ProgramData {
     fn truncate_initialized_cnt(&mut self, len: usize) {
         self.initialized_vars.truncate(len)
     }
+
+    fn mark_property_mutattion(&mut self, id: Id, ctx: Ctx) {
+        let e = self.vars.entry(id).or_default();
+        e.has_property_mutation = true;
+
+        let mut to_mark_mutate = Vec::new();
+        for (other, kind) in &e.infects_to {
+            if *kind == AccessKind::Reference {
+                to_mark_mutate.push(other.clone())
+            }
+        }
+
+        for other in to_mark_mutate {
+            let other = self.vars.entry(other).or_insert_with(|| {
+                let simple_assign = ctx.is_exact_reassignment && !ctx.is_op_assign;
+
+                VarUsageInfo {
+                    used_above_decl: !simple_assign,
+                    ..Default::default()
+                }
+            });
+
+            other.has_property_mutation = true;
+        }
+    }
 }
 
 impl ScopeDataLike for ScopeData {
@@ -447,10 +472,6 @@ impl VarDataLike for VarUsageInfo {
         self.has_property_access = true;
     }
 
-    fn mark_has_property_mutation(&mut self) {
-        self.has_property_mutation = true;
-    }
-
     fn mark_used_as_callee(&mut self) {
         self.callee_count += 1;
     }
@@ -650,15 +671,6 @@ impl ProgramData {
 
         // Passing object as a argument is possibly modification.
         e.mutated |= is_modify || (call_may_mutate && ctx.is_exact_arg);
-        let mut to_mark_mutate = Vec::new();
-        if call_may_mutate && ctx.is_exact_arg {
-            e.has_property_mutation = true;
-            for (other, kind) in e.infects_to.clone() {
-                if kind == AccessKind::Reference {
-                    to_mark_mutate.push(other)
-                }
-            }
-        }
 
         e.executed_multiple_time |= ctx.executed_multiple_time;
         e.used_in_cond |= ctx.in_cond;
@@ -676,7 +688,7 @@ impl ProgramData {
                     && e.var_kind != Some(VarDeclKind::Const)
                     && !inited
                 {
-                    self.initialized_vars.insert(i);
+                    self.initialized_vars.insert(i.clone());
                     e.assign_count -= 1;
                     e.var_initialized = true;
                 } else {
@@ -695,19 +707,8 @@ impl ProgramData {
             e.usage_count += 1;
         }
 
-        for other in to_mark_mutate {
-            let other = self.vars.entry(other).or_insert_with(|| {
-                // trace!("insert({}{:?})", i.0, i.1);
-
-                let simple_assign = ctx.is_exact_reassignment && !ctx.is_op_assign;
-
-                VarUsageInfo {
-                    used_above_decl: !simple_assign,
-                    ..Default::default()
-                }
-            });
-
-            other.has_property_mutation = true;
+        if call_may_mutate && ctx.is_exact_arg {
+            self.mark_property_mutattion(i, ctx)
         }
     }
 }

crates/swc_ecma_minifier/tests/benches-full/echarts.js

@@ -18546,11 +18546,13 @@
                 layout.r = layout.r0, layout.r0 = tmp;
             }
             var r = mathMin$6(layout.r, coordSysClipArea.r), r0 = mathMax$6(layout.r0, coordSysClipArea.r0);
-            if (layout.r = r, layout.r0 = r0, signR < 0) {
+            layout.r = r, layout.r0 = r0;
+            var clipped = r - r0 < 0;
+            if (signR < 0) {
                 var tmp = layout.r;
                 layout.r = layout.r0, layout.r0 = tmp;
             }
-            return r - r0 < 0;
+            return clipped;
         }
     }, elementCreator = {
         cartesian2d: function(seriesModel, data, newIndex, layout, isHorizontal, animationModel, axisModel, isUpdate, roundCap) {

crates/swc_ecma_minifier/tests/fixture/issues/7500/input.js

@@ -0,0 +1,6 @@
+var globalArray = [1, 1, 1];
+module.exports = function () {
+    var localArray = globalArray;
+    localArray[0] = localArray[1] = localArray[2] = 0;
+    return localArray;
+};

crates/swc_ecma_minifier/tests/fixture/issues/7500/output.js

@@ -0,0 +1,8 @@
+var globalArray = [
+    1,
+    1,
+    1
+];
+module.exports = function() {
+    return globalArray[0] = globalArray[1] = globalArray[2] = 0, globalArray;
+};

crates/swc_ecma_minifier/tests/fixture/next/feedback-2/codemirror/output.js

@@ -1823,7 +1823,8 @@
     }, NativeScrollbars.prototype.setScrollTop = function(pos) {
         this.vert.scrollTop != pos && (this.vert.scrollTop = pos), this.disableVert && this.enableZeroWidthBar(this.vert, this.disableVert, "vert");
     }, NativeScrollbars.prototype.zeroWidthHack = function() {
-        this.horiz.style.height = this.vert.style.width = mac && !mac_geMountainLion ? "12px" : "18px", this.horiz.style.pointerEvents = this.vert.style.pointerEvents = "none", this.disableHoriz = new Delayed(), this.disableVert = new Delayed();
+        var w = mac && !mac_geMountainLion ? "12px" : "18px";
+        this.horiz.style.height = this.vert.style.width = w, this.horiz.style.pointerEvents = this.vert.style.pointerEvents = "none", this.disableHoriz = new Delayed(), this.disableVert = new Delayed();
     }, NativeScrollbars.prototype.enableZeroWidthBar = function(bar, delay, type) {
         bar.style.pointerEvents = "auto", delay.set(1000, function maybeDisable() {
             var box = bar.getBoundingClientRect();

crates/swc_ecma_usage_analyzer/src/analyzer/mod.rs

@@ -850,13 +850,13 @@ where
                 v.mark_indexed_with_dynamic_key();
             }
 
-            if self.ctx.in_assign_lhs || self.ctx.is_delete_arg {
-                v.mark_has_property_mutation();
-            }
-
             if let MemberProp::Ident(prop) = &e.prop {
                 v.add_accessed_property(prop.sym.clone());
             }
+
+            if self.ctx.in_assign_lhs || self.ctx.is_delete_arg {
+                self.data.mark_property_mutattion(obj.to_id(), self.ctx)
+            }
         }
     }
 

crates/swc_ecma_usage_analyzer/src/analyzer/storage.rs

@@ -29,6 +29,8 @@ pub trait Storage: Sized + Default {
 
     fn get_initialized_cnt(&self) -> usize;
     fn truncate_initialized_cnt(&mut self, len: usize);
+
+    fn mark_property_mutattion(&mut self, id: Id, ctx: Ctx);
 }
 
 pub trait ScopeDataLike: Sized + Default + Clone {
@@ -55,8 +57,6 @@ pub trait VarDataLike: Sized {
 
     fn mark_has_property_access(&mut self);
 
-    fn mark_has_property_mutation(&mut self);
-
     fn mark_used_as_callee(&mut self);
 
     fn mark_used_as_arg(&mut self);