From 7f4c883b27b7326bf7661556231e6e189ffcd30b Mon Sep 17 00:00:00 2001 From: Boshen Date: Sat, 20 Jul 2024 15:19:18 +0800 Subject: [PATCH] feat(codegen): print shorthand for all `{ x }` variants closes #4340 --- Cargo.lock | 4 + crates/oxc_codegen/src/gen.rs | 75 +++++++++++++------ crates/oxc_codegen/src/lib.rs | 32 +++++--- crates/oxc_codegen/tests/integration/unit.rs | 7 ++ .../tests/snapshots/function-parameters.snap | 4 +- crates/oxc_mangler/Cargo.toml | 7 ++ crates/oxc_mangler/tests/integration/main.rs | 24 ++++++ .../tests/integration/snapshots/mangler.snap | 23 ++++++ .../oxc_mangler/tests/integration/tester.rs | 14 ++++ tasks/coverage/codegen_sourcemap.snap | 43 ++++------- 10 files changed, 170 insertions(+), 63 deletions(-) create mode 100644 crates/oxc_mangler/tests/integration/main.rs create mode 100644 crates/oxc_mangler/tests/integration/snapshots/mangler.snap create mode 100644 crates/oxc_mangler/tests/integration/tester.rs diff --git a/Cargo.lock b/Cargo.lock index 6092220ad973a..b6d458b139518 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1560,9 +1560,13 @@ dependencies = [ name = "oxc_mangler" version = "0.21.0" dependencies = [ + "insta", "itertools 0.13.0", + "oxc_allocator", "oxc_ast", + "oxc_codegen", "oxc_index", + "oxc_parser", "oxc_semantic", "oxc_span", ] diff --git a/crates/oxc_codegen/src/gen.rs b/crates/oxc_codegen/src/gen.rs index 85af160bce68c..e05b47b03546b 100644 --- a/crates/oxc_codegen/src/gen.rs +++ b/crates/oxc_codegen/src/gen.rs @@ -1079,18 +1079,10 @@ impl<'a, const MINIFY: bool> GenExpr for ParenthesizedExpression<'a> { impl<'a, const MINIFY: bool> Gen 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 for IdentifierName<'a> { impl<'a, const MINIFY: bool> Gen 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 for ObjectPropertyKind<'a> { } } -// TODO: only print shorthand if key value are the same. impl<'a, const MINIFY: bool> Gen 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 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 for AssignmentTargetProperty<'a> { impl<'a, const MINIFY: bool> Gen 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 for ObjectPattern<'a> { } } -// TODO: only print shorthand if key value are the same. impl<'a, const MINIFY: bool> Gen 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); } } diff --git a/crates/oxc_codegen/src/lib.rs b/crates/oxc_codegen/src/lib.rs index e6fdeefce4aef..dc7ac422d6772 100644 --- a/crates/oxc_codegen/src/lib.rs +++ b/crates/oxc_codegen/src/lib.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, 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) { diff --git a/crates/oxc_codegen/tests/integration/unit.rs b/crates/oxc_codegen/tests/integration/unit.rs index 7ad988a7a9690..670ac475d434c 100644 --- a/crates/oxc_codegen/tests/integration/unit.rs +++ b/crates/oxc_codegen/tests/integration/unit.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"); diff --git a/crates/oxc_isolated_declarations/tests/snapshots/function-parameters.snap b/crates/oxc_isolated_declarations/tests/snapshots/function-parameters.snap index ab84d4ebf1128..c006d77d4c613 100644 --- a/crates/oxc_isolated_declarations/tests/snapshots/function-parameters.snap +++ b/crates/oxc_isolated_declarations/tests/snapshots/function-parameters.snap @@ -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(p: T, rParam: T, r2: T): void; export declare function fnDeclBad2(p: T, r2: T): void; export declare function fnDeclBad3(p: T, rParam?: T, r2: T): void; diff --git a/crates/oxc_mangler/Cargo.toml b/crates/oxc_mangler/Cargo.toml index 3a8d3819cc8c2..e5717b71d7068 100644 --- a/crates/oxc_mangler/Cargo.toml +++ b/crates/oxc_mangler/Cargo.toml @@ -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 } diff --git a/crates/oxc_mangler/tests/integration/main.rs b/crates/oxc_mangler/tests/integration/main.rs new file mode 100644 index 0000000000000..c746940bf172b --- /dev/null +++ b/crates/oxc_mangler/tests/integration/main.rs @@ -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); + }); +} diff --git a/crates/oxc_mangler/tests/integration/snapshots/mangler.snap b/crates/oxc_mangler/tests/integration/snapshots/mangler.snap new file mode 100644 index 0000000000000..acef0c9a68e02 --- /dev/null +++ b/crates/oxc_mangler/tests/integration/snapshots/mangler.snap @@ -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); +} diff --git a/crates/oxc_mangler/tests/integration/tester.rs b/crates/oxc_mangler/tests/integration/tester.rs new file mode 100644 index 0000000000000..cdfdcc82cabce --- /dev/null +++ b/crates/oxc_mangler/tests/integration/tester.rs @@ -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 +} diff --git a/tasks/coverage/codegen_sourcemap.snap b/tasks/coverage/codegen_sourcemap.snap index 91044e0b321bd..ab1b06065fd72 100644 --- a/tasks/coverage/codegen_sourcemap.snap +++ b/tasks/coverage/codegen_sourcemap.snap @@ -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};"