fix(es/minifier): Abort seq inliner if a same var is defined in outer…

… scope (#7772)

**Description:**

The algorithm here is directly copied from terser, I don't if it's correct or can be improved, but it does fix the issue.


**Related issue:**

 - Closes #7749

crates/swc_ecma_ast/src/expr.rs

@@ -613,6 +613,12 @@ impl Take for AssignExpr {
     }
 }
 
+impl AssignExpr {
+    pub fn is_simple_assign(&self) -> bool {
+        self.op == op!("=") && self.left.as_ident().is_some()
+    }
+}
+
 // Custom deserializer to convert `PatOrExpr::Pat(Box<Pat::Ident>)`
 // to `PatOrExpr::Expr(Box<Expr::Ident>)` when `op` is not `=`.
 // Same logic as parser:

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

@@ -869,9 +869,21 @@ impl Optimizer<'_> {
                         let ids: Vec<Id> = find_pat_ids(&decl.name);
 
                         for id in ids {
-                            remap
-                                .entry(id)
+                            let ctx = remap
+                                .entry(id.clone())
                                 .or_insert_with(|| SyntaxContext::empty().apply_mark(Mark::new()));
+
+                            // [is_skippable_for_seq] would check fn scope
+                            if let Some(usage) = self.data.vars.get(&id) {
+                                let mut usage = usage.clone();
+                                usage.in_fn_scope_of = self.ctx.fn_scope;
+                                // as we turn var declaration into assignment
+                                // we need to maintain correct var usage
+                                if decl.init.is_some() {
+                                    usage.ref_count += 1;
+                                }
+                                self.data.vars.insert((id.0, *ctx), usage);
+                            }
                         }
                     }
                 }

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

@@ -174,6 +174,9 @@ struct Ctx {
 
     /// Current scope.
     scope: SyntaxContext,
+
+    /// Current function scope
+    fn_scope: SyntaxContext,
 }
 
 impl Ctx {
@@ -2134,6 +2137,7 @@ impl VisitMut for Optimizer<'_> {
                 skip_standalone: self.ctx.skip_standalone || is_standalone,
                 in_fn_like: true,
                 scope: n.span.ctxt,
+                fn_scope: n.span.ctxt,
                 top_level: false,
 
                 ..self.ctx

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

@@ -1158,25 +1158,14 @@ impl Optimizer<'_> {
                             )
                         }),
                         Mergable::Expr(a) => match a {
-                            Expr::Assign(AssignExpr {
-                                left,
-                                right,
-                                op: op!("="),
-                                ..
-                            }) => {
-                                if left.as_ident().is_some() {
-                                    Some(collect_infects_from(
-                                        right,
-                                        AliasConfig {
-                                            marks: Some(self.marks),
-                                            ignore_nested: true,
-                                            need_all: true,
-                                        },
-                                    ))
-                                } else {
-                                    None
-                                }
-                            }
+                            Expr::Assign(a) if a.is_simple_assign() => Some(collect_infects_from(
+                                &a.right,
+                                AliasConfig {
+                                    marks: Some(self.marks),
+                                    ignore_nested: true,
+                                    need_all: true,
+                                },
+                            )),
 
                             _ => None,
                         },
