refact(es/minifier): Inline into shorthand prop early

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

@@ -912,7 +912,6 @@ impl Optimizer<'_> {
         }
     }
 
-    /// Actually inlines variables.
     pub(super) fn inline(&mut self, e: &mut Expr) {
         if self.ctx.bit_ctx.contains(BitCtx::IsExactLhsOfAssign) {
             return;
@@ -936,81 +935,89 @@ impl Optimizer<'_> {
                 }
             }
             Expr::Ident(i) => {
-                let id = i.to_id();
-                if let Some(value) = self.vars.lits.get(&id).or_else(|| {
-                    if self.ctx.bit_ctx.contains(BitCtx::IsCallee) {
-                        self.vars.simple_functions.get(&id)
-                    } else {
-                        None
-                    }
-                }) {
-                    if !matches!(**value, Expr::Ident(..) | Expr::Member(..))
-                        && self.ctx.bit_ctx.contains(BitCtx::IsUpdateArg)
-                    {
-                        return;
-                    }
-
-                    // currently renamer relies on the fact no distinct var has same ctxt, we need
-                    // to remap all new bindings.
-                    let bindings: FxHashSet<Id> = collect_decls(value);
-                    let new_mark = Mark::new();
-                    let mut cache = FxHashMap::default();
-                    let mut remap = FxHashMap::default();
-
-                    for id in bindings {
-                        let new_ctxt = cache
-                            .entry(id.1)
-                            .or_insert_with(|| id.1.apply_mark(new_mark));
+                if let Some(value) = self.inline_ident(i) {
+                    *e = *value
+                }
+            }
+            _ => (),
+        }
+    }
 
-                        let new_ctxt = *new_ctxt;
+    /// Actually inlines variables.
+    pub(super) fn inline_ident(&mut self, ident: &Ident) -> Option<Box<Expr>> {
+        let id = ident.to_id();
 
-                        if let Some(usage) = self.data.vars.get(&id).cloned() {
-                            let new_id = (id.0.clone(), new_ctxt);
-                            self.data.vars.insert(new_id, usage);
-                        }
+        if let Some(value) = self.vars.lits.get(&id).or_else(|| {
+            if self.ctx.bit_ctx.contains(BitCtx::IsCallee) {
+                self.vars.simple_functions.get(&id)
+            } else {
+                None
+            }
+        }) {
+            if !matches!(**value, Expr::Ident(..) | Expr::Member(..))
+                && self.ctx.bit_ctx.contains(BitCtx::IsUpdateArg)
+            {
+                return None;
+            }
 
-                        remap.insert(id, new_ctxt);
-                    }
+            // currently renamer relies on the fact no distinct var has same ctxt, we need
+            // to remap all new bindings.
+            let bindings: FxHashSet<Id> = collect_decls(value);
+            let new_mark = Mark::new();
+            let mut cache = FxHashMap::default();
+            let mut remap = FxHashMap::default();
 
-                    let mut value = value.clone();
-                    if !remap.is_empty() {
-                        let mut remapper = Remapper::new(&remap);
-                        value.visit_mut_with(&mut remapper);
-                    }
+            for id in bindings {
+                let new_ctxt = cache
+                    .entry(id.1)
+                    .or_insert_with(|| id.1.apply_mark(new_mark));
 
-                    self.changed = true;
-                    report_change!("inline: Replacing a variable `{}` with cheap expression", i);
+                let new_ctxt = *new_ctxt;
 
-                    *e = *value;
-                    return;
+                if let Some(usage) = self.data.vars.get(&id).cloned() {
+                    let new_id = (id.0.clone(), new_ctxt);
+                    self.data.vars.insert(new_id, usage);
                 }
 
-                // Check without cloning
-                if let Some(value) = self.vars.vars_for_inlining.get(&id) {
-                    if self.ctx.bit_ctx.contains(BitCtx::IsExactLhsOfAssign)
-                        && !is_valid_for_lhs(value)
-                    {
-                        return;
-                    }
+                remap.insert(id, new_ctxt);
+            }
 
-                    if let Expr::Member(..) = &**value {
-                        if self.ctx.bit_ctx.contains(BitCtx::ExecutedMultipleTime) {
-                            return;
-                        }
-                    }
-                }
+            let mut value = value.clone();
+            if !remap.is_empty() {
+                let mut remapper = Remapper::new(&remap);
+                value.visit_mut_with(&mut remapper);
+            }
 
-                if let Some(value) = self.vars.vars_for_inlining.remove(&id) {
-                    self.changed = true;
-                    report_change!("inline: Replacing '{}' with an expression", i);
+            self.changed = true;
+            report_change!(
+                "inline: Replacing a variable `{}` with cheap expression",
+                ident
+            );
 
-                    *e = *value;
+            return Some(value);
+        }
+
+        // Check without cloning
+        if let Some(value) = self.vars.vars_for_inlining.get(&id) {
+            if self.ctx.bit_ctx.contains(BitCtx::IsExactLhsOfAssign) && !is_valid_for_lhs(value) {
+                return None;
+            }
 
-                    log_abort!("inline: [Change] {}", crate::debug::dump(&*e, false))
+            if let Expr::Member(..) = &**value {
+                if self.ctx.bit_ctx.contains(BitCtx::ExecutedMultipleTime) {
+                    return None;
                 }
             }
-            _ => (),
         }
+
+        if let Some(value) = self.vars.vars_for_inlining.remove(&id) {
+            self.changed = true;
+            report_change!("inline: Replacing '{}' with an expression", ident);
+
+            return Some(value);
+        }
+
+        None
     }
 }
 

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

