Skip to content

Commit c6f300e

Browse files
authored
Suppress empty builder for all-required dictionaries (#17)
When every property is required, the generated <Foo>Builder carries only build() with nothing to chain - dead weight. ts-gen now emits just new(reqs) directly on the dictionary type with construction inlined, skipping the wrapper struct. Before: impl EmailSendResult { pub fn new(message_id: &str) -> EmailSendResult { Self::builder(message_id).build() } pub fn builder(message_id: &str) -> EmailSendResultBuilder { let inner: Self = JsCast::unchecked_into(js_sys::Object::new()); inner.set_message_id(message_id); EmailSendResultBuilder { inner } } } pub struct EmailSendResultBuilder { inner: EmailSendResult } impl EmailSendResultBuilder { pub fn build(self) -> EmailSendResult { self.inner } } After: impl EmailSendResult { pub fn new(message_id: &str) -> EmailSendResult { let inner: Self = JsCast::unchecked_into(js_sys::Object::new()); inner.set_message_id(message_id); inner } } Dictionaries with at least one optional field still get the full new(reqs) + builder(reqs) + chainable wrapper treatment.
1 parent a38a67c commit c6f300e

7 files changed

Lines changed: 2189 additions & 13475 deletions

File tree

CONVENTIONS.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,10 @@ let init = ResponseInit::builder().status(200.0).build();
262262
let init = ResponseInit::new(); // empty object
263263
```
264264

265+
If every property is required (no optionals), the builder would carry
266+
only `build()` and is suppressed — only `new(reqs)` is emitted, with
267+
construction inlined.
268+
265269
### Required-property cartesian product across union types
266270

267271
When a required property has union-typed setter overloads (e.g.

src/codegen/classes.rs

Lines changed: 84 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,13 @@ fn generate_dictionary_factory(config: &ClassConfig) -> TokenStream {
298298
}
299299
}
300300

301+
// When every property is required, the builder type is dead weight: it
302+
// would carry only `pub fn build(self) -> Foo` with nothing to chain,
303+
// so we suppress it. `new(reqs)` still gets emitted but inlines the
304+
// construction directly. See the dictionary-builder convention in
305+
// `CONVENTIONS.md` for the rule.
306+
let emit_builder = !optional_getters.is_empty();
307+
301308
// Resolve each getter through the setter pipeline. Multi-overload
302309
// setters (e.g. `set_from(&str)` + `set_from_with_email_address(&EmailAddress)`
303310
// for `from: string | EmailAddress`) produce multiple sigs — we
@@ -607,7 +614,6 @@ fn generate_dictionary_factory(config: &ClassConfig) -> TokenStream {
607614
if !emitted_names.insert(full_suffix.clone()) {
608615
continue;
609616
}
610-
let builder_ident = super::typemap::make_ident(&format!("builder{full_suffix}"));
611617
let new_ident = super::typemap::make_ident(&format!("new{full_suffix}"));
612618
let (generics, params_tokens) = generate_dictionary_params(
613619
&plan.value_params,
@@ -640,39 +646,67 @@ fn generate_dictionary_factory(config: &ClassConfig) -> TokenStream {
640646
super::doc_tokens(&Some(doc_text))
641647
};
642648

643-
// No-required case has zero `init_calls`, so we inline the
644-
// factory directly into the struct literal where the field type
645-
// pins inference. The required-args case keeps a `let inner:
646-
// Self` so setter calls type-resolve before construction.
647-
let body = if init_calls.is_empty() {
648-
quote! {
649-
#builder_name {
650-
inner: JsCast::unchecked_into(js_sys::Object::new()),
649+
if emit_builder {
650+
// `builder*` returns the wrapper struct so callers can chain
651+
// setters; `new*` is just `Self::builder(reqs).build()`.
652+
//
653+
// No-required case has zero `init_calls`, so we inline the
654+
// factory directly into the struct literal where the field type
655+
// pins inference. The required-args case keeps a `let inner:
656+
// Self` so setter calls type-resolve before construction.
657+
let builder_body = if init_calls.is_empty() {
658+
quote! {
659+
#builder_name {
660+
inner: JsCast::unchecked_into(js_sys::Object::new()),
661+
}
651662
}
652-
}
663+
} else {
664+
quote! {
665+
let inner: Self = JsCast::unchecked_into(js_sys::Object::new());
666+
#(#init_calls)*
667+
#builder_name { inner }
668+
}
669+
};
670+
let builder_ident = super::typemap::make_ident(&format!("builder{full_suffix}"));
671+
builder_variants.push(quote! {
672+
#doc_attr
673+
pub fn #builder_ident #generics (#params_tokens) -> #builder_name {
674+
#builder_body
675+
}
676+
});
677+
new_variants.push(quote! {
678+
#doc_attr
679+
pub fn #new_ident #generics (#params_tokens) -> #rust_type {
680+
Self::#builder_ident(#(#arg_idents),*).build()
681+
}
682+
});
653683
} else {
654-
quote! {
655-
let inner: Self = JsCast::unchecked_into(js_sys::Object::new());
656-
#(#init_calls)*
657-
#builder_name { inner }
658-
}
659-
};
660-
builder_variants.push(quote! {
661-
#doc_attr
662-
pub fn #builder_ident #generics (#params_tokens) -> #builder_name {
663-
#body
664-
}
665-
});
666-
new_variants.push(quote! {
667-
#doc_attr
668-
pub fn #new_ident #generics (#params_tokens) -> #rust_type {
669-
Self::#builder_ident(#(#arg_idents),*).build()
670-
}
671-
});
684+
// All-required dictionary: no builder to delegate to, so
685+
// construction is inlined directly into `new*`. Returns
686+
// `Self` rather than the wrapper struct.
687+
let body = if init_calls.is_empty() {
688+
quote! {
689+
JsCast::unchecked_into(js_sys::Object::new())
690+
}
691+
} else {
692+
quote! {
693+
let inner: Self = JsCast::unchecked_into(js_sys::Object::new());
694+
#(#init_calls)*
695+
inner
696+
}
697+
};
698+
new_variants.push(quote! {
699+
#doc_attr
700+
pub fn #new_ident #generics (#params_tokens) -> #rust_type {
701+
#body
702+
}
703+
});
704+
}
672705
}
673706

