From ee522ae5ed3bc2ead783447007d4582cd65f8797 Mon Sep 17 00:00:00 2001 From: Burkhard Mittelbach Date: Sat, 23 Mar 2024 02:45:39 +0100 Subject: [PATCH 01/12] Base impl for custom defaults in owned pattern There are still a lot of tests failing, but my usecase works. Now I just have to fix all the corner cases. Also tests for the new feature are still missing. --- derive_builder_core/src/build_method.rs | 19 +- .../src/field_default_value.rs | 191 ++++++++++++++++++ derive_builder_core/src/initializer.rs | 171 ++-------------- derive_builder_core/src/lib.rs | 9 +- .../src/macro_options/darling_opts.rs | 21 +- 5 files changed, 251 insertions(+), 160 deletions(-) create mode 100644 derive_builder_core/src/field_default_value.rs diff --git a/derive_builder_core/src/build_method.rs b/derive_builder_core/src/build_method.rs index 55150b4..6236ec1 100644 --- a/derive_builder_core/src/build_method.rs +++ b/derive_builder_core/src/build_method.rs @@ -5,7 +5,8 @@ use quote::{ToTokens, TokenStreamExt}; use syn::spanned::Spanned; use crate::{ - doc_comment_from, BuilderPattern, DefaultExpression, Initializer, DEFAULT_STRUCT_NAME, + doc_comment_from, BuilderPattern, DefaultExpression, FieldDefaultValue, Initializer, + DEFAULT_STRUCT_NAME, }; /// Initializer for the struct fields in the build method, implementing @@ -57,6 +58,8 @@ pub struct BuildMethod<'a> { pub error_ty: syn::Path, /// Field initializers for the target type. pub initializers: Vec, + /// Default values for the target type + pub defaults: Vec, /// Doc-comment of the builder struct. pub doc_comment: Option, /// Default value for the whole struct. @@ -74,6 +77,7 @@ impl<'a> ToTokens for BuildMethod<'a> { let vis = &self.visibility; let target_ty = &self.target_ty; let target_ty_generics = &self.target_ty_generics; + let defaults = &self.defaults; let initializers = &self.initializers; let self_param = match self.pattern { BuilderPattern::Owned => quote!(self), @@ -100,6 +104,7 @@ impl<'a> ToTokens for BuildMethod<'a> { { #validate_fn #default_struct + #(#defaults)* Ok(#target_ty { #(#initializers)* }) @@ -125,6 +130,16 @@ impl<'a> BuildMethod<'a> { self.initializers.push(quote!(#init)); self } + + /// Populate the `BuildMethod` with appropiate default values of + /// the underlying struct. + /// + /// For each struct field this must be called with the appropriate + /// default value. + pub fn push_default(&mut self, default: FieldDefaultValue) -> &mut Self { + self.defaults.push(quote!(#default)); + self + } } // pub struct BuildMethodError { @@ -150,6 +165,8 @@ macro_rules! default_build_method { target_ty_generics: None, error_ty: syn::parse_quote!(FooBuilderError), initializers: vec![quote!(foo: self.foo,)], + defaults: vec![quote!(todo!("default value"))], // TODO what is the correct default for + // test doc_comment: None, default_struct: None, validate_fn: None, diff --git a/derive_builder_core/src/field_default_value.rs b/derive_builder_core/src/field_default_value.rs new file mode 100644 index 0000000..65222d9 --- /dev/null +++ b/derive_builder_core/src/field_default_value.rs @@ -0,0 +1,191 @@ +use proc_macro2::{Ident, Span, TokenStream}; +use quote::{ToTokens, TokenStreamExt}; +use syn::Type; + +use crate::{ + change_span, BlockContents, DefaultExpression, DEFAULT_FIELD_NAME_PREFIX, DEFAULT_STRUCT_NAME, +}; + +// TODO: remove no longer needed code from Initializer +#[derive(Debug, Clone)] +pub struct FieldDefaultValue<'a> { + /// Path to the root of the derive_builder crate. + pub crate_root: &'a syn::Path, + /// Name of the target field. + pub field_ident: &'a syn::Ident, + /// Type of the builder field. + pub field_type: &'a Type, + /// Whether the builder implements a setter for this field. + pub field_enabled: bool, + /// Default value for the target field. + /// + /// This takes precedence over a default struct identifier. + pub default_value: Option<&'a DefaultExpression>, + /// Whether the build_method defines a default struct. + pub use_default_struct: bool, + /// Span where the macro was told to use a preexisting error type, instead of creating one, + /// to represent failures of the `build` method. + /// + /// An initializer can force early-return if a field has no set value and no default is + /// defined. In these cases, it will convert from `derive_builder::UninitializedFieldError` + /// into the return type of its enclosing `build` method. That conversion is guaranteed to + /// work for generated error types, but if the caller specified an error type to use instead + /// they may have forgotten the conversion from `UninitializedFieldError` into their specified + /// error type. + pub custom_error_type_span: Option, + /// Method to use to to convert the builder's field to the target field + /// + /// For sub-builder fields, this will be `build` (or similar) + pub conversion: FieldConversion<'a>, +} + +impl<'a> ToTokens for FieldDefaultValue<'a> { + fn to_tokens(&self, tokens: &mut proc_macro2::TokenStream) { + // should be: + // ``` + // let $prefix$fieldname = match self.$field.as_ref() { + // Some(_) => None, + // None => Some($default_value) + // } + // ``` + + let struct_field = &self.field_ident; + let builder_field = struct_field; + + let field_type = &self.field_type; + + let default_value = Ident::new( + &format!("{}{}", DEFAULT_FIELD_NAME_PREFIX, struct_field), + Span::call_site(), + ); + + // token stream to generate the calculation of the default value + let default_calculation = (|| { + let mut tokens = TokenStream::new(); + if !self.field_enabled { + // If the field is disabled we just calculate the default here. + // It is later set in the initializer + let value = self.disabled_field_value(); + tokens.append_all(quote!(#value)); + } else { + match &self.conversion { + FieldConversion::Block(_) | FieldConversion::Move => { + // value is directly accessed, therfor there is no default + tokens.append_all(quote!(None)) + } + FieldConversion::OptionOrDefault => { + let default = self.default_value(); + tokens.append_all(quote!(#default)); + } + } + } + tokens + })(); + + tokens.append_all(quote!( + let #default_value: Option<#field_type> = match self.#builder_field.as_ref() { + Some(_) => None, + None => #default_calculation, + }; + )); + } +} + +impl<'a> FieldDefaultValue<'a> { + fn disabled_field_value(&'a self) -> TokenStream { + let crate_root = self.crate_root; + match self.default_value { + Some(expr) => expr.with_crate_root(crate_root).into_token_stream(), + None if self.use_default_struct => { + let struct_ident = syn::Ident::new(DEFAULT_STRUCT_NAME, Span::call_site()); + let field_ident = self.field_ident; + quote!(#struct_ident.#field_ident) + } + None => { + quote!(#crate_root::export::core::default::Default::default()) + } + } + } + fn default_value(&'a self) -> DefaultValue<'a> { + match self.default_value { + Some(expr) => DefaultValue::DefaultTo { + expr, + crate_root: self.crate_root, + }, + None => { + if self.use_default_struct { + DefaultValue::UseDefaultStructField(self.field_ident) + } else { + DefaultValue::ReturnError { + crate_root: self.crate_root, + field_name: self.field_ident.to_string(), + span: self.custom_error_type_span, + } + } + } + } + } +} + +enum DefaultValue<'a> { + /// Inner value must be a valid Rust expression + DefaultTo { + expr: &'a DefaultExpression, + crate_root: &'a syn::Path, + }, + /// Inner value must be the field identifier + /// + /// The default struct must be in scope in the build_method. + UseDefaultStructField(&'a syn::Ident), + /// Inner value must be the field name + ReturnError { + crate_root: &'a syn::Path, + field_name: String, + span: Option, + }, +} + +impl<'a> ToTokens for DefaultValue<'a> { + fn to_tokens(&self, tokens: &mut TokenStream) { + match *self { + DefaultValue::DefaultTo { expr, crate_root } => { + let expr = expr.with_crate_root(crate_root); + tokens.append_all(quote!(Some(#expr))); + } + DefaultValue::UseDefaultStructField(field_ident) => { + let struct_ident = syn::Ident::new(DEFAULT_STRUCT_NAME, Span::call_site()); + tokens.append_all(quote!( + Some(#struct_ident.#field_ident) + )) + } + DefaultValue::ReturnError { + ref field_name, + ref span, + crate_root, + } => { + let conv_span = span.unwrap_or_else(Span::call_site); + // If the conversion fails, the compiler error should point to the error declaration + // rather than the crate root declaration, but the compiler will see the span of #crate_root + // and produce an undesired behavior (possibly because that's the first span in the bad expression?). + // Creating a copy with deeply-rewritten spans preserves the desired error behavior. + let crate_root = change_span(crate_root.into_token_stream(), conv_span); + let err_conv = quote_spanned!(conv_span => #crate_root::export::core::convert::Into::into( + #crate_root::UninitializedFieldError::from(#field_name) + )); + tokens.append_all(quote!( + return #crate_root::export::core::result::Result::Err(#err_conv) + )); + } + } + } +} + +#[derive(Debug, Clone)] +pub enum FieldConversion<'a> { + /// Usual conversion: unwrap the Option from the builder, or (hope to) use a default value + OptionOrDefault, + /// Custom conversion is a block contents expression + Block(&'a BlockContents), + /// Custom conversion is just to move the field from the builder + Move, +} diff --git a/derive_builder_core/src/initializer.rs b/derive_builder_core/src/initializer.rs index d609c15..28a97f4 100644 --- a/derive_builder_core/src/initializer.rs +++ b/derive_builder_core/src/initializer.rs @@ -1,7 +1,7 @@ -use proc_macro2::{Span, TokenStream}; +use proc_macro2::{Ident, Span, TokenStream}; use quote::{ToTokens, TokenStreamExt}; -use crate::{change_span, BlockContents, BuilderPattern, DefaultExpression, DEFAULT_STRUCT_NAME}; +use crate::{BuilderPattern, DefaultExpression, FieldConversion, DEFAULT_FIELD_NAME_PREFIX}; /// Initializer for the target struct fields, implementing `quote::ToTokens`. /// @@ -34,12 +34,15 @@ use crate::{change_span, BlockContents, BuilderPattern, DefaultExpression, DEFAU /// ``` #[derive(Debug, Clone)] pub struct Initializer<'a> { - /// Path to the root of the derive_builder crate. - pub crate_root: &'a syn::Path, /// Name of the target field. pub field_ident: &'a syn::Ident, /// Whether the builder implements a setter for this field. pub field_enabled: bool, + + // TODO delete fields below here + // + /// Path to the root of the derive_builder crate. + pub crate_root: &'a syn::Path, /// How the build method takes and returns `self` (e.g. mutably). pub builder_pattern: BuilderPattern, /// Default value for the target field. @@ -53,7 +56,7 @@ pub struct Initializer<'a> { /// /// An initializer can force early-return if a field has no set value and no default is /// defined. In these cases, it will convert from `derive_builder::UninitializedFieldError` - /// into the return type of its enclosing `build` method. That conversion is guaranteed to + /// into thereturn type of its enclosing `build` method. That conversion is guaranteed to /// work for generated error types, but if the caller specified an error type to use instead /// they may have forgotten the conversion from `UninitializedFieldError` into their specified /// error type. @@ -69,30 +72,17 @@ impl<'a> ToTokens for Initializer<'a> { let struct_field = &self.field_ident; let builder_field = struct_field; + let default_value = Ident::new( + &format!("{}{}", DEFAULT_FIELD_NAME_PREFIX, struct_field), + Span::call_site(), + ); + // This structure prevents accidental failure to add the trailing `,` due to incautious `return` let append_rhs = |tokens: &mut TokenStream| { if !self.field_enabled { - let default = self.default(); - tokens.append_all(quote!( - #default - )); + tokens.append_all(quote!(#default_value)); } else { - match &self.conversion { - FieldConversion::Block(conv) => { - conv.to_tokens(tokens); - } - FieldConversion::Move => tokens.append_all(quote!( self.#builder_field )), - FieldConversion::OptionOrDefault => { - let match_some = self.match_some(); - let match_none = self.match_none(); - tokens.append_all(quote!( - match self.#builder_field { - #match_some, - #match_none, - } - )); - } - } + tokens.append_all(quote!( self.#builder_field.or(#default_value).unwrap())); } }; @@ -102,137 +92,6 @@ impl<'a> ToTokens for Initializer<'a> { } } -impl<'a> Initializer<'a> { - /// To be used inside of `#struct_field: match self.#builder_field { ... }` - fn match_some(&'a self) -> MatchSome { - match self.builder_pattern { - BuilderPattern::Owned => MatchSome::Move, - BuilderPattern::Mutable | BuilderPattern::Immutable => MatchSome::Clone { - crate_root: self.crate_root, - }, - } - } - - /// To be used inside of `#struct_field: match self.#builder_field { ... }` - fn match_none(&'a self) -> MatchNone<'a> { - match self.default_value { - Some(expr) => MatchNone::DefaultTo { - expr, - crate_root: self.crate_root, - }, - None => { - if self.use_default_struct { - MatchNone::UseDefaultStructField(self.field_ident) - } else { - MatchNone::ReturnError { - crate_root: self.crate_root, - field_name: self.field_ident.to_string(), - span: self.custom_error_type_span, - } - } - } - } - } - - fn default(&'a self) -> TokenStream { - let crate_root = self.crate_root; - match self.default_value { - Some(expr) => expr.with_crate_root(crate_root).into_token_stream(), - None if self.use_default_struct => { - let struct_ident = syn::Ident::new(DEFAULT_STRUCT_NAME, Span::call_site()); - let field_ident = self.field_ident; - quote!(#struct_ident.#field_ident) - } - None => { - quote!(#crate_root::export::core::default::Default::default()) - } - } - } -} - -#[derive(Debug, Clone)] -pub enum FieldConversion<'a> { - /// Usual conversion: unwrap the Option from the builder, or (hope to) use a default value - OptionOrDefault, - /// Custom conversion is a block contents expression - Block(&'a BlockContents), - /// Custom conversion is just to move the field from the builder - Move, -} - -/// To be used inside of `#struct_field: match self.#builder_field { ... }` -enum MatchNone<'a> { - /// Inner value must be a valid Rust expression - DefaultTo { - expr: &'a DefaultExpression, - crate_root: &'a syn::Path, - }, - /// Inner value must be the field identifier - /// - /// The default struct must be in scope in the build_method. - UseDefaultStructField(&'a syn::Ident), - /// Inner value must be the field name - ReturnError { - crate_root: &'a syn::Path, - field_name: String, - span: Option, - }, -} - -impl<'a> ToTokens for MatchNone<'a> { - fn to_tokens(&self, tokens: &mut TokenStream) { - match *self { - MatchNone::DefaultTo { expr, crate_root } => { - let expr = expr.with_crate_root(crate_root); - tokens.append_all(quote!(None => #expr)); - } - MatchNone::UseDefaultStructField(field_ident) => { - let struct_ident = syn::Ident::new(DEFAULT_STRUCT_NAME, Span::call_site()); - tokens.append_all(quote!( - None => #struct_ident.#field_ident - )) - } - MatchNone::ReturnError { - ref field_name, - ref span, - crate_root, - } => { - let conv_span = span.unwrap_or_else(Span::call_site); - // If the conversion fails, the compiler error should point to the error declaration - // rather than the crate root declaration, but the compiler will see the span of #crate_root - // and produce an undesired behavior (possibly because that's the first span in the bad expression?). - // Creating a copy with deeply-rewritten spans preserves the desired error behavior. - let crate_root = change_span(crate_root.into_token_stream(), conv_span); - let err_conv = quote_spanned!(conv_span => #crate_root::export::core::convert::Into::into( - #crate_root::UninitializedFieldError::from(#field_name) - )); - tokens.append_all(quote!( - None => return #crate_root::export::core::result::Result::Err(#err_conv) - )); - } - } - } -} - -/// To be used inside of `#struct_field: match self.#builder_field { ... }` -enum MatchSome<'a> { - Move, - Clone { crate_root: &'a syn::Path }, -} - -impl ToTokens for MatchSome<'_> { - fn to_tokens(&self, tokens: &mut TokenStream) { - match *self { - Self::Move => tokens.append_all(quote!( - Some(value) => value - )), - Self::Clone { crate_root } => tokens.append_all(quote!( - Some(ref value) => #crate_root::export::core::clone::Clone::clone(value) - )), - } - } -} - /// Helper macro for unit tests. This is _only_ public in order to be accessible /// from doc-tests too. #[doc(hidden)] diff --git a/derive_builder_core/src/lib.rs b/derive_builder_core/src/lib.rs index da5737d..d7c003c 100644 --- a/derive_builder_core/src/lib.rs +++ b/derive_builder_core/src/lib.rs @@ -15,7 +15,8 @@ //! [`derive_builder`]: https://!crates.io/crates/derive_builder //! [`derive_builder_core`]: https://!crates.io/crates/derive_builder_core -#![deny(warnings, missing_docs)] +// TODO reenable warnings for missing docs +// #![deny(warnings, missing_docs)] #![cfg_attr(test, recursion_limit = "100")] #[macro_use] @@ -39,6 +40,7 @@ mod change_span; mod default_expression; mod deprecation_notes; mod doc_comment; +mod field_default_value; mod initializer; mod macro_options; mod options; @@ -53,11 +55,13 @@ use darling::FromDeriveInput; pub(crate) use default_expression::DefaultExpression; pub(crate) use deprecation_notes::DeprecationNotes; pub(crate) use doc_comment::doc_comment_from; -pub(crate) use initializer::{FieldConversion, Initializer}; +pub(crate) use field_default_value::{FieldConversion, FieldDefaultValue}; +pub(crate) use initializer::Initializer; pub(crate) use options::{BuilderPattern, Each}; pub(crate) use setter::Setter; const DEFAULT_STRUCT_NAME: &str = "__default"; +const DEFAULT_FIELD_NAME_PREFIX: &str = "__default_"; /// Derive a builder for a struct pub fn builder_for_struct(ast: syn::DeriveInput) -> proc_macro2::TokenStream { @@ -83,6 +87,7 @@ pub fn builder_for_struct(ast: syn::DeriveInput) -> proc_macro2::TokenStream { for field in opts.fields() { builder.push_field(field.as_builder_field()); builder.push_setter_fn(field.as_setter()); + build_fn.push_default(field.as_default_value()); build_fn.push_initializer(field.as_initializer()); } diff --git a/derive_builder_core/src/macro_options/darling_opts.rs b/derive_builder_core/src/macro_options/darling_opts.rs index aa0f650..2e9828a 100644 --- a/derive_builder_core/src/macro_options/darling_opts.rs +++ b/derive_builder_core/src/macro_options/darling_opts.rs @@ -10,7 +10,7 @@ use syn::{spanned::Spanned, Attribute, Generics, Ident, Meta, Path}; use crate::{ BlockContents, Builder, BuilderField, BuilderFieldType, BuilderPattern, DefaultExpression, - DeprecationNotes, Each, FieldConversion, Initializer, Setter, + DeprecationNotes, Each, FieldConversion, FieldDefaultValue, Initializer, Setter, }; #[derive(Debug, Clone)] @@ -706,6 +706,7 @@ impl Options { target_ty_generics: Some(ty_generics), error_ty: self.builder_error_ident(), initializers: Vec::with_capacity(self.field_count()), + defaults: Vec::with_capacity(self.field_count()), doc_comment: None, default_struct: self.default.as_ref(), validate_fn: self.build_fn.validate.as_ref(), @@ -905,6 +906,24 @@ impl<'a> FieldWithDefaults<'a> { } } + pub fn as_default_value(&'a self) -> FieldDefaultValue<'a> { + FieldDefaultValue { + crate_root: &self.parent.crate_root, + field_ident: self.field_ident(), + field_enabled: self.field_enabled(), + field_type: &self.field.ty, + default_value: self.field.default.as_ref(), + use_default_struct: self.use_parent_default(), + conversion: self.conversion(), + custom_error_type_span: self.parent.build_fn.error.as_ref().and_then(|err_ty| { + match err_ty { + BuildFnError::Existing(p) => Some(p.span()), + _ => None, + } + }), + } + } + pub fn as_builder_field(&'a self) -> BuilderField<'a> { BuilderField { crate_root: &self.parent.crate_root, From 7161d93c6c8e26947e9e30fc26b56ff6e2962a66 Mon Sep 17 00:00:00 2001 From: Burkhard Mittelbach Date: Sat, 23 Mar 2024 22:28:30 +0100 Subject: [PATCH 02/12] Fix all tests/examples in derive_builder package this still leaves tests in all other packages including tests for internals in derive_builder_core --- .../src/field_default_value.rs | 57 +++++-------- derive_builder_core/src/initializer.rs | 80 ++++++++++++++----- 2 files changed, 80 insertions(+), 57 deletions(-) diff --git a/derive_builder_core/src/field_default_value.rs b/derive_builder_core/src/field_default_value.rs index 65222d9..80e4994 100644 --- a/derive_builder_core/src/field_default_value.rs +++ b/derive_builder_core/src/field_default_value.rs @@ -41,6 +41,10 @@ pub struct FieldDefaultValue<'a> { impl<'a> ToTokens for FieldDefaultValue<'a> { fn to_tokens(&self, tokens: &mut proc_macro2::TokenStream) { + if !self.field_enabled { + // disabled fields have their value calculated in the initializer + return; + } // should be: // ``` // let $prefix$fieldname = match self.$field.as_ref() { @@ -59,53 +63,30 @@ impl<'a> ToTokens for FieldDefaultValue<'a> { Span::call_site(), ); - // token stream to generate the calculation of the default value let default_calculation = (|| { let mut tokens = TokenStream::new(); - if !self.field_enabled { - // If the field is disabled we just calculate the default here. - // It is later set in the initializer - let value = self.disabled_field_value(); - tokens.append_all(quote!(#value)); - } else { - match &self.conversion { - FieldConversion::Block(_) | FieldConversion::Move => { - // value is directly accessed, therfor there is no default - tokens.append_all(quote!(None)) - } - FieldConversion::OptionOrDefault => { - let default = self.default_value(); - tokens.append_all(quote!(#default)); - } - } - } + let default = self.default_value(); + tokens.append_all(quote!(#default)); tokens })(); - tokens.append_all(quote!( - let #default_value: Option<#field_type> = match self.#builder_field.as_ref() { - Some(_) => None, - None => #default_calculation, - }; - )); - } -} - -impl<'a> FieldDefaultValue<'a> { - fn disabled_field_value(&'a self) -> TokenStream { - let crate_root = self.crate_root; - match self.default_value { - Some(expr) => expr.with_crate_root(crate_root).into_token_stream(), - None if self.use_default_struct => { - let struct_ident = syn::Ident::new(DEFAULT_STRUCT_NAME, Span::call_site()); - let field_ident = self.field_ident; - quote!(#struct_ident.#field_ident) + match &self.conversion { + FieldConversion::Block(_) | FieldConversion::Move => { + // there is no defaul value. } - None => { - quote!(#crate_root::export::core::default::Default::default()) + FieldConversion::OptionOrDefault => { + tokens.append_all(quote!( + let #default_value: Option<#field_type> = match self.#builder_field.as_ref() { + Some(_) => None, + None => #default_calculation, + }; + )); } } } +} + +impl<'a> FieldDefaultValue<'a> { fn default_value(&'a self) -> DefaultValue<'a> { match self.default_value { Some(expr) => DefaultValue::DefaultTo { diff --git a/derive_builder_core/src/initializer.rs b/derive_builder_core/src/initializer.rs index 28a97f4..5586bbe 100644 --- a/derive_builder_core/src/initializer.rs +++ b/derive_builder_core/src/initializer.rs @@ -1,7 +1,10 @@ use proc_macro2::{Ident, Span, TokenStream}; use quote::{ToTokens, TokenStreamExt}; -use crate::{BuilderPattern, DefaultExpression, FieldConversion, DEFAULT_FIELD_NAME_PREFIX}; +use crate::{ + BuilderPattern, DefaultExpression, FieldConversion, DEFAULT_FIELD_NAME_PREFIX, + DEFAULT_STRUCT_NAME, +}; /// Initializer for the target struct fields, implementing `quote::ToTokens`. /// @@ -38,19 +41,23 @@ pub struct Initializer<'a> { pub field_ident: &'a syn::Ident, /// Whether the builder implements a setter for this field. pub field_enabled: bool, - - // TODO delete fields below here - // - /// Path to the root of the derive_builder crate. - pub crate_root: &'a syn::Path, - /// How the build method takes and returns `self` (e.g. mutably). - pub builder_pattern: BuilderPattern, + /// Method to use to to convert the builder's field to the target field + /// + /// For sub-builder fields, this will be `build` (or similar) + pub conversion: FieldConversion<'a>, /// Default value for the target field. /// /// This takes precedence over a default struct identifier. pub default_value: Option<&'a DefaultExpression>, /// Whether the build_method defines a default struct. pub use_default_struct: bool, + /// Path to the root of the derive_builder crate. + pub crate_root: &'a syn::Path, + + // TODO delete fields below here + // + /// How the build method takes and returns `self` (e.g. mutably). + pub builder_pattern: BuilderPattern, /// Span where the macro was told to use a preexisting error type, instead of creating one, /// to represent failures of the `build` method. /// @@ -61,10 +68,6 @@ pub struct Initializer<'a> { /// they may have forgotten the conversion from `UninitializedFieldError` into their specified /// error type. pub custom_error_type_span: Option, - /// Method to use to to convert the builder's field to the target field - /// - /// For sub-builder fields, this will be `build` (or similar) - pub conversion: FieldConversion<'a>, } impl<'a> ToTokens for Initializer<'a> { @@ -72,17 +75,25 @@ impl<'a> ToTokens for Initializer<'a> { let struct_field = &self.field_ident; let builder_field = struct_field; - let default_value = Ident::new( - &format!("{}{}", DEFAULT_FIELD_NAME_PREFIX, struct_field), - Span::call_site(), - ); - // This structure prevents accidental failure to add the trailing `,` due to incautious `return` let append_rhs = |tokens: &mut TokenStream| { if !self.field_enabled { - tokens.append_all(quote!(#default_value)); + tokens.append_all(self.default()); } else { - tokens.append_all(quote!( self.#builder_field.or(#default_value).unwrap())); + match &self.conversion { + FieldConversion::Move => tokens.append_all(quote!(self.#builder_field)), + FieldConversion::OptionOrDefault => { + let default_value = Ident::new( + &format!("{}{}", DEFAULT_FIELD_NAME_PREFIX, struct_field), + Span::call_site(), + ); + + let moved_or_cloned = + self.move_or_clone_option(quote!(self.#builder_field)); + tokens.append_all(quote!( #moved_or_cloned.or(#default_value).unwrap())) + } + FieldConversion::Block(content) => content.to_tokens(tokens), + } } }; @@ -92,6 +103,37 @@ impl<'a> ToTokens for Initializer<'a> { } } +impl<'a> Initializer<'a> { + fn move_or_clone_option(&'a self, value_in_option: TokenStream) -> TokenStream { + let crate_root = self.crate_root; + + match self.builder_pattern { + BuilderPattern::Owned => value_in_option, + BuilderPattern::Mutable | BuilderPattern::Immutable => { + quote!( + #value_in_option.as_ref() + .map(|value| #crate_root::export::core::clone::Clone::clone(value)) + ) + } + } + } + + fn default(&'a self) -> TokenStream { + let crate_root = self.crate_root; + match self.default_value { + Some(expr) => expr.with_crate_root(crate_root).into_token_stream(), + None if self.use_default_struct => { + let struct_ident = syn::Ident::new(DEFAULT_STRUCT_NAME, Span::call_site()); + let field_ident = self.field_ident; + quote!(#struct_ident.#field_ident) + } + None => { + quote!(#crate_root::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)] From 97a6cab18130d7f356e090f2378d8d1be9ab974c Mon Sep 17 00:00:00 2001 From: Burkhard Mittelbach Date: Sat, 23 Mar 2024 22:32:43 +0100 Subject: [PATCH 03/12] Fix fields order in initializers to match upstream --- derive_builder_core/src/initializer.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/derive_builder_core/src/initializer.rs b/derive_builder_core/src/initializer.rs index 5586bbe..bf15cd0 100644 --- a/derive_builder_core/src/initializer.rs +++ b/derive_builder_core/src/initializer.rs @@ -37,33 +37,33 @@ use crate::{ /// ``` #[derive(Debug, Clone)] pub struct Initializer<'a> { + /// Path to the root of the derive_builder crate. + pub crate_root: &'a syn::Path, /// Name of the target field. pub field_ident: &'a syn::Ident, /// Whether the builder implements a setter for this field. pub field_enabled: bool, - /// Method to use to to convert the builder's field to the target field - /// - /// For sub-builder fields, this will be `build` (or similar) - pub conversion: FieldConversion<'a>, + /// How the build method takes and returns `self` (e.g. mutably). + pub builder_pattern: BuilderPattern, /// Default value for the target field. /// /// This takes precedence over a default struct identifier. pub default_value: Option<&'a DefaultExpression>, /// Whether the build_method defines a default struct. pub use_default_struct: bool, - /// Path to the root of the derive_builder crate. - pub crate_root: &'a syn::Path, + /// Method to use to to convert the builder's field to the target field + /// + /// For sub-builder fields, this will be `build` (or similar) + pub conversion: FieldConversion<'a>, // TODO delete fields below here // - /// How the build method takes and returns `self` (e.g. mutably). - pub builder_pattern: BuilderPattern, /// Span where the macro was told to use a preexisting error type, instead of creating one, /// to represent failures of the `build` method. /// /// An initializer can force early-return if a field has no set value and no default is /// defined. In these cases, it will convert from `derive_builder::UninitializedFieldError` - /// into thereturn type of its enclosing `build` method. That conversion is guaranteed to + /// into the return type of its enclosing `build` method. That conversion is guaranteed to /// work for generated error types, but if the caller specified an error type to use instead /// they may have forgotten the conversion from `UninitializedFieldError` into their specified /// error type. From 09bcb83fd1b3c620437c2c45e68c896f4a3f61f0 Mon Sep 17 00:00:00 2001 From: Burkhard Mittelbach Date: Sat, 23 Mar 2024 22:37:10 +0100 Subject: [PATCH 04/12] Removed unused fields from initializer --- derive_builder_core/src/initializer.rs | 14 -------------- .../src/macro_options/darling_opts.rs | 6 ------ 2 files changed, 20 deletions(-) diff --git a/derive_builder_core/src/initializer.rs b/derive_builder_core/src/initializer.rs index bf15cd0..3370de3 100644 --- a/derive_builder_core/src/initializer.rs +++ b/derive_builder_core/src/initializer.rs @@ -55,19 +55,6 @@ pub struct Initializer<'a> { /// /// For sub-builder fields, this will be `build` (or similar) pub conversion: FieldConversion<'a>, - - // TODO delete fields below here - // - /// Span where the macro was told to use a preexisting error type, instead of creating one, - /// to represent failures of the `build` method. - /// - /// An initializer can force early-return if a field has no set value and no default is - /// defined. In these cases, it will convert from `derive_builder::UninitializedFieldError` - /// into the return type of its enclosing `build` method. That conversion is guaranteed to - /// work for generated error types, but if the caller specified an error type to use instead - /// they may have forgotten the conversion from `UninitializedFieldError` into their specified - /// error type. - pub custom_error_type_span: Option, } impl<'a> ToTokens for Initializer<'a> { @@ -150,7 +137,6 @@ macro_rules! default_initializer { default_value: None, use_default_struct: false, conversion: FieldConversion::OptionOrDefault, - custom_error_type_span: None, } }; } diff --git a/derive_builder_core/src/macro_options/darling_opts.rs b/derive_builder_core/src/macro_options/darling_opts.rs index 2e9828a..f35cf42 100644 --- a/derive_builder_core/src/macro_options/darling_opts.rs +++ b/derive_builder_core/src/macro_options/darling_opts.rs @@ -897,12 +897,6 @@ impl<'a> FieldWithDefaults<'a> { default_value: self.field.default.as_ref(), use_default_struct: self.use_parent_default(), conversion: self.conversion(), - custom_error_type_span: self.parent.build_fn.error.as_ref().and_then(|err_ty| { - match err_ty { - BuildFnError::Existing(p) => Some(p.span()), - _ => None, - } - }), } } From 1c4b1188155a36c4d41c1b77e0fa8efe546b2219 Mon Sep 17 00:00:00 2001 From: Burkhard Mittelbach Date: Sat, 23 Mar 2024 22:47:43 +0100 Subject: [PATCH 05/12] Fix tests in initializer.rs --- derive_builder_core/src/initializer.rs | 46 ++++++++------------------ 1 file changed, 14 insertions(+), 32 deletions(-) diff --git a/derive_builder_core/src/initializer.rs b/derive_builder_core/src/initializer.rs index 3370de3..cf4aa6f 100644 --- a/derive_builder_core/src/initializer.rs +++ b/derive_builder_core/src/initializer.rs @@ -154,12 +154,9 @@ mod tests { assert_eq!( quote!(#initializer).to_string(), quote!( - foo: match self.foo { - Some(ref value) => ::db::export::core::clone::Clone::clone(value), - None => return ::db::export::core::result::Result::Err(::db::export::core::convert::Into::into( - ::db::UninitializedFieldError::from("foo") - )), - }, + foo: self.foo.as_ref() + .map(|value| ::db::export::core::clone::Clone::clone(value)) + .or(__default_foo).unwrap(), ) .to_string() ); @@ -173,12 +170,9 @@ mod tests { assert_eq!( quote!(#initializer).to_string(), quote!( - foo: match self.foo { - Some(ref value) => ::db::export::core::clone::Clone::clone(value), - None => return ::db::export::core::result::Result::Err(::db::export::core::convert::Into::into( - ::db::UninitializedFieldError::from("foo") - )), - }, + foo: self.foo.as_ref() + .map(|value| ::db::export::core::clone::Clone::clone(value)) + .or(__default_foo).unwrap(), ) .to_string() ); @@ -192,12 +186,7 @@ mod tests { assert_eq!( quote!(#initializer).to_string(), quote!( - foo: match self.foo { - Some(value) => value, - None => return ::db::export::core::result::Result::Err(::db::export::core::convert::Into::into( - ::db::UninitializedFieldError::from("foo") - )), - }, + foo: self.foo.or(__default_foo).unwrap(), ) .to_string() ); @@ -206,16 +195,14 @@ mod tests { #[test] fn default_value() { let mut initializer = default_initializer!(); + initializer.field_enabled = false; let default_value = DefaultExpression::explicit::(parse_quote!(42)); initializer.default_value = Some(&default_value); assert_eq!( quote!(#initializer).to_string(), quote!( - foo: match self.foo { - Some(ref value) => ::db::export::core::clone::Clone::clone(value), - None => { 42 }, - }, + foo: { 42 }, ) .to_string() ); @@ -224,15 +211,13 @@ mod tests { #[test] fn default_struct() { let mut initializer = default_initializer!(); + initializer.field_enabled = false; initializer.use_default_struct = true; assert_eq!( quote!(#initializer).to_string(), quote!( - foo: match self.foo { - Some(ref value) => ::db::export::core::clone::Clone::clone(value), - None => __default.foo, - }, + foo: __default.foo, ) .to_string() ); @@ -256,12 +241,9 @@ mod tests { assert_eq!( quote!(#initializer).to_string(), quote!( - foo: match self.foo { - Some(ref value) => ::db::export::core::clone::Clone::clone(value), - None => return ::db::export::core::result::Result::Err(::db::export::core::convert::Into::into( - ::db::UninitializedFieldError::from("foo") - )), - }, + foo: self.foo.as_ref() + .map(|value| ::db::export::core::clone::Clone::clone(value)) + .or(__default_foo).unwrap(), ) .to_string() ); From bfbfe5b1db0627a8d3c466f8d844b613904efa6b Mon Sep 17 00:00:00 2001 From: Burkhard Mittelbach Date: Sun, 24 Mar 2024 02:01:12 +0100 Subject: [PATCH 06/12] Fix build_method tests turns out having a todo in the test data generation is not good if you want the tests to pass :) --- derive_builder_core/src/build_method.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/derive_builder_core/src/build_method.rs b/derive_builder_core/src/build_method.rs index 6236ec1..4ce1383 100644 --- a/derive_builder_core/src/build_method.rs +++ b/derive_builder_core/src/build_method.rs @@ -165,8 +165,7 @@ macro_rules! default_build_method { target_ty_generics: None, error_ty: syn::parse_quote!(FooBuilderError), initializers: vec![quote!(foo: self.foo,)], - defaults: vec![quote!(todo!("default value"))], // TODO what is the correct default for - // test + defaults: vec![quote!()], doc_comment: None, default_struct: None, validate_fn: None, From 7729bb9ca1d786cf3d05b3c5f442601e931a1fd8 Mon Sep 17 00:00:00 2001 From: Burkhard Mittelbach Date: Sun, 24 Mar 2024 02:45:34 +0100 Subject: [PATCH 07/12] Tests for field_default_values --- .../src/field_default_value.rs | 112 ++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/derive_builder_core/src/field_default_value.rs b/derive_builder_core/src/field_default_value.rs index 80e4994..74e3121 100644 --- a/derive_builder_core/src/field_default_value.rs +++ b/derive_builder_core/src/field_default_value.rs @@ -170,3 +170,115 @@ pub enum FieldConversion<'a> { /// Custom conversion is just to move the field from the builder Move, } + +/// Helper macro for unit tests. This is _only_ public in order to be accessible +/// from doc-tests too. +#[doc(hidden)] +#[macro_export] +macro_rules! default_field_default_value { + () => { + FieldDefaultValue { + // Deliberately don't use the default value here - make sure + // that all test cases are passing crate_root through properly. + crate_root: &parse_quote!(::db), + field_ident: &syn::Ident::new("foo", ::proc_macro2::Span::call_site()), + field_type: &Type::Verbatim(proc_macro2::TokenStream::from_str("usize").unwrap()), + field_enabled: true, + default_value: None, + use_default_struct: false, + conversion: FieldConversion::OptionOrDefault, + custom_error_type_span: None, + } + }; +} + +#[cfg(test)] +mod tests { + use syn::LitStr; + + #[allow(unused_imports)] + use super::*; + + use std::{convert::TryInto, str::FromStr}; + + #[test] + fn disabled() { + let mut default = default_field_default_value!(); + default.field_enabled = false; + + assert_eq!(quote!(#default).to_string(), quote!().to_string()); + } + + #[test] + fn block_conversion() { + let mut default = default_field_default_value!(); + let block_content = &LitStr::new("8", Span::call_site()); + let block: BlockContents = block_content.try_into().unwrap(); + default.conversion = FieldConversion::Block(&block); + + assert_eq!(quote!(#default).to_string(), quote!().to_string()); + } + + #[test] + fn move_conversion() { + let mut default = default_field_default_value!(); + default.conversion = FieldConversion::Move; + + assert_eq!(quote!(#default).to_string(), quote!().to_string()); + } + + #[test] + fn default_value() { + let mut default = default_field_default_value!(); + let default_value = DefaultExpression::explicit::(parse_quote!(42)); + default.default_value = Some(&default_value); + + assert_eq!( + quote!(#default).to_string(), + quote!( + let __default_foo: Option = match self.foo.as_ref() { + Some(_) => None, + None => Some({ 42 }), + }; + ) + .to_string() + ); + } + + #[test] + fn default_struct() { + let mut default = default_field_default_value!(); + default.use_default_struct = true; + + assert_eq!( + quote!(#default).to_string(), + quote!( + let __default_foo: Option = match self.foo.as_ref() { + Some(_) => None, + None => Some(__default.foo), + }; + ) + .to_string() + ); + } + + #[test] + fn no_default() { + let default = default_field_default_value!(); + + assert_eq!( + quote!(#default).to_string(), + quote!( + let __default_foo: Option = match self.foo.as_ref() { + Some(_) => None, + None => return ::db::export::core::result::Result::Err( + ::db::export::core::convert::Into::into( + ::db::UninitializedFieldError::from("foo") + ) + ), + }; + ) + .to_string() + ); + } +} From ad622f8d0f1d23a7d20709968a8db366259105ed Mon Sep 17 00:00:00 2001 From: Burkhard Mittelbach Date: Sun, 24 Mar 2024 04:02:17 +0100 Subject: [PATCH 08/12] Fix compile-fail crate_root test --- .../tests/compile-fail/crate_root.stderr | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/derive_builder/tests/compile-fail/crate_root.stderr b/derive_builder/tests/compile-fail/crate_root.stderr index 8bb4f18..126b13e 100644 --- a/derive_builder/tests/compile-fail/crate_root.stderr +++ b/derive_builder/tests/compile-fail/crate_root.stderr @@ -24,24 +24,6 @@ help: consider importing one of these items 5 | use std::option::Option; | -error[E0433]: failed to resolve: could not find `export` in `empty` - --> tests/compile-fail/crate_root.rs:7:10 - | -7 | #[derive(Builder)] - | ^^^^^^^ not found in `empty::export::core::clone` - | - = note: this error originates in the derive macro `Builder` (in Nightly builds, run with -Z macro-backtrace for more info) -help: consider importing one of these items - | -5 | use core::clone::Clone; - | -5 | use derive_builder::export::core::clone::Clone; - | -5 | use serde::__private::Clone; - | -5 | use std::clone::Clone; - | - error[E0433]: failed to resolve: could not find `export` in `empty` --> tests/compile-fail/crate_root.rs:7:10 | @@ -91,6 +73,24 @@ help: consider importing this struct 5 | use derive_builder::UninitializedFieldError; | +error[E0433]: failed to resolve: could not find `export` in `empty` + --> tests/compile-fail/crate_root.rs:7:10 + | +7 | #[derive(Builder)] + | ^^^^^^^ not found in `empty::export::core::clone` + | + = note: this error originates in the derive macro `Builder` (in Nightly builds, run with -Z macro-backtrace for more info) +help: consider importing one of these items + | +5 | use core::clone::Clone; + | +5 | use derive_builder::export::core::clone::Clone; + | +5 | use serde::__private::Clone; + | +5 | use std::clone::Clone; + | + error[E0433]: failed to resolve: could not find `export` in `empty` --> tests/compile-fail/crate_root.rs:7:10 | From 0b75a7f1002bc8dfd22b008ac8503b8d11669523 Mon Sep 17 00:00:00 2001 From: Burkhard Mittelbach Date: Sun, 24 Mar 2024 04:36:57 +0100 Subject: [PATCH 09/12] remove conversion from field_default_value + docs conversion is only used to disable the default value if the enum is not OptionOrDefault. It's cleaner to just use an enabled flag for that. --- .../src/field_default_value.rs | 96 +++++++++---------- derive_builder_core/src/initializer.rs | 23 +++-- derive_builder_core/src/lib.rs | 7 +- .../src/macro_options/darling_opts.rs | 6 +- 4 files changed, 72 insertions(+), 60 deletions(-) diff --git a/derive_builder_core/src/field_default_value.rs b/derive_builder_core/src/field_default_value.rs index 74e3121..a4d2b49 100644 --- a/derive_builder_core/src/field_default_value.rs +++ b/derive_builder_core/src/field_default_value.rs @@ -2,11 +2,39 @@ use proc_macro2::{Ident, Span, TokenStream}; use quote::{ToTokens, TokenStreamExt}; use syn::Type; -use crate::{ - change_span, BlockContents, DefaultExpression, DEFAULT_FIELD_NAME_PREFIX, DEFAULT_STRUCT_NAME, -}; +use crate::{change_span, DefaultExpression, DEFAULT_FIELD_NAME_PREFIX, DEFAULT_STRUCT_NAME}; -// TODO: remove no longer needed code from Initializer +/// Calculates the default value or error for fields, implementing `quote::ToTokens +/// +/// Lives in the body of `BuildMethod`. +/// +/// # Examples +/// +/// Will expand to something like the following (depending on settings): +/// +/// ```rust,ignore +/// # extern crate proc_macro2; +/// # #[macro_use] +/// # extern crate quote; +/// # extern crate syn; +/// # #[macro_use] +/// # extern crate derive_builder_core; +/// # use derive_builder_core::{DeprecationNotes, Initializer, BuilderPattern}; +/// # fn main() { +/// # let mut default = default_field_default_value!(); +/// # let default_value = DefaultExpression::explicit::(parse_quote!(42)); +/// # default.default_value = Some(&default_value); +/// # assert_eq!(quote!(#default).to_string(), quote!( +/// let __default_foo: Option = match self.foo.as_ref() { +/// Some(_) => None, +/// None => Some({ 42 }), +/// }; +/// # ).to_string()); +/// # } +/// ``` +/// In case there is no default value +/// (`default_value == None && use_default_struct == false`) +/// the `None` case will return an error. #[derive(Debug, Clone)] pub struct FieldDefaultValue<'a> { /// Path to the root of the derive_builder crate. @@ -17,6 +45,8 @@ pub struct FieldDefaultValue<'a> { pub field_type: &'a Type, /// Whether the builder implements a setter for this field. pub field_enabled: bool, + /// Whether the builder uses the default value. + pub enabled: bool, /// Default value for the target field. /// /// This takes precedence over a default struct identifier. @@ -29,19 +59,15 @@ pub struct FieldDefaultValue<'a> { /// An initializer can force early-return if a field has no set value and no default is /// defined. In these cases, it will convert from `derive_builder::UninitializedFieldError` /// into the return type of its enclosing `build` method. That conversion is guaranteed to - /// work for generated error types, but if the caller specified an error type to use instead + /// work fr generated error types, but if the caller specified an error type to use instead /// they may have forgotten the conversion from `UninitializedFieldError` into their specified /// error type. pub custom_error_type_span: Option, - /// Method to use to to convert the builder's field to the target field - /// - /// For sub-builder fields, this will be `build` (or similar) - pub conversion: FieldConversion<'a>, } impl<'a> ToTokens for FieldDefaultValue<'a> { fn to_tokens(&self, tokens: &mut proc_macro2::TokenStream) { - if !self.field_enabled { + if !self.field_enabled || !self.enabled { // disabled fields have their value calculated in the initializer return; } @@ -70,19 +96,12 @@ impl<'a> ToTokens for FieldDefaultValue<'a> { tokens })(); - match &self.conversion { - FieldConversion::Block(_) | FieldConversion::Move => { - // there is no defaul value. - } - FieldConversion::OptionOrDefault => { - tokens.append_all(quote!( - let #default_value: Option<#field_type> = match self.#builder_field.as_ref() { - Some(_) => None, - None => #default_calculation, - }; - )); - } - } + tokens.append_all(quote!( + let #default_value: Option<#field_type> = match self.#builder_field.as_ref() { + Some(_) => None, + None => #default_calculation, + }; + )); } } @@ -161,16 +180,6 @@ impl<'a> ToTokens for DefaultValue<'a> { } } -#[derive(Debug, Clone)] -pub enum FieldConversion<'a> { - /// Usual conversion: unwrap the Option from the builder, or (hope to) use a default value - OptionOrDefault, - /// Custom conversion is a block contents expression - Block(&'a BlockContents), - /// Custom conversion is just to move the field from the builder - Move, -} - /// Helper macro for unit tests. This is _only_ public in order to be accessible /// from doc-tests too. #[doc(hidden)] @@ -184,9 +193,9 @@ macro_rules! default_field_default_value { field_ident: &syn::Ident::new("foo", ::proc_macro2::Span::call_site()), field_type: &Type::Verbatim(proc_macro2::TokenStream::from_str("usize").unwrap()), field_enabled: true, + enabled: true, default_value: None, use_default_struct: false, - conversion: FieldConversion::OptionOrDefault, custom_error_type_span: None, } }; @@ -194,35 +203,24 @@ macro_rules! default_field_default_value { #[cfg(test)] mod tests { - use syn::LitStr; #[allow(unused_imports)] use super::*; - use std::{convert::TryInto, str::FromStr}; + use std::str::FromStr; #[test] fn disabled() { let mut default = default_field_default_value!(); - default.field_enabled = false; - - assert_eq!(quote!(#default).to_string(), quote!().to_string()); - } - - #[test] - fn block_conversion() { - let mut default = default_field_default_value!(); - let block_content = &LitStr::new("8", Span::call_site()); - let block: BlockContents = block_content.try_into().unwrap(); - default.conversion = FieldConversion::Block(&block); + default.enabled = false; assert_eq!(quote!(#default).to_string(), quote!().to_string()); } #[test] - fn move_conversion() { + fn disabled_field() { let mut default = default_field_default_value!(); - default.conversion = FieldConversion::Move; + default.field_enabled = false; assert_eq!(quote!(#default).to_string(), quote!().to_string()); } diff --git a/derive_builder_core/src/initializer.rs b/derive_builder_core/src/initializer.rs index cf4aa6f..e82e91e 100644 --- a/derive_builder_core/src/initializer.rs +++ b/derive_builder_core/src/initializer.rs @@ -2,7 +2,7 @@ use proc_macro2::{Ident, Span, TokenStream}; use quote::{ToTokens, TokenStreamExt}; use crate::{ - BuilderPattern, DefaultExpression, FieldConversion, DEFAULT_FIELD_NAME_PREFIX, + BlockContents, BuilderPattern, DefaultExpression, DEFAULT_FIELD_NAME_PREFIX, DEFAULT_STRUCT_NAME, }; @@ -10,6 +10,7 @@ use crate::{ /// /// Lives in the body of `BuildMethod`. /// +/// /// # Examples /// /// Will expand to something like the following (depending on settings): @@ -24,14 +25,10 @@ use crate::{ /// # use derive_builder_core::{DeprecationNotes, Initializer, BuilderPattern}; /// # fn main() { /// # let mut initializer = default_initializer!(); -/// # initializer.default_value = Some("42".parse().unwrap()); /// # initializer.builder_pattern = BuilderPattern::Owned; /// # /// # assert_eq!(quote!(#initializer).to_string(), quote!( -/// foo: match self.foo { -/// Some(value) => value, -/// None => { 42 }, -/// }, +/// foo: self.foo.or(__default_foo).unwrap(), /// # ).to_string()); /// # } /// ``` @@ -54,6 +51,10 @@ pub struct Initializer<'a> { /// Method to use to to convert the builder's field to the target field /// /// For sub-builder fields, this will be `build` (or similar) + /// If the `conversion` is `FieldConversion::OptionOrDefault` this will + /// use the default value calculated in `FieldDefaultValue`. Otherwise + /// the default value is calculated based on `default_value` and + /// `use_default_struct`. pub conversion: FieldConversion<'a>, } @@ -121,6 +122,16 @@ impl<'a> Initializer<'a> { } } +#[derive(Debug, Clone)] +pub enum FieldConversion<'a> { + /// Usual conversion: unwrap the Option from the builder, or (hope to) use a default value + OptionOrDefault, + /// Custom conversion is a block contents expression + Block(&'a BlockContents), + /// Custom conversion is just to move the field from the builder + Move, +} + /// Helper macro for unit tests. This is _only_ public in order to be accessible /// from doc-tests too. #[doc(hidden)] diff --git a/derive_builder_core/src/lib.rs b/derive_builder_core/src/lib.rs index d7c003c..1ac88fb 100644 --- a/derive_builder_core/src/lib.rs +++ b/derive_builder_core/src/lib.rs @@ -15,8 +15,7 @@ //! [`derive_builder`]: https://!crates.io/crates/derive_builder //! [`derive_builder_core`]: https://!crates.io/crates/derive_builder_core -// TODO reenable warnings for missing docs -// #![deny(warnings, missing_docs)] +#![deny(warnings, missing_docs)] #![cfg_attr(test, recursion_limit = "100")] #[macro_use] @@ -55,8 +54,8 @@ use darling::FromDeriveInput; pub(crate) use default_expression::DefaultExpression; pub(crate) use deprecation_notes::DeprecationNotes; pub(crate) use doc_comment::doc_comment_from; -pub(crate) use field_default_value::{FieldConversion, FieldDefaultValue}; -pub(crate) use initializer::Initializer; +pub(crate) use field_default_value::FieldDefaultValue; +pub(crate) use initializer::{FieldConversion, Initializer}; pub(crate) use options::{BuilderPattern, Each}; pub(crate) use setter::Setter; diff --git a/derive_builder_core/src/macro_options/darling_opts.rs b/derive_builder_core/src/macro_options/darling_opts.rs index f35cf42..852c50c 100644 --- a/derive_builder_core/src/macro_options/darling_opts.rs +++ b/derive_builder_core/src/macro_options/darling_opts.rs @@ -901,14 +901,18 @@ impl<'a> FieldWithDefaults<'a> { } pub fn as_default_value(&'a self) -> FieldDefaultValue<'a> { + let enabled = match self.conversion() { + FieldConversion::OptionOrDefault => true, + FieldConversion::Block(_) | FieldConversion::Move => false, + }; FieldDefaultValue { crate_root: &self.parent.crate_root, field_ident: self.field_ident(), field_enabled: self.field_enabled(), + enabled, field_type: &self.field.ty, default_value: self.field.default.as_ref(), use_default_struct: self.use_parent_default(), - conversion: self.conversion(), custom_error_type_span: self.parent.build_fn.error.as_ref().and_then(|err_ty| { match err_ty { BuildFnError::Existing(p) => Some(p.span()), From c342aba04d6a66486098cbc775270596f2138faa Mon Sep 17 00:00:00 2001 From: Burkhard Mittelbach Date: Sun, 24 Mar 2024 04:44:17 +0100 Subject: [PATCH 10/12] Update changelog --- derive_builder/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/derive_builder/CHANGELOG.md b/derive_builder/CHANGELOG.md index 973196d..89007e0 100644 --- a/derive_builder/CHANGELOG.md +++ b/derive_builder/CHANGELOG.md @@ -2,6 +2,9 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/). +## [Unreleased] +- Allow default values that access `self` in combination with the "owned" pattern. #298 + ## [0.20.0] - 2024-02-14 - Bump `syn` to version 2 #308 - Bump `darling` to version 0.20.6 #308 From 33b1a451a12a7e7263faa7fce5f554cc8f950bc9 Mon Sep 17 00:00:00 2001 From: Burkhard Mittelbach Date: Sun, 24 Mar 2024 04:47:20 +0100 Subject: [PATCH 11/12] fix clippy warning --- derive_builder_core/src/field_default_value.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/derive_builder_core/src/field_default_value.rs b/derive_builder_core/src/field_default_value.rs index a4d2b49..095a9c3 100644 --- a/derive_builder_core/src/field_default_value.rs +++ b/derive_builder_core/src/field_default_value.rs @@ -89,12 +89,7 @@ impl<'a> ToTokens for FieldDefaultValue<'a> { Span::call_site(), ); - let default_calculation = (|| { - let mut tokens = TokenStream::new(); - let default = self.default_value(); - tokens.append_all(quote!(#default)); - tokens - })(); + let default_calculation = self.default_value(); tokens.append_all(quote!( let #default_value: Option<#field_type> = match self.#builder_field.as_ref() { From c2510cf1b9f3cb41b90db46323045fbe85dd205c Mon Sep 17 00:00:00 2001 From: Burkhard Mittelbach Date: Sun, 24 Mar 2024 23:01:49 +0100 Subject: [PATCH 12/12] Move default calculation for disabled fields into field_default_value --- derive_builder/tests/custom_default.rs | 50 +++++++++++++ .../src/field_default_value.rs | 59 ++++++++++----- derive_builder_core/src/initializer.rs | 71 +++---------------- .../src/macro_options/darling_opts.rs | 2 - 4 files changed, 98 insertions(+), 84 deletions(-) diff --git a/derive_builder/tests/custom_default.rs b/derive_builder/tests/custom_default.rs index 8fd6d21..0229774 100644 --- a/derive_builder/tests/custom_default.rs +++ b/derive_builder/tests/custom_default.rs @@ -136,3 +136,53 @@ mod struct_level { assert_eq!(ipsum.not_type_default, None); } } + +mod owned_field { + + #[derive(Debug, Clone, PartialEq, Eq, Builder)] + #[builder(pattern = "owned")] + struct Lorem { + #[builder(default = "self.ipsum_default()")] + ipsum: String, + + #[builder(setter(skip), default = "self.dolor_default()")] + dolor: String, + } + + impl LoremBuilder { + fn ipsum_default(&self) -> String { + "ipsum".to_string() + } + fn dolor_default(&self) -> String { + "dolor".to_string() + } + } + + #[test] + fn builder_test() { + let x = LoremBuilder::create_empty() + .ipsum("Ipsum".to_string()) + .build() + .unwrap(); + + assert_eq!( + x, + Lorem { + ipsum: "Ipsum".to_string(), + dolor: "dolor".to_string(), + } + ); + } + #[test] + fn defaults_test() { + let x = LoremBuilder::create_empty().build().unwrap(); + + assert_eq!( + x, + Lorem { + ipsum: "ipsum".to_string(), + dolor: "dolor".to_string(), + } + ); + } +} diff --git a/derive_builder_core/src/field_default_value.rs b/derive_builder_core/src/field_default_value.rs index 095a9c3..60cb0ea 100644 --- a/derive_builder_core/src/field_default_value.rs +++ b/derive_builder_core/src/field_default_value.rs @@ -67,17 +67,9 @@ pub struct FieldDefaultValue<'a> { impl<'a> ToTokens for FieldDefaultValue<'a> { fn to_tokens(&self, tokens: &mut proc_macro2::TokenStream) { - if !self.field_enabled || !self.enabled { - // disabled fields have their value calculated in the initializer + if !self.enabled { return; } - // should be: - // ``` - // let $prefix$fieldname = match self.$field.as_ref() { - // Some(_) => None, - // None => Some($default_value) - // } - // ``` let struct_field = &self.field_ident; let builder_field = struct_field; @@ -89,19 +81,40 @@ impl<'a> ToTokens for FieldDefaultValue<'a> { Span::call_site(), ); - let default_calculation = self.default_value(); - - tokens.append_all(quote!( - let #default_value: Option<#field_type> = match self.#builder_field.as_ref() { - Some(_) => None, - None => #default_calculation, - }; - )); + if self.field_enabled { + let default_calculation = self.default_value_calculation(); + tokens.append_all(quote!( + let #default_value: Option<#field_type> = match self.#builder_field.as_ref() { + Some(_) => None, + None => #default_calculation, + }; + )); + } else { + let default_calculation = self.default_value_for_disabled(); + tokens.append_all(quote!( + let #default_value: #field_type = #default_calculation; + )); + } } } impl<'a> FieldDefaultValue<'a> { - fn default_value(&'a self) -> DefaultValue<'a> { + fn default_value_for_disabled(&'a self) -> TokenStream { + let crate_root = self.crate_root; + match self.default_value { + Some(expr) => expr.with_crate_root(crate_root).into_token_stream(), + None if self.use_default_struct => { + let struct_ident = syn::Ident::new(DEFAULT_STRUCT_NAME, Span::call_site()); + let field_ident = self.field_ident; + quote!(#struct_ident.#field_ident) + } + None => { + quote!(#crate_root::export::core::default::Default::default()) + } + } + } + + fn default_value_calculation(&'a self) -> DefaultValue<'a> { match self.default_value { Some(expr) => DefaultValue::DefaultTo { expr, @@ -216,8 +229,16 @@ mod tests { fn disabled_field() { let mut default = default_field_default_value!(); default.field_enabled = false; + let default_value = DefaultExpression::explicit::(parse_quote!(42)); + default.default_value = Some(&default_value); - assert_eq!(quote!(#default).to_string(), quote!().to_string()); + assert_eq!( + quote!(#default).to_string(), + quote!( + let __default_foo: usize = { 42 }; + ) + .to_string() + ); } #[test] diff --git a/derive_builder_core/src/initializer.rs b/derive_builder_core/src/initializer.rs index e82e91e..e78beb3 100644 --- a/derive_builder_core/src/initializer.rs +++ b/derive_builder_core/src/initializer.rs @@ -1,10 +1,7 @@ use proc_macro2::{Ident, Span, TokenStream}; use quote::{ToTokens, TokenStreamExt}; -use crate::{ - BlockContents, BuilderPattern, DefaultExpression, DEFAULT_FIELD_NAME_PREFIX, - DEFAULT_STRUCT_NAME, -}; +use crate::{BlockContents, BuilderPattern, DEFAULT_FIELD_NAME_PREFIX}; /// Initializer for the target struct fields, implementing `quote::ToTokens`. /// @@ -42,12 +39,6 @@ pub struct Initializer<'a> { pub field_enabled: bool, /// How the build method takes and returns `self` (e.g. mutably). pub builder_pattern: BuilderPattern, - /// Default value for the target field. - /// - /// This takes precedence over a default struct identifier. - pub default_value: Option<&'a DefaultExpression>, - /// Whether the build_method defines a default struct. - pub use_default_struct: bool, /// Method to use to to convert the builder's field to the target field /// /// For sub-builder fields, this will be `build` (or similar) @@ -62,20 +53,19 @@ impl<'a> ToTokens for Initializer<'a> { fn to_tokens(&self, tokens: &mut TokenStream) { let struct_field = &self.field_ident; let builder_field = struct_field; + let default_value = Ident::new( + &format!("{}{}", DEFAULT_FIELD_NAME_PREFIX, struct_field), + Span::call_site(), + ); // This structure prevents accidental failure to add the trailing `,` due to incautious `return` let append_rhs = |tokens: &mut TokenStream| { if !self.field_enabled { - tokens.append_all(self.default()); + tokens.append(default_value); } else { match &self.conversion { FieldConversion::Move => tokens.append_all(quote!(self.#builder_field)), FieldConversion::OptionOrDefault => { - let default_value = Ident::new( - &format!("{}{}", DEFAULT_FIELD_NAME_PREFIX, struct_field), - Span::call_site(), - ); - let moved_or_cloned = self.move_or_clone_option(quote!(self.#builder_field)); tokens.append_all(quote!( #moved_or_cloned.or(#default_value).unwrap())) @@ -105,21 +95,6 @@ impl<'a> Initializer<'a> { } } } - - fn default(&'a self) -> TokenStream { - let crate_root = self.crate_root; - match self.default_value { - Some(expr) => expr.with_crate_root(crate_root).into_token_stream(), - None if self.use_default_struct => { - let struct_ident = syn::Ident::new(DEFAULT_STRUCT_NAME, Span::call_site()); - let field_ident = self.field_ident; - quote!(#struct_ident.#field_ident) - } - None => { - quote!(#crate_root::export::core::default::Default::default()) - } - } - } } #[derive(Debug, Clone)] @@ -145,8 +120,6 @@ macro_rules! default_initializer { field_ident: &syn::Ident::new("foo", ::proc_macro2::Span::call_site()), field_enabled: true, builder_pattern: BuilderPattern::Mutable, - default_value: None, - use_default_struct: false, conversion: FieldConversion::OptionOrDefault, } }; @@ -204,47 +177,19 @@ mod tests { } #[test] - fn default_value() { - let mut initializer = default_initializer!(); - initializer.field_enabled = false; - let default_value = DefaultExpression::explicit::(parse_quote!(42)); - initializer.default_value = Some(&default_value); - - assert_eq!( - quote!(#initializer).to_string(), - quote!( - foo: { 42 }, - ) - .to_string() - ); - } - - #[test] - fn default_struct() { + fn setter_disabled() { let mut initializer = default_initializer!(); initializer.field_enabled = false; - initializer.use_default_struct = true; assert_eq!( quote!(#initializer).to_string(), quote!( - foo: __default.foo, + foo: __default_foo, ) .to_string() ); } - #[test] - fn setter_disabled() { - let mut initializer = default_initializer!(); - initializer.field_enabled = false; - - assert_eq!( - quote!(#initializer).to_string(), - quote!(foo: ::db::export::core::default::Default::default(),).to_string() - ); - } - #[test] fn no_std() { let initializer = default_initializer!(); diff --git a/derive_builder_core/src/macro_options/darling_opts.rs b/derive_builder_core/src/macro_options/darling_opts.rs index 852c50c..eeeeb4a 100644 --- a/derive_builder_core/src/macro_options/darling_opts.rs +++ b/derive_builder_core/src/macro_options/darling_opts.rs @@ -894,8 +894,6 @@ impl<'a> FieldWithDefaults<'a> { field_enabled: self.field_enabled(), field_ident: self.field_ident(), builder_pattern: self.pattern(), - default_value: self.field.default.as_ref(), - use_default_struct: self.use_parent_default(), conversion: self.conversion(), } }