fix(es/minifier): Check type of assignment target before merging assignments (#9617)

**Description:**

Collect types of vars, maybe other optimization could benefit from this:
`merged_var_type: Option<Value<Type>>`

When the variable is reassigned, the we merge the types with some simple
rules:
`None` + `None` = `None`
`None` + `Some(ty)` = `Some(ty)`
`Some(ty1)` + `Some(ty2)` if `ty1` == `ty2` = `Some(ty1)`
Otherwise = Unknown

**Related issue:**

 - Closes #8718

Author: CPunisher

.changeset/honest-cooks-dream.md

@@ -0,0 +1,7 @@
+---
+swc_ecma_minifier: patch
+swc_ecma_utils: patch
+swc_ecma_usage_analyzer: patch
+---
+
+fix(es/minifier): Check type of assignment target before merging assignments

crates/swc/tests/tsc-references/compoundAdditionAssignmentLHSCanBeAssigned.2.minified.js

@@ -1,3 +1,3 @@
 //// [compoundAdditionAssignmentWithInvalidOperands.ts]
 var E, a, x1, x2, x3, x4, x5, E1 = ((E = E1 || {})[E.a = 0] = "a", E[E.b = 1] = "b", E);
-x1 += a, x1 += !0, x1 += 0, x1 += {}, x1 += null, x1 += void 0, x2 += a, x2 += !0, x2 += 0, x2 += {}, x2 += null, x2 += void 0, x3 += a, x3 += !0, x3 += 0, x3 += {}, x3 += null, x3 += void 0, x4 += a, x4 += !0, x4 += {}, x5 += a, x5 += !0;
+x1 += a, x1 += !0, x1 += 0, x1 += 0, x1 += {}, x1 += null, x1 += void 0, x2 += a, x2 += !0, x2 += 0, x2 += 0, x2 += {}, x2 += null, x2 += void 0, x3 += a, x3 += !0, x3 += 0, x3 += 0, x3 += {}, x3 += null, x3 += void 0, x4 += a, x4 += !0, x4 += {}, x5 += a, x5 += !0;

crates/swc/tests/tsc-references/compoundAdditionAssignmentWithInvalidOperands.2.minified.js

@@ -2479,13 +2479,18 @@ impl Optimizer<'_> {
                         _ => None,
                     };
 
+                    let var_type = self
+                        .data
+                        .vars
+                        .get(&left_id.to_id())
+                        .and_then(|info| info.merged_var_type);
                     let Some(a_type) = a_type else {
                         return Ok(false);
                     };
                     let b_type = b.right.get_type();
 
                     if let Some(a_op) = a_op {
-                        if can_drop_op_for(a_op, b.op, a_type, b_type) {
+                        if can_drop_op_for(a_op, b.op, var_type, a_type, b_type) {
                             if b_left.to_id() == left_id.to_id() {
                                 if let Some(bin_op) = b.op.to_update() {
                                     report_change!(
@@ -2717,13 +2722,26 @@ pub(crate) fn is_trivial_lit(e: &Expr) -> bool {
 }
 
 /// This assumes `a.left.to_id() == b.left.to_id()`
-fn can_drop_op_for(a: AssignOp, b: AssignOp, a_type: Value<Type>, b_type: Value<Type>) -> bool {
+fn can_drop_op_for(
+    a: AssignOp,
+    b: AssignOp,
+    var_type: Option<Value<Type>>,
+    a_type: Value<Type>,
+    b_type: Value<Type>,
+) -> bool {
     if a == op!("=") {
         return true;
     }
 
     if a == b {
-        if a == op!("+=") && a_type.is_known() && a_type == b_type {
+        if a == op!("+=")
+            && a_type.is_known()
+            && a_type == b_type
+            && (match var_type {
+                Some(ty) => a_type == ty,
+                None => true,
+            })
+        {
             return true;
         }
 

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

@@ -18,6 +18,7 @@ use swc_ecma_usage_analyzer::{
     marks::Marks,
     util::is_global_var_with_pure_property_access,
 };
+use swc_ecma_utils::{Merge, Type, Value};
 use swc_ecma_visit::VisitWith;
 
 pub(crate) fn analyze<N>(n: &N, marks: Option<Marks>) -> ProgramData
@@ -98,6 +99,7 @@ pub(crate) struct VarUsageInfo {
 
     pub(crate) var_kind: Option<VarDeclKind>,
     pub(crate) var_initialized: bool,
+    pub(crate) merged_var_type: Option<Value<Type>>,
 
     pub(crate) declared_as_catch_param: bool,
 
@@ -149,6 +151,7 @@ impl Default for VarUsageInfo {
             used_in_cond: Default::default(),
             var_kind: Default::default(),
             var_initialized: Default::default(),
+            merged_var_type: Default::default(),
             declared_as_catch_param: Default::default(),
             no_side_effect_for_member_access: Default::default(),
             callee_count: Default::default(),
@@ -248,6 +251,8 @@ impl Storage for ProgramData {
                         }
                     }
 
+                    e.get_mut().merged_var_type.merge(var_info.merged_var_type);
+
                     e.get_mut().ref_count += var_info.ref_count;
 
                     e.get_mut().reassigned |= var_info.reassigned;
@@ -352,7 +357,7 @@ impl Storage for ProgramData {
         e.used_in_cond |= ctx.in_cond;
     }
 
-    fn report_assign(&mut self, ctx: Ctx, i: Id, is_op: bool) {
+    fn report_assign(&mut self, ctx: Ctx, i: Id, is_op: bool, ty: Value<Type>) {
         let e = self.vars.entry(i.clone()).or_default();
 
         let inited = self.initialized_vars.contains(&i);
@@ -361,14 +366,15 @@ impl Storage for ProgramData {
             e.reassigned = true
         }
 
+        e.merged_var_type.merge(Some(ty));
         e.assign_count += 1;
 
         if !is_op {
             self.initialized_vars.insert(i.clone());
             if e.ref_count == 1 && e.var_kind != Some(VarDeclKind::Const) && !inited {
                 e.var_initialized = true;
             } else {
-                e.reassigned = true
+                e.reassigned = true;
             }
 
             if e.ref_count == 1 && e.used_above_decl {
@@ -406,7 +412,7 @@ impl Storage for ProgramData {
         &mut self,
         ctx: Ctx,
         i: &Ident,
-        has_init: bool,
+        init_type: Option<Value<Type>>,
         kind: Option<VarDeclKind>,
     ) -> &mut VarUsageInfo {
         // if cfg!(feature = "debug") {
@@ -417,7 +423,7 @@ impl Storage for ProgramData {
         v.is_top_level |= ctx.is_top_level;
 
         // assigned or declared before this declaration
-        if has_init {
+        if init_type.is_some() {
             if v.declared || v.var_initialized || v.assign_count > 0 {
                 #[cfg(feature = "debug")]
                 {
@@ -440,12 +446,13 @@ impl Storage for ProgramData {
             v.is_fn_local = false;
         }
 
-        v.var_initialized |= has_init;
+        v.var_initialized |= init_type.is_some();
+        v.merged_var_type.merge(init_type);
 
         v.declared_count += 1;
         v.declared = true;
         // not a VarDecl, thus always inited
-        if has_init || kind.is_none() {
+        if init_type.is_some() || kind.is_none() {
             self.initialized_vars.insert(i.to_id());
         }
         v.declared_as_catch_param |= ctx.in_catch_param;

crates/swc_ecma_minifier/src/program_data.rs

@@ -0,0 +1,2 @@
+let a = "";
+console.log(((a += 1), (a += 2)));

crates/swc_ecma_minifier/tests/fixture/issues/8718/1/input.js

@@ -0,0 +1,2 @@
+let a = "";
+console.log((a += 1, a += 2));

crates/swc_ecma_minifier/tests/fixture/issues/8718/1/output.js

@@ -0,0 +1,3 @@
+let a = 0;
+a = "";
+console.log(((a += 1), (a += 2)));

crates/swc_ecma_minifier/tests/fixture/issues/8718/2/input.js

@@ -0,0 +1 @@
+console.log("12");

crates/swc_ecma_minifier/tests/fixture/issues/8718/2/output.js

@@ -0,0 +1,8 @@
+let a;
+function f() {
+    a = "123";
+    console.log(a);
+}
+
+f();
+console.log(((a += 1), (a += 2)));

crates/swc_ecma_minifier/tests/fixture/issues/8718/3/input.js

@@ -0,0 +1,2 @@
+let a;
+console.log(a = "123"), console.log((a += 1, a += 2));

crates/swc_ecma_minifier/tests/fixture/issues/8718/3/output.js

@@ -0,0 +1,11 @@
+let a;
+function g() {
+    a = "123";
+    console.log(a);
+}
+function f() {
+    // a = "123";
+    console.log(a);
+}
+f(), g();
+console.log(((a += 1), (a += 2)));

crates/swc_ecma_minifier/tests/fixture/issues/8718/4/input.js

@@ -0,0 +1,3 @@
+let a;
+// a = "123";
+console.log(a), console.log(a = "123"), console.log((a += 1, a += 2));

crates/swc_ecma_minifier/tests/fixture/issues/8718/4/output.js

@@ -0,0 +1,7 @@
+let a = 0;
+function f() {
+    a = "123";
+    console.log(a);
+}
+
+console.log(((a += 1), (a += 2)));

crates/swc_ecma_minifier/tests/fixture/issues/8718/5/input.js

@@ -0,0 +1 @@
+console.log(3);

crates/swc_ecma_minifier/tests/fixture/issues/8718/5/output.js

@@ -0,0 +1,10 @@
+let a = 0;
+function g() {
+    a = "123";
+    console.log(a);
+}
+function f() {
+    // a = "123";
+    console.log(a);
+}
+console.log(((a += 1), (a += 2)));

crates/swc_ecma_minifier/tests/fixture/issues/8718/6/input.js

@@ -0,0 +1 @@
+console.log(3);

crates/swc_ecma_minifier/tests/fixture/issues/8718/6/output.js

@@ -3,6 +3,7 @@
 use std::ops::{Deref, DerefMut};
 
 use swc_ecma_ast::VarDeclKind;
+use swc_ecma_utils::{Type, Value};
 
 use super::{storage::Storage, UsageAnalyzer};
 
@@ -28,7 +29,7 @@ pub struct Ctx {
     pub in_decl_with_no_side_effect_for_member_access: bool,
 
     pub in_pat_of_var_decl: bool,
-    pub in_pat_of_var_decl_with_init: bool,
+    pub in_pat_of_var_decl_with_init: Option<Value<Type>>,
     pub in_pat_of_param: bool,
     pub in_catch_param: bool,
 

crates/swc_ecma_usage_analyzer/src/analyzer/ctx.rs

@@ -1,6 +1,6 @@
 use swc_common::{collections::AHashMap, SyntaxContext};
 use swc_ecma_ast::*;
-use swc_ecma_utils::{find_pat_ids, ExprCtx, ExprExt, IsEmpty, StmtExt};
+use swc_ecma_utils::{find_pat_ids, ExprCtx, ExprExt, IsEmpty, StmtExt, Type, Value};
 use swc_ecma_visit::{noop_visit_type, Visit, VisitWith};
 use swc_timer::timer;
 
@@ -155,33 +155,38 @@ where
 
     fn report_assign_pat(&mut self, p: &Pat, is_read_modify: bool) {
         for id in find_pat_ids(p) {
-            self.data.report_assign(self.ctx, id, is_read_modify)
+            // It's hard to determined the type of pat assignment
+            self.data
+                .report_assign(self.ctx, id, is_read_modify, Value::Unknown)
         }
 
         if let Pat::Expr(e) = p {
             match &**e {
-                Expr::Ident(i) => self.data.report_assign(self.ctx, i.to_id(), is_read_modify),
+                Expr::Ident(i) => {
+                    self.data
+                        .report_assign(self.ctx, i.to_id(), is_read_modify, Value::Unknown)
+                }
                 _ => self.mark_mutation_if_member(e.as_member()),
             }
         }
     }
 
-    fn report_assign_expr_if_ident(&mut self, e: Option<&Ident>, is_op: bool) {
+    fn report_assign_expr_if_ident(&mut self, e: Option<&Ident>, is_op: bool, ty: Value<Type>) {
         if let Some(i) = e {
-            self.data.report_assign(self.ctx, i.to_id(), is_op)
+            self.data.report_assign(self.ctx, i.to_id(), is_op, ty)
         }
     }
 
     fn declare_decl(
         &mut self,
         i: &Ident,
-        has_init: bool,
+        init_type: Option<Value<Type>>,
         kind: Option<VarDeclKind>,
         is_fn_decl: bool,
     ) -> &mut S::VarData {
         self.scope.add_declared_symbol(i);
 
-        let v = self.data.declare_decl(self.ctx, i, has_init, kind);
+        let v = self.data.declare_decl(self.ctx, i, init_type, kind);
 
         if is_fn_decl {
             v.mark_declared_as_fn_decl();
@@ -267,13 +272,15 @@ where
         match &n.left {
             AssignTarget::Pat(p) => {
                 for id in find_pat_ids(p) {
-                    self.data.report_assign(self.ctx, id, is_op_assign)
+                    self.data
+                        .report_assign(self.ctx, id, is_op_assign, n.right.get_type())
                 }
             }
             AssignTarget::Simple(e) => {
                 self.report_assign_expr_if_ident(
                     e.as_ident().map(Ident::from).as_ref(),
                     is_op_assign,
+                    n.right.get_type(),
                 );
                 self.mark_mutation_if_member(e.as_member())
             }
@@ -516,7 +523,7 @@ where
 
     #[cfg_attr(feature = "tracing-spans", tracing::instrument(skip_all))]
     fn visit_class_decl(&mut self, n: &ClassDecl) {
-        self.declare_decl(&n.ident, true, None, false);
+        self.declare_decl(&n.ident, Some(Value::Unknown), None, false);
 
         n.visit_children_with(self);
     }
@@ -526,7 +533,7 @@ where
         n.visit_children_with(self);
 
         if let Some(id) = &n.ident {
-            self.declare_decl(id, true, None, false);
+            self.declare_decl(id, Some(Value::Unknown), None, false);
         }
     }
 
@@ -681,7 +688,7 @@ where
             in_pat_of_param: false,
             in_catch_param: false,
             var_decl_kind_of_pat: None,
-            in_pat_of_var_decl_with_init: false,
+            in_pat_of_var_decl_with_init: None,
             ..self.ctx
         };
 
@@ -721,7 +728,8 @@ where
             in_decl_with_no_side_effect_for_member_access: true,
             ..self.ctx
         };
-        self.with_ctx(ctx).declare_decl(&n.ident, true, None, true);
+        self.with_ctx(ctx)
+            .declare_decl(&n.ident, Some(Value::Known(Type::Obj)), None, true);
 
         if n.function.body.is_empty() {
             self.data.var_or_default(n.ident.to_id()).mark_as_pure_fn();
@@ -889,15 +897,15 @@ where
     }
 
     fn visit_import_default_specifier(&mut self, n: &ImportDefaultSpecifier) {
-        self.declare_decl(&n.local, true, None, false);
+        self.declare_decl(&n.local, Some(Value::Unknown), None, false);
     }
 
     fn visit_import_named_specifier(&mut self, n: &ImportNamedSpecifier) {
-        self.declare_decl(&n.local, true, None, false);
+        self.declare_decl(&n.local, Some(Value::Unknown), None, false);
     }
 
     fn visit_import_star_as_specifier(&mut self, n: &ImportStarAsSpecifier) {
-        self.declare_decl(&n.local, true, None, false);
+        self.declare_decl(&n.local, Some(Value::Unknown), None, false);
     }
 
     #[cfg_attr(feature = "tracing-spans", tracing::instrument(skip_all))]
@@ -907,7 +915,7 @@ where
             in_pat_of_param: false,
             in_catch_param: false,
             var_decl_kind_of_pat: None,
-            in_pat_of_var_decl_with_init: false,
+            in_pat_of_var_decl_with_init: None,
             ..self.ctx
         };
 
@@ -1238,7 +1246,7 @@ where
     fn visit_update_expr(&mut self, n: &UpdateExpr) {
         n.visit_children_with(self);
 
-        self.report_assign_expr_if_ident(n.arg.as_ident(), true);
+        self.report_assign_expr_if_ident(n.arg.as_ident(), true, Value::Known(Type::Num));
         self.mark_mutation_if_member(n.arg.as_member());
     }
 
@@ -1278,7 +1286,7 @@ where
             let ctx = Ctx {
                 inline_prevented: self.ctx.inline_prevented || prevent_inline,
                 in_pat_of_var_decl: true,
-                in_pat_of_var_decl_with_init: e.init.is_some(),
+                in_pat_of_var_decl_with_init: e.init.as_ref().map(|init| init.get_type()),
                 in_decl_with_no_side_effect_for_member_access: e
                     .init
                     .as_deref()

crates/swc_ecma_usage_analyzer/src/analyzer/mod.rs

@@ -1,6 +1,7 @@
 use swc_atoms::JsWord;
 use swc_common::SyntaxContext;
 use swc_ecma_ast::*;
+use swc_ecma_utils::{Type, Value};
 
 use super::{ctx::Ctx, ScopeKind};
 use crate::alias::Access;
@@ -19,13 +20,13 @@ pub trait Storage: Sized + Default {
 
     fn report_usage(&mut self, ctx: Ctx, i: Id);
 
-    fn report_assign(&mut self, ctx: Ctx, i: Id, is_op: bool);
+    fn report_assign(&mut self, ctx: Ctx, i: Id, is_op: bool, ty: Value<Type>);
 
     fn declare_decl(
         &mut self,
         ctx: Ctx,
         i: &Ident,
-        has_init: bool,
+        init_type: Option<Value<Type>>,
         kind: Option<VarDeclKind>,
     ) -> &mut Self::VarData;
 

crates/swc_ecma_usage_analyzer/src/analyzer/storage.rs

@@ -31,6 +31,7 @@ use tracing::trace;
 pub use self::{
     factory::{ExprFactory, FunctionFactory, IntoIndirectCall},
     value::{
+        Merge,
         Type::{
             self, Bool as BoolType, Null as NullType, Num as NumberType, Obj as ObjectType,
             Str as StringType, Symbol as SymbolType, Undefined as UndefinedType,

crates/swc_ecma_utils/src/lib.rs

@@ -108,3 +108,18 @@ impl Not for Value<bool> {
         }
     }
 }
+
+pub trait Merge {
+    fn merge(&mut self, rhs: Self);
+}
+
+impl Merge for Option<Value<Type>> {
+    fn merge(&mut self, rhs: Self) {
+        *self = match (*self, rhs) {
+            (None, None) => None,
+            (None, Some(ty)) | (Some(ty), None) => Some(ty),
+            (Some(ty1), Some(ty2)) if ty1 == ty2 => Some(ty1),
+            _ => Some(Unknown),
+        }
+    }
+}