674707
// Fluent methods on the wrapper for optional fields. Required fields
675708
// are intentionally absent — they're only settable through `builder`.
709+
// (Empty when `!emit_builder`, since there's nothing to chain.)
676710
let mut builder_methods: Vec<TokenStream> = Vec::new();
677711
for g in &optional_getters {
678712
for sig in resolve_setter_sigs(g) {
@@ -700,21 +734,31 @@ fn generate_dictionary_factory(config: &ClassConfig) -> TokenStream {
700734
}
701735
}
702736

703-
quote! {
704-
impl #rust_type {
705-
#(#new_variants)*
706-
#(#builder_variants)*
707-
}
737+
if emit_builder {
738+
quote! {
739+
impl #rust_type {
740+
#(#new_variants)*
741+
#(#builder_variants)*
742+
}
708743

709-
pub struct #builder_name {
710-
inner: #rust_type,
711-
}
744+
pub struct #builder_name {
745+
inner: #rust_type,
746+
}
712747

713-
impl #builder_name {
714-
#(#builder_methods)*
748+
impl #builder_name {
749+
#(#builder_methods)*
715750

716-
pub fn build(self) -> #rust_type {
717-
self.inner
751+
pub fn build(self) -> #rust_type {
752+
self.inner
753+
}
754+
}
755+
}
756+
} else {
757+
// All-required dictionary: the builder would be empty, so we
758+
// emit only `new*` factories on the dictionary type itself.
759+
quote! {
760+
impl #rust_type {
761+
#(#new_variants)*
718762
}
719763
}
720764
}
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// Fixture: dictionary builder is suppressed when there are no optional
2+
// fields to chain.
3+
//
4+
// When every property is required, the generated `FooBuilder` would only
5+
// have `pub fn build(self) -> Foo` — pure dead weight. ts-gen detects
6+
// this and emits only `new(reqs) -> Foo` with the construction inlined.
7+
8+
/** All fields required → no builder, only `new`. */
9+
interface AllRequired {
10+
/** The thing's name. */
11+
name: string;
12+
/** The thing's count. */
13+
count: number;
14+
}
15+
16+
/** Has at least one optional field → builder still emitted. */
17+
interface HasOptional {
18+
/** Required. */
19+
name: string;
20+
/** Optional. */
21+
count?: number;
22+
}
23+
24+
/** All optional → zero-arg `new()` and `builder()` with chainable setters. */
25+
interface AllOptional {
26+
/** Optional. */
27+
name?: string;
28+
/** Optional. */
29+
count?: number;
30+
}
31+
32+
/** Single required field, no optionals → no builder, just `new`. */
33+
interface SingleRequired {
34+
/** The id. */
35+
id: string;
36+
}

tests/snapshots/coverage.rs

