feat(codegen): print shorthand for all { x } variants

closes #4340

Author: Boshen

Cargo.lock

@@ -1079,18 +1079,10 @@ impl<'a, const MINIFY: bool> GenExpr<MINIFY> for ParenthesizedExpression<'a> {
 
 impl<'a, const MINIFY: bool> Gen<MINIFY> for IdentifierReference<'a> {
     fn gen(&self, p: &mut Codegen<{ MINIFY }>, _ctx: Context) {
-        if let Some(mangler) = &p.mangler {
-            if let Some(reference_id) = self.reference_id.get() {
-                if let Some(name) = mangler.get_reference_name(reference_id) {
-                    let name = CompactStr::new(name);
-                    p.add_source_mapping_for_name(self.span, &name);
-                    p.print_str(&name);
-                    return;
-                }
-            }
-        }
-        p.add_source_mapping_for_name(self.span, &self.name);
-        p.print_str(&self.name);
+        let name = p.get_identifier_reference_name(self);
+        let name = CompactStr::new(name);
+        p.add_source_mapping_for_name(self.span, &name);
+        p.print_str(&name);
     }
 }
 
@@ -1103,7 +1095,10 @@ impl<'a, const MINIFY: bool> Gen<MINIFY> for IdentifierName<'a> {
 
 impl<'a, const MINIFY: bool> Gen<MINIFY> for BindingIdentifier<'a> {
     fn gen(&self, p: &mut Codegen<{ MINIFY }>, _ctx: Context) {
-        p.print_symbol(self.span, self.symbol_id.get(), self.name.as_str());
+        let name = p.get_binding_identifier_name(self);
+        let name = CompactStr::new(name);
+        p.add_source_mapping_for_name(self.span, &name);
+        p.print_str(&name);
     }
 }
 
@@ -1514,7 +1509,6 @@ impl<'a, const MINIFY: bool> Gen<MINIFY> for ObjectPropertyKind<'a> {
     }
 }
 
-// TODO: only print shorthand if key value are the same.
 impl<'a, const MINIFY: bool> Gen<MINIFY> for ObjectProperty<'a> {
     fn gen(&self, p: &mut Codegen<{ MINIFY }>, ctx: Context) {
         if let Expression::FunctionExpression(func) = &self.value {
@@ -1559,15 +1553,29 @@ impl<'a, const MINIFY: bool> Gen<MINIFY> for ObjectProperty<'a> {
                 return;
             }
         }
+
+        let mut shorthand = false;
+        if let PropertyKey::StaticIdentifier(key) = &self.key {
+            if let Expression::Identifier(ident) = &self.value {
+                if key.name == p.get_identifier_reference_name(ident) {
+                    shorthand = true;
+                }
+            }
+        }
+
         if self.computed {
             p.print_char(b'[');
         }
-        self.key.gen(p, ctx);
+        if !shorthand {
+            self.key.gen(p, ctx);
+        }
         if self.computed {
             p.print_char(b']');
         }
-        p.print_colon();
-        p.print_soft_space();
+        if !shorthand {
+            p.print_colon();
+            p.print_soft_space();
+        }
         self.value.gen_expr(p, Precedence::Assign, Context::default());
     }
 }
@@ -1925,7 +1933,17 @@ impl<'a, const MINIFY: bool> Gen<MINIFY> for AssignmentTargetProperty<'a> {
 
 impl<'a, const MINIFY: bool> Gen<MINIFY> for AssignmentTargetPropertyIdentifier<'a> {
     fn gen(&self, p: &mut Codegen<{ MINIFY }>, ctx: Context) {
-        self.binding.gen(p, ctx);
+        let ident_name = p.get_identifier_reference_name(&self.binding);
+        if ident_name == self.binding.name.as_str() {
+            self.binding.gen(p, ctx);
+        } else {
+            // `({x: a} = y);`
+            let ident_name = CompactStr::new(ident_name);
+            p.print_str(self.binding.name.as_str());
+            p.print_colon();
+            p.print_soft_space();
+            p.print_str(ident_name.as_str());
+        }
         if let Some(expr) = &self.init {
             p.print_soft_space();
             p.print_equal();
@@ -2535,19 +2553,32 @@ impl<'a, const MINIFY: bool> Gen<MINIFY> for ObjectPattern<'a> {
     }
 }
 
-// TODO: only print shorthand if key value are the same.
 impl<'a, const MINIFY: bool> Gen<MINIFY> for BindingProperty<'a> {
     fn gen(&self, p: &mut Codegen<{ MINIFY }>, ctx: Context) {
         p.add_source_mapping(self.span.start);
         if self.computed {
             p.print_char(b'[');
         }
-        self.key.gen(p, ctx);
+
+        let mut shorthand = false;
+        if let PropertyKey::StaticIdentifier(key) = &self.key {
+            if let BindingPatternKind::BindingIdentifier(ident) = &self.value.kind {
+                if key.name == p.get_binding_identifier_name(ident) {
+                    shorthand = true;
+                }
+            }
+        }
+
+        if !shorthand {
+            self.key.gen(p, ctx);
+        }
         if self.computed {
             p.print_char(b']');
         }
-        p.print_colon();
-        p.print_soft_space();
+        if !shorthand {
+            p.print_colon();
+            p.print_soft_space();
+        }
         self.value.gen(p, ctx);
     }
 }

crates/oxc_codegen/src/gen.rs

@@ -14,16 +14,18 @@ use std::{borrow::Cow, ops::Range};
 use rustc_hash::FxHashMap;
 
 use oxc_ast::{
-    ast::{BlockStatement, Directive, Expression, Program, Statement},
+    ast::{
+        BindingIdentifier, BlockStatement, Directive, Expression, IdentifierReference, Program,
+        Statement,
+    },
     Comment, Trivias,
 };
 use oxc_mangler::Mangler;
-use oxc_span::{CompactStr, Span};
+use oxc_span::Span;
 use oxc_syntax::{
     identifier::is_identifier_part,
     operator::{BinaryOperator, UnaryOperator, UpdateOperator},
     precedence::Precedence,
-    symbol::SymbolId,
 };
 
 pub use crate::{
@@ -425,19 +427,25 @@ impl<'a, const MINIFY: bool> Codegen<'a, MINIFY> {
         }
     }
 
-    #[allow(clippy::needless_pass_by_value)]
-    fn print_symbol(&mut self, span: Span, symbol_id: Option<SymbolId>, fallback: &str) {
+    fn get_identifier_reference_name(&self, reference: &IdentifierReference<'a>) -> &str {
         if let Some(mangler) = &self.mangler {
-            if let Some(symbol_id) = symbol_id {
+            if let Some(reference_id) = reference.reference_id.get() {
+                if let Some(name) = mangler.get_reference_name(reference_id) {
+                    return name;
+                }
+            }
+        }
+        reference.name.as_str()
+    }
+
+    fn get_binding_identifier_name(&self, ident: &BindingIdentifier<'a>) -> &str {
+        if let Some(mangler) = &self.mangler {
+            if let Some(symbol_id) = ident.symbol_id.get() {
                 let name = mangler.get_symbol_name(symbol_id);
-                let name = CompactStr::new(name);
-                self.add_source_mapping_for_name(span, &name);
-                self.print_str(&name);
-                return;
+                return name;
             }
         }
-        self.add_source_mapping_for_name(span, fallback);
-        self.print_str(fallback);
+        ident.name.as_str()
     }
 
     fn print_space_before_operator(&mut self, next: Operator) {

crates/oxc_codegen/src/lib.rs

@@ -126,6 +126,13 @@ fn for_stmt() {
     // );
 }
 
+#[test]
+fn shorthand() {
+    test("let _ = { x }", "let _ = {x};\n");
+    test("let { x } = y", "let { x } = y;\n");
+    test("({ x } = y)", "({x} = y);\n");
+}
+
 #[test]
 fn unicode_escape() {
     test("console.log('你好');", "console.log('你好');\n");

crates/oxc_codegen/tests/integration/unit.rs

@@ -7,8 +7,8 @@ input_file: crates/oxc_isolated_declarations/tests/fixtures/function-parameters.
 export declare function fnDeclGood(p?: T, rParam?: string): void;
 export declare function fnDeclGood2(p?: T, rParam?: number): void;
 export declare function fooGood([a, b]?: any[]): number;
-export declare const fooGood2: ({ a: a, b: b }?: object) => number;
-export declare function fooGood3({ a: a, b: [{ c: c }] }: object): void;
+export declare const fooGood2: ({ a, b }?: object) => number;
+export declare function fooGood3({ a, b: [{ c }] }: object): void;
 export declare function fnDeclBad<T>(p: T, rParam: T, r2: T): void;
 export declare function fnDeclBad2<T>(p: T, r2: T): void;
 export declare function fnDeclBad3<T>(p: T, rParam?: T, r2: T): void;

crates/oxc_isolated_declarations/tests/snapshots/function-parameters.snap

@@ -26,3 +26,10 @@ oxc_ast      = { workspace = true }
 oxc_semantic = { workspace = true }
 oxc_index    = { workspace = true }
 itertools    = { workspace = true }
+
+[dev-dependencies]
+oxc_codegen   = { workspace = true }
+oxc_parser    = { workspace = true }
+oxc_span      = { workspace = true }
+oxc_allocator = { workspace = true }
+insta         = { workspace = true }

crates/oxc_mangler/Cargo.toml

@@ -0,0 +1,24 @@
+mod tester;
+
+use std::fmt::Write;
+
+use tester::mangle;
+
+#[test]
+fn mangler() {
+    let cases = [
+        "function foo(a) {a}",
+        "function foo(a) { let _ = { x } }",
+        "function foo(a) { let { x } = y }",
+        "var x; function foo(a) { ({ x } = y) }",
+    ];
+
+    let snapshot = cases.into_iter().fold(String::new(), |mut w, case| {
+        write!(w, "{case}\n{}\n", mangle(case)).unwrap();
+        w
+    });
+
+    insta::with_settings!({ prepend_module_to_snapshot => false, omit_expression => true }, {
+        insta::assert_snapshot!("mangler", snapshot);
+    });
+}

crates/oxc_mangler/tests/integration/main.rs

@@ -0,0 +1,23 @@
+---
+source: crates/oxc_mangler/tests/integration/main.rs
+---
+function foo(a) {a}
+function a(b) {
+	b;
+}
+
+function foo(a) { let _ = { x } }
+function a(b) {
+	let c = {x};
+}
+
+function foo(a) { let { x } = y }
+function a(b) {
+	let { x: c } = y;
+}
+
+var x; function foo(a) { ({ x } = y) }
+var a;
+function b(c) {
+	({x: a} = y);
+}

crates/oxc_mangler/tests/integration/snapshots/mangler.snap

@@ -0,0 +1,14 @@
+use oxc_allocator::Allocator;
+use oxc_codegen::CodeGenerator;
+use oxc_mangler::ManglerBuilder;
+use oxc_parser::Parser;
+use oxc_span::SourceType;
+
+pub fn mangle(source_text: &str) -> String {
+    let allocator = Allocator::default();
+    let source_type = SourceType::default().with_module(true);
+    let ret = Parser::new(&allocator, source_text, source_type).parse();
+    let program = ret.program;
+    let mangler = ManglerBuilder::default().build(&program);
+    CodeGenerator::new().with_mangler(Some(mangler)).build(&program).source_text
+}

crates/oxc_mangler/tests/integration/tester.rs

@@ -714,15 +714,11 @@ Invalid Character `[`
 (160:1-162:0) "};\n" --> (90:44-91:2) "e};\n\t"
 (162:0-162:4) "\nvar" --> (91:2-91:6) "\tvar"
 (162:4-162:27) " ReactSharedInternals =" --> (91:6-91:29) " ReactSharedInternals ="
-(162:27-163:2) " {\n " --> (91:29-92:3) " {\n\t\t"
-(163:2-163:26) " ReactCurrentDispatcher:" --> (92:3-92:27) "\tReactCurrentDispatcher:"
-(163:26-164:2) " ReactCurrentDispatcher,\n " --> (92:27-93:3) " ReactCurrentDispatcher,\n\t\t"
-(164:2-164:27) " ReactCurrentBatchConfig:" --> (93:3-93:28) "\tReactCurrentBatchConfig:"
-(164:27-165:2) " ReactCurrentBatchConfig,\n " --> (93:28-94:3) " ReactCurrentBatchConfig,\n\t\t"
-(165:2-165:21) " ReactCurrentOwner:" --> (94:3-94:22) "\tReactCurrentOwner:"
-(165:21-166:2) " ReactCurrentOwner,\n " --> (94:22-95:3) " ReactCurrentOwner,\n\t\t"
-(166:2-166:24) " IsSomeRendererActing:" --> (95:3-95:25) "\tIsSomeRendererActing:"
-(166:24-168:2) " IsSomeRendererActing,\n  // Used by renderers to avoid bundling object-assign twice in UMD bundles:\n " --> (95:25-96:3) " IsSomeRendererActing,\n\t\t"
+(162:27-163:26) " {\n  ReactCurrentDispatcher:" --> (91:29-92:3) " {\n\t\t"
+(163:26-164:27) " ReactCurrentDispatcher,\n  ReactCurrentBatchConfig:" --> (92:3-93:3) "\tReactCurrentDispatcher,\n\t\t"
+(164:27-165:21) " ReactCurrentBatchConfig,\n  ReactCurrentOwner:" --> (93:3-94:3) "\tReactCurrentBatchConfig,\n\t\t"
+(165:21-166:24) " ReactCurrentOwner,\n  IsSomeRendererActing:" --> (94:3-95:3) "\tReactCurrentOwner,\n\t\t"
+(166:24-168:2) " IsSomeRendererActing,\n  // Used by renderers to avoid bundling object-assign twice in UMD bundles:\n " --> (95:3-96:3) "\tIsSomeRendererActing,\n\t\t"
 (168:2-168:10) " assign:" --> (96:3-96:11) "\tassign:"
 (168:10-169:1) " _assign\n" --> (96:11-97:2) " _assign\n\t"
 (169:1-171:0) "};\n" --> (97:2-98:2) "\t};\n\t"
@@ -1593,15 +1589,11 @@ Invalid Character `[`
 (649:6-649:16) " element =" --> (347:7-347:17) " element ="
 (649:16-651:4) " {\n    // This tag allows us to uniquely identify this as a React Element\n   " --> (347:17-348:4) " {\n\t\t\t"
 (651:4-651:14) " $$typeof:" --> (348:4-348:14) "\t$$typeof:"
-(651:14-653:4) " REACT_ELEMENT_TYPE,\n    // Built-in properties that belong on the element\n   " --> (348:14-349:4) " REACT_ELEMENT_TYPE,\n\t\t\t"
-(653:4-653:10) " type:" --> (349:4-349:10) "\ttype:"
-(653:10-654:4) " type,\n   " --> (349:10-350:4) " type,\n\t\t\t"
-(654:4-654:9) " key:" --> (350:4-350:9) "\tkey:"
-(654:9-655:4) " key,\n   " --> (350:9-351:4) " key,\n\t\t\t"
-(655:4-655:9) " ref:" --> (351:4-351:9) "\tref:"
-(655:9-656:4) " ref,\n   " --> (351:9-352:4) " ref,\n\t\t\t"
-(656:4-656:11) " props:" --> (352:4-352:11) "\tprops:"
-(656:11-658:4) " props,\n    // Record the component responsible for creating this element.\n   " --> (352:11-353:4) " props,\n\t\t\t"
+(651:14-653:10) " REACT_ELEMENT_TYPE,\n    // Built-in properties that belong on the element\n    type:" --> (348:14-349:4) " REACT_ELEMENT_TYPE,\n\t\t\t"
+(653:10-654:9) " type,\n    key:" --> (349:4-350:4) "\ttype,\n\t\t\t"
+(654:9-655:9) " key,\n    ref:" --> (350:4-351:4) "\tkey,\n\t\t\t"
+(655:9-656:11) " ref,\n    props:" --> (351:4-352:4) "\tref,\n\t\t\t"
+(656:11-658:4) " props,\n    // Record the component responsible for creating this element.\n   " --> (352:4-353:4) "\tprops,\n\t\t\t"
 (658:4-658:12) " _owner:" --> (353:4-353:12) "\t_owner:"
 (658:12-659:3) " owner\n  " --> (353:12-354:3) " owner\n\t\t"
 (659:3-661:2) "};\n\n " --> (354:3-355:3) "\t};\n\t\t"
@@ -3096,9 +3088,8 @@ Invalid Character `[`
 (1387:6-1387:20) " elementType =" --> (828:7-828:21) " elementType ="
 (1387:20-1388:4) " {\n   " --> (828:21-829:4) " {\n\t\t\t"
 (1388:4-1388:14) " $$typeof:" --> (829:4-829:14) "\t$$typeof:"
-(1388:14-1389:4) " REACT_FORWARD_REF_TYPE,\n   " --> (829:14-830:4) " REACT_FORWARD_REF_TYPE,\n\t\t\t"
-(1389:4-1389:12) " render:" --> (830:4-830:12) "\trender:"
-(1389:12-1390:3) " render\n  " --> (830:12-831:3) " render\n\t\t"
+(1388:14-1389:12) " REACT_FORWARD_REF_TYPE,\n    render:" --> (829:14-830:4) " REACT_FORWARD_REF_TYPE,\n\t\t\t"
+(1389:12-1390:3) " render\n  " --> (830:4-831:3) "\trender\n\t\t"
 (1390:3-1392:2) "};\n\n " --> (831:3-832:3) "\t};\n\t\t"
 (1392:2-1393:4) " {\n   " --> (832:3-833:4) "\t{\n\t\t\t"
 (1393:4-1393:8) " var" --> (833:4-833:8) "\tvar"
@@ -3244,9 +3235,8 @@ Invalid Character `[`
 (1443:6-1443:20) " elementType =" --> (871:7-871:21) " elementType ="
 (1443:20-1444:4) " {\n   " --> (871:21-872:4) " {\n\t\t\t"
 (1444:4-1444:14) " $$typeof:" --> (872:4-872:14) "\t$$typeof:"
-(1444:14-1445:4) " REACT_MEMO_TYPE,\n   " --> (872:14-873:4) " REACT_MEMO_TYPE,\n\t\t\t"
-(1445:4-1445:10) " type:" --> (873:4-873:10) "\ttype:"
-(1445:10-1446:4) " type,\n   " --> (873:10-874:4) " type,\n\t\t\t"
+(1444:14-1445:10) " REACT_MEMO_TYPE,\n    type:" --> (872:14-873:4) " REACT_MEMO_TYPE,\n\t\t\t"
+(1445:10-1446:4) " type,\n   " --> (873:4-874:4) "\ttype,\n\t\t\t"
 (1446:4-1446:13) " compare:" --> (874:4-874:13) "\tcompare:"
 (1446:13-1446:25) " compare ===" --> (874:13-874:25) " compare ==="
 (1446:25-1446:37) " undefined ?" --> (874:25-874:37) " undefined ?"
@@ -5179,9 +5169,8 @@ Invalid Character `[`
 (2301:2-2301:11) " forEach:" --> (1456:3-1456:12) "\tforEach:"
 (2301:11-2302:2) " forEachChildren,\n " --> (1456:12-1457:3) " forEachChildren,\n\t\t"
 (2302:2-2302:9) " count:" --> (1457:3-1457:10) "\tcount:"
-(2302:9-2303:2) " countChildren,\n " --> (1457:10-1458:3) " countChildren,\n\t\t"
-(2303:2-2303:11) " toArray:" --> (1458:3-1458:12) "\ttoArray:"
-(2303:11-2304:2) " toArray,\n " --> (1458:12-1459:3) " toArray,\n\t\t"
+(2302:9-2303:11) " countChildren,\n  toArray:" --> (1457:10-1458:3) " countChildren,\n\t\t"
+(2303:11-2304:2) " toArray,\n " --> (1458:3-1459:3) "\ttoArray,\n\t\t"
 (2304:2-2304:8) " only:" --> (1459:3-1459:9) "\tonly:"
 (2304:8-2305:1) " onlyChild\n" --> (1459:9-1460:2) " onlyChild\n\t"
 (2305:1-2307:0) "};\n" --> (1460:2-1461:0) "\t};"