From 006258e6e9109ca7580baf8dab74a83ecffc0f41 Mon Sep 17 00:00:00 2001 From: YunfeiHe Date: Fri, 18 Nov 2022 17:30:57 +0800 Subject: [PATCH 1/9] fix(es/minifier): should not drop statements which has side-effects --- .../src/compress/optimize/mod.rs | 25 ++++++++++++------- .../tests/fixture/issues/6422/2/config.json | 4 +++ .../tests/fixture/issues/6422/2/input.js | 10 ++++++++ .../tests/fixture/issues/6422/2/output.js | 10 ++++++++ .../tests/fixture/issues/6422/3/config.json | 4 +++ .../tests/fixture/issues/6422/3/input.js | 18 +++++++++++++ .../tests/fixture/issues/6422/3/output.js | 18 +++++++++++++ 7 files changed, 80 insertions(+), 9 deletions(-) create mode 100644 crates/swc_ecma_minifier/tests/fixture/issues/6422/2/config.json create mode 100644 crates/swc_ecma_minifier/tests/fixture/issues/6422/2/input.js create mode 100644 crates/swc_ecma_minifier/tests/fixture/issues/6422/2/output.js create mode 100644 crates/swc_ecma_minifier/tests/fixture/issues/6422/3/config.json create mode 100644 crates/swc_ecma_minifier/tests/fixture/issues/6422/3/input.js create mode 100644 crates/swc_ecma_minifier/tests/fixture/issues/6422/3/output.js diff --git a/crates/swc_ecma_minifier/src/compress/optimize/mod.rs b/crates/swc_ecma_minifier/src/compress/optimize/mod.rs index 8f3cd3d4a68a..1da8a3bde6af 100644 --- a/crates/swc_ecma_minifier/src/compress/optimize/mod.rs +++ b/crates/swc_ecma_minifier/src/compress/optimize/mod.rs @@ -1918,15 +1918,22 @@ where #[cfg(feature = "debug")] let start = dump(&n.expr, true); - let expr = self.ignore_return_value(&mut n.expr); - n.expr = expr.map(Box::new).unwrap_or_else(|| { - report_change!("visit_mut_expr_stmt: Dropped an expression statement"); - - #[cfg(feature = "debug")] - dump_change_detail!("Removed {}", start); - - undefined(DUMMY_SP) - }); + let is_object_lit_with_spread = n + .expr + .as_object() + .map(|object_lit| object_lit.props.iter().any(|prop| prop.is_spread())) + .unwrap_or(false); + + if !is_object_lit_with_spread { + let expr = self.ignore_return_value(&mut n.expr); + n.expr = expr.map(Box::new).unwrap_or_else(|| { + report_change!("visit_mut_expr_stmt: Dropped an expression statement"); + #[cfg(feature = "debug")] + dump_change_detail!("Removed {}", start); + + undefined(DUMMY_SP) + }); + } } else { match &mut *n.expr { Expr::Seq(e) => { diff --git a/crates/swc_ecma_minifier/tests/fixture/issues/6422/2/config.json b/crates/swc_ecma_minifier/tests/fixture/issues/6422/2/config.json new file mode 100644 index 000000000000..8e1ba5e679a7 --- /dev/null +++ b/crates/swc_ecma_minifier/tests/fixture/issues/6422/2/config.json @@ -0,0 +1,4 @@ +{ + "toplevel": true, + "unused": true +} \ No newline at end of file diff --git a/crates/swc_ecma_minifier/tests/fixture/issues/6422/2/input.js b/crates/swc_ecma_minifier/tests/fixture/issues/6422/2/input.js new file mode 100644 index 000000000000..373eacbebfc1 --- /dev/null +++ b/crates/swc_ecma_minifier/tests/fixture/issues/6422/2/input.js @@ -0,0 +1,10 @@ +import assert from 'assert' +let result = 'FAIL'; +const unused = { + ...{ + get prop() { + result = 'PASS'; + } + } +}; +assert.strictEqual(result, 'PASS'); \ No newline at end of file diff --git a/crates/swc_ecma_minifier/tests/fixture/issues/6422/2/output.js b/crates/swc_ecma_minifier/tests/fixture/issues/6422/2/output.js new file mode 100644 index 000000000000..d899d5cc5b61 --- /dev/null +++ b/crates/swc_ecma_minifier/tests/fixture/issues/6422/2/output.js @@ -0,0 +1,10 @@ +import assert from 'assert'; +let result = 'FAIL'; +({ + ...{ + get prop () { + result = 'PASS'; + } + } +}); +assert.strictEqual(result, 'PASS'); diff --git a/crates/swc_ecma_minifier/tests/fixture/issues/6422/3/config.json b/crates/swc_ecma_minifier/tests/fixture/issues/6422/3/config.json new file mode 100644 index 000000000000..8e1ba5e679a7 --- /dev/null +++ b/crates/swc_ecma_minifier/tests/fixture/issues/6422/3/config.json @@ -0,0 +1,4 @@ +{ + "toplevel": true, + "unused": true +} \ No newline at end of file diff --git a/crates/swc_ecma_minifier/tests/fixture/issues/6422/3/input.js b/crates/swc_ecma_minifier/tests/fixture/issues/6422/3/input.js new file mode 100644 index 000000000000..da075da2c7d7 --- /dev/null +++ b/crates/swc_ecma_minifier/tests/fixture/issues/6422/3/input.js @@ -0,0 +1,18 @@ +import assert from 'assert' +let result = 0; +const unused = { + ...{ + get prop() { + result = 1; + } + }, + [assert.strictEqual(result, 1)]: null, + [result = 2]: null, + [assert.strictEqual(result, 2)]: null, + ...{ + get prop() { + result = 3; + } + } +}; +assert.strictEqual(result, 3); \ No newline at end of file diff --git a/crates/swc_ecma_minifier/tests/fixture/issues/6422/3/output.js b/crates/swc_ecma_minifier/tests/fixture/issues/6422/3/output.js new file mode 100644 index 000000000000..eaeef3af06c2 --- /dev/null +++ b/crates/swc_ecma_minifier/tests/fixture/issues/6422/3/output.js @@ -0,0 +1,18 @@ +import assert from 'assert'; +let result = 0; +({ + ...{ + get prop () { + result = 1; + } + }, + [assert.strictEqual(result, 1)]: null, + [result = 2]: null, + [assert.strictEqual(result, 2)]: null, + ...{ + get prop () { + result = 3; + } + } +}); +assert.strictEqual(result, 3); From 790fed90e9c06af8debd99c7d4b549350faaaebd Mon Sep 17 00:00:00 2001 From: YunfeiHe Date: Fri, 18 Nov 2022 19:01:03 +0800 Subject: [PATCH 2/9] Add case for respecting side-effect of getter and setter --- crates/swc_atoms/words.txt | 1 + .../src/compress/optimize/mod.rs | 2 ++ .../tests/fixture/issues/6422/1/config.json | 5 +++ .../tests/fixture/issues/6422/1/input.js | 21 +++++++++++++ .../tests/fixture/issues/6422/1/output.js | 18 +++++++++++ crates/swc_ecma_utils/src/lib.rs | 31 +++++++++++++------ 6 files changed, 68 insertions(+), 10 deletions(-) create mode 100644 crates/swc_ecma_minifier/tests/fixture/issues/6422/1/config.json create mode 100644 crates/swc_ecma_minifier/tests/fixture/issues/6422/1/input.js create mode 100644 crates/swc_ecma_minifier/tests/fixture/issues/6422/1/output.js diff --git a/crates/swc_atoms/words.txt b/crates/swc_atoms/words.txt index 07b7a5fe01b0..dcdf635a8ace 100644 --- a/crates/swc_atoms/words.txt +++ b/crates/swc_atoms/words.txt @@ -1369,3 +1369,4 @@ ychannelselector yield zoomAndPan zoomandpan +__proto__ \ No newline at end of file diff --git a/crates/swc_ecma_minifier/src/compress/optimize/mod.rs b/crates/swc_ecma_minifier/src/compress/optimize/mod.rs index 1da8a3bde6af..9435775c5f80 100644 --- a/crates/swc_ecma_minifier/src/compress/optimize/mod.rs +++ b/crates/swc_ecma_minifier/src/compress/optimize/mod.rs @@ -1918,6 +1918,7 @@ where #[cfg(feature = "debug")] let start = dump(&n.expr, true); + // Fix https://github.com/swc-project/swc/issues/6422 let is_object_lit_with_spread = n .expr .as_object() @@ -2572,6 +2573,7 @@ where if can_be_removed { self.changed = true; report_change!("unused: Dropping an expression without side effect"); + tracing::debug!("unused: Dropping \n{}\n", dump(&*expr, false)); dump_change_detail!("unused: Dropping \n{}\n", dump(&*expr, false)); *s = Stmt::Empty(EmptyStmt { span: DUMMY_SP }); return; diff --git a/crates/swc_ecma_minifier/tests/fixture/issues/6422/1/config.json b/crates/swc_ecma_minifier/tests/fixture/issues/6422/1/config.json new file mode 100644 index 000000000000..480beee76639 --- /dev/null +++ b/crates/swc_ecma_minifier/tests/fixture/issues/6422/1/config.json @@ -0,0 +1,5 @@ +{ + "collapse_vars": true, + "unused": true, + "toplevel": true +} \ No newline at end of file diff --git a/crates/swc_ecma_minifier/tests/fixture/issues/6422/1/input.js b/crates/swc_ecma_minifier/tests/fixture/issues/6422/1/input.js new file mode 100644 index 000000000000..84de0d303429 --- /dev/null +++ b/crates/swc_ecma_minifier/tests/fixture/issues/6422/1/input.js @@ -0,0 +1,21 @@ +let getter_effect = 'FAIL'; +let setter_effect = 'FAIL'; +let proto = { + get foo() { + getter_effect = 'PASS'; + }, + set bar(value) { + setter_effect = 'PASS'; + } +}; +let obj1 = { + __proto__: proto +}; +let obj2 = { + __proto__: proto +}; +let unused = obj1.foo; +obj2.bar = 0; + +assert.strictEqual(getter_effect, 'PASS'); +assert.strictEqual(setter_effect, 'PASS'); \ No newline at end of file diff --git a/crates/swc_ecma_minifier/tests/fixture/issues/6422/1/output.js b/crates/swc_ecma_minifier/tests/fixture/issues/6422/1/output.js new file mode 100644 index 000000000000..e3d96e8a2cbc --- /dev/null +++ b/crates/swc_ecma_minifier/tests/fixture/issues/6422/1/output.js @@ -0,0 +1,18 @@ +let getter_effect = 'FAIL'; +let setter_effect = 'FAIL'; +let proto = { + get foo () { + getter_effect = 'PASS'; + }, + set bar (value){ + setter_effect = 'PASS'; + } +}; +({ + __proto__: proto +}).foo; +({ + __proto__: proto +}).bar = 0; +assert.strictEqual(getter_effect, 'PASS'); +assert.strictEqual(setter_effect, 'PASS'); diff --git a/crates/swc_ecma_utils/src/lib.rs b/crates/swc_ecma_utils/src/lib.rs index f2aa790f538a..e16f3c2beaf0 100644 --- a/crates/swc_ecma_utils/src/lib.rs +++ b/crates/swc_ecma_utils/src/lib.rs @@ -1375,18 +1375,29 @@ pub trait ExprExt { } } Expr::Object(obj) => { - let is_static_accessor = |prop: &PropOrSpread| { - if let PropOrSpread::Prop(prop) = prop { - if let Prop::Getter(_) | Prop::Setter(_) = &**prop { - true - } else { - false - } - } else { - false + let is_prop_could_has_side_effect = |prop: &PropOrSpread| { + match prop { + // ({ ...{ __proto__: { get foo() { a = 1; } } } }).foo + PropOrSpread::Spread(_) => true, + PropOrSpread::Prop(prop) => match prop.as_ref() { + Prop::Shorthand(ident) => ident.sym == js_word!("__proto__"), + Prop::KeyValue(KeyValueProp { key, value, .. }) => match key { + PropName::Ident(ident) => { + ident.sym == js_word!("__proto__") + } + PropName::Str(str) => str.value == js_word!("__proto__"), + PropName::Computed(_) => true, + _ => false, + }, + Prop::Getter(..) | Prop::Setter(..) => true, + Prop::Method(prop) => true, + Prop::Assign(_) => { + unreachable!("This is **invalid** for object literal") + } + }, } }; - if obj.props.iter().any(is_static_accessor) { + if obj.props.iter().any(is_prop_could_has_side_effect) { return true; } } From 8e8fab7e40786b15ff653f575a97278d5c6cfccb Mon Sep 17 00:00:00 2001 From: YunfeiHe Date: Fri, 18 Nov 2022 19:24:47 +0800 Subject: [PATCH 3/9] Commit changed snapshots --- .../tsc-references/objectSpreadSetonlyAccessor.2.minified.js | 5 +++++ crates/swc_ecma_minifier/src/compress/optimize/mod.rs | 1 - 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/swc/tests/tsc-references/objectSpreadSetonlyAccessor.2.minified.js b/crates/swc/tests/tsc-references/objectSpreadSetonlyAccessor.2.minified.js index b71455030685..80186d638761 100644 --- a/crates/swc/tests/tsc-references/objectSpreadSetonlyAccessor.2.minified.js +++ b/crates/swc/tests/tsc-references/objectSpreadSetonlyAccessor.2.minified.js @@ -1 +1,6 @@ //// [objectSpreadSetonlyAccessor.ts] +({ + ...{ + set foo (_v){} + } +}); diff --git a/crates/swc_ecma_minifier/src/compress/optimize/mod.rs b/crates/swc_ecma_minifier/src/compress/optimize/mod.rs index 9435775c5f80..a140edf0915f 100644 --- a/crates/swc_ecma_minifier/src/compress/optimize/mod.rs +++ b/crates/swc_ecma_minifier/src/compress/optimize/mod.rs @@ -2573,7 +2573,6 @@ where if can_be_removed { self.changed = true; report_change!("unused: Dropping an expression without side effect"); - tracing::debug!("unused: Dropping \n{}\n", dump(&*expr, false)); dump_change_detail!("unused: Dropping \n{}\n", dump(&*expr, false)); *s = Stmt::Empty(EmptyStmt { span: DUMMY_SP }); return; From b9ebf40addde0a953a8f78471aadbf73287a03b3 Mon Sep 17 00:00:00 2001 From: YunfeiHe Date: Fri, 18 Nov 2022 20:38:44 +0800 Subject: [PATCH 4/9] Fix clippy --- crates/swc_ecma_utils/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/swc_ecma_utils/src/lib.rs b/crates/swc_ecma_utils/src/lib.rs index e16f3c2beaf0..387c7e542c52 100644 --- a/crates/swc_ecma_utils/src/lib.rs +++ b/crates/swc_ecma_utils/src/lib.rs @@ -1381,16 +1381,16 @@ pub trait ExprExt { PropOrSpread::Spread(_) => true, PropOrSpread::Prop(prop) => match prop.as_ref() { Prop::Shorthand(ident) => ident.sym == js_word!("__proto__"), - Prop::KeyValue(KeyValueProp { key, value, .. }) => match key { + Prop::KeyValue(KeyValueProp { key, .. }) => match key { PropName::Ident(ident) => { ident.sym == js_word!("__proto__") } - PropName::Str(str) => str.value == js_word!("__proto__"), + PropName::Str(s) => s.value == js_word!("__proto__"), PropName::Computed(_) => true, _ => false, }, Prop::Getter(..) | Prop::Setter(..) => true, - Prop::Method(prop) => true, + Prop::Method(_) => true, Prop::Assign(_) => { unreachable!("This is **invalid** for object literal") } From be1ac30d93587e4ed108990fef6432e2d043e96d Mon Sep 17 00:00:00 2001 From: YunfeiHe Date: Sat, 19 Nov 2022 11:39:27 +0800 Subject: [PATCH 5/9] Rename variable --- crates/swc_ecma_utils/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/swc_ecma_utils/src/lib.rs b/crates/swc_ecma_utils/src/lib.rs index 387c7e542c52..06671530e8f5 100644 --- a/crates/swc_ecma_utils/src/lib.rs +++ b/crates/swc_ecma_utils/src/lib.rs @@ -1375,7 +1375,7 @@ pub trait ExprExt { } } Expr::Object(obj) => { - let is_prop_could_has_side_effect = |prop: &PropOrSpread| { + let can_have_side_effect = |prop: &PropOrSpread| { match prop { // ({ ...{ __proto__: { get foo() { a = 1; } } } }).foo PropOrSpread::Spread(_) => true, @@ -1397,7 +1397,7 @@ pub trait ExprExt { }, } }; - if obj.props.iter().any(is_prop_could_has_side_effect) { + if obj.props.iter().any(can_have_side_effect) { return true; } } From 4952c38f71396f57a23768b8e7d56a06a0e268fc Mon Sep 17 00:00:00 2001 From: YunfeiHe Date: Sat, 19 Nov 2022 16:28:18 +0800 Subject: [PATCH 6/9] Remove comment --- crates/swc_ecma_utils/src/lib.rs | 35 ++++++++++++++------------------ 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/crates/swc_ecma_utils/src/lib.rs b/crates/swc_ecma_utils/src/lib.rs index 06671530e8f5..cfa04b74257b 100644 --- a/crates/swc_ecma_utils/src/lib.rs +++ b/crates/swc_ecma_utils/src/lib.rs @@ -1375,27 +1375,22 @@ pub trait ExprExt { } } Expr::Object(obj) => { - let can_have_side_effect = |prop: &PropOrSpread| { - match prop { - // ({ ...{ __proto__: { get foo() { a = 1; } } } }).foo - PropOrSpread::Spread(_) => true, - PropOrSpread::Prop(prop) => match prop.as_ref() { - Prop::Shorthand(ident) => ident.sym == js_word!("__proto__"), - Prop::KeyValue(KeyValueProp { key, .. }) => match key { - PropName::Ident(ident) => { - ident.sym == js_word!("__proto__") - } - PropName::Str(s) => s.value == js_word!("__proto__"), - PropName::Computed(_) => true, - _ => false, - }, - Prop::Getter(..) | Prop::Setter(..) => true, - Prop::Method(_) => true, - Prop::Assign(_) => { - unreachable!("This is **invalid** for object literal") - } + let can_have_side_effect = |prop: &PropOrSpread| match prop { + PropOrSpread::Spread(_) => true, + PropOrSpread::Prop(prop) => match prop.as_ref() { + Prop::Shorthand(i) => i.sym == js_word!("__proto__"), + Prop::KeyValue(KeyValueProp { key, .. }) => match key { + PropName::Ident(i) => i.sym == js_word!("__proto__"), + PropName::Str(s) => s.value == js_word!("__proto__"), + PropName::Computed(_) => true, + _ => false, }, - } + Prop::Getter(..) | Prop::Setter(..) => true, + Prop::Method(_) => true, + Prop::Assign(_) => { + unreachable!("This is **invalid** for object literal") + } + }, }; if obj.props.iter().any(can_have_side_effect) { return true; From 952af6ee18d7d16b05326a2a88dbb28ac8fb2e87 Mon Sep 17 00:00:00 2001 From: YunfeiHe Date: Sat, 19 Nov 2022 21:16:33 +0800 Subject: [PATCH 7/9] Code style --- crates/swc_ecma_utils/src/lib.rs | 36 ++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/crates/swc_ecma_utils/src/lib.rs b/crates/swc_ecma_utils/src/lib.rs index cfa04b74257b..cec60d8fedb7 100644 --- a/crates/swc_ecma_utils/src/lib.rs +++ b/crates/swc_ecma_utils/src/lib.rs @@ -1376,21 +1376,31 @@ pub trait ExprExt { } Expr::Object(obj) => { let can_have_side_effect = |prop: &PropOrSpread| match prop { - PropOrSpread::Spread(_) => true, - PropOrSpread::Prop(prop) => match prop.as_ref() { - Prop::Shorthand(i) => i.sym == js_word!("__proto__"), - Prop::KeyValue(KeyValueProp { key, .. }) => match key { - PropName::Ident(i) => i.sym == js_word!("__proto__"), - PropName::Str(s) => s.value == js_word!("__proto__"), - PropName::Computed(_) => true, + PropOrSpread::Spread(_) | PropOrSpread::Prop(prop) => { + match prop.as_ref() { + Prop::Getter(_) + | Prop::Setter(_) + | Prop::Method(_) + | Prop::Shorthand(Ident { + sym: js_word!("__proto__"), + .. + }) + | Prop::KeyValue(KeyValueProp { + key: + PropName::Ident(Ident { + sym: js_word!("__proto__"), + .. + }) + | PropName::Str(Str { + value: js_word!("__proto__"), + .. + }) + | PropName::Computed(_), + .. + }) => true, _ => false, - }, - Prop::Getter(..) | Prop::Setter(..) => true, - Prop::Method(_) => true, - Prop::Assign(_) => { - unreachable!("This is **invalid** for object literal") } - }, + } }; if obj.props.iter().any(can_have_side_effect) { return true; From 9f328a3e2cffcf4ab3bf61f37c72bd8744def508 Mon Sep 17 00:00:00 2001 From: YunfeiHe Date: Sat, 19 Nov 2022 21:19:06 +0800 Subject: [PATCH 8/9] Code style --- crates/swc_ecma_utils/src/lib.rs | 49 ++++++++++++++++---------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/crates/swc_ecma_utils/src/lib.rs b/crates/swc_ecma_utils/src/lib.rs index cec60d8fedb7..b6af8b5dbfe4 100644 --- a/crates/swc_ecma_utils/src/lib.rs +++ b/crates/swc_ecma_utils/src/lib.rs @@ -1376,31 +1376,30 @@ pub trait ExprExt { } Expr::Object(obj) => { let can_have_side_effect = |prop: &PropOrSpread| match prop { - PropOrSpread::Spread(_) | PropOrSpread::Prop(prop) => { - match prop.as_ref() { - Prop::Getter(_) - | Prop::Setter(_) - | Prop::Method(_) - | Prop::Shorthand(Ident { - sym: js_word!("__proto__"), - .. - }) - | Prop::KeyValue(KeyValueProp { - key: - PropName::Ident(Ident { - sym: js_word!("__proto__"), - .. - }) - | PropName::Str(Str { - value: js_word!("__proto__"), - .. - }) - | PropName::Computed(_), - .. - }) => true, - _ => false, - } - } + PropOrSpread::Spread(_) + | PropOrSpread::Prop( + Prop::Getter(_) + | Prop::Setter(_) + | Prop::Method(_) + | Prop::Shorthand(Ident { + sym: js_word!("__proto__"), + .. + }) + | Prop::KeyValue(KeyValueProp { + key: + PropName::Ident(Ident { + sym: js_word!("__proto__"), + .. + }) + | PropName::Str(Str { + value: js_word!("__proto__"), + .. + }) + | PropName::Computed(_), + .. + }), + ) => true, + _ => false, }; if obj.props.iter().any(can_have_side_effect) { return true; From f9c07f6e8e5ce91f6dc100f0facb15cd634dea53 Mon Sep 17 00:00:00 2001 From: YunfeiHe Date: Sat, 19 Nov 2022 21:39:48 +0800 Subject: [PATCH 9/9] Code style --- crates/swc_ecma_utils/src/lib.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/swc_ecma_utils/src/lib.rs b/crates/swc_ecma_utils/src/lib.rs index b6af8b5dbfe4..c11d36ebb596 100644 --- a/crates/swc_ecma_utils/src/lib.rs +++ b/crates/swc_ecma_utils/src/lib.rs @@ -1376,8 +1376,8 @@ pub trait ExprExt { } Expr::Object(obj) => { let can_have_side_effect = |prop: &PropOrSpread| match prop { - PropOrSpread::Spread(_) - | PropOrSpread::Prop( + PropOrSpread::Spread(_) => true, + PropOrSpread::Prop(prop) => match prop.as_ref() { Prop::Getter(_) | Prop::Setter(_) | Prop::Method(_) @@ -1397,9 +1397,9 @@ pub trait ExprExt { }) | PropName::Computed(_), .. - }), - ) => true, - _ => false, + }) => true, + _ => false, + }, }; if obj.props.iter().any(can_have_side_effect) { return true;