Lines changed: 6 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -24,20 +24,9 @@ extern "C" {
2424
}
2525
impl NumberIndexed {
2626
pub fn new(length: f64) -> NumberIndexed {
27-
Self::builder(length).build()
28-
}
29-
pub fn builder(length: f64) -> NumberIndexedBuilder {
3027
let inner: Self = JsCast::unchecked_into(js_sys::Object::new());
3128
inner.set_length(length);
32-
NumberIndexedBuilder { inner }
33-
}
34-
}
35-
pub struct NumberIndexedBuilder {
36-
inner: NumberIndexed,
37-
}
38-
impl NumberIndexedBuilder {
39-
pub fn build(self) -> NumberIndexed {
40-
self.inner
29+
inner
4130
}
4231
}
4332
#[wasm_bindgen]
@@ -52,20 +41,9 @@ extern "C" {
5241
}
5342
impl MixedWithIndex {
5443
pub fn new(name: &str) -> MixedWithIndex {
55-
Self::builder(name).build()
56-
}
57-
pub fn builder(name: &str) -> MixedWithIndexBuilder {
5844
let inner: Self = JsCast::unchecked_into(js_sys::Object::new());
5945
inner.set_name(name);
60-
MixedWithIndexBuilder { inner }
61-
}
62-
}
63-
pub struct MixedWithIndexBuilder {
64-
inner: MixedWithIndex,
65-
}
66-
impl MixedWithIndexBuilder {
67-
pub fn build(self) -> MixedWithIndex {
68-
self.inner
46+
inner
6947
}
7048
}
7149
#[wasm_bindgen]
@@ -80,20 +58,9 @@ extern "C" {
8058
}
8159
impl HasName {
8260
pub fn new(name: &str) -> HasName {
83-
Self::builder(name).build()
84-
}
85-
pub fn builder(name: &str) -> HasNameBuilder {
8661
let inner: Self = JsCast::unchecked_into(js_sys::Object::new());
8762
inner.set_name(name);
88-
HasNameBuilder { inner }
89-
}
90-
}
91-
pub struct HasNameBuilder {
92-
inner: HasName,
93-
}
94-
impl HasNameBuilder {
95-
pub fn build(self) -> HasName {
96-
self.inner
63+
inner
9764
}
9865
}
9966
#[wasm_bindgen]
@@ -108,20 +75,9 @@ extern "C" {
10875
}
10976
impl HasAge {
11077
pub fn new(age: f64) -> HasAge {
111-
Self::builder(age).build()
112-
}
113-
pub fn builder(age: f64) -> HasAgeBuilder {
11478
let inner: Self = JsCast::unchecked_into(js_sys::Object::new());
11579
inner.set_age(age);
116-
HasAgeBuilder { inner }
117-
}
118-
}
119-
pub struct HasAgeBuilder {
120-
inner: HasAge,
121-
}
122-
impl HasAgeBuilder {
123-
pub fn build(self) -> HasAge {
124-
self.inner
80+
inner
12581
}
12682
}
12783
#[allow(dead_code)]
@@ -261,21 +217,10 @@ extern "C" {
261217
}
262218
impl LinkedList {
263219
pub fn new(data: &JsValue, next: Option<&LinkedList>) -> LinkedList {
264-
Self::builder(data, next).build()
265-
}
266-
pub fn builder(data: &JsValue, next: Option<&LinkedList>) -> LinkedListBuilder {
267220
let inner: Self = JsCast::unchecked_into(js_sys::Object::new());
268221
inner.set_data(data);
269222
inner.set_next(next);
270-
LinkedListBuilder { inner }
271-
}
272-
}
273-
pub struct LinkedListBuilder {
274-
inner: LinkedList,
275-
}
276-
impl LinkedListBuilder {
277-
pub fn build(self) -> LinkedList {
278-
self.inner
223+
inner
279224
}
280225
}
281226
#[wasm_bindgen]
@@ -333,20 +278,9 @@ extern "C" {
333278
}
334279
impl MultiExtend {
335280
pub fn new(id: &str) -> MultiExtend {
336-
Self::builder(id).build()
337-
}
338-
pub fn builder(id: &str) -> MultiExtendBuilder {
339281
let inner: Self = JsCast::unchecked_into(js_sys::Object::new());
340282
inner.set_id(id);
341-
MultiExtendBuilder { inner }
342-
}
343-
}
344-
pub struct MultiExtendBuilder {
345-
inner: MultiExtend,
346-
}
347-
impl MultiExtendBuilder {
348-
pub fn build(self) -> MultiExtend {
349-
self.inner
283+
inner
350284
}
351285
}
352286
#[wasm_bindgen]

0 commit comments

Comments
 (0)