@@ -10,8 +10,8 @@ use swc_ecma_ast::*;
 use swc_ecma_transforms_base::rename::contains_eval;
 use swc_ecma_transforms_optimization::debug_assert_valid;
 use swc_ecma_utils::{
-    prepend_stmts, ExprCtx, ExprExt, ExprFactory, IdentUsageFinder, IsEmpty, ModuleItemLike,
-    StmtLike, Type, Value,
+    prepend_stmts, prop_name_from_ident, ExprCtx, ExprExt, ExprFactory, IdentUsageFinder, IsEmpty,
+    ModuleItemLike, StmtLike, Type, Value,
 };
 use swc_ecma_visit::{noop_visit_mut_type, VisitMut, VisitMutWith, VisitWith};
 #[cfg(feature = "debug")]
@@ -2489,6 +2489,18 @@ impl VisitMut for Optimizer<'_> {
         n.retain(|p| !p.pat.is_invalid());
     }
 
+    fn visit_mut_prop(&mut self, n: &mut Prop) {
+        n.visit_mut_children_with(self);
+
+        if let Prop::Shorthand(i) = n {
+            if let Some(expr) = self.inline_ident(i) {
+                let key = prop_name_from_ident(i.take());
+                *n = Prop::KeyValue(KeyValueProp { key, value: expr });
+                self.changed = true;
+            }
+        }
+    }
+
     #[cfg_attr(feature = "debug", tracing::instrument(level = "debug", skip_all))]
     fn visit_mut_return_stmt(&mut self, n: &mut ReturnStmt) {
         n.visit_mut_children_with(self);

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

@@ -9,7 +9,9 @@ use swc_atoms::{Atom, Wtf8Atom};
 use swc_common::{util::take::Take, Mark, SyntaxContext, DUMMY_SP};
 use swc_ecma_ast::*;
 use swc_ecma_transforms_base::perf::{Parallel, ParallelExt};
-use swc_ecma_utils::{collect_decls, contains_this_expr, ExprCtx, ExprExt, Remapper};
+use swc_ecma_utils::{
+    collect_decls, contains_this_expr, prop_name_from_ident, ExprCtx, ExprExt, Remapper,
+};
 use swc_ecma_visit::{noop_visit_mut_type, VisitMut, VisitMutWith};
 use tracing::debug;
 
@@ -822,21 +824,3 @@ pub fn get_ids_of_pat(pat: &Pat) -> Vec<Id> {
     append(pat, &mut idents);
     idents
 }
-
-/// Creates a PropName for a shorthand property, handling the special case of
-/// `__proto__`. When the property name is `__proto__`, it must be converted to
-/// a computed property to preserve JavaScript semantics.
-fn prop_name_from_ident(ident: Ident) -> PropName {
-    if ident.sym == "__proto__" {
-        PropName::Computed(ComputedPropName {
-            span: ident.span,
-            expr: Box::new(Expr::Lit(Lit::Str(Str {
-                span: ident.span,
-                value: ident.sym.clone().into(),
-                raw: None,
-            }))),
-        })
-    } else {
-        ident.into()
-    }
-}

crates/swc_ecma_minifier/tests/exec.rs

@@ -5484,6 +5484,129 @@ fn terser_reduce_vars_issue_3110_3() {
     run_exec_test(src, config, false);
 }
 
+#[test]
+fn terser_reduce_vars_shorthand_proto_null() {
+    let src = r#"(function () {
+    var __proto__ = null;
+    var o = { __proto__ };
+    console.log(
+        Object.getPrototypeOf(o) === Object.prototype,
+        Object.prototype.hasOwnProperty.call(o, "__proto__"),
+        o.__proto__ === null
+    );
+})();"#;
+    let config = r#"{
+    "evaluate": true,
+    "inline": true,
+    "passes": 2,
+    "properties": true,
+    "reduce_vars": true,
+    "side_effects": true,
+    "unused": true
+}"#;
+
+    run_exec_test(src, config, false);
+}
+
+#[test]
+fn terser_reduce_vars_shorthand_proto_object() {
+    let src = r#"(function () {
+    var __proto__ = { marker: 1 };
+    var o = { __proto__ };
+    console.log(
+        Object.getPrototypeOf(o) === Object.prototype,
+        Object.prototype.hasOwnProperty.call(o, "__proto__"),
+        o.__proto__ === __proto__,
+        o.__proto__.marker
+    );
+})();"#;
+    let config = r#"{
+    "evaluate": true,
+    "inline": true,
+    "passes": 2,
+    "properties": true,
+    "reduce_vars": true,
+    "side_effects": true,
+    "unused": true
+}"#;
+
+    run_exec_test(src, config, false);
+}
+
+#[test]
+fn terser_reduce_vars_shorthand_member_not_reexecuted_in_loop() {
+    let src = r#"(function () {
+    var calls = 0;
+    var source = {
+        get value() {
+            calls++;
+            return { n: calls };
+        },
+    };
+    var value = source.value;
+    for (var i = 0; i < 3; i++) {
+        console.log(({ value }).value.n);
+    }
+    console.log(calls);
+})();"#;
+    let config = r#"{
+    "evaluate": true,
+    "inline": true,
+    "passes": 2,
+    "properties": true,
+    "reduce_vars": true,
+    "side_effects": true,
+    "unused": true
+}"#;
+
+    run_exec_test(src, config, false);
+}
+
+#[test]
+fn terser_reduce_vars_shorthand_method_this_not_collapsed() {
+    let src = r#"(function () {
+    var fn = function () {
+        return this.x;
+    };
+    console.log(({ fn, x: "PASS" }).fn());
+})();"#;
+    let config = r#"{
+    "collapse_vars": true,
+    "evaluate": true,
+    "inline": true,
+    "passes": 2,
+    "properties": true,
+    "reduce_vars": true,
+    "side_effects": true,
+    "unsafe": true,
+    "unused": true
+}"#;
+
+    run_exec_test(src, config, false);
+}
+
+#[test]
+fn terser_reduce_vars_shorthand_multi_use_function_identity() {
+    let src = r#"(function () {
+    function foo() {
+        return "PASS";
+    }
+    var o = { foo };
+    console.log(o.foo === foo, o.foo());
+})();"#;
+    let config = r#"{
+    "evaluate": true,
+    "inline": true,
+    "passes": 2,
+    "properties": true,
+    "reduce_vars": true,
+    "side_effects": true,
+    "unused": true
+}"#;
+
+    run_exec_test(src, config, false);
+}
+
 #[test]
 fn terser_reduce_vars_defun_redefine() {
     let src = r###"function f() {

crates/swc_ecma_minifier/tests/terser/compress/reduce_vars/issue_3110_shorthand_1/config.json

@@ -0,0 +1,11 @@
+{
+    "conditionals": true,
+    "evaluate": true,
+    "inline": true,
+    "passes": 3,
+    "properties": true,
+    "reduce_vars": true,
+    "sequences": true,
+    "side_effects": true,
+    "unused": true
+}

crates/swc_ecma_minifier/tests/terser/compress/reduce_vars/issue_3110_shorthand_1/expected.stdout

@@ -0,0 +1,2 @@
+foo
+foo

crates/swc_ecma_minifier/tests/terser/compress/reduce_vars/issue_3110_shorthand_1/input.js

@@ -0,0 +1,9 @@
+(function () {
+    function foo() {
+        return isDev ? "foo" : "bar";
+    }
+    var isDev = true;
+    var obj = { foo };
+    console.log(foo());
+    console.log(obj.foo());
+})();

crates/swc_ecma_minifier/tests/terser/compress/reduce_vars/issue_3110_shorthand_1/output.js

@@ -0,0 +1 @@
+

crates/swc_ecma_minifier/tests/terser/compress/reduce_vars/issue_3110_shorthand_1/output.mangleOnly.js

@@ -0,0 +1,11 @@
+(function() {
+    function o() {
+        return n ? "foo" : "bar";
+    }
+    var n = true;
+    var r = {
+        foo: o
+    };
+    console.log(o());
+    console.log(r.foo());
+})();