Skip to content

Commit

Permalink
Fix requiring generic parameters to implement Default (#179)
Browse files Browse the repository at this point in the history
Currently the builder uses `#[derive(Default)]`, which adds additional bounds to the type parameters
requiring them to implement Default. This is not required since the fields are all either `Option`
or `PhantomData`, which always have defaults, and means that Builders cannot be instantiated for
non-Default generics.

This commit makes the builder implement Default manually, without any additional bounds.

Fixes #178
  • Loading branch information
ColonelThirtyTwo committed Jan 12, 2021
1 parent ea372ba commit 3899b89
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 12 deletions.
89 changes: 77 additions & 12 deletions derive_builder_core/src/builder.rs
Expand Up @@ -36,7 +36,7 @@ use Setter;
/// # result.append_all(quote!(#[allow(clippy::all)]));
/// #
/// # result.append_all(quote!(
/// #[derive(Default, Clone)]
/// #[derive(Clone)]
/// pub struct FooBuilder {
/// foo: u32,
/// }
Expand Down Expand Up @@ -86,6 +86,15 @@ use Setter;
/// unimplemented!()
/// }
/// }
///
/// impl ::derive_builder::export::core::default::Default for FooBuilder {
/// fn default() -> Self {
/// Self {
/// foo: ::derive_builder::export::core::default::Default::default(),
/// }
/// }
/// }
///
/// # ));
/// # result
/// # }.to_string()
Expand All @@ -111,6 +120,10 @@ pub struct Builder<'a> {
///
/// Expects each entry to be terminated by a comma.
pub fields: Vec<TokenStream>,
/// Builder field initializers, e.g. `foo: Default::default(),`
///
/// Expects each entry to be terminated by a comma.
pub field_initializers: Vec<TokenStream>,
/// Functions of the builder struct, e.g. `fn bar() -> { unimplemented!() }`
pub functions: Vec<TokenStream>,
/// Whether this builder must derive `Clone`.
Expand Down Expand Up @@ -138,22 +151,24 @@ impl<'a> ToTokens for Builder<'a> {
.map(|(i, t, w)| (Some(i), Some(t), Some(w)))
.unwrap_or((None, None, None));
let builder_fields = &self.fields;
let builder_field_initializers = &self.field_initializers;
let functions = &self.functions;

// Create the comma-separated set of derived traits for the builder
let derived_traits = {
let default_trait: Path = parse_quote!(Default);
let derive_attr = {
let clone_trait: Path = parse_quote!(Clone);

let mut traits: Punctuated<&Path, Token![,]> = Default::default();
traits.push(&default_trait);

if self.must_derive_clone {
traits.push(&clone_trait);
}
traits.extend(self.derives);

quote!(#traits)
if traits.is_empty() {
quote!()
} else {
quote!(#[derive(#traits)])
}
};

let builder_doc_comment = &self.doc_comment;
Expand All @@ -171,7 +186,7 @@ impl<'a> ToTokens for Builder<'a> {
let builder_error_doc = format!("Error type for {}", builder_ident);

tokens.append_all(quote!(
#[derive(#derived_traits)]
#derive_attr
#builder_doc_comment
#builder_vis struct #builder_ident #struct_generics #where_clause {
#(#builder_fields)*
Expand Down Expand Up @@ -221,6 +236,14 @@ impl<'a> ToTokens for Builder<'a> {
#(#functions)*
#deprecation_notes
}

impl #impl_generics ::derive_builder::export::core::default::Default for #builder_ident #ty_generics #where_clause {
fn default() -> Self {
Self {
#(#builder_field_initializers)*
}
}
}
));
} else {
trace!("Skipping builder `{}`.", self.ident);
Expand All @@ -238,6 +261,7 @@ impl<'a> Builder<'a> {
/// Add a field to the builder
pub fn push_field(&mut self, f: BuilderField) -> &mut Self {
self.fields.push(quote!(#f));
self.field_initializers.push(f.default_initializer_tokens());
self
}

Expand Down Expand Up @@ -297,6 +321,7 @@ macro_rules! default_builder {
generics: None,
visibility: syn::parse_str("pub").unwrap(),
fields: vec![quote!(foo: u32,)],
field_initializers: vec![quote!(foo: ::derive_builder::export::core::default::Default::default(), )],
functions: vec![quote!(fn bar() -> { unimplemented!() })],
must_derive_clone: true,
doc_comment: None,
Expand All @@ -323,7 +348,7 @@ mod tests {
result.append_all(quote!(#[allow(clippy::all)]));

result.append_all(quote!(
#[derive(Default, Clone)]
#[derive(Clone)]
pub struct FooBuilder {
foo: u32,
}
Expand Down Expand Up @@ -375,6 +400,14 @@ mod tests {
unimplemented!()
}
}

impl ::derive_builder::export::core::default::Default for FooBuilder {
fn default() -> Self {
Self {
foo: ::derive_builder::export::core::default::Default::default(),
}
}
}
));

result
Expand Down Expand Up @@ -404,7 +437,7 @@ mod tests {
result.append_all(quote!(#[allow(clippy::all)]));

result.append_all(quote!(
#[derive(Default, Clone)]
#[derive(Clone)]
pub struct FooBuilder<'a, T: Debug> where T: PartialEq {
foo: u32,
}
Expand Down Expand Up @@ -456,6 +489,14 @@ mod tests {
unimplemented!()
}
}

impl<'a, T: Debug + ::derive_builder::export::core::clone::Clone> ::derive_builder::export::core::default::Default for FooBuilder<'a, T> where T: PartialEq {
fn default() -> Self {
Self {
foo: ::derive_builder::export::core::default::Default::default(),
}
}
}
));

result
Expand Down Expand Up @@ -485,7 +526,7 @@ mod tests {
result.append_all(quote!(#[allow(clippy::all)]));

result.append_all(quote!(
#[derive(Default, Clone)]
#[derive(Clone)]
pub struct FooBuilder<'a, T: 'a + Default> where T: PartialEq {
foo: u32,
}
Expand Down Expand Up @@ -540,6 +581,14 @@ mod tests {
unimplemented!()
}
}

impl<'a, T: 'a + Default + ::derive_builder::export::core::clone::Clone> ::derive_builder::export::core::default::Default for FooBuilder<'a, T> where T: PartialEq {
fn default() -> Self {
Self {
foo: ::derive_builder::export::core::default::Default::default(),
}
}
}
));

result
Expand Down Expand Up @@ -570,7 +619,6 @@ mod tests {
result.append_all(quote!(#[allow(clippy::all)]));

result.append_all(quote!(
#[derive(Default)]
pub struct FooBuilder<'a, T: Debug> where T: PartialEq {
foo: u32,
}
Expand Down Expand Up @@ -622,6 +670,15 @@ mod tests {
unimplemented!()
}
}

impl<'a, T: Debug> ::derive_builder::export::core::default::Default for FooBuilder<'a, T>
where T: PartialEq {
fn default() -> Self {
Self {
foo: ::derive_builder::export::core::default::Default::default(),
}
}
}
));

result
Expand Down Expand Up @@ -652,7 +709,7 @@ mod tests {
result.append_all(quote!(#[allow(clippy::all)]));

result.append_all(quote!(
#[derive(Default, Clone, Serialize)]
#[derive(Clone, Serialize)]
pub struct FooBuilder {
foo: u32,
}
Expand Down Expand Up @@ -704,6 +761,14 @@ mod tests {
unimplemented!()
}
}

impl ::derive_builder::export::core::default::Default for FooBuilder {
fn default() -> Self {
Self {
foo: ::derive_builder::export::core::default::Default::default(),
}
}
}
));

result
Expand Down
8 changes: 8 additions & 0 deletions derive_builder_core/src/builder_field.rs
Expand Up @@ -75,6 +75,14 @@ impl<'a> ToTokens for BuilderField<'a> {
}
}

impl<'a> BuilderField<'a> {
/// Emits a struct field initializer that initializes the field to `Default::default`.
pub fn default_initializer_tokens(&self) -> TokenStream {
let ident = self.field_ident;
quote!{ #ident : ::derive_builder::export::core::default::Default::default(), }
}
}

/// Helper macro for unit tests. This is _only_ public in order to be accessible
/// from doc-tests too.
#[doc(hidden)]
Expand Down
1 change: 1 addition & 0 deletions derive_builder_core/src/macro_options/darling_opts.rs
Expand Up @@ -376,6 +376,7 @@ impl Options {
generics: Some(&self.generics),
visibility: self.builder_vis(),
fields: Vec::with_capacity(self.field_count()),
field_initializers: Vec::with_capacity(self.field_count()),
functions: Vec::with_capacity(self.field_count()),
must_derive_clone: self.requires_clone(),
doc_comment: None,
Expand Down

0 comments on commit 3899b89

Please sign in to comment.