@@ -1270,18 +1259,43 @@ impl Optimizer<'_> {
                 };
 
                 if let Some(a) = a {
+                    let left_fn_scope = self
+                        .data
+                        .vars
+                        .get(&left_id.to_id())
+                        .map(|u| u.in_fn_scope_of)
+                        .unwrap_or(self.expr_ctx.unresolved_ctxt);
                     match a {
                         Mergable::Var(a) => {
                             if is_ident_used_by(left_id.to_id(), &**a) {
                                 log_abort!("e.left is used by a (var)");
                                 return false;
                             }
+
+                            if let Some(init) = &a.init {
+                                if init.may_have_side_effects(&self.expr_ctx)
+                                    && self.ctx.fn_scope != left_fn_scope
+                                {
+                                    log_abort!("a (var) init has side effect");
+                                    return false;
+                                }
+                            }
                         }
                         Mergable::Expr(a) => {
                             if is_ident_used_by(left_id.to_id(), &**a) {
                                 log_abort!("e.left is used by a (expr)");
                                 return false;
                             }
+                            let has_side_effect = match a {
+                                Expr::Assign(a) if a.is_simple_assign() => {
+                                    a.right.may_have_side_effects(&self.expr_ctx)
+                                }
+                                _ => a.may_have_side_effects(&self.expr_ctx),
+                            };
+                            if has_side_effect && self.ctx.fn_scope != left_fn_scope {
+                                log_abort!("a (expr) has side effect");
+                                return false;
+                            }
                         }
                         Mergable::FnDecl(a) => {
                             // TODO(kdy1): I'm not sure if this check is required.
@@ -1725,9 +1739,10 @@ impl Optimizer<'_> {
                 }
 
                 let b_left = b_assign.left.as_ident();
-                let b_left = match b_left {
-                    Some(v) => v.clone(),
-                    None => return Ok(false),
+                let b_left = if let Some(v) = b_left {
+                    v.clone()
+                } else {
+                    return Ok(false);
                 };
 
                 if !self.is_skippable_for_seq(Some(a), &Expr::Ident(b_left.clone())) {
@@ -2213,7 +2228,11 @@ impl Optimizer<'_> {
                             }
 
                             // We can remove this variable same as unused pass
-                            if !usage.reassigned() && usage.usage_count == 1 && usage.declared {
+                            if !usage.reassigned()
+                                && usage.usage_count == 1
+                                && usage.declared
+                                && !usage.used_recursively
+                            {
                                 can_remove = true;
                             }
                         } else {

crates/swc_ecma_minifier/src/program_data.rs

@@ -137,6 +137,8 @@ pub(crate) struct VarUsageInfo {
     pub(crate) accessed_props: Box<AHashMap<JsWord, u32>>,
 
     pub(crate) used_recursively: bool,
+
+    pub(crate) in_fn_scope_of: SyntaxContext,
 }
 
 impl Default for VarUsageInfo {
@@ -177,6 +179,7 @@ impl Default for VarUsageInfo {
             is_top_level: Default::default(),
             assigned_fn_local: true,
             used_as_ref: false,
+            in_fn_scope_of: Default::default(),
         }
     }
 }
@@ -364,6 +367,7 @@ impl Storage for ProgramData {
         // }
 
         let v = self.vars.entry(i.to_id()).or_default();
+        v.in_fn_scope_of = ctx.fn_scope;
         v.is_top_level |= ctx.is_top_level;
 
         if has_init && (v.declared || v.var_initialized) {

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

@@ -26730,21 +26730,21 @@
     }
     function updateRectShape(controller, cover, name, x, y, w, h) {
         var points, xmin, ymin, el = cover.childOfName(name);
-        el && el.setShape((xmin = mathMin$9((points = clipByPanel(controller, cover, [
-            [
-                x,
-                y
-            ],
-            [
-                x + w,
-                y + h
-            ]
-        ]))[0][0], points[1][0]), {
-            x: xmin,
+        el && el.setShape({
+            x: xmin = mathMin$9((points = clipByPanel(controller, cover, [
+                [
+                    x,
+                    y
+                ],
+                [
+                    x + w,
+                    y + h
+                ]
+            ]))[0][0], points[1][0]),
             y: ymin = mathMin$9(points[0][1], points[1][1]),
             width: mathMax$9(points[0][0], points[1][0]) - xmin,
             height: mathMax$9(points[0][1], points[1][1]) - ymin
-        }));
+        });
     }
     function makeStyle(brushOption) {
         return defaults({
@@ -34700,25 +34700,25 @@
             header.innerHTML = lang[0] || model.get('title'), header.style.cssText = 'margin: 10px 20px;', header.style.color = model.get('textColor');
             var viewMain = document.createElement('div'), textarea = document.createElement('textarea');
             viewMain.style.cssText = 'display:block;width:100%;overflow:auto;';
-            var optionToContent = model.get('optionToContent'), contentToOption = model.get('contentToOption'), result1 = (seriesGroupByCategoryAxis = {}, otherSeries = [], meta = [], ecModel.eachRawSeries(function(seriesModel) {
-                var coordSys = seriesModel.coordinateSystem;
-                if (coordSys && ('cartesian2d' === coordSys.type || 'polar' === coordSys.type)) {
-                    var baseAxis = coordSys.getBaseAxis();
-                    if ('category' === baseAxis.type) {
-                        var key = baseAxis.dim + '_' + baseAxis.index;
-                        seriesGroupByCategoryAxis[key] || (seriesGroupByCategoryAxis[key] = {
-                            categoryAxis: baseAxis,
-                            valueAxis: coordSys.getOtherAxis(baseAxis),
-                            series: []
-                        }, meta.push({
-                            axisDim: baseAxis.dim,
-                            axisIndex: baseAxis.index
-                        })), seriesGroupByCategoryAxis[key].series.push(seriesModel);
-                    } else otherSeries.push(seriesModel);
-                } else otherSeries.push(seriesModel);
-            }), {
+            var optionToContent = model.get('optionToContent'), contentToOption = model.get('contentToOption'), result1 = {
                 value: filter([
-                    (groups = (result = {
+                    (groups = (seriesGroupByCategoryAxis = {}, otherSeries = [], meta = [], ecModel.eachRawSeries(function(seriesModel) {
+                        var coordSys = seriesModel.coordinateSystem;
+                        if (coordSys && ('cartesian2d' === coordSys.type || 'polar' === coordSys.type)) {
+                            var baseAxis = coordSys.getBaseAxis();
+                            if ('category' === baseAxis.type) {
+                                var key = baseAxis.dim + '_' + baseAxis.index;
+                                seriesGroupByCategoryAxis[key] || (seriesGroupByCategoryAxis[key] = {
+                                    categoryAxis: baseAxis,
+                                    valueAxis: coordSys.getOtherAxis(baseAxis),
+                                    series: []
+                                }, meta.push({
+                                    axisDim: baseAxis.dim,
+                                    axisIndex: baseAxis.index
+                                })), seriesGroupByCategoryAxis[key].series.push(seriesModel);
+                            } else otherSeries.push(seriesModel);
+                        } else otherSeries.push(seriesModel);
+                    }), result = {
                         seriesGroupByCategoryAxis: seriesGroupByCategoryAxis,
                         other: otherSeries,
                         meta: meta
@@ -34757,7 +34757,7 @@
                     return !!str.replace(/[\n\t\s]/g, '');
                 }).join('\n\n' + BLOCK_SPLITER + '\n\n'),
                 meta: result.meta
-            });
+            };
             if ('function' == typeof optionToContent) {
                 var htmlOrDom = optionToContent(api.getOption());
                 'string' == typeof htmlOrDom ? viewMain.innerHTML = htmlOrDom : isDom(htmlOrDom) && viewMain.appendChild(htmlOrDom);

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

@@ -13437,9 +13437,9 @@
                 return childComponents.map(function(child, index) {
                     var child1, props1, calculatedProps1, domain, scale, stringMap, categories, axisChild, role = child.type && child.type.role, style = Array.isArray(child.props.style) ? child.props.style : lodash_defaults__WEBPACK_IMPORTED_MODULE_1___default()({}, child.props.style, {
                         parent: baseStyle
-                    }), childProps = (child1 = child, props1 = props, calculatedProps1 = calculatedProps, axisChild = victory_core__WEBPACK_IMPORTED_MODULE_3__.Axis.findAxisComponents([
+                    }), childProps = (child1 = child, props1 = props, calculatedProps1 = calculatedProps, (axisChild = victory_core__WEBPACK_IMPORTED_MODULE_3__.Axis.findAxisComponents([
                         child1
-                    ]), axisChild.length > 0 ? (axisChild[0], domain = calculatedProps1.domain, scale = calculatedProps1.scale, stringMap = calculatedProps1.stringMap, categories = calculatedProps1.categories, {
+                    ])).length > 0 ? (axisChild[0], domain = calculatedProps1.domain, scale = calculatedProps1.scale, stringMap = calculatedProps1.stringMap, categories = calculatedProps1.categories, {
                         stringMap: stringMap,
                         horizontal: calculatedProps1.horizontal,
                         categories: categories,
@@ -18467,8 +18467,8 @@
                             x: x,
                             y: y
                         };
-                        var polarPadding = (style = props.style, degrees = getDegrees(props, datum), labelStyle = style.labels || {}, padding = _helpers__WEBPACK_IMPORTED_MODULE_1__.default.evaluateProp(labelStyle.padding, props) || 0, {
-                            x: padding * Math.cos(angle = _helpers__WEBPACK_IMPORTED_MODULE_1__.default.degreesToRadians(degrees)),
+                        var polarPadding = (style = props.style, degrees = getDegrees(props, datum), labelStyle = style.labels || {}, {
+                            x: (padding = _helpers__WEBPACK_IMPORTED_MODULE_1__.default.evaluateProp(labelStyle.padding, props) || 0) * Math.cos(angle = _helpers__WEBPACK_IMPORTED_MODULE_1__.default.degreesToRadians(degrees)),
                             y: -padding * Math.sin(angle)
                         });
                         return {
@@ -28248,9 +28248,9 @@
                     {
                         key: "getCachedSharedEvents",
                         value: function(name, cacheValues) {
-                            var arr, _ref2 = function(arr) {
+                            var _ref = this.sharedEventsCache[name] || [], _ref2 = function(arr) {
                                 if (Array.isArray(arr)) return arr;
-                            }(arr = this.sharedEventsCache[name] || []) || function(arr, i) {
+                            }(_ref) || function(arr, i) {
                                 var _arr = [], _n = !0, _d = !1, _e = void 0;
                                 try {
                                     for(var _s, _i = arr[Symbol.iterator](); !(_n = (_s = _i.next()).done) && (_arr.push(_s.value), !i || _arr.length !== i); _n = !0);
@@ -28264,7 +28264,7 @@
                                     }
                                 }
                                 return _arr;
-                            }(arr, 2) || function() {
+                            }(_ref, 2) || function() {
                                 throw TypeError("Invalid attempt to destructure non-iterable instance");
                             }(), sharedEvents = _ref2[0], prevCacheValues = _ref2[1];
                             if (sharedEvents && react_fast_compare__WEBPACK_IMPORTED_MODULE_10___default()(cacheValues, prevCacheValues)) return sharedEvents;
@@ -28468,13 +28468,13 @@
                     return dataset.filter(function(datum) {
                         return null !== datum._x && null !== datum._y;
                     });
-                }), fillInMissingData = props1.fillInMissingData, xMap = filterNullChildData.reduce(function(prev, dataset) {
+                }), (fillInMissingData = props1.fillInMissingData, xMap = filterNullChildData.reduce(function(prev, dataset) {
                     return dataset.forEach(function(datum) {
                         prev[datum._x instanceof Date ? datum._x.getTime() : datum._x] = !0;
                     }), prev;
                 }, {}), xKeys = lodash_keys__WEBPACK_IMPORTED_MODULE_1___default()(xMap).map(function(k) {
                     return +k;
-                }), xArr = lodash_orderBy__WEBPACK_IMPORTED_MODULE_0___default()(xKeys), (datasets = filterNullChildData.map(function(dataset) {
+                }), xArr = lodash_orderBy__WEBPACK_IMPORTED_MODULE_0___default()(xKeys), datasets = filterNullChildData.map(function(dataset) {
                     var indexOffset = 0, isDate = dataset[0] && dataset[0]._x instanceof Date;
                     return xArr.map(function(x, index) {
                         x = +x;