From 0745d901a793a92f7680eef4efd5d959da40f264 Mon Sep 17 00:00:00 2001 From: Veetaha Date: Fri, 25 Oct 2024 18:58:18 +0000 Subject: [PATCH 01/10] Update changelog for the `#[builder(on(_, transparent))]` --- website/changelog.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/changelog.md b/website/changelog.md index 4445ffe0..c0f901ad 100644 --- a/website/changelog.md +++ b/website/changelog.md @@ -23,7 +23,7 @@ All the breaking changes are very unlikely to actually break your code that was - Add `#[builder(builder_type(vis = "..."))]` that allows overriding the visibility of the builder struct ([#145](https://github.com/elastio/bon/pull/145)) - Add `#[builder(finish_fn(vis = "..."))]` that allows overriding the visibility of the finishing function ([#145](https://github.com/elastio/bon/pull/145)) - Add `#[builder(with = closure)]` syntax to customize setters with a custom closure. If the closure returns a `Result<_, E>` the setters become fallible ([#145](https://github.com/elastio/bon/pull/145)) -- Add `#[builder(transparent)]` for `Option` fields to opt out from their special handling which makes `bon` treat them as regular required fields ([#145](https://github.com/elastio/bon/pull/145)) +- Add `#[builder(transparent)]` for `Option` fields to opt out from their special handling which makes `bon` treat them as regular required fields. It's also available at the top-level via `#[builder(on(_, transparent))]` ([#145](https://github.com/elastio/bon/pull/145), [#155](https://github.com/elastio/bon/pull/155)) - Add `#[builder(state_mod)]` to configure the builder's type state API module name, visibility and docs ([#145](https://github.com/elastio/bon/pull/145)) - Add `#[builder(overwritable)]` and `#[builder(on(..., overwritable)]` to make it possible to call setters multiple times for the same member ([#145](https://github.com/elastio/bon/pull/145)) - Add `#[builder(setters)]` to fine-tune the setters names, visibility and docs ([#145](https://github.com/elastio/bon/pull/145)) From 1f116393ac38390de4b2d75faf056014580807cf Mon Sep 17 00:00:00 2001 From: Veetaha Date: Fri, 25 Oct 2024 21:48:08 +0000 Subject: [PATCH 02/10] Initial impl, tests for `with = Some` --- .../builder/builder_gen/member/config/mod.rs | 15 +- .../{setter_closure.rs => with/closure.rs} | 0 .../builder_gen/member/config/with/mod.rs | 70 +++++ bon-macros/src/builder/builder_gen/mod.rs | 11 +- .../src/builder/builder_gen/setters/mod.rs | 246 +++++++++++++----- .../integration/builder/attr_with/mod.rs | 1 + .../integration/builder/attr_with/some.rs | 148 +++++++++++ .../ui/compile_fail/attr_transparent.rs | 4 + .../ui/compile_fail/attr_transparent.stderr | 34 ++- .../ui/compile_fail/attr_with.stderr | 5 +- 10 files changed, 451 insertions(+), 83 deletions(-) rename bon-macros/src/builder/builder_gen/member/config/{setter_closure.rs => with/closure.rs} (100%) create mode 100644 bon-macros/src/builder/builder_gen/member/config/with/mod.rs create mode 100644 bon/tests/integration/builder/attr_with/some.rs diff --git a/bon-macros/src/builder/builder_gen/member/config/mod.rs b/bon-macros/src/builder/builder_gen/member/config/mod.rs index e2c9a552..10bf68f7 100644 --- a/bon-macros/src/builder/builder_gen/member/config/mod.rs +++ b/bon-macros/src/builder/builder_gen/member/config/mod.rs @@ -1,9 +1,9 @@ mod blanket; -mod setter_closure; +mod with; mod setters; pub(crate) use blanket::*; -pub(crate) use setter_closure::*; +pub(crate) use with::*; pub(crate) use setters::*; use super::MemberOrigin; @@ -52,11 +52,12 @@ pub(crate) struct MemberConfig { /// this option to see if it's worth it. pub(crate) overwritable: darling::util::Flag, - /// Customize the setter signature and body with a custom closure. The closure - /// must return the value of the type of the member, or optionally a `Result<_>` - /// type where `_` is used to mark the type of the member. In this case the - /// generated setters will be fallible (they'll propagate the `Result`). - pub(crate) with: Option>, + /// Customize the setter signature and body with a custom closure or a well-known + /// function. The closure/function must return the value of the type of the member, + /// or optionally a `Result<_>` type where `_` is used to mark the type of + /// the member. In this case the generated setters will be fallible + /// (they'll propagate the `Result`). + pub(crate) with: Option>, /// Disables the special handling for a member of type `Option`. The /// member no longer has the default of `None`. It also becomes a required diff --git a/bon-macros/src/builder/builder_gen/member/config/setter_closure.rs b/bon-macros/src/builder/builder_gen/member/config/with/closure.rs similarity index 100% rename from bon-macros/src/builder/builder_gen/member/config/setter_closure.rs rename to bon-macros/src/builder/builder_gen/member/config/with/closure.rs diff --git a/bon-macros/src/builder/builder_gen/member/config/with/mod.rs b/bon-macros/src/builder/builder_gen/member/config/with/mod.rs new file mode 100644 index 00000000..7e19dfd6 --- /dev/null +++ b/bon-macros/src/builder/builder_gen/member/config/with/mod.rs @@ -0,0 +1,70 @@ +mod closure; + +pub(crate) use closure::*; + +use crate::util::prelude::*; +use darling::FromMeta; + +#[derive(Debug)] +pub(crate) enum WithConfig { + /// Closure syntax e.g. `#[builder(with = |param: Type| body)]` + Closure(SetterClosure), + + /// Well-known path [`Option::Some`] + Some(syn::Path), + + /// Well-known path [`FromIterator::from_iter`] or `<_>::from_iter` + FromIter(syn::ExprPath), +} + +impl WithConfig { + pub(crate) fn as_closure(&self) -> Option<&SetterClosure> { + match self { + Self::Closure(closure) => Some(closure), + _ => None, + } + } +} + +impl FromMeta for WithConfig { + fn from_meta(meta: &syn::Meta) -> darling::Result { + let err = || { + err!( + meta, + "expected a closure e.g. `#[builder(with = |param: T| expression)]` or \ + a well-known function path which could be one of:\n\ + - #[builder(with = Some)]\n\ + - #[builder(with = FromIterator::from_iter)]\n\ + - #[builder(with = <_>::from_iter)] (same as above, but shorter)", + ) + }; + + let name_val = match meta { + syn::Meta::NameValue(meta) => meta, + _ => return Err(err()), + }; + + if let syn::Expr::Closure(_) = name_val.value { + return SetterClosure::from_meta(meta).map(Self::Closure); + } + + let path = match &name_val.value { + syn::Expr::Path(path) => path, + _ => return Err(err()), + }; + + crate::parsing::reject_syntax("attribute", &path.attrs.first())?; + + if *path == syn::parse_quote!(Some) { + return Ok(Self::Some(path.path.clone())); + } + + if *path == syn::parse_quote!(FromIterator::from_iter) + || *path == syn::parse_quote!(<_>::from_iter) + { + return Ok(Self::FromIter(path.clone())); + } + + Err(err()) + } +} diff --git a/bon-macros/src/builder/builder_gen/mod.rs b/bon-macros/src/builder/builder_gen/mod.rs index 8fe45b57..588fd41a 100644 --- a/bon-macros/src/builder/builder_gen/mod.rs +++ b/bon-macros/src/builder/builder_gen/mod.rs @@ -42,7 +42,7 @@ impl BuilderGenCtx { let mut start_fn = self.start_fn(); let state_mod = state_mod::StateModGenCtx::new(&self).state_mod(); let builder_decl = self.builder_decl(); - let builder_impl = self.builder_impl(); + let builder_impl = self.builder_impl()?; let builder_derives = self.builder_derives(); let default_allows = syn::parse_quote!(#[allow( @@ -94,11 +94,12 @@ impl BuilderGenCtx { }) } - fn builder_impl(&self) -> TokenStream { + fn builder_impl(&self) -> Result { let finish_fn = self.finish_fn(); let setter_methods = self .named_members() - .map(|member| SettersCtx::new(self, member).setter_methods()); + .map(|member| SettersCtx::new(self, member).setter_methods()) + .collect::>>()?; let generics_decl = &self.generics.decl_without_defaults; let generic_args = &self.generics.args; @@ -109,7 +110,7 @@ impl BuilderGenCtx { let allows = allow_warnings_on_member_types(); - quote! { + Ok(quote! { #allows #[automatically_derived] impl< @@ -125,7 +126,7 @@ impl BuilderGenCtx { #finish_fn #(#setter_methods)* } - } + }) } /// Generates code that has no meaning to the compiler, but it helps diff --git a/bon-macros/src/builder/builder_gen/setters/mod.rs b/bon-macros/src/builder/builder_gen/setters/mod.rs index 48bf088b..991dd5a4 100644 --- a/bon-macros/src/builder/builder_gen/setters/mod.rs +++ b/bon-macros/src/builder/builder_gen/setters/mod.rs @@ -1,4 +1,4 @@ -use super::member::SetterClosure; +use super::member::WithConfig; use super::{BuilderGenCtx, NamedMember}; use crate::parsing::ItemSigConfig; use crate::util::prelude::*; @@ -14,27 +14,30 @@ impl<'a> SettersCtx<'a> { Self { base, member } } - pub(crate) fn setter_methods(&self) -> TokenStream { + pub(crate) fn setter_methods(&self) -> Result { match SettersItems::new(self) { SettersItems::Required(item) => self.setter_for_required_member(item), SettersItems::Optional(setters) => self.setters_for_optional_member(setters), } } - fn setter_for_required_member(&self, item: SetterItem) -> TokenStream { - let input; + fn setter_for_required_member(&self, item: SetterItem) -> Result { + let inputs; let expr; let member_type = self.member.ty.norm.as_ref(); - if let Some(closure) = &self.member.config.with { - input = Self::underlying_input_from_closure(closure); - expr = self.member_expr_from_closure(closure); + if let Some(with) = &self.member.config.with { + inputs = self.underlying_inputs_from_with(with)?; + expr = self.member_expr_from_with(with); } else if self.member.config.into.is_present() { - input = quote!(value: impl Into<#member_type>); + inputs = vec![( + pat_ident("value"), + syn::parse_quote!(impl Into<#member_type>), + )]; expr = quote!(Into::into(value)); } else { - input = quote!(value: #member_type); + inputs = vec![(pat_ident("value"), member_type.clone())]; expr = quote!(value); }; @@ -42,28 +45,28 @@ impl<'a> SettersCtx<'a> { expr: quote!(::core::option::Option::Some(#expr)), }; - self.setter_method(Setter { + Ok(self.setter_method(Setter { item, - imp: SetterImpl { input, body }, - }) + imp: SetterImpl { inputs, body }, + })) } - fn setters_for_optional_member(&self, items: OptionalSettersItems) -> TokenStream { + fn setters_for_optional_member(&self, items: OptionalSettersItems) -> Result { if let Some(closure) = &self.member.config.with { - return self.setters_for_optional_member_with_closure(closure, items); + return self.setters_for_optional_member_having_with(closure, items); } let underlying_ty = self.member.underlying_norm_ty(); - let underlying_ty = if self.member.config.into.is_present() { - quote!(impl Into<#underlying_ty>) + let underlying_ty: syn::Type = if self.member.config.into.is_present() { + syn::parse_quote!(impl Into<#underlying_ty>) } else { - quote!(#underlying_ty) + underlying_ty.clone() }; let some_fn = Setter { item: items.some_fn, imp: SetterImpl { - input: quote!(value: #underlying_ty), + inputs: vec![(pat_ident("value"), underlying_ty.clone())], body: SetterBody::Forward { body: { let option_fn_name = &items.option_fn.name; @@ -78,7 +81,10 @@ impl<'a> SettersCtx<'a> { let option_fn = Setter { item: items.option_fn, imp: SetterImpl { - input: quote!(value: Option<#underlying_ty>), + inputs: vec![( + pat_ident("value"), + syn::parse_quote!(Option<#underlying_ty>), + )], body: SetterBody::SetMember { expr: if self.member.config.into.is_present() { quote! { @@ -91,20 +97,22 @@ impl<'a> SettersCtx<'a> { }, }; - [self.setter_method(some_fn), self.setter_method(option_fn)].concat() + Ok([self.setter_method(some_fn), self.setter_method(option_fn)].concat()) } - fn setters_for_optional_member_with_closure( + fn setters_for_optional_member_having_with( &self, - closure: &SetterClosure, + with: &WithConfig, items: OptionalSettersItems, - ) -> TokenStream { - let idents = closure.inputs.iter().map(|input| &input.pat.ident); + ) -> Result { + let inputs = self.underlying_inputs_from_with(with)?; + + let idents = inputs.iter().map(|(pat, _)| &pat.ident); // If the closure accepts just a single input avoid wrapping it // in a tuple in the `option_fn` setter. let tuple_if_many = |val: TokenStream| -> TokenStream { - if closure.inputs.len() == 1 { + if inputs.len() == 1 { val } else { quote!((#val)) @@ -116,7 +124,7 @@ impl<'a> SettersCtx<'a> { let some_fn = Setter { item: items.some_fn, imp: SetterImpl { - input: Self::underlying_input_from_closure(closure), + inputs: self.underlying_inputs_from_with(with)?, body: SetterBody::Forward { body: { let option_fn_name = &items.option_fn.name; @@ -129,14 +137,15 @@ impl<'a> SettersCtx<'a> { }; let option_fn_impl = SetterImpl { - input: { - let input_types = closure.inputs.iter().map(|input| &input.ty); + inputs: { + let input_types = inputs.iter().map(|(_, ty)| ty); let input_types = tuple_if_many(quote!(#( #input_types, )*)); - quote!(value: Option<#input_types>) + + vec![(pat_ident("value"), syn::parse_quote!(Option<#input_types>))] }, body: SetterBody::SetMember { expr: { - let expr = self.member_expr_from_closure(closure); + let expr = self.member_expr_from_with(with); quote! { // Not using `Option::map` here because the `#expr` // can contain a `?` operator for a fallible operation. @@ -154,7 +163,7 @@ impl<'a> SettersCtx<'a> { imp: option_fn_impl, }; - [self.setter_method(some_fn), self.setter_method(option_fn)].concat() + Ok([self.setter_method(some_fn), self.setter_method(option_fn)].concat()) } /// This method is reused between the setter for the required member and @@ -166,39 +175,144 @@ impl<'a> SettersCtx<'a> { /// of the builder compatible when a required member becomes optional. /// To be able to explicitly pass an `Option` value to the setter method /// users need to use the `maybe_{member_ident}` method. - fn underlying_input_from_closure(closure: &SetterClosure) -> TokenStream { - let pats = closure.inputs.iter().map(|input| &input.pat); - let types = closure.inputs.iter().map(|input| &input.ty); - quote! { - #( #pats: #types ),* - } - } + fn underlying_inputs_from_with( + &self, + with: &WithConfig, + ) -> Result> { + let inputs = match with { + WithConfig::Closure(closure) => closure + .inputs + .iter() + .map(|input| (input.pat.clone(), (*input.ty).clone())) + .collect(), + WithConfig::Some(some) => { + let input_ty = self + .member + .underlying_norm_ty() + .option_type_param() + .ok_or_else(|| { + if self.member.ty.norm.is_option() { + err!( + some, + "the underlying type of this member is not `Option`; \ + by default, members of type `Option` are optional and their \ + 'underlying type' is the type under the `Option`; \ + you might be missing #[builder(transparent)]` annotation \ + for this member" + ) + } else { + err!( + &self.member.underlying_norm_ty(), + "`with = Some` only works for members with the underlying \ + type of `Option`;" + ) + } + })?; - fn member_expr_from_closure(&self, closure: &SetterClosure) -> TokenStream { - let body = &closure.body; + vec![(pat_ident("value"), input_ty.clone())] + } + WithConfig::FromIter(from_iter) => { + let collection_ty = self.member.underlying_norm_ty(); + + let well_known_single_arg_suffixes = ["Vec", "Set", "Deque", "Heap"]; + + let err = || { + err!( + collection_ty, + "the underlying type of this member is not a known collection type; \ + only a collection type that matches the following patterns will be \ + accepted by `#[builder(with = {})], where * at the beginning means \ + the collection type may start with any prefix:\n\ + - *Map\n\ + {}", + quote!(#from_iter), + well_known_single_arg_suffixes + .iter() + .map(|suffix| { format!("- *{suffix}") }) + .join("\n") + ) + }; - let ty = self.member.underlying_norm_ty().to_token_stream(); + let path = collection_ty.as_path_no_qself().ok_or_else(err)?; - let output = Self::maybe_wrap_in_result(closure, ty); + let last_segment = path.segments.last().ok_or_else(err)?; + let args = match &last_segment.arguments { + syn::PathArguments::AngleBracketed(args) => &args.args, + _ => return Err(err()), + }; - // Avoid wrapping the body in a block if it's already a block. - let body = if matches!(body.as_ref(), syn::Expr::Block(_)) { - body.to_token_stream() - } else { - quote!({ #body }) + let last_segment_ident_str = last_segment.ident.to_string(); + + let item_ty = + if well_known_single_arg_suffixes.contains(&last_segment_ident_str.as_str()) { + // We don't compare for `len == 1` because there may be an optional last + // type argument for the allocator + if args.len() < 1 { + return Err(err()); + } + + let arg = args.first().ok_or_else(err)?; + + syn::parse_quote!(#arg) + } else if last_segment_ident_str == "Map" { + // We don't compare for `len == 2` because there may be an optional last + // type argument for the allocator + if args.len() < 2 { + return Err(err()); + } + + let mut args = args.iter(); + let key = args.next().ok_or_else(err)?; + let value = args.next().ok_or_else(err)?; + + syn::parse_quote!((#key, #value)) + } else { + return Err(err()); + }; + + vec![(pat_ident("iter"), item_ty)] + } }; - let question_mark = closure - .output - .is_some() - .then(|| syn::Token![?](Span::call_site())); + Ok(inputs) + } - quote! { - (move || -> #output #body)() #question_mark + fn member_expr_from_with(&self, with: &WithConfig) -> TokenStream { + match with { + WithConfig::Closure(closure) => { + let body = &closure.body; + + let ty = self.member.underlying_norm_ty().to_token_stream(); + + let output = Self::maybe_wrap_in_result(with, ty); + + // Avoid wrapping the body in a block if it's already a block. + let body = if matches!(body.as_ref(), syn::Expr::Block(_)) { + body.to_token_stream() + } else { + quote!({ #body }) + }; + + let question_mark = closure + .output + .is_some() + .then(|| syn::Token![?](Span::call_site())); + + quote! { + (move || -> #output #body)() #question_mark + } + } + WithConfig::Some(some) => quote!(#some(value)), + WithConfig::FromIter(from_iter) => quote!(#from_iter(iter)), } } - fn maybe_wrap_in_result(closure: &SetterClosure, ty: TokenStream) -> TokenStream { + fn maybe_wrap_in_result(with: &WithConfig, ty: TokenStream) -> TokenStream { + let closure = match with { + WithConfig::Closure(closure) => closure, + _ => return ty, + }; + let output = match closure.output.as_ref() { Some(output) => output, None => return ty, @@ -260,7 +374,7 @@ impl<'a> SettersCtx<'a> { .config .with .as_ref() - .and_then(|closure| closure.output.as_ref()); + .and_then(|with| with.as_closure()?.output.as_ref()); if let Some(result_output) = result_output { let result_path = &result_output.result_path; @@ -290,8 +404,8 @@ impl<'a> SettersCtx<'a> { } }; - if let Some(closure) = &self.member.config.with { - return_type = Self::maybe_wrap_in_result(closure, return_type); + if let Some(with) = &self.member.config.with { + return_type = Self::maybe_wrap_in_result(with, return_type); } let where_clause = (!self.member.config.overwritable.is_present()).then(|| { @@ -303,7 +417,8 @@ impl<'a> SettersCtx<'a> { }); let SetterItem { name, vis, docs } = item; - let input = imp.input; + let pats = imp.inputs.iter().map(|(pat, _)| pat); + let types = imp.inputs.iter().map(|(_, ty)| ty); quote! { #( #docs )* @@ -318,7 +433,7 @@ impl<'a> SettersCtx<'a> { clippy::missing_const_for_fn, )] #[inline(always)] - #vis fn #name(#maybe_mut self, #input) -> #return_type + #vis fn #name(#maybe_mut self, #( #pats: #types ),*) -> #return_type #where_clause { #body @@ -333,7 +448,7 @@ struct Setter { } struct SetterImpl { - input: TokenStream, + inputs: Vec<(syn::PatIdent, syn::Type)>, body: SetterBody, } @@ -532,3 +647,14 @@ fn well_known_default(ty: &syn::Type) -> Option { Some(value) } + +/// Unfortunately there is no `syn::Parse` impl on `PatIdent` directly, +/// so we use this workaround instead +fn pat_ident(ident_name: &'static str) -> syn::PatIdent { + let ident = syn::Ident::new(ident_name, Span::call_site()); + let pat: syn::Pat = syn::parse_quote!(#ident); + match pat { + syn::Pat::Ident(pat_ident) => pat_ident, + _ => unreachable!("can't parse something else than PatIdent here: {pat:?}"), + } +} diff --git a/bon/tests/integration/builder/attr_with/mod.rs b/bon/tests/integration/builder/attr_with/mod.rs index c28189c6..de3542c6 100644 --- a/bon/tests/integration/builder/attr_with/mod.rs +++ b/bon/tests/integration/builder/attr_with/mod.rs @@ -1,6 +1,7 @@ mod multi_arg; mod overwritable; mod single_arg; +mod some; struct IntoStrRef<'a>(&'a str); diff --git a/bon/tests/integration/builder/attr_with/some.rs b/bon/tests/integration/builder/attr_with/some.rs new file mode 100644 index 00000000..ba1d14d1 --- /dev/null +++ b/bon/tests/integration/builder/attr_with/some.rs @@ -0,0 +1,148 @@ +use crate::prelude::*; +use core::fmt; + +#[test] +fn test_struct() { + #[derive(Debug, Builder)] + #[builder(derive(Clone))] + struct Sut { + #[builder(transparent, with = Some)] + _required: Option, + + #[builder(transparent, with = Some, default = Some(()))] + _optional: Option<()>, + + #[builder(transparent, with = Some)] + _generic: Option, + + #[builder(transparent, with = Some, default = None)] + _optional_generic: Option, + } + + let builder = Sut::builder(); + + let builder = builder.required(99); + + let _ignore = builder.clone().optional(()); + let builder = builder.maybe_optional(None); + + let builder = builder.generic(2); + + let _ignore = builder.clone().optional_generic(21); + let builder = builder.maybe_optional_generic(Some(22)); + + let sut = builder.build(); + + assert_debug_eq(&sut, expect![[r#" + Sut { + _required: Some( + 99, + ), + _optional: Some( + (), + ), + _generic: Some( + 2, + ), + _optional_generic: Some( + 22, + ), + }"#]]); +} + +#[test] +fn test_free_fn() { + #[builder(derive(Clone))] + fn sut( + #[builder(transparent, with = Some)] required: Option, + #[builder(transparent, with = Some, default = Some(()))] optional: Option<()>, + #[builder(transparent, with = Some)] generic: Option, + #[builder(transparent, with = Some)] impl_trait: Option, + #[builder(transparent, with = Some, default = None)] optional_generic: Option, + ) -> impl fmt::Debug { + (required, optional, generic, impl_trait, optional_generic) + } + + let builder = sut(); + + let builder = builder.required(99); + + let _ignore = builder.clone().optional(()); + let builder = builder.maybe_optional(None); + + let builder = builder.generic(2); + let builder = builder.impl_trait("impl Trait"); + + let _ignore = builder.clone().optional_generic(21); + let builder = builder.maybe_optional_generic(Some(22)); + + let sut = builder.call(); + + assert_debug_eq(&sut, expect![[r#"(Some(99), Some(()), Some(2), Some("impl Trait"), Some(22))"#]]); +} + +#[test] +fn test_assoc_method() { + struct Sut; + + #[bon] + impl Sut { + #[builder(derive(Clone))] + fn sut( + #[builder(transparent, with = Some)] required: Option, + #[builder(transparent, with = Some, default = Some(()))] optional: Option<()>, + #[builder(transparent, with = Some)] generic: Option, + #[builder(transparent, with = Some)] impl_trait: Option, + #[builder(transparent, with = Some, default = None)] optional_generic: Option, + ) -> impl fmt::Debug { + (required, optional, generic, impl_trait, optional_generic) + } + + #[builder(derive(Clone))] + fn with_self( + &self, + #[builder(transparent, with = Some)] required: Option, + #[builder(transparent, with = Some, default = Some(()))] optional: Option<()>, + #[builder(transparent, with = Some)] generic: Option, + #[builder(transparent, with = Some)] impl_trait: Option, + #[builder(transparent, with = Some, default = None)] optional_generic: Option, + ) -> impl fmt::Debug { + let _ = self; + (required, optional, generic, impl_trait, optional_generic) + } + } + + let builder = Sut::sut(); + + let builder = builder.required(99); + + let _ignore = builder.clone().optional(()); + let builder = builder.maybe_optional(None); + + let builder = builder.generic(2); + let builder = builder.impl_trait("impl Trait"); + + let _ignore = builder.clone().optional_generic(21); + let builder = builder.maybe_optional_generic(Some(22)); + + let sut = builder.call(); + + assert_debug_eq(&sut, expect![[r#"(Some(99), Some(()), Some(2), Some("impl Trait"), Some(22))"#]]); + + let builder = Sut.with_self(); + + let builder = builder.required(99); + + let _ignore = builder.clone().optional(()); + let builder = builder.maybe_optional(None); + + let builder = builder.generic(2); + let builder = builder.impl_trait("impl Trait"); + + let _ignore = builder.clone().optional_generic(21); + let builder = builder.maybe_optional_generic(Some(22)); + + let sut = builder.call(); + + assert_debug_eq(&sut, expect![[r#"(Some(99), Some(()), Some(2), Some("impl Trait"), Some(22))"#]]); +} diff --git a/bon/tests/integration/ui/compile_fail/attr_transparent.rs b/bon/tests/integration/ui/compile_fail/attr_transparent.rs index 0fbc4236..733ab844 100644 --- a/bon/tests/integration/ui/compile_fail/attr_transparent.rs +++ b/bon/tests/integration/ui/compile_fail/attr_transparent.rs @@ -28,11 +28,15 @@ struct InvalidOnSkippedMember { struct Valid { #[builder(transparent)] member: Option, + + #[builder(transparent, with = Some)] + some_member: Option<()>, } fn main() { // Make sure there is no `maybe_` setter generated let _ = Valid::builder().maybe_member(Some(42)); + let _ = Valid::builder().maybe_some_member(Some(())); // Another way to get transparency { diff --git a/bon/tests/integration/ui/compile_fail/attr_transparent.stderr b/bon/tests/integration/ui/compile_fail/attr_transparent.stderr index 75366b28..44a38390 100644 --- a/bon/tests/integration/ui/compile_fail/attr_transparent.stderr +++ b/bon/tests/integration/ui/compile_fail/attr_transparent.stderr @@ -23,37 +23,51 @@ error: `skip` attribute can't be specified together with `transparent` | ^^^^ error[E0599]: no method named `maybe_member` found for struct `ValidBuilder` in the current scope - --> tests/integration/ui/compile_fail/attr_transparent.rs:35:30 + --> tests/integration/ui/compile_fail/attr_transparent.rs:38:30 | 27 | #[derive(Builder)] | ------- method `maybe_member` not found for this struct ... -35 | let _ = Valid::builder().maybe_member(Some(42)); +38 | let _ = Valid::builder().maybe_member(Some(42)); | ^^^^^^^^^^^^ | help: there is a method `member` with a similar name | -35 | let _ = Valid::builder().member(Some(42)); +38 | let _ = Valid::builder().member(Some(42)); | ~~~~~~ +error[E0599]: no method named `maybe_some_member` found for struct `ValidBuilder` in the current scope + --> tests/integration/ui/compile_fail/attr_transparent.rs:39:30 + | +27 | #[derive(Builder)] + | ------- method `maybe_some_member` not found for this struct +... +39 | let _ = Valid::builder().maybe_some_member(Some(())); + | ^^^^^^^^^^^^^^^^^ + | +help: there is a method `some_member` with a similar name + | +39 | let _ = Valid::builder().some_member(Some(())); + | ~~~~~~~~~~~ + error[E0277]: the member `Unset` was not set, but this method requires it to be set - --> tests/integration/ui/compile_fail/attr_transparent.rs:47:32 + --> tests/integration/ui/compile_fail/attr_transparent.rs:51:32 | -47 | let _ = Sut::builder().build(); +51 | let _ = Sut::builder().build(); | ^^^^^ the member `Unset` was not set, but this method requires it to be set | = help: the trait `IsSet` is not implemented for `Unset`, which is required by `sut_builder::Empty: sut_builder::IsComplete` = help: the trait `IsSet` is implemented for `Set` note: required for `sut_builder::Empty` to implement `sut_builder::IsComplete` - --> tests/integration/ui/compile_fail/attr_transparent.rs:41:18 + --> tests/integration/ui/compile_fail/attr_transparent.rs:45:18 | -41 | #[derive(Builder)] +45 | #[derive(Builder)] | ^^^^^^^ unsatisfied trait bound introduced in this `derive` macro note: required by a bound in `SutBuilder::::build` - --> tests/integration/ui/compile_fail/attr_transparent.rs:41:18 + --> tests/integration/ui/compile_fail/attr_transparent.rs:45:18 | -41 | #[derive(Builder)] +45 | #[derive(Builder)] | ^^^^^^^ required by this bound in `SutBuilder::::build` -42 | struct Sut { +46 | struct Sut { | --- required by a bound in this associated function = note: this error originates in the derive macro `Builder` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/bon/tests/integration/ui/compile_fail/attr_with.stderr b/bon/tests/integration/ui/compile_fail/attr_with.stderr index f9954c81..dc202fe1 100644 --- a/bon/tests/integration/ui/compile_fail/attr_with.stderr +++ b/bon/tests/integration/ui/compile_fail/attr_with.stderr @@ -1,4 +1,7 @@ -error: expected a closure e.g. `with = |param: T| expression` +error: expected a closure e.g. `#[builder(with = |param: T| expression)]` or a well-known function path which could be one of: + - #[builder(with = Some)] + - #[builder(with = FromIterator::from_iter)] + - #[builder(with = <_>::from_iter)] (same as above, but shorter) --> tests/integration/ui/compile_fail/attr_with.rs:5:15 | 5 | #[builder(with = 42)] From 7240e657f67da1b78a8e5d54c0a4b753f1b72d55 Mon Sep 17 00:00:00 2001 From: Veetaha Date: Fri, 25 Oct 2024 22:59:50 +0000 Subject: [PATCH 03/10] Tests and fixes for `#[builder(with = FromIterator/<_>::from_iter)]` --- .../src/builder/builder_gen/setters/mod.rs | 50 ++-- .../builder/attr_with/from_iter.rs | 248 ++++++++++++++++++ .../integration/builder/attr_with/mod.rs | 1 + 3 files changed, 275 insertions(+), 24 deletions(-) create mode 100644 bon/tests/integration/builder/attr_with/from_iter.rs diff --git a/bon-macros/src/builder/builder_gen/setters/mod.rs b/bon-macros/src/builder/builder_gen/setters/mod.rs index 991dd5a4..d8384b3c 100644 --- a/bon-macros/src/builder/builder_gen/setters/mod.rs +++ b/bon-macros/src/builder/builder_gen/setters/mod.rs @@ -214,7 +214,7 @@ impl<'a> SettersCtx<'a> { WithConfig::FromIter(from_iter) => { let collection_ty = self.member.underlying_norm_ty(); - let well_known_single_arg_suffixes = ["Vec", "Set", "Deque", "Heap"]; + let well_known_single_arg_suffixes = ["Vec", "Set", "Deque", "Heap", "List"]; let err = || { err!( @@ -243,34 +243,36 @@ impl<'a> SettersCtx<'a> { let last_segment_ident_str = last_segment.ident.to_string(); - let item_ty = - if well_known_single_arg_suffixes.contains(&last_segment_ident_str.as_str()) { - // We don't compare for `len == 1` because there may be an optional last - // type argument for the allocator - if args.len() < 1 { - return Err(err()); - } + let item_ty = if well_known_single_arg_suffixes + .iter() + .any(|suffix| last_segment_ident_str.ends_with(suffix)) + { + // We don't compare for `len == 1` because there may be an optional last + // type argument for the allocator + if args.is_empty() { + return Err(err()); + } - let arg = args.first().ok_or_else(err)?; + let arg = args.first().ok_or_else(err)?; - syn::parse_quote!(#arg) - } else if last_segment_ident_str == "Map" { - // We don't compare for `len == 2` because there may be an optional last - // type argument for the allocator - if args.len() < 2 { - return Err(err()); - } + quote!(#arg) + } else if last_segment_ident_str.ends_with("Map") { + // We don't compare for `len == 2` because there may be an optional last + // type argument for the allocator + if args.len() < 2 { + return Err(err()); + } - let mut args = args.iter(); - let key = args.next().ok_or_else(err)?; - let value = args.next().ok_or_else(err)?; + let mut args = args.iter(); + let key = args.next().ok_or_else(err)?; + let value = args.next().ok_or_else(err)?; - syn::parse_quote!((#key, #value)) - } else { - return Err(err()); - }; + quote!((#key, #value)) + } else { + return Err(err()); + }; - vec![(pat_ident("iter"), item_ty)] + vec![(pat_ident("iter"), syn::parse_quote!(impl IntoIterator))] } }; diff --git a/bon/tests/integration/builder/attr_with/from_iter.rs b/bon/tests/integration/builder/attr_with/from_iter.rs new file mode 100644 index 00000000..81c92261 --- /dev/null +++ b/bon/tests/integration/builder/attr_with/from_iter.rs @@ -0,0 +1,248 @@ +#![cfg(feature = "alloc")] +#![allow(clippy::linkedlist)] + +use crate::prelude::*; +use core::fmt; +use std::collections::{BinaryHeap, HashMap, HashSet, VecDeque}; + +type RenamedVec = Vec; + +#[test] +fn test_struct() { + #[derive(Debug, Builder)] + #[builder(derive(Clone))] + struct Sut { + #[builder(with = <_>::from_iter)] + _vec: Vec, + + #[builder(with = FromIterator::from_iter)] + _optional_vec: Option<::alloc::vec::Vec>, + + #[builder(with = <_>::from_iter, default)] + _default_vec: Vec, + + #[builder(with = FromIterator::from_iter)] + _renamed_vec: RenamedVec, + + #[builder(with = <_>::from_iter)] + _hash_set: HashSet, + + #[builder(with = FromIterator::from_iter)] + _btree_set: BTreeSet, + + #[builder(with = <_>::from_iter)] + _vec_deque: VecDeque, + + #[builder(with = FromIterator::from_iter)] + _binary_heap: BinaryHeap, + + #[builder(with = <_>::from_iter)] + _linked_list: std::collections::LinkedList, + + #[builder(with = FromIterator::from_iter)] + _hash_map: HashMap, + + #[builder(with = <_>::from_iter)] + _btree_map: std::collections::BTreeMap, + } + + let builder = Sut::builder(); + + let _ignore = builder.clone().optional_vec([1, 2, 3]); + let builder = builder.maybe_optional_vec(Some([4, 5, 6])); + + let _ignore = builder.clone().default_vec([7, 8, 9]); + let builder = builder.maybe_default_vec(Some([10, 11, 12])); + + // `Hash*`` collections have random order of iteration, so their debug + // output is unstable. To work around this instability, we just specify + // a single element for `Hash*` collections. + let sut = builder + .vec([13, 14, 15]) + .renamed_vec([16, 17, 18]) + .hash_set(std::iter::once(19)) + .btree_set([20, 21, 22, 22, 21]) + .vec_deque([23, 24, 25]) + .binary_heap([26, 27, 28]) + .linked_list([29, 30, 31]) + .hash_map(std::iter::once((32, 33))) + .btree_map([(34, 35), (34, 36), (37, 38)]) + .build(); + + assert_debug_eq(&sut, expect![]); +} + +#[test] +fn test_free_fn() { + #[builder(derive(Clone))] + fn sut( + #[builder(with = <_>::from_iter)] vec: Vec, + #[builder(with = FromIterator::from_iter)] optional_vec: Option<::alloc::vec::Vec>, + #[builder(with = <_>::from_iter, default)] default_vec: Vec, + #[builder(with = FromIterator::from_iter)] renamed_vec: RenamedVec, + #[builder(with = <_>::from_iter)] hash_set: HashSet, + #[builder(with = FromIterator::from_iter)] btree_set: BTreeSet, + #[builder(with = <_>::from_iter)] vec_deque: VecDeque, + #[builder(with = FromIterator::from_iter)] binary_heap: BinaryHeap, + #[builder(with = <_>::from_iter)] linked_list: std::collections::LinkedList, + #[builder(with = FromIterator::from_iter)] hash_map: HashMap, + #[builder(with = <_>::from_iter)] btree_map: std::collections::BTreeMap, + ) -> impl fmt::Debug { + ( + vec, + optional_vec, + default_vec, + renamed_vec, + hash_set, + btree_set, + vec_deque, + binary_heap, + linked_list, + hash_map, + btree_map, + ) + } + + let builder = sut(); + + let _ignore = builder.clone().optional_vec([1, 2, 3]); + let builder = builder.maybe_optional_vec(Some([4, 5, 6])); + + let _ignore = builder.clone().default_vec([7, 8, 9]); + let builder = builder.maybe_default_vec(Some([10, 11, 12])); + + // `Hash*`` collections have random order of iteration, so their debug + // output is unstable. To work around this instability, we just specify + // a single element for `Hash*` collections. + let sut = builder + .vec([13, 14, 15]) + .renamed_vec([16, 17, 18]) + .hash_set(std::iter::once(19)) + .btree_set([20, 21, 22, 22, 21]) + .vec_deque([23, 24, 25]) + .binary_heap([26, 27, 28]) + .linked_list([29, 30, 31]) + .hash_map(std::iter::once((32, 33))) + .btree_map([(34, 35), (34, 36), (37, 38)]) + .call(); + + assert_debug_eq(&sut, expect![]); +} + +#[test] +fn test_assoc_method() { + struct Sut; + + #[bon] + impl Sut { + #[builder(derive(Clone))] + fn sut( + #[builder(with = <_>::from_iter)] vec: Vec, + #[builder(with = FromIterator::from_iter)] optional_vec: Option<::alloc::vec::Vec>, + #[builder(with = <_>::from_iter, default)] default_vec: Vec, + #[builder(with = FromIterator::from_iter)] renamed_vec: RenamedVec, + #[builder(with = <_>::from_iter)] hash_set: HashSet, + #[builder(with = FromIterator::from_iter)] btree_set: BTreeSet, + #[builder(with = <_>::from_iter)] vec_deque: VecDeque, + #[builder(with = FromIterator::from_iter)] binary_heap: BinaryHeap, + #[builder(with = <_>::from_iter)] linked_list: std::collections::LinkedList, + #[builder(with = FromIterator::from_iter)] hash_map: HashMap, + #[builder(with = <_>::from_iter)] btree_map: std::collections::BTreeMap, + ) -> impl fmt::Debug { + ( + vec, + optional_vec, + default_vec, + renamed_vec, + hash_set, + btree_set, + vec_deque, + binary_heap, + linked_list, + hash_map, + btree_map, + ) + } + + #[builder(derive(Clone))] + fn with_self( + &self, + #[builder(with = <_>::from_iter)] vec: Vec, + #[builder(with = FromIterator::from_iter)] optional_vec: Option<::alloc::vec::Vec>, + #[builder(with = <_>::from_iter, default)] default_vec: Vec, + #[builder(with = FromIterator::from_iter)] renamed_vec: RenamedVec, + #[builder(with = <_>::from_iter)] hash_set: HashSet, + #[builder(with = FromIterator::from_iter)] btree_set: BTreeSet, + #[builder(with = <_>::from_iter)] vec_deque: VecDeque, + #[builder(with = FromIterator::from_iter)] binary_heap: BinaryHeap, + #[builder(with = <_>::from_iter)] linked_list: std::collections::LinkedList, + #[builder(with = FromIterator::from_iter)] hash_map: HashMap, + #[builder(with = <_>::from_iter)] btree_map: std::collections::BTreeMap, + ) -> impl fmt::Debug { + let _ = self; + ( + vec, + optional_vec, + default_vec, + renamed_vec, + hash_set, + btree_set, + vec_deque, + binary_heap, + linked_list, + hash_map, + btree_map, + ) + } + } + + let builder = Sut::sut(); + + let _ignore = builder.clone().optional_vec([1, 2, 3]); + let builder = builder.maybe_optional_vec(Some([4, 5, 6])); + + let _ignore = builder.clone().default_vec([7, 8, 9]); + let builder = builder.maybe_default_vec(Some([10, 11, 12])); + + // `Hash*`` collections have random order of iteration, so their debug + // output is unstable. To work around this instability, we just specify + // a single element for `Hash*` collections. + let sut = builder + .vec([13, 14, 15]) + .renamed_vec([16, 17, 18]) + .hash_set(std::iter::once(19)) + .btree_set([20, 21, 22, 22, 21]) + .vec_deque([23, 24, 25]) + .binary_heap([26, 27, 28]) + .linked_list([29, 30, 31]) + .hash_map(std::iter::once((32, 33))) + .btree_map([(34, 35), (34, 36), (37, 38)]) + .call(); + + assert_debug_eq(&sut, expect![]); + + let builder = Sut.with_self(); + + let _ignore = builder.clone().optional_vec([1, 2, 3]); + let builder = builder.maybe_optional_vec(Some([4, 5, 6])); + + let _ignore = builder.clone().default_vec([7, 8, 9]); + let builder = builder.maybe_default_vec(Some([10, 11, 12])); + + // `Hash*`` collections have random order of iteration, so their debug + // output is unstable. To work around this instability, we just specify + // a single element for `Hash*` collections. + let sut = builder + .vec([13, 14, 15]) + .renamed_vec([16, 17, 18]) + .hash_set(std::iter::once(19)) + .btree_set([20, 21, 22, 22, 21]) + .vec_deque([23, 24, 25]) + .binary_heap([26, 27, 28]) + .linked_list([29, 30, 31]) + .hash_map(std::iter::once((32, 33))) + .btree_map([(34, 35), (34, 36), (37, 38)]) + .call(); + + assert_debug_eq(&sut, expect![]); +} diff --git a/bon/tests/integration/builder/attr_with/mod.rs b/bon/tests/integration/builder/attr_with/mod.rs index de3542c6..b0c39087 100644 --- a/bon/tests/integration/builder/attr_with/mod.rs +++ b/bon/tests/integration/builder/attr_with/mod.rs @@ -2,6 +2,7 @@ mod multi_arg; mod overwritable; mod single_arg; mod some; +mod from_iter; struct IntoStrRef<'a>(&'a str); From c54ff9419676583c8bdc6d08d695ec4dc2797544 Mon Sep 17 00:00:00 2001 From: Veetaha Date: Fri, 25 Oct 2024 23:04:45 +0000 Subject: [PATCH 04/10] Fix tests --- .../builder/builder_gen/member/config/mod.rs | 4 +- .../src/builder/builder_gen/setters/mod.rs | 5 +- .../builder/attr_with/from_iter.rs | 224 +++++++++++++++++- .../integration/builder/attr_with/mod.rs | 2 +- .../integration/builder/attr_with/some.rs | 22 +- 5 files changed, 244 insertions(+), 13 deletions(-) diff --git a/bon-macros/src/builder/builder_gen/member/config/mod.rs b/bon-macros/src/builder/builder_gen/member/config/mod.rs index 10bf68f7..f212ce85 100644 --- a/bon-macros/src/builder/builder_gen/member/config/mod.rs +++ b/bon-macros/src/builder/builder_gen/member/config/mod.rs @@ -1,10 +1,10 @@ mod blanket; -mod with; mod setters; +mod with; pub(crate) use blanket::*; -pub(crate) use with::*; pub(crate) use setters::*; +pub(crate) use with::*; use super::MemberOrigin; use crate::parsing::SpannedKey; diff --git a/bon-macros/src/builder/builder_gen/setters/mod.rs b/bon-macros/src/builder/builder_gen/setters/mod.rs index d8384b3c..1f9ccd81 100644 --- a/bon-macros/src/builder/builder_gen/setters/mod.rs +++ b/bon-macros/src/builder/builder_gen/setters/mod.rs @@ -272,7 +272,10 @@ impl<'a> SettersCtx<'a> { return Err(err()); }; - vec![(pat_ident("iter"), syn::parse_quote!(impl IntoIterator))] + vec![( + pat_ident("iter"), + syn::parse_quote!(impl IntoIterator), + )] } }; diff --git a/bon/tests/integration/builder/attr_with/from_iter.rs b/bon/tests/integration/builder/attr_with/from_iter.rs index 81c92261..41e910b5 100644 --- a/bon/tests/integration/builder/attr_with/from_iter.rs +++ b/bon/tests/integration/builder/attr_with/from_iter.rs @@ -69,7 +69,61 @@ fn test_struct() { .btree_map([(34, 35), (34, 36), (37, 38)]) .build(); - assert_debug_eq(&sut, expect![]); + assert_debug_eq(&sut, expect![[r#" + Sut { + _vec: [ + 13, + 14, + 15, + ], + _optional_vec: Some( + [ + 4, + 5, + 6, + ], + ), + _default_vec: [ + 10, + 11, + 12, + ], + _renamed_vec: [ + 16, + 17, + 18, + ], + _hash_set: { + 19, + }, + _btree_set: { + 20, + 21, + 22, + }, + _vec_deque: [ + 23, + 24, + 25, + ], + _binary_heap: [ + 28, + 27, + 26, + ], + _linked_list: [ + 29, + 30, + 31, + ], + _hash_map: { + 32: 33, + }, + _btree_map: { + 34: 36, + 37: 38, + }, + }"#]]); } #[test] @@ -126,7 +180,61 @@ fn test_free_fn() { .btree_map([(34, 35), (34, 36), (37, 38)]) .call(); - assert_debug_eq(&sut, expect![]); + assert_debug_eq(&sut, expect![[r#" + ( + [ + 13, + 14, + 15, + ], + Some( + [ + 4, + 5, + 6, + ], + ), + [ + 10, + 11, + 12, + ], + [ + 16, + 17, + 18, + ], + { + 19, + }, + { + 20, + 21, + 22, + }, + [ + 23, + 24, + 25, + ], + [ + 28, + 27, + 26, + ], + [ + 29, + 30, + 31, + ], + { + 32: 33, + }, + { + 34: 36, + 37: 38, + }, + )"#]]); } #[test] @@ -219,7 +327,61 @@ fn test_assoc_method() { .btree_map([(34, 35), (34, 36), (37, 38)]) .call(); - assert_debug_eq(&sut, expect![]); + assert_debug_eq(&sut, expect![[r#" + ( + [ + 13, + 14, + 15, + ], + Some( + [ + 4, + 5, + 6, + ], + ), + [ + 10, + 11, + 12, + ], + [ + 16, + 17, + 18, + ], + { + 19, + }, + { + 20, + 21, + 22, + }, + [ + 23, + 24, + 25, + ], + [ + 28, + 27, + 26, + ], + [ + 29, + 30, + 31, + ], + { + 32: 33, + }, + { + 34: 36, + 37: 38, + }, + )"#]]); let builder = Sut.with_self(); @@ -244,5 +406,59 @@ fn test_assoc_method() { .btree_map([(34, 35), (34, 36), (37, 38)]) .call(); - assert_debug_eq(&sut, expect![]); + assert_debug_eq(&sut, expect![[r#" + ( + [ + 13, + 14, + 15, + ], + Some( + [ + 4, + 5, + 6, + ], + ), + [ + 10, + 11, + 12, + ], + [ + 16, + 17, + 18, + ], + { + 19, + }, + { + 20, + 21, + 22, + }, + [ + 23, + 24, + 25, + ], + [ + 28, + 27, + 26, + ], + [ + 29, + 30, + 31, + ], + { + 32: 33, + }, + { + 34: 36, + 37: 38, + }, + )"#]]); } diff --git a/bon/tests/integration/builder/attr_with/mod.rs b/bon/tests/integration/builder/attr_with/mod.rs index b0c39087..ac7eb0bf 100644 --- a/bon/tests/integration/builder/attr_with/mod.rs +++ b/bon/tests/integration/builder/attr_with/mod.rs @@ -1,8 +1,8 @@ +mod from_iter; mod multi_arg; mod overwritable; mod single_arg; mod some; -mod from_iter; struct IntoStrRef<'a>(&'a str); diff --git a/bon/tests/integration/builder/attr_with/some.rs b/bon/tests/integration/builder/attr_with/some.rs index ba1d14d1..7b36755b 100644 --- a/bon/tests/integration/builder/attr_with/some.rs +++ b/bon/tests/integration/builder/attr_with/some.rs @@ -33,7 +33,9 @@ fn test_struct() { let sut = builder.build(); - assert_debug_eq(&sut, expect![[r#" + assert_debug_eq( + &sut, + expect![[r#" Sut { _required: Some( 99, @@ -47,7 +49,8 @@ fn test_struct() { _optional_generic: Some( 22, ), - }"#]]); + }"#]], + ); } #[test] @@ -78,7 +81,10 @@ fn test_free_fn() { let sut = builder.call(); - assert_debug_eq(&sut, expect![[r#"(Some(99), Some(()), Some(2), Some("impl Trait"), Some(22))"#]]); + assert_debug_eq( + &sut, + expect![[r#"(Some(99), Some(()), Some(2), Some("impl Trait"), Some(22))"#]], + ); } #[test] @@ -127,7 +133,10 @@ fn test_assoc_method() { let sut = builder.call(); - assert_debug_eq(&sut, expect![[r#"(Some(99), Some(()), Some(2), Some("impl Trait"), Some(22))"#]]); + assert_debug_eq( + &sut, + expect![[r#"(Some(99), Some(()), Some(2), Some("impl Trait"), Some(22))"#]], + ); let builder = Sut.with_self(); @@ -144,5 +153,8 @@ fn test_assoc_method() { let sut = builder.call(); - assert_debug_eq(&sut, expect![[r#"(Some(99), Some(()), Some(2), Some("impl Trait"), Some(22))"#]]); + assert_debug_eq( + &sut, + expect![[r#"(Some(99), Some(()), Some(2), Some("impl Trait"), Some(22))"#]], + ); } From a97abd4f1da52f354bae0c0e05b331816a8bb314 Mon Sep 17 00:00:00 2001 From: Veetaha Date: Fri, 25 Oct 2024 23:13:01 +0000 Subject: [PATCH 05/10] Use `std` --- bon/tests/integration/builder/attr_with/from_iter.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bon/tests/integration/builder/attr_with/from_iter.rs b/bon/tests/integration/builder/attr_with/from_iter.rs index 41e910b5..9922d267 100644 --- a/bon/tests/integration/builder/attr_with/from_iter.rs +++ b/bon/tests/integration/builder/attr_with/from_iter.rs @@ -1,4 +1,4 @@ -#![cfg(feature = "alloc")] +#![cfg(feature = "std")] #![allow(clippy::linkedlist)] use crate::prelude::*; From 4f60dc8b164be335d517eabeb189b991f4362ca9 Mon Sep 17 00:00:00 2001 From: Veetaha Date: Fri, 25 Oct 2024 23:45:28 +0000 Subject: [PATCH 06/10] cargo fmt --- .../builder/attr_with/from_iter.rs | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/bon/tests/integration/builder/attr_with/from_iter.rs b/bon/tests/integration/builder/attr_with/from_iter.rs index 9922d267..b38a628f 100644 --- a/bon/tests/integration/builder/attr_with/from_iter.rs +++ b/bon/tests/integration/builder/attr_with/from_iter.rs @@ -69,7 +69,9 @@ fn test_struct() { .btree_map([(34, 35), (34, 36), (37, 38)]) .build(); - assert_debug_eq(&sut, expect![[r#" + assert_debug_eq( + &sut, + expect![[r#" Sut { _vec: [ 13, @@ -123,7 +125,8 @@ fn test_struct() { 34: 36, 37: 38, }, - }"#]]); + }"#]], + ); } #[test] @@ -180,7 +183,9 @@ fn test_free_fn() { .btree_map([(34, 35), (34, 36), (37, 38)]) .call(); - assert_debug_eq(&sut, expect![[r#" + assert_debug_eq( + &sut, + expect![[r#" ( [ 13, @@ -234,7 +239,8 @@ fn test_free_fn() { 34: 36, 37: 38, }, - )"#]]); + )"#]], + ); } #[test] @@ -327,7 +333,9 @@ fn test_assoc_method() { .btree_map([(34, 35), (34, 36), (37, 38)]) .call(); - assert_debug_eq(&sut, expect![[r#" + assert_debug_eq( + &sut, + expect![[r#" ( [ 13, @@ -381,7 +389,8 @@ fn test_assoc_method() { 34: 36, 37: 38, }, - )"#]]); + )"#]], + ); let builder = Sut.with_self(); @@ -406,7 +415,9 @@ fn test_assoc_method() { .btree_map([(34, 35), (34, 36), (37, 38)]) .call(); - assert_debug_eq(&sut, expect![[r#" + assert_debug_eq( + &sut, + expect![[r#" ( [ 13, @@ -460,5 +471,6 @@ fn test_assoc_method() { 34: 36, 37: 38, }, - )"#]]); + )"#]], + ); } From 55b5bfefa2b9d11d295d3163f58b988f4562a7b8 Mon Sep 17 00:00:00 2001 From: Veetaha Date: Fri, 25 Oct 2024 23:50:09 +0000 Subject: [PATCH 07/10] Fix tests --- .../integration/builder/attr_with/some.rs | 89 +++++++++++++------ 1 file changed, 60 insertions(+), 29 deletions(-) diff --git a/bon/tests/integration/builder/attr_with/some.rs b/bon/tests/integration/builder/attr_with/some.rs index 7b36755b..cf89fa8e 100644 --- a/bon/tests/integration/builder/attr_with/some.rs +++ b/bon/tests/integration/builder/attr_with/some.rs @@ -55,36 +55,46 @@ fn test_struct() { #[test] fn test_free_fn() { - #[builder(derive(Clone))] - fn sut( - #[builder(transparent, with = Some)] required: Option, - #[builder(transparent, with = Some, default = Some(()))] optional: Option<()>, - #[builder(transparent, with = Some)] generic: Option, - #[builder(transparent, with = Some)] impl_trait: Option, - #[builder(transparent, with = Some, default = None)] optional_generic: Option, - ) -> impl fmt::Debug { - (required, optional, generic, impl_trait, optional_generic) - } + { + #[builder(derive(Clone))] + fn sut( + #[builder(transparent, with = Some)] required: Option, + #[builder(transparent, with = Some, default = Some(()))] optional: Option<()>, + #[builder(transparent, with = Some)] generic: Option, + #[builder(transparent, with = Some, default = None)] optional_generic: Option, + ) -> impl fmt::Debug { + (required, optional, generic, optional_generic) + } - let builder = sut(); + let builder = sut(); - let builder = builder.required(99); + let builder = builder.required(99); - let _ignore = builder.clone().optional(()); - let builder = builder.maybe_optional(None); + let _ignore = builder.clone().optional(()); + let builder = builder.maybe_optional(None); - let builder = builder.generic(2); - let builder = builder.impl_trait("impl Trait"); + let builder = builder.generic(2); - let _ignore = builder.clone().optional_generic(21); - let builder = builder.maybe_optional_generic(Some(22)); + let _ignore = builder.clone().optional_generic(21); + let builder = builder.maybe_optional_generic(Some(22)); - let sut = builder.call(); + let sut = builder.call(); - assert_debug_eq( - &sut, - expect![[r#"(Some(99), Some(()), Some(2), Some("impl Trait"), Some(22))"#]], - ); + assert_debug_eq( + &sut, + expect![[r#"(Some(99), Some(()), Some(2), Some("impl Trait"), Some(22))"#]], + ); + } + { + #[builder] + fn sut( + #[builder(transparent, with = Some)] impl_trait: Option, + ) -> impl fmt::Debug { + impl_trait + } + + assert_debug_eq(sut().impl_trait("impl Trait").call(), expect![]); + } } #[test] @@ -98,10 +108,9 @@ fn test_assoc_method() { #[builder(transparent, with = Some)] required: Option, #[builder(transparent, with = Some, default = Some(()))] optional: Option<()>, #[builder(transparent, with = Some)] generic: Option, - #[builder(transparent, with = Some)] impl_trait: Option, #[builder(transparent, with = Some, default = None)] optional_generic: Option, ) -> impl fmt::Debug { - (required, optional, generic, impl_trait, optional_generic) + (required, optional, generic, optional_generic) } #[builder(derive(Clone))] @@ -110,11 +119,26 @@ fn test_assoc_method() { #[builder(transparent, with = Some)] required: Option, #[builder(transparent, with = Some, default = Some(()))] optional: Option<()>, #[builder(transparent, with = Some)] generic: Option, - #[builder(transparent, with = Some)] impl_trait: Option, #[builder(transparent, with = Some, default = None)] optional_generic: Option, ) -> impl fmt::Debug { let _ = self; - (required, optional, generic, impl_trait, optional_generic) + (required, optional, generic, optional_generic) + } + + #[builder] + fn sut_impl_trait( + #[builder(transparent, with = Some)] impl_trait: Option, + ) -> impl fmt::Debug { + impl_trait + } + + #[builder] + fn with_self_impl_trait( + &self, + #[builder(transparent, with = Some)] impl_trait: Option, + ) -> impl fmt::Debug { + let _ = self; + impl_trait } } @@ -126,7 +150,6 @@ fn test_assoc_method() { let builder = builder.maybe_optional(None); let builder = builder.generic(2); - let builder = builder.impl_trait("impl Trait"); let _ignore = builder.clone().optional_generic(21); let builder = builder.maybe_optional_generic(Some(22)); @@ -146,7 +169,6 @@ fn test_assoc_method() { let builder = builder.maybe_optional(None); let builder = builder.generic(2); - let builder = builder.impl_trait("impl Trait"); let _ignore = builder.clone().optional_generic(21); let builder = builder.maybe_optional_generic(Some(22)); @@ -157,4 +179,13 @@ fn test_assoc_method() { &sut, expect![[r#"(Some(99), Some(()), Some(2), Some("impl Trait"), Some(22))"#]], ); + + assert_debug_eq( + Sut::sut_impl_trait().impl_trait("impl Trait").call(), + expect![], + ); + assert_debug_eq( + Sut.with_self_impl_trait().impl_trait("impl Trait").call(), + expect![], + ); } From 3fa76b0b8124c77f57ada25dfbef5fb321d03543 Mon Sep 17 00:00:00 2001 From: Veetaha Date: Fri, 25 Oct 2024 23:52:13 +0000 Subject: [PATCH 08/10] Update snapshots --- bon/tests/integration/builder/attr_with/some.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/bon/tests/integration/builder/attr_with/some.rs b/bon/tests/integration/builder/attr_with/some.rs index cf89fa8e..9bd41c39 100644 --- a/bon/tests/integration/builder/attr_with/some.rs +++ b/bon/tests/integration/builder/attr_with/some.rs @@ -82,7 +82,7 @@ fn test_free_fn() { assert_debug_eq( &sut, - expect![[r#"(Some(99), Some(()), Some(2), Some("impl Trait"), Some(22))"#]], + expect!["(Some(99), Some(()), Some(2), Some(22))"], ); } { @@ -93,7 +93,7 @@ fn test_free_fn() { impl_trait } - assert_debug_eq(sut().impl_trait("impl Trait").call(), expect![]); + assert_debug_eq(sut().impl_trait("impl Trait").call(), expect![[r#"Some("impl Trait")"#]]); } } @@ -158,7 +158,7 @@ fn test_assoc_method() { assert_debug_eq( &sut, - expect![[r#"(Some(99), Some(()), Some(2), Some("impl Trait"), Some(22))"#]], + expect!["(Some(99), Some(()), Some(2), Some(22))"], ); let builder = Sut.with_self(); @@ -177,15 +177,15 @@ fn test_assoc_method() { assert_debug_eq( &sut, - expect![[r#"(Some(99), Some(()), Some(2), Some("impl Trait"), Some(22))"#]], + expect!["(Some(99), Some(()), Some(2), Some(22))"], ); assert_debug_eq( Sut::sut_impl_trait().impl_trait("impl Trait").call(), - expect![], + expect![[r#"Some("impl Trait")"#]], ); assert_debug_eq( Sut.with_self_impl_trait().impl_trait("impl Trait").call(), - expect![], + expect![[r#"Some("impl Trait")"#]], ); } From 5cafdd53625a5ca521d63cf8e9f314781e24107a Mon Sep 17 00:00:00 2001 From: Veetaha Date: Fri, 25 Oct 2024 23:54:27 +0000 Subject: [PATCH 09/10] cargo fmt --- .../integration/builder/attr_with/some.rs | 20 +++++++------------ 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/bon/tests/integration/builder/attr_with/some.rs b/bon/tests/integration/builder/attr_with/some.rs index 9bd41c39..c7b160d2 100644 --- a/bon/tests/integration/builder/attr_with/some.rs +++ b/bon/tests/integration/builder/attr_with/some.rs @@ -80,10 +80,7 @@ fn test_free_fn() { let sut = builder.call(); - assert_debug_eq( - &sut, - expect!["(Some(99), Some(()), Some(2), Some(22))"], - ); + assert_debug_eq(&sut, expect!["(Some(99), Some(()), Some(2), Some(22))"]); } { #[builder] @@ -93,7 +90,10 @@ fn test_free_fn() { impl_trait } - assert_debug_eq(sut().impl_trait("impl Trait").call(), expect![[r#"Some("impl Trait")"#]]); + assert_debug_eq( + sut().impl_trait("impl Trait").call(), + expect![[r#"Some("impl Trait")"#]], + ); } } @@ -156,10 +156,7 @@ fn test_assoc_method() { let sut = builder.call(); - assert_debug_eq( - &sut, - expect!["(Some(99), Some(()), Some(2), Some(22))"], - ); + assert_debug_eq(&sut, expect!["(Some(99), Some(()), Some(2), Some(22))"]); let builder = Sut.with_self(); @@ -175,10 +172,7 @@ fn test_assoc_method() { let sut = builder.call(); - assert_debug_eq( - &sut, - expect!["(Some(99), Some(()), Some(2), Some(22))"], - ); + assert_debug_eq(&sut, expect!["(Some(99), Some(()), Some(2), Some(22))"]); assert_debug_eq( Sut::sut_impl_trait().impl_trait("impl Trait").call(), From c469cf827cf304c87cb4e669cdcf695ec26b9133 Mon Sep 17 00:00:00 2001 From: Veetaha Date: Sat, 26 Oct 2024 00:14:05 +0000 Subject: [PATCH 10/10] Self-review, more testc --- .../src/builder/builder_gen/setters/mod.rs | 18 +++++----- .../integration/ui/compile_fail/attr_with.rs | 23 ++++++++++++ .../ui/compile_fail/attr_with.stderr | 36 +++++++++++++++++++ website/changelog.md | 1 + 4 files changed, 70 insertions(+), 8 deletions(-) diff --git a/bon-macros/src/builder/builder_gen/setters/mod.rs b/bon-macros/src/builder/builder_gen/setters/mod.rs index 1f9ccd81..3fbc78ac 100644 --- a/bon-macros/src/builder/builder_gen/setters/mod.rs +++ b/bon-macros/src/builder/builder_gen/setters/mod.rs @@ -52,8 +52,8 @@ impl<'a> SettersCtx<'a> { } fn setters_for_optional_member(&self, items: OptionalSettersItems) -> Result { - if let Some(closure) = &self.member.config.with { - return self.setters_for_optional_member_having_with(closure, items); + if let Some(with) = &self.member.config.with { + return self.setters_for_optional_member_having_with(with, items); } let underlying_ty = self.member.underlying_norm_ty(); @@ -124,7 +124,7 @@ impl<'a> SettersCtx<'a> { let some_fn = Setter { item: items.some_fn, imp: SetterImpl { - inputs: self.underlying_inputs_from_with(with)?, + inputs: inputs.clone(), body: SetterBody::Forward { body: { let option_fn_name = &items.option_fn.name; @@ -217,15 +217,17 @@ impl<'a> SettersCtx<'a> { let well_known_single_arg_suffixes = ["Vec", "Set", "Deque", "Heap", "List"]; let err = || { + let mut from_iter_path = quote!(#from_iter).to_string(); + from_iter_path.retain(|c| !c.is_whitespace()); + err!( collection_ty, "the underlying type of this member is not a known collection type; \ only a collection type that matches the following patterns will be \ - accepted by `#[builder(with = {})], where * at the beginning means \ - the collection type may start with any prefix:\n\ + accepted by `#[builder(with = {from_iter_path})], where * at \ + the beginning means the collection type may start with any prefix:\n\ - *Map\n\ {}", - quote!(#from_iter), well_known_single_arg_suffixes .iter() .map(|suffix| { format!("- *{suffix}") }) @@ -653,8 +655,8 @@ fn well_known_default(ty: &syn::Type) -> Option { Some(value) } -/// Unfortunately there is no `syn::Parse` impl on `PatIdent` directly, -/// so we use this workaround instead +/// Unfortunately there is no `syn::Parse` impl for `PatIdent` directly, +/// so we use this workaround instead. fn pat_ident(ident_name: &'static str) -> syn::PatIdent { let ident = syn::Ident::new(ident_name, Span::call_site()); let pat: syn::Pat = syn::parse_quote!(#ident); diff --git a/bon/tests/integration/ui/compile_fail/attr_with.rs b/bon/tests/integration/ui/compile_fail/attr_with.rs index c1a0d1c6..07fa7409 100644 --- a/bon/tests/integration/ui/compile_fail/attr_with.rs +++ b/bon/tests/integration/ui/compile_fail/attr_with.rs @@ -80,5 +80,28 @@ struct ThreeGenericArgsResultReturnType { value: u32, } +#[derive(Builder)] +struct NonOptionWithSome1 { + #[builder(with = Some)] + value: u32, +} + +#[derive(Builder)] +struct NonOptionWithSome2 { + #[builder(with = Some)] + value: Option, +} + +#[derive(Builder)] +struct NonCollectionWithFromIter1 { + #[builder(with = FromIterator::from_iter)] + value: Option, +} + +#[derive(Builder)] +struct NonCollectionWithFromIter2 { + #[builder(with = <_>::from_iter)] + value: Option, +} fn main() {} diff --git a/bon/tests/integration/ui/compile_fail/attr_with.stderr b/bon/tests/integration/ui/compile_fail/attr_with.stderr index dc202fe1..41ce2a12 100644 --- a/bon/tests/integration/ui/compile_fail/attr_with.stderr +++ b/bon/tests/integration/ui/compile_fail/attr_with.stderr @@ -147,6 +147,42 @@ error: expected one of the following: 79 | #[builder(with = |value: u32| -> ::core::result::Result {})] | ^ +error: `with = Some` only works for members with the underlying type of `Option`; + --> tests/integration/ui/compile_fail/attr_with.rs:86:12 + | +86 | value: u32, + | ^^^ + +error: the underlying type of this member is not `Option`; by default, members of type `Option` are optional and their 'underlying type' is the type under the `Option`; you might be missing #[builder(transparent)]` annotation for this member + --> tests/integration/ui/compile_fail/attr_with.rs:91:22 + | +91 | #[builder(with = Some)] + | ^^^^ + +error: the underlying type of this member is not a known collection type; only a collection type that matches the following patterns will be accepted by `#[builder(with = FromIterator::from_iter)], where * at the beginning means the collection type may start with any prefix: + - *Map + - *Vec + - *Set + - *Deque + - *Heap + - *List + --> tests/integration/ui/compile_fail/attr_with.rs:98:19 + | +98 | value: Option, + | ^^^ + +error: the underlying type of this member is not a known collection type; only a collection type that matches the following patterns will be accepted by `#[builder(with = <_>::from_iter)], where * at the beginning means the collection type may start with any prefix: + - *Map + - *Vec + - *Set + - *Deque + - *Heap + - *List + --> tests/integration/ui/compile_fail/attr_with.rs:104:19 + | +104 | value: Option, + | ^^^ + error[E0308]: mismatched types --> tests/integration/ui/compile_fail/attr_with.rs:54:12 | diff --git a/website/changelog.md b/website/changelog.md index c0f901ad..03d829e8 100644 --- a/website/changelog.md +++ b/website/changelog.md @@ -23,6 +23,7 @@ All the breaking changes are very unlikely to actually break your code that was - Add `#[builder(builder_type(vis = "..."))]` that allows overriding the visibility of the builder struct ([#145](https://github.com/elastio/bon/pull/145)) - Add `#[builder(finish_fn(vis = "..."))]` that allows overriding the visibility of the finishing function ([#145](https://github.com/elastio/bon/pull/145)) - Add `#[builder(with = closure)]` syntax to customize setters with a custom closure. If the closure returns a `Result<_, E>` the setters become fallible ([#145](https://github.com/elastio/bon/pull/145)) +- Add `#[builder(with = Some)]`, `#[builder(with = FromIterator::from_iter)]`, `#[builder(with = <_>::from_iter)]` support for two well-known functions that will probably be used frequently ([#157](https://github.com/elastio/bon/pull/157)) - Add `#[builder(transparent)]` for `Option` fields to opt out from their special handling which makes `bon` treat them as regular required fields. It's also available at the top-level via `#[builder(on(_, transparent))]` ([#145](https://github.com/elastio/bon/pull/145), [#155](https://github.com/elastio/bon/pull/155)) - Add `#[builder(state_mod)]` to configure the builder's type state API module name, visibility and docs ([#145](https://github.com/elastio/bon/pull/145)) - Add `#[builder(overwritable)]` and `#[builder(on(..., overwritable)]` to make it possible to call setters multiple times for the same member ([#145](https://github.com/elastio/bon/pull/145))