diff --git a/bon-macros/src/builder/builder_gen/input_func.rs b/bon-macros/src/builder/builder_gen/input_func.rs index 566ce258..3b2682f6 100644 --- a/bon-macros/src/builder/builder_gen/input_func.rs +++ b/bon-macros/src/builder/builder_gen/input_func.rs @@ -1,6 +1,6 @@ use super::{ generic_param_to_arg, AssocMethodCtx, AssocMethodReceiverCtx, BuilderGenCtx, FinishFunc, - FinishFuncBody, Generics, Member, MemberExpr, MemberOrigin, StartFunc, + FinishFuncBody, Generics, Member, MemberOrigin, RawMember, StartFunc, }; use crate::builder::params::BuilderParams; use crate::normalization::NormalizeSelfTy; @@ -283,23 +283,37 @@ impl FuncInputCtx { } let builder_ident = self.builder_ident(); - let builder_private_impl_ident = - quote::format_ident!("__{}PrivateImpl", builder_ident.raw_name()); - let builder_state_trait_ident = quote::format_ident!("__{}State", builder_ident.raw_name()); fn typed_args(func: &syn::ItemFn) -> impl Iterator { func.sig.inputs.iter().filter_map(syn::FnArg::as_typed) } - let members: Vec<_> = typed_args(&self.norm_func) + let members = typed_args(&self.norm_func) .zip(typed_args(&self.orig_func)) - .map(Member::from_typed_fn_arg) - .try_collect()?; + .map(|(norm_arg, orig_arg)| { + let syn::Pat::Ident(pat) = norm_arg.pat.as_ref() else { + bail!( + &orig_arg.pat, + "use a simple `identifier: type` syntax for the function argument; \ + destructuring patterns in arguments aren't supported by the `#[builder]`", + ) + }; + + Ok(RawMember { + attrs: &norm_arg.attrs, + ident: pat.ident.clone(), + norm_ty: norm_arg.ty.clone(), + orig_ty: orig_arg.ty.clone(), + }) + }) + .collect::>>()?; + + let members = Member::from_raw(MemberOrigin::FnArg, members)?; let generics = self.generics(); let finish_func_body = FnCallBody { - func: self.adapted_func()?, + sig: self.adapted_func()?.sig, impl_ctx: self.impl_ctx.clone(), }; @@ -358,8 +372,6 @@ impl FuncInputCtx { conditional_params: self.params.base.on, builder_ident, - builder_private_impl_ident, - builder_state_trait_ident, assoc_method_ctx: receiver, generics, @@ -374,13 +386,13 @@ impl FuncInputCtx { } struct FnCallBody { - func: syn::ItemFn, + sig: syn::Signature, impl_ctx: Option>, } impl FinishFuncBody for FnCallBody { - fn generate(&self, member_exprs: &[MemberExpr<'_>]) -> TokenStream2 { - let asyncness = &self.func.sig.asyncness; + fn generate(&self, members: &[Member]) -> TokenStream2 { + let asyncness = &self.sig.asyncness; let maybe_await = asyncness.is_some().then(|| quote!(.await)); // Filter out lifetime generic arguments, because they are not needed @@ -389,7 +401,6 @@ impl FinishFuncBody for FnCallBody { // the turbofish syntax. See the problem of late-bound lifetimes specification // in the issue https://github.com/rust-lang/rust/issues/42868 let generic_args = self - .func .sig .generics .params @@ -398,25 +409,25 @@ impl FinishFuncBody for FnCallBody { .map(generic_param_to_arg); let prefix = self - .func .sig .receiver() .map(|receiver| { let self_token = &receiver.self_token; - quote!(#self_token.__private_impl.receiver.) + quote!(#self_token.__private_receiver.) }) .or_else(|| { let self_ty = &self.impl_ctx.as_deref()?.self_ty; Some(quote!(<#self_ty>::)) }); - let func_ident = &self.func.sig.ident; + let func_ident = &self.sig.ident; - let member_exprs = member_exprs.iter().map(|member| &member.expr); + // The variables with values of members in scope for this expression. + let member_vars = members.iter().map(Member::ident); quote! { #prefix #func_ident::<#(#generic_args,)*>( - #( #member_exprs ),* + #( #member_vars ),* ) #maybe_await } @@ -458,26 +469,6 @@ fn merge_generic_params( .collect() } -impl Member { - fn from_typed_fn_arg((norm_arg, orig_arg): (&syn::PatType, &syn::PatType)) -> Result { - let syn::Pat::Ident(pat) = norm_arg.pat.as_ref() else { - bail!( - &orig_arg.pat, - "use a simple `identifier: type` syntax for the function argument; \ - destructuring patterns in arguments aren't supported by the `#[builder]`", - ) - }; - - Member::new( - MemberOrigin::FnArg, - &norm_arg.attrs, - pat.ident.clone(), - norm_arg.ty.clone(), - orig_arg.ty.clone(), - ) - } -} - #[derive(Default)] struct FindSelfReference { self_span: Option, diff --git a/bon-macros/src/builder/builder_gen/input_struct.rs b/bon-macros/src/builder/builder_gen/input_struct.rs index 42e15a57..2b4cfa10 100644 --- a/bon-macros/src/builder/builder_gen/input_struct.rs +++ b/bon-macros/src/builder/builder_gen/input_struct.rs @@ -1,6 +1,6 @@ use super::{ - AssocMethodCtx, BuilderGenCtx, FinishFunc, FinishFuncBody, Generics, Member, MemberExpr, - MemberOrigin, StartFunc, + AssocMethodCtx, BuilderGenCtx, FinishFunc, FinishFuncBody, Generics, Member, MemberOrigin, + RawMember, StartFunc, }; use crate::builder::params::{BuilderParams, ItemParams}; use crate::util::prelude::*; @@ -74,10 +74,6 @@ impl StructInputCtx { pub(crate) fn into_builder_gen_ctx(self) -> Result { let builder_ident = self.builder_ident(); - let builder_private_impl_ident = - quote::format_ident!("__{}PrivateImpl", builder_ident.raw_name()); - - let builder_state_trait_ident = quote::format_ident!("__{}State", builder_ident.raw_name()); fn fields(struct_item: &syn::ItemStruct) -> Result<&syn::FieldsNamed> { match &struct_item.fields { @@ -91,12 +87,25 @@ impl StructInputCtx { let norm_fields = fields(&self.norm_struct)?; let orig_fields = fields(&self.orig_struct)?; - let members: Vec<_> = norm_fields + let members = norm_fields .named .iter() .zip(&orig_fields.named) - .map(Member::from_syn_field) - .try_collect()?; + .map(|(norm_field, orig_field)| { + let ident = norm_field.ident.clone().ok_or_else(|| { + err!(norm_field, "only structs with named fields are supported") + })?; + + Ok(RawMember { + attrs: &norm_field.attrs, + ident, + norm_ty: Box::new(norm_field.ty.clone()), + orig_ty: Box::new(orig_field.ty.clone()), + }) + }) + .collect::>>()?; + + let members = Member::from_raw(MemberOrigin::StructField, members)?; let generics = Generics { params: Vec::from_iter(self.norm_struct.generics.params.iter().cloned()), @@ -157,8 +166,6 @@ impl StructInputCtx { conditional_params: self.params.base.on, builder_ident, - builder_private_impl_ident, - builder_state_trait_ident, assoc_method_ctx, generics, @@ -177,37 +184,16 @@ struct StructLiteralBody { } impl FinishFuncBody for StructLiteralBody { - fn generate(&self, member_exprs: &[MemberExpr<'_>]) -> TokenStream2 { + fn generate(&self, member_exprs: &[Member]) -> TokenStream2 { let Self { struct_ident } = self; - let member_exprs = member_exprs.iter().map(|MemberExpr { member, expr }| { - let ident = member.ident(); - quote! { - #ident: #expr - } - }); + // The variables with values of members in scope for this expression. + let member_vars = member_exprs.iter().map(Member::ident); quote! { #struct_ident { - #(#member_exprs,)* + #(#member_vars,)* } } } } - -impl Member { - fn from_syn_field((norm_field, orig_field): (&syn::Field, &syn::Field)) -> Result { - let ident = norm_field - .ident - .clone() - .ok_or_else(|| err!(norm_field, "only structs with named fields are supported"))?; - - Member::new( - MemberOrigin::StructField, - &norm_field.attrs, - ident, - Box::new(norm_field.ty.clone()), - Box::new(orig_field.ty.clone()), - ) - } -} diff --git a/bon-macros/src/builder/builder_gen/member.rs b/bon-macros/src/builder/builder_gen/member.rs index 41d76930..af72b641 100644 --- a/bon-macros/src/builder/builder_gen/member.rs +++ b/bon-macros/src/builder/builder_gen/member.rs @@ -6,7 +6,7 @@ use quote::quote; use std::fmt; use syn::spanned::Spanned; -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Copy)] pub(crate) enum MemberOrigin { FnArg, StructField, @@ -42,6 +42,9 @@ pub(crate) struct RegularMember { /// Specifies what syntax the member comes from. pub(crate) origin: MemberOrigin, + /// Index of the member relative to other regular members. The index is 0-based. + pub(crate) index: syn::Index, + /// Original name of the member is used as the name of the builder field and /// in its setter methods. Struct field/fn arg names conventionally use `snake_case` /// in Rust, but this isn't enforced, so this member isn't guaranteed to be in @@ -58,9 +61,9 @@ pub(crate) struct RegularMember { /// Original type of the member (not normalized) pub(crate) orig_ty: Box, - /// The name of the associated type in the builder state trait that corresponds - /// to this member. - pub(crate) state_assoc_type_ident: syn::Ident, + /// The name of the type variable that can be used as the type of this + /// member in contexts where it should be generic. + pub(crate) generic_var_ident: syn::Ident, /// Parameters configured by the user explicitly via attributes pub(crate) params: MemberParams, @@ -159,66 +162,6 @@ fn parse_optional_expression(meta: &syn::Meta) -> Result, - orig_ty: Box, - ) -> Result { - let docs = attrs.iter().filter(|attr| attr.is_doc()).cloned().collect(); - - let params = MemberParams::from_attributes(attrs)?; - params.validate(&origin)?; - - if let Some(value) = params.skip { - return Ok(Self::Skipped(SkippedMember { - ident, - norm_ty, - value, - })); - } - - let me = RegularMember { - origin, - state_assoc_type_ident: ident.snake_to_pascal_case(), - ident, - norm_ty, - orig_ty, - params, - docs, - }; - - me.validate()?; - - Ok(Self::Regular(me)) - } -} - -impl Member { - pub(crate) fn norm_ty(&self) -> &syn::Type { - match self { - Self::Regular(me) => &me.norm_ty, - Self::Skipped(me) => &me.norm_ty, - } - } - - pub(crate) fn ident(&self) -> &syn::Ident { - match self { - Self::Regular(me) => &me.ident, - Self::Skipped(me) => &me.ident, - } - } - - pub(crate) fn as_regular(&self) -> Option<&RegularMember> { - match self { - Self::Regular(me) => Some(me), - Self::Skipped(_) => None, - } - } -} - impl RegularMember { fn validate(&self) -> Result { super::reject_self_references_in_docs(&self.docs)?; @@ -245,32 +188,16 @@ impl RegularMember { Self::as_optional_with_ty(self, &self.norm_ty) } - pub(crate) fn init_expr(&self) -> TokenStream2 { - self.as_optional_norm_ty() - .map(|_| quote!(::bon::private::Optional(::core::marker::PhantomData))) - .unwrap_or_else(|| quote!(::bon::private::Required(::core::marker::PhantomData))) - } - - pub(crate) fn unset_state_type(&self) -> TokenStream2 { - let ty = &self.norm_ty; - - if let Some(inner_type) = self.as_optional_norm_ty() { - quote!(::bon::private::Optional<#inner_type>) - } else { - quote!(::bon::private::Required<#ty>) - } + pub(crate) fn is_optional(&self) -> bool { + self.as_optional_norm_ty().is_some() } - pub(crate) fn set_state_type_param(&self) -> TokenStream2 { + pub(crate) fn set_state_type(&self) -> TokenStream2 { let ty = &self.norm_ty; - - self.as_optional_norm_ty() + let ty = self + .as_optional_norm_ty() .map(|ty| quote!(Option<#ty>)) - .unwrap_or_else(|| quote!(#ty)) - } - - pub(crate) fn set_state_type(&self) -> TokenStream2 { - let ty = self.set_state_type_param(); + .unwrap_or_else(|| quote!(#ty)); quote!(::bon::private::Set<#ty>) } @@ -310,3 +237,89 @@ impl RegularMember { Ok(verdict_from_override || verdict_from_defaults) } } + +pub(crate) struct RawMember<'a> { + pub(crate) attrs: &'a [syn::Attribute], + pub(crate) ident: syn::Ident, + pub(crate) norm_ty: Box, + pub(crate) orig_ty: Box, +} + +impl Member { + // False-positive lint. We can't elide the lifetime in `RawMember` because + // anonymous lifetimes in impl traits are unstable, and we shouldn't omit + // the lifetime parameter because we want to be explicit about its existence + // (there is an other lint that checks for this). + #[allow(single_use_lifetimes)] + pub(crate) fn from_raw<'a>( + origin: MemberOrigin, + members: impl IntoIterator>, + ) -> Result> { + let mut regular_members_count = 0; + + members + .into_iter() + .map(|member| { + let RawMember { + attrs, + ident, + norm_ty, + orig_ty, + } = member; + + let params = MemberParams::from_attributes(attrs)?; + params.validate(&origin)?; + + if let Some(value) = params.skip { + return Ok(Member::Skipped(SkippedMember { + ident, + norm_ty, + value, + })); + } + + let docs = attrs.iter().filter(|attr| attr.is_doc()).cloned().collect(); + + let me = RegularMember { + index: regular_members_count.into(), + origin, + generic_var_ident: quote::format_ident!("__{}", ident.snake_to_pascal_case()), + ident, + norm_ty, + orig_ty, + params, + docs, + }; + + regular_members_count += 1; + + me.validate()?; + + Ok(Member::Regular(me)) + }) + .collect() + } +} + +impl Member { + pub(crate) fn norm_ty(&self) -> &syn::Type { + match self { + Self::Regular(me) => &me.norm_ty, + Self::Skipped(me) => &me.norm_ty, + } + } + + pub(crate) fn ident(&self) -> &syn::Ident { + match self { + Self::Regular(me) => &me.ident, + Self::Skipped(me) => &me.ident, + } + } + + pub(crate) fn as_regular(&self) -> Option<&RegularMember> { + match self { + Self::Regular(me) => Some(me), + Self::Skipped(_) => None, + } + } +} diff --git a/bon-macros/src/builder/builder_gen/mod.rs b/bon-macros/src/builder/builder_gen/mod.rs index 6104f388..aad74890 100644 --- a/bon-macros/src/builder/builder_gen/mod.rs +++ b/bon-macros/src/builder/builder_gen/mod.rs @@ -8,7 +8,7 @@ use member::*; use super::params::ConditionalParams; use crate::util::prelude::*; -use quote::quote; +use quote::{quote, ToTokens}; pub(crate) struct AssocMethodReceiverCtx { pub(crate) with_self_keyword: syn::Receiver, @@ -38,8 +38,6 @@ pub(crate) struct BuilderGenCtx { pub(crate) finish_func: FinishFunc, pub(crate) builder_ident: syn::Ident, - pub(crate) builder_private_impl_ident: syn::Ident, - pub(crate) builder_state_trait_ident: syn::Ident, } pub(crate) struct FinishFunc { @@ -65,13 +63,10 @@ pub(crate) struct StartFunc { } pub(crate) trait FinishFuncBody { - /// Generate `finish` function body from ready-made expressions. - fn generate(&self, member_exprs: &[MemberExpr<'_>]) -> TokenStream2; -} - -pub(crate) struct MemberExpr<'a> { - pub(crate) member: &'a Member, - pub(crate) expr: TokenStream2, + /// Generate `finish` function body from ready-made variables. + /// The generated function body may assume that there are variables + /// named the same as the members in scope. + fn generate(&self, members: &[Member]) -> TokenStream2; } pub(crate) struct Generics { @@ -95,13 +90,11 @@ impl BuilderGenCtx { pub(crate) fn output(self) -> Result { let start_func = self.start_func(); - let builder_state_trait_decl = self.builder_state_trait_decl(); let builder_decl = self.builder_decl(); let call_method_impl = self.finish_method_impl()?; let setter_methods_impls = self.setter_methods_impls()?; let other_items = quote! { - #builder_state_trait_decl #builder_decl #call_method_impl #setter_methods_impls @@ -158,11 +151,10 @@ impl BuilderGenCtx { let docs = &self.start_func.attrs; let vis = self.start_func.vis.as_ref().unwrap_or(&self.vis); - let builder_private_impl_ident = &self.builder_private_impl_ident; let start_func_ident = &self.start_func.ident; // TODO: we can use a shorter syntax with anonymous lifetimes to make - // the generate code and function signature displayed by rust-analyzer + // the generated code and function signature displayed by rust-analyzer // a bit shorter and easier to read. However, the caveat is that we can // do this only for lifetimes that have no bounds and if they don't appear // in the where clause. Research `darling`'s lifetime tracking API and @@ -182,19 +174,15 @@ impl BuilderGenCtx { let receiver_field_init = receiver.map(|receiver| { let self_token = &receiver.with_self_keyword.self_token; quote! { - receiver: #self_token, + __private_receiver: #self_token, } }); let receiver = receiver.map(|receiver| &receiver.with_self_keyword); - let member_inits = self.regular_members().map(|member| { - let expr = member.init_expr(); - let ident = &member.ident; - quote! { - #ident: #expr - } - }); + let unset_state_literals = self + .regular_members() + .map(|_| quote!(::bon::private::Unset)); let ide_hints = self.ide_hints(); @@ -203,19 +191,15 @@ impl BuilderGenCtx { #[inline(always)] #vis fn #start_func_ident<#(#generics_decl),*>( #receiver - ) -> #builder_ident< - #(#generic_args,)* - > + ) -> #builder_ident<#(#generic_args,)*> #where_clause { #ide_hints #builder_ident { - __private_impl: #builder_private_impl_ident { - _phantom: ::core::marker::PhantomData, - #receiver_field_init - #( #member_inits, )* - } + __private_phantom: ::core::marker::PhantomData, + #receiver_field_init + __private_members: (#( #unset_state_literals, )*) } } }; @@ -224,10 +208,6 @@ impl BuilderGenCtx { } fn phantom_data(&self) -> TokenStream2 { - self.phantom_data_with_state(true) - } - - fn phantom_data_with_state(&self, state: bool) -> TokenStream2 { let member_types = self.members.iter().map(Member::norm_ty); let receiver_ty = self .assoc_method_ctx @@ -251,8 +231,6 @@ impl BuilderGenCtx { quote!(::core::marker::PhantomData<#ty>) }); - let state = state.then(|| quote!(::core::marker::PhantomData<__State>)); - quote! { ::core::marker::PhantomData<( // There is an interesting quirk with lifetimes in Rust, which is the @@ -278,60 +256,36 @@ impl BuilderGenCtx { // explanation for it, I just didn't care to research it yet ¯\_(ツ)_/¯. #(#types,)* - // A special case of zero members requires storing `__State` in phantom data + // A special case of zero members requires storing `_State` in phantom data // otherwise it would be reported as an unused type parameter. - #state + ::core::marker::PhantomData<_State> )> } } - fn builder_state_trait_decl(&self) -> TokenStream2 { - let trait_ident = &self.builder_state_trait_ident; - let assoc_types_idents: Vec<_> = self - .regular_members() - .map(|member| &member.state_assoc_type_ident) - .collect(); - - let vis = &self.vis; - - quote! { - #[doc(hidden)] - #vis trait #trait_ident { - #( type #assoc_types_idents; )* - } - - impl<#(#assoc_types_idents),*> #trait_ident for (#(#assoc_types_idents,)*) { - #( type #assoc_types_idents = #assoc_types_idents; )* - } - } - } - fn builder_decl(&self) -> TokenStream2 { let vis = &self.vis; let builder_ident = &self.builder_ident; - let builder_private_impl_ident = &self.builder_private_impl_ident; - let builder_state_trait_ident = &self.builder_state_trait_ident; let generics_decl = &self.generics.params; let where_clause = &self.generics.where_clause; - let generic_args: Vec<_> = self.generic_args().collect(); - let unset_state_types = self.regular_members().map(|arg| arg.unset_state_type()); let phantom_data = self.phantom_data(); + let private_field_doc = "\ + Please don't touch this field. It's an implementation \ + detail that is exempt from the API stability guarantees. \ + This field couldn't be hidden using Rust's privacy syntax. \ + The details about this are described in [the blog post]\ + (https://elastio.github.io/bon/blog/the-weird-of-function-local-types-in-rust). + "; + let receiver_field = self.assoc_method_ctx.as_ref().and_then(|receiver| { let ty = &receiver.receiver.as_ref()?.without_self_keyword; Some(quote! { - receiver: #ty, + #[doc = #private_field_doc] + __private_receiver: #ty, }) }); - let members = self.regular_members().map(|member| { - let ident = &member.ident; - let assoc_type_ident = &member.state_assoc_type_ident; - quote! { - #ident: __State::#assoc_type_ident, - } - }); - let must_use_message = format!( "the builder does nothing until you call `{}()` on it to finish building", self.finish_func.ident @@ -345,144 +299,52 @@ impl BuilderGenCtx { let allows = allow_warnings_on_member_types(); - // This is the workaround for the bug in rustc: - // https://github.com/rust-lang/rust/issues/87682 - // - // The main purpose of this workaround is to avoid using associated - // types projections in the default value for the generic parameter. - // - // We can't control the user code, so we do a trick where we capture - // all the needed generic parameters in an intermediate structs (without - // the __State generic). Then we implement a trait for this struct where - // the assoc type of that trait is what we want to have in place of the - // default generic parameter for the builder. - // - // You don't even know how much I hate this workaround, and how hard it - // was for me to come up with it. - let initial_state_workaround_trait = + let initial_state_type_alias_ident = quote::format_ident!("__{}InitialState", builder_ident.raw_name()); - let initial_state_workaround_trait_impl = - quote::format_ident!("__{}InitialStateImpl", builder_ident.raw_name()); - - let phantom_data_without_state = self.phantom_data_with_state(false); + let unset_state_types = self + .regular_members() + .map(|_| quote!(::bon::private::Unset)); quote! { - /// Workaround for - #[doc(hidden)] - #vis trait #initial_state_workaround_trait { - type State; - } - - /// Workaround for + // This type alias exists just to shorten the type signature of + // the default generic argument of the builder struct. It's not + // really important for users to see what this type alias expands to. + // + // If they want to see how "bon works" they should just expand the + // macro manually where they'll see this type alias. #[doc(hidden)] - #allows - #vis struct #initial_state_workaround_trait_impl<#(#generics_decl,)*> - #where_clause - { - #receiver_field - _phantom: #phantom_data_without_state, - } - - #allows - impl<#(#generics_decl,)*> #initial_state_workaround_trait for - #initial_state_workaround_trait_impl<#(#generic_args,)*> - #where_clause - { - type State = (#(#unset_state_types,)*); - } + #vis type #initial_state_type_alias_ident = (#(#unset_state_types,)*); #[must_use = #must_use_message] #[doc = #docs] #allows #vis struct #builder_ident< #(#generics_decl,)* - __State: #builder_state_trait_ident = < - #initial_state_workaround_trait_impl<#(#generic_args,)*> - as #initial_state_workaround_trait - >::State + _State = #initial_state_type_alias_ident > #where_clause { - /// Please don't touch this field. It's an implementation - /// detail that is exempt from the API stability guarantees. - /// It's visible to you only because of the limitations of - /// the Rust language. - /// - /// The limitation is that we can't make the fields of the - /// generated struct private other than by placing its - /// declaration inside of a nested sub-module. However, we - /// can't do that because this breaks support for fn items - /// declared inside of other fn items like this: - /// - /// ```rustdoc_hidden - /// use bon::builder; - /// - /// fn foo() { - /// struct Foo; - /// - /// #[builder] - /// fn nested(foo: Foo) {} - /// - /// nested().foo(Foo).call(); - /// } - /// ``` - /// - /// If we were to generate a child module like this then code - /// in that child module would lose access to the symbol `Foo` - /// in the parent module. The following code doesn't compile. - /// - /// ```rustdoc_hidden - /// fn foo() { - /// struct Foo; - /// - /// mod __private_child_module { - /// use super::*; - /// - /// pub(super) struct Builder { - /// foo: Foo, - /// } - /// } - /// } - /// ``` - /// - /// `Foo` symbol is inaccessible inside of `__private_child_module` - /// because it is defined inside of the function `foo()` and not - /// inside of the parent module. - /// - /// Child modules are kinda implicitly "hoisted" to the top-level of - /// the module and they can't see the local symbols defined inside - /// of the same function scope. - __private_impl: #builder_private_impl_ident< - #(#generic_args,)* - __State - > - } + // We could use `#[cfg(not(rust_analyzer))]` to hide these. + // However, RA would then not be able to type-check the generated + // code, which may or may not be a problem, because the main thing + // is that the type signatures would still work in RA. + #[doc = #private_field_doc] + __private_phantom: #phantom_data, - /// This struct exists only to reduce the number of private fields - /// that pop up in IDE completions for developers. It groups all - /// the private fields in it leaving the builder type higher with - /// just a single field of this type that documents the fact that - /// the developers shouldn't touch it. - #allows - struct #builder_private_impl_ident< - #(#generics_decl,)* - __State: #builder_state_trait_ident - > - #where_clause - { - _phantom: #phantom_data, #receiver_field - #(#members)* + + #[doc = #private_field_doc] + __private_members: _State } } } fn member_expr(&self, member: &Member) -> Result { - let regular = match member { - Member::Regular(regular) => regular, - Member::Skipped(skipped) => { - let expr = skipped + let member = match member { + Member::Regular(member) => member, + Member::Skipped(member) => { + let expr = member .value .as_ref() .as_ref() @@ -493,17 +355,18 @@ impl BuilderGenCtx { } }; - let maybe_default = regular + let maybe_default = member .as_optional_norm_ty() // For `Option` members we don't need any `unwrap_or_[else/default]`. - // We pass them directly to the function unchanged. - .filter(|_| !regular.norm_ty.is_option()) + // The implementation of `From for Set>` already + // returns an `Option`. + .filter(|_| !member.norm_ty.is_option()) .map(|_| { - regular + member .param_default() .flatten() .map(|default| { - let has_into = regular.param_into(&self.conditional_params)?; + let has_into = member.param_into(&self.conditional_params)?; let default = if has_into { quote! { ::core::convert::Into::into((|| #default)()) } } else { @@ -516,27 +379,31 @@ impl BuilderGenCtx { }) .transpose()?; - let member_ident = ®ular.ident; + let index = &member.index; - let expr = quote! { - ::core::convert::Into::<::bon::private::Set<_>>::into( - self.__private_impl.#member_ident - ) - .0 - #maybe_default + let expr = if member.is_optional() { + quote! { + ::core::convert::Into::<::bon::private::Set<_>>::into( + self.__private_members.#index + ) + .0 + #maybe_default + } + } else { + quote! { + self.__private_members.#index.0 + } }; Ok(expr) } fn finish_method_impl(&self) -> Result { - let (members_vars_decls, member_exprs): (Vec<_>, Vec<_>) = self + let members_vars_decls = self .members .iter() - .map(|member| Ok((member, self.member_expr(member)?))) - .collect::>>()? - .into_iter() - .map(|(member, expr)| { + .map(|member| { + let expr = self.member_expr(member)?; let var_ident = member.ident(); // The type hint is necessary in some cases to assist the compiler @@ -549,62 +416,60 @@ impl BuilderGenCtx { // intermediate variable here. let ty = member.norm_ty(); - let var_decl = quote! { + Ok(quote! { let #var_ident: #ty = #expr; - }; - - let member_expr = MemberExpr { - member, - expr: quote! { #var_ident }, - }; - - (var_decl, member_expr) + }) }) - .unzip(); + .collect::>>()?; - let body = &self.finish_func.body.generate(&member_exprs); + let body = &self.finish_func.body.generate(&self.members); let asyncness = &self.finish_func.asyncness; let unsafety = &self.finish_func.unsafety; let must_use = &self.finish_func.must_use; let docs = &self.finish_func.docs; let vis = &self.vis; let builder_ident = &self.builder_ident; - let builder_state_trait_ident = &self.builder_state_trait_ident; let finish_func_ident = &self.finish_func.ident; let output = &self.finish_func.output; let generics_decl = &self.generics.params; let generic_builder_args = self.generic_args(); - let where_clause_predicates = self - .generics - .where_clause - .as_ref() - .into_iter() - .flat_map(|where_clause| &where_clause.predicates); + let where_clause = &self.generics.where_clause; - let state_where_predicates = self.regular_members().map(|member| { - let member_assoc_type_ident = &member.state_assoc_type_ident; - let set_state_type_param = member.set_state_type_param(); - quote! { - __State::#member_assoc_type_ident: - ::core::convert::Into<::bon::private::Set<#set_state_type_param>> + let builder_state_types = self.regular_members().map(|member| { + if member.is_optional() { + member.generic_var_ident.to_token_stream() + } else { + member.set_state_type() } }); + let optional_members_generics_decls = self + .regular_members() + // Only optional members can have two source states, that we + // we need to represent with a trait bound. They can either + // be `Unset` (setter never called) or `Set` (setter was called) + .filter(|member| member.is_optional()) + .map(|member| { + let ident = &member.generic_var_ident; + let set_state_type = member.set_state_type(); + Some(quote! { + #ident: ::core::convert::Into<#set_state_type> + }) + }); + let allows = allow_warnings_on_member_types(); Ok(quote! { #allows impl< #(#generics_decl,)* - __State: #builder_state_trait_ident + #(#optional_members_generics_decls,)* > #builder_ident< #(#generic_builder_args,)* - __State + (#(#builder_state_types,)*) > - where - #( #where_clause_predicates, )* - #( #state_where_predicates, )* + #where_clause { #[doc = #docs] #[inline(always)] diff --git a/bon-macros/src/builder/builder_gen/setter_methods.rs b/bon-macros/src/builder/builder_gen/setter_methods.rs index 3a054a32..ced468f9 100644 --- a/bon-macros/src/builder/builder_gen/setter_methods.rs +++ b/bon-macros/src/builder/builder_gen/setter_methods.rs @@ -9,100 +9,62 @@ impl BuilderGenCtx { ) -> Result { let output_members_states = self.regular_members().map(|other_member| { if other_member.ident == member.ident { - return member.set_state_type().to_token_stream(); + member.set_state_type().to_token_stream() + } else { + other_member.generic_var_ident.to_token_stream() } - - let state_assoc_type_ident = &other_member.state_assoc_type_ident; - quote!(__State::#state_assoc_type_ident) }); - let state_assoc_type_ident = &member.state_assoc_type_ident; let builder_ident = &self.builder_ident; - let builder_state_trait_ident = &self.builder_state_trait_ident; let generics_decl = &self.generics.params; let generic_args: Vec<_> = self.generic_args().collect(); - let where_clause = &self.generics.where_clause; - let unset_state_type = member.unset_state_type(); - let output_builder_alias_ident = quote::format_ident!( - "__{}Set{}", - builder_ident.raw_name(), - state_assoc_type_ident.raw_name() - ); + let where_clause_predicates: Vec<_> = self + .generics + .where_clause + .as_ref() + .into_iter() + .flat_map(|where_clause| &where_clause.predicates) + .collect(); + + let member_state_vars = self + .regular_members() + .filter(|other_member| other_member.ident != member.ident) + .map(|other_member| &other_member.generic_var_ident); - // A case where there is just one member is special, because the type alias would - // receive a generic `__State` parameter that it wouldn't use, so we create it - // only if there are 2 or more members. - let (output_builder_alias_state_var_decl, output_builder_alias_state_arg) = - (self.regular_members().count() > 1) - .then(|| (quote!(__State: #builder_state_trait_ident), quote!(__State))) - .unzip(); + let input_member_states = self.regular_members().map(|other_member| { + if other_member.ident == member.ident { + quote!(::bon::private::Unset) + } else { + other_member.generic_var_ident.to_token_stream() + } + }); let setter_methods = MemberSettersCtx::new( self, member, quote! { - #output_builder_alias_ident< + #builder_ident< #(#generic_args,)* - #output_builder_alias_state_arg + (#(#output_members_states,)*) > }, ) .setter_methods()?; - let vis = &self.vis; - let allows = super::allow_warnings_on_member_types(); Ok(quote! { - // This lint is ignored, because bounds in type aliases are still useful - // to make the following example usage compile: - // ``` - // type Bar = T::Item; - // ``` - // In this case the bound is necessary for `T::Item` access to be valid. - // The compiler proposes this: - // - // > use fully disambiguated paths (i.e., `::Assoc`) to refer - // > to associated types in type aliases - // - // But, come on... Why would you want to write `T::Item` as `::Item` - // especially if that `T::Item` access is used in multiple places? It's a waste of time - // to implement logic that rewrites the user's type expressions to that syntax when just - // having bounds on the type alias is enough already. - #[allow(type_alias_bounds)] - #allows - // This is `doc(hidden)` with the same visibility as the setter to reduce the noise in - // the docs generated by `rustdoc`. Rustdoc auto-inlines type aliases if they aren't exposed - // as part of the public API of the crate. This is a workaround to prevent that. - #[doc(hidden)] - #vis type #output_builder_alias_ident< - #(#generics_decl,)* - #output_builder_alias_state_var_decl - > - // The where clause in this position will be deprecated. The preferred - // position will be at the end of the entire type alias syntax construct. - // See details at https://github.com/rust-lang/rust/issues/112792. - // - // However, at the time of this writing the only way to put the where - // clause on a type alias is here. - #where_clause - = #builder_ident< - #(#generic_args,)* - ( #(#output_members_states,)* ) - >; - #allows impl< #(#generics_decl,)* - __State: #builder_state_trait_ident< - #state_assoc_type_ident = #unset_state_type - > + #(#member_state_vars,)* > #builder_ident< #(#generic_args,)* - __State + (#(#input_member_states,)*) > - #where_clause + where + #(#where_clause_predicates,)* { #setter_methods } @@ -171,8 +133,10 @@ impl<'a> MemberSettersCtx<'a> { Ok(self.setter_method(MemberSetterMethod { method_name: self.setter_method_core_name(), fn_params: quote!(value: #fn_param_type), - member_init: quote!(::bon::private::Set(value #maybe_into_call)), overwrite_docs: None, + body: SetterBody::Default { + member_init: quote!(::bon::private::Set(value #maybe_into_call)), + }, })) } @@ -180,14 +144,10 @@ impl<'a> MemberSettersCtx<'a> { let has_into = self .member .param_into(&self.builder_gen.conditional_params)?; - let (inner_type, maybe_conv_call, maybe_map_conv_call) = if has_into { - ( - quote!(impl Into<#inner_type>), - quote!(.into()), - quote!(.map(Into::into)), - ) + let (inner_type, maybe_map_conv_call) = if has_into { + (quote!(impl Into<#inner_type>), quote!(.map(Into::into))) } else { - (quote!(#inner_type), quote!(), quote!()) + (quote!(#inner_type), quote!()) }; let setter_method_name = self.setter_method_core_name(); @@ -198,16 +158,23 @@ impl<'a> MemberSettersCtx<'a> { setter_method_name.span(), ); + // Option-less setter is just a shortcut for wrapping the value in `Some`. + let optionless_setter_body = quote! { + self.#option_method_name(Some(value)) + }; + let methods = [ MemberSetterMethod { method_name: option_method_name, fn_params: quote!(value: Option<#inner_type>), - member_init: quote!(::bon::private::Set(value #maybe_map_conv_call)), overwrite_docs: Some(format!( "Same as [`Self::{setter_method_name}`], but accepts \ an `Option` as input. See that method's documentation for \ more details.", )), + body: SetterBody::Default { + member_init: quote!(::bon::private::Set(value #maybe_map_conv_call)), + }, }, // We intentionally keep the name and signature of the setter method // for an optional member that accepts the value under the option the @@ -218,8 +185,8 @@ impl<'a> MemberSettersCtx<'a> { MemberSetterMethod { method_name: setter_method_name, fn_params: quote!(value: #inner_type), - member_init: quote!(::bon::private::Set(Some(value #maybe_conv_call))), overwrite_docs: None, + body: SetterBody::Custom(optionless_setter_body), }, ]; @@ -234,8 +201,8 @@ impl<'a> MemberSettersCtx<'a> { let MemberSetterMethod { method_name, fn_params, - member_init, overwrite_docs, + body, } = method; let docs = match overwrite_docs { @@ -246,41 +213,42 @@ impl<'a> MemberSettersCtx<'a> { let vis = &self.builder_gen.vis; - let builder_ident = &self.builder_gen.builder_ident; - let builder_private_impl_ident = &self.builder_gen.builder_private_impl_ident; - let member_idents = self - .builder_gen - .regular_members() - .map(|member| member.ident.clone()); - - let maybe_receiver_field = self - .builder_gen - .assoc_method_ctx - .as_ref() - .is_some_and(|ctx| ctx.receiver.is_some()) - .then(|| quote!(receiver: self.__private_impl.receiver,)); + let body = match body { + SetterBody::Custom(body) => body, + SetterBody::Default { member_init } => { + let maybe_receiver_field = self + .builder_gen + .assoc_method_ctx + .as_ref() + .is_some_and(|ctx| ctx.receiver.is_some()) + .then(|| quote!(__private_receiver: self.__private_receiver,)); + + let builder_ident = &self.builder_gen.builder_ident; + + let member_exprs = self.builder_gen.regular_members().map(|other_member| { + if other_member.ident == self.member.ident { + return member_init.clone(); + } + let index = &other_member.index; + quote!(self.__private_members.#index) + }); - let member_exprs = self.builder_gen.regular_members().map(|other_member| { - if other_member.ident == self.member.ident { - return member_init.clone(); + quote! { + #builder_ident { + __private_phantom: ::core::marker::PhantomData, + #maybe_receiver_field + __private_members: (#( #member_exprs, )*) + } + } } - - let ident = &other_member.ident; - quote!(self.__private_impl.#ident) - }); + }; quote! { #( #docs )* #[allow(clippy::impl_trait_in_params)] #[inline(always)] #vis fn #method_name(self, #fn_params) -> #return_type { - #builder_ident { - __private_impl: #builder_private_impl_ident { - _phantom: ::core::marker::PhantomData, - #maybe_receiver_field - #( #member_idents: #member_exprs, )* - } - } + #body } } } @@ -317,9 +285,14 @@ impl<'a> MemberSettersCtx<'a> { } } +enum SetterBody { + Custom(TokenStream2), + Default { member_init: TokenStream2 }, +} + struct MemberSetterMethod { method_name: syn::Ident, fn_params: TokenStream2, - member_init: TokenStream2, overwrite_docs: Option, + body: SetterBody, } diff --git a/bon-macros/src/util/fn_arg.rs b/bon-macros/src/util/fn_arg.rs index 4e2fcb09..0fe0c8d7 100644 --- a/bon-macros/src/util/fn_arg.rs +++ b/bon-macros/src/util/fn_arg.rs @@ -8,29 +8,29 @@ pub(crate) trait FnArgExt { impl FnArgExt for syn::FnArg { fn attrs_mut(&mut self) -> &mut Vec { match self { - syn::FnArg::Receiver(arg) => &mut arg.attrs, - syn::FnArg::Typed(arg) => &mut arg.attrs, + Self::Receiver(arg) => &mut arg.attrs, + Self::Typed(arg) => &mut arg.attrs, } } fn ty_mut(&mut self) -> &mut syn::Type { match self { - syn::FnArg::Receiver(arg) => &mut arg.ty, - syn::FnArg::Typed(arg) => &mut arg.ty, + Self::Receiver(arg) => &mut arg.ty, + Self::Typed(arg) => &mut arg.ty, } } fn as_receiver(&self) -> Option<&syn::Receiver> { match self { - syn::FnArg::Typed(_) => None, - syn::FnArg::Receiver(arg) => Some(arg), + Self::Typed(_) => None, + Self::Receiver(arg) => Some(arg), } } fn as_typed(&self) -> Option<&syn::PatType> { match self { - syn::FnArg::Typed(arg) => Some(arg), - syn::FnArg::Receiver(_) => None, + Self::Typed(arg) => Some(arg), + Self::Receiver(_) => None, } } } diff --git a/bon/src/private/mod.rs b/bon/src/private/mod.rs index 62432fb0..1ae5c08a 100644 --- a/bon/src/private/mod.rs +++ b/bon/src/private/mod.rs @@ -6,14 +6,11 @@ pub mod ide; pub extern crate alloc; #[derive(Debug)] -pub struct Required(pub core::marker::PhantomData); +pub struct Unset; -#[derive(Debug)] -pub struct Optional(pub core::marker::PhantomData>); - -impl From> for Set> { +impl From for Set> { #[inline(always)] - fn from(_: Optional) -> Set> { + fn from(_: Unset) -> Set> { Set(None) } } diff --git a/bon/tests/integration/builder/generics.rs b/bon/tests/integration/builder/generics.rs index b6bd43c8..06954099 100644 --- a/bon/tests/integration/builder/generics.rs +++ b/bon/tests/integration/builder/generics.rs @@ -41,14 +41,15 @@ fn unsized_generics_in_params() { sut().arg(&42).call(); } +#[cfg(feature = "alloc")] #[test] fn unsized_generics_in_return_type() { #[builder] - fn sut(arg: &T) { - let _ = arg; + fn sut() -> Box { + Box::default() } - sut().arg(&42).call(); + sut::().call(); } // This is based on the issue https://github.com/rust-lang/rust/issues/129701 diff --git a/bon/tests/integration/builder/mod.rs b/bon/tests/integration/builder/mod.rs index 8f186382..1cfa222a 100644 --- a/bon/tests/integration/builder/mod.rs +++ b/bon/tests/integration/builder/mod.rs @@ -6,6 +6,7 @@ mod attr_skip; mod generics; mod init_order; mod lints; +mod name_conflicts; mod raw_idents; mod smoke; diff --git a/bon/tests/integration/builder/name_conflicts.rs b/bon/tests/integration/builder/name_conflicts.rs new file mode 100644 index 00000000..77ae108e --- /dev/null +++ b/bon/tests/integration/builder/name_conflicts.rs @@ -0,0 +1,32 @@ +//! @Veetaha encountered a bug when working on big changes to `bon` where +//! the generic params generated by the macro were named the same as the +//! type of the member, and thus leading to a name conflict. This test is +//! here to catch any such simple regression. +use crate::prelude::*; + +struct User; + +#[test] +fn member_and_type_named_the_same_fn() { + #[builder] + fn sut(_user: User) {} + + sut().user(User).call(); +} + +#[test] +fn member_and_type_named_the_same_struct() { + #[builder] + struct Sut { + _user: User, + } + + #[bon] + impl Sut { + #[builder] + fn sut(_user: User) {} + } + + let _ = Sut::builder().user(User).build(); + Sut::sut().user(User).call(); +} diff --git a/bon/tests/integration/main.rs b/bon/tests/integration/main.rs index 11647ffb..37a0988c 100644 --- a/bon/tests/integration/main.rs +++ b/bon/tests/integration/main.rs @@ -6,9 +6,8 @@ extern crate alloc; mod prelude { #[cfg(feature = "alloc")] - pub(crate) use { - alloc::borrow::ToOwned, alloc::collections::BTreeSet, alloc::format, alloc::string::String, - alloc::vec, alloc::vec::Vec, + pub(crate) use alloc::{ + borrow::ToOwned, boxed::Box, collections::BTreeSet, format, string::String, vec, vec::Vec, }; pub(crate) use super::assert_debug_eq; diff --git a/e2e-tests/src/lib.rs b/e2e-tests/src/lib.rs index c42318b8..e660f376 100644 --- a/e2e-tests/src/lib.rs +++ b/e2e-tests/src/lib.rs @@ -87,3 +87,44 @@ pub fn greet( ) -> String { format!("Hello {name} with age {age}!") } + +#[builder] +pub fn many_function_parameters( + _id: Option<&str>, + _keyword: Option<&str>, + _attraction_id: Option<&str>, + _venue_id: Option<&str>, + _postal_code: Option<&str>, + _latlong: Option<&str>, + _radius: Option<&str>, + _unit: Option<&str>, + _source: Option<&str>, + _locale: Option<&str>, + _market_id: Option<&str>, + _start_date_time: Option<&str>, + _end_date_time: Option<&str>, + _include_tba: Option<&str>, + _include_tbd: Option<&str>, + _include_test: Option<&str>, + _size: Option<&str>, + _page: Option<&str>, + _sort: Option<&str>, + _onsale_start_date_time: Option<&str>, + _onsale_end_date_time: Option<&str>, + _city: Option<&str>, + _country_code: Option<&str>, + _state_code: Option<&str>, + _classification_name: Option<&str>, + _classification_id: Option<&str>, + _dma_id: Option<&str>, + _onsale_on_start_date: Option<&str>, + _onsale_on_after_start_date: Option<&str>, + _segment_id: Option<&str>, + _segment_name: Option<&str>, + _promoter_id: Option<&str>, + _client_visibility: Option<&str>, + _nlp: Option<&str>, + _include_licensed_content: Option<&str>, + _geopoint: Option<&str>, +) { +}