diff --git a/.changeset/honest-cooks-dream.md b/.changeset/honest-cooks-dream.md new file mode 100644 index 000000000000..da76f4566aeb --- /dev/null +++ b/.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 diff --git a/crates/swc/tests/tsc-references/compoundAdditionAssignmentLHSCanBeAssigned.2.minified.js b/crates/swc/tests/tsc-references/compoundAdditionAssignmentLHSCanBeAssigned.2.minified.js index 4e016e020263..b0c3c701bfb7 100644 --- a/crates/swc/tests/tsc-references/compoundAdditionAssignmentLHSCanBeAssigned.2.minified.js +++ b/crates/swc/tests/tsc-references/compoundAdditionAssignmentLHSCanBeAssigned.2.minified.js @@ -1,3 +1,3 @@ //// [compoundAdditionAssignmentLHSCanBeAssigned.ts] var E, a, b, x1, x2, x3, x4, x6, E1 = ((E = E1 || {})[E.a = 0] = "a", E[E.b = 1] = "b", E); -x1 += a, x1 += b, x1 += !0, x1 += 0, x1 += '', x1 += 0, x1 += {}, x1 += null, x1 += void 0, x2 += a, x2 += b, x2 += !0, x2 += 0, x2 += '', x2 += 0, x2 += {}, x2 += null, x2 += void 0, x3 += a, x3 += 0, x3 += null, x3 += void 0, x4 += a, x4 += 0, x4 += null, x4 += void 0, x6 += a, x6 += ''; +x1 += a, x1 += b, x1 += !0, x1 += 0, x1 += '', x1 += 0, x1 += {}, x1 += null, x1 += void 0, x2 += a, x2 += b, x2 += !0, x2 += 0, x2 += '', x2 += 0, x2 += {}, x2 += null, x2 += void 0, x3 += a, x3 += 0, x3 += 0, x3 += null, x3 += void 0, x4 += a, x4 += 0, x4 += 0, x4 += null, x4 += void 0, x6 += a, x6 += ''; diff --git a/crates/swc/tests/tsc-references/compoundAdditionAssignmentWithInvalidOperands.2.minified.js b/crates/swc/tests/tsc-references/compoundAdditionAssignmentWithInvalidOperands.2.minified.js index a7fc39bdaa50..21819cd63ee4 100644 --- a/crates/swc/tests/tsc-references/compoundAdditionAssignmentWithInvalidOperands.2.minified.js +++ b/crates/swc/tests/tsc-references/compoundAdditionAssignmentWithInvalidOperands.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; diff --git a/crates/swc_ecma_minifier/src/compress/optimize/sequences.rs b/crates/swc_ecma_minifier/src/compress/optimize/sequences.rs index 893f2267319b..0ad883c1de93 100644 --- a/crates/swc_ecma_minifier/src/compress/optimize/sequences.rs +++ b/crates/swc_ecma_minifier/src/compress/optimize/sequences.rs @@ -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, b_type: Value) -> bool { +fn can_drop_op_for( + a: AssignOp, + b: AssignOp, + var_type: Option>, + a_type: Value, + b_type: Value, +) -> 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; } diff --git a/crates/swc_ecma_minifier/src/program_data.rs b/crates/swc_ecma_minifier/src/program_data.rs index aba70b53b36a..2106dc21ccbc 100644 --- a/crates/swc_ecma_minifier/src/program_data.rs +++ b/crates/swc_ecma_minifier/src/program_data.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, marks: Option) -> ProgramData @@ -98,6 +99,7 @@ pub(crate) struct VarUsageInfo { pub(crate) var_kind: Option, pub(crate) var_initialized: bool, + pub(crate) merged_var_type: Option>, 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) { let e = self.vars.entry(i.clone()).or_default(); let inited = self.initialized_vars.contains(&i); @@ -361,6 +366,7 @@ impl Storage for ProgramData { e.reassigned = true } + e.merged_var_type.merge(Some(ty)); e.assign_count += 1; if !is_op { @@ -368,7 +374,7 @@ impl Storage for ProgramData { 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>, kind: Option, ) -> &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; diff --git a/crates/swc_ecma_minifier/tests/fixture/issues/8718/1/input.js b/crates/swc_ecma_minifier/tests/fixture/issues/8718/1/input.js new file mode 100644 index 000000000000..a728e31fd802 --- /dev/null +++ b/crates/swc_ecma_minifier/tests/fixture/issues/8718/1/input.js @@ -0,0 +1,2 @@ +let a = ""; +console.log(((a += 1), (a += 2))); diff --git a/crates/swc_ecma_minifier/tests/fixture/issues/8718/1/output.js b/crates/swc_ecma_minifier/tests/fixture/issues/8718/1/output.js new file mode 100644 index 000000000000..49e0386ac402 --- /dev/null +++ b/crates/swc_ecma_minifier/tests/fixture/issues/8718/1/output.js @@ -0,0 +1,2 @@ +let a = ""; +console.log((a += 1, a += 2)); diff --git a/crates/swc_ecma_minifier/tests/fixture/issues/8718/2/input.js b/crates/swc_ecma_minifier/tests/fixture/issues/8718/2/input.js new file mode 100644 index 000000000000..e024d91aa4b2 --- /dev/null +++ b/crates/swc_ecma_minifier/tests/fixture/issues/8718/2/input.js @@ -0,0 +1,3 @@ +let a = 0; +a = ""; +console.log(((a += 1), (a += 2))); diff --git a/crates/swc_ecma_minifier/tests/fixture/issues/8718/2/output.js b/crates/swc_ecma_minifier/tests/fixture/issues/8718/2/output.js new file mode 100644 index 000000000000..79e3914a9e44 --- /dev/null +++ b/crates/swc_ecma_minifier/tests/fixture/issues/8718/2/output.js @@ -0,0 +1 @@ +console.log("12"); diff --git a/crates/swc_ecma_minifier/tests/fixture/issues/8718/3/input.js b/crates/swc_ecma_minifier/tests/fixture/issues/8718/3/input.js new file mode 100644 index 000000000000..51381b2570af --- /dev/null +++ b/crates/swc_ecma_minifier/tests/fixture/issues/8718/3/input.js @@ -0,0 +1,8 @@ +let a; +function f() { + a = "123"; + console.log(a); +} + +f(); +console.log(((a += 1), (a += 2))); diff --git a/crates/swc_ecma_minifier/tests/fixture/issues/8718/3/output.js b/crates/swc_ecma_minifier/tests/fixture/issues/8718/3/output.js new file mode 100644 index 000000000000..238f38fb3dd4 --- /dev/null +++ b/crates/swc_ecma_minifier/tests/fixture/issues/8718/3/output.js @@ -0,0 +1,2 @@ +let a; +console.log(a = "123"), console.log((a += 1, a += 2)); diff --git a/crates/swc_ecma_minifier/tests/fixture/issues/8718/4/input.js b/crates/swc_ecma_minifier/tests/fixture/issues/8718/4/input.js new file mode 100644 index 000000000000..3a89b22dfe15 --- /dev/null +++ b/crates/swc_ecma_minifier/tests/fixture/issues/8718/4/input.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))); diff --git a/crates/swc_ecma_minifier/tests/fixture/issues/8718/4/output.js b/crates/swc_ecma_minifier/tests/fixture/issues/8718/4/output.js new file mode 100644 index 000000000000..6366991f8001 --- /dev/null +++ b/crates/swc_ecma_minifier/tests/fixture/issues/8718/4/output.js @@ -0,0 +1,3 @@ +let a; +// a = "123"; +console.log(a), console.log(a = "123"), console.log((a += 1, a += 2)); diff --git a/crates/swc_ecma_minifier/tests/fixture/issues/8718/5/input.js b/crates/swc_ecma_minifier/tests/fixture/issues/8718/5/input.js new file mode 100644 index 000000000000..a19317780bcc --- /dev/null +++ b/crates/swc_ecma_minifier/tests/fixture/issues/8718/5/input.js @@ -0,0 +1,7 @@ +let a = 0; +function f() { + a = "123"; + console.log(a); +} + +console.log(((a += 1), (a += 2))); diff --git a/crates/swc_ecma_minifier/tests/fixture/issues/8718/5/output.js b/crates/swc_ecma_minifier/tests/fixture/issues/8718/5/output.js new file mode 100644 index 000000000000..3bd241b50e7c --- /dev/null +++ b/crates/swc_ecma_minifier/tests/fixture/issues/8718/5/output.js @@ -0,0 +1 @@ +console.log(3); diff --git a/crates/swc_ecma_minifier/tests/fixture/issues/8718/6/input.js b/crates/swc_ecma_minifier/tests/fixture/issues/8718/6/input.js new file mode 100644 index 000000000000..d22825361528 --- /dev/null +++ b/crates/swc_ecma_minifier/tests/fixture/issues/8718/6/input.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))); diff --git a/crates/swc_ecma_minifier/tests/fixture/issues/8718/6/output.js b/crates/swc_ecma_minifier/tests/fixture/issues/8718/6/output.js new file mode 100644 index 000000000000..3bd241b50e7c --- /dev/null +++ b/crates/swc_ecma_minifier/tests/fixture/issues/8718/6/output.js @@ -0,0 +1 @@ +console.log(3); diff --git a/crates/swc_ecma_usage_analyzer/src/analyzer/ctx.rs b/crates/swc_ecma_usage_analyzer/src/analyzer/ctx.rs index 422600fee872..dae2dc1c5414 100644 --- a/crates/swc_ecma_usage_analyzer/src/analyzer/ctx.rs +++ b/crates/swc_ecma_usage_analyzer/src/analyzer/ctx.rs @@ -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>, pub in_pat_of_param: bool, pub in_catch_param: bool, diff --git a/crates/swc_ecma_usage_analyzer/src/analyzer/mod.rs b/crates/swc_ecma_usage_analyzer/src/analyzer/mod.rs index 37d3eb8a6699..4cb660d462b8 100644 --- a/crates/swc_ecma_usage_analyzer/src/analyzer/mod.rs +++ b/crates/swc_ecma_usage_analyzer/src/analyzer/mod.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) { 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>, kind: Option, 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() diff --git a/crates/swc_ecma_usage_analyzer/src/analyzer/storage.rs b/crates/swc_ecma_usage_analyzer/src/analyzer/storage.rs index 5b2209c89dbb..49fea01d998b 100644 --- a/crates/swc_ecma_usage_analyzer/src/analyzer/storage.rs +++ b/crates/swc_ecma_usage_analyzer/src/analyzer/storage.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); fn declare_decl( &mut self, ctx: Ctx, i: &Ident, - has_init: bool, + init_type: Option>, kind: Option, ) -> &mut Self::VarData; diff --git a/crates/swc_ecma_utils/src/lib.rs b/crates/swc_ecma_utils/src/lib.rs index 9de7b817aef2..a93096a861b5 100644 --- a/crates/swc_ecma_utils/src/lib.rs +++ b/crates/swc_ecma_utils/src/lib.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, diff --git a/crates/swc_ecma_utils/src/value.rs b/crates/swc_ecma_utils/src/value.rs index 3fc655935090..09bcedbbd8b3 100644 --- a/crates/swc_ecma_utils/src/value.rs +++ b/crates/swc_ecma_utils/src/value.rs @@ -108,3 +108,18 @@ impl Not for Value { } } } + +pub trait Merge { + fn merge(&mut self, rhs: Self); +} + +impl Merge for Option> { + 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), + } + } +}