From 0d2cc2ec8dbb92a18ff95918bebd047dadf1be32 Mon Sep 17 00:00:00 2001 From: Veetaha Date: Tue, 10 Sep 2024 23:24:24 +0000 Subject: [PATCH 01/21] Initial attempt --- .github/workflows/ci.yml | 4 - .../builder/builder_gen/builder_derives.rs | 12 +- .../src/builder/builder_gen/input_func.rs | 2 +- .../src/builder/builder_gen/input_struct.rs | 2 +- .../builder_gen/member/into_conversion.rs | 96 ++++++ .../builder_gen/{member.rs => member/mod.rs} | 307 +++++++++++++----- bon-macros/src/builder/builder_gen/mod.rs | 106 ++++-- .../src/builder/builder_gen/setter_methods.rs | 12 +- bon-macros/src/builder/item_impl.rs | 4 +- bon-macros/src/util/iterator.rs | 7 - bon-macros/src/util/ty/mod.rs | 4 +- bon/src/private/mod.rs | 2 +- bon/tests/integration/builder/mod.rs | 22 +- .../integration/builder/positional_members.rs | 23 ++ 14 files changed, 470 insertions(+), 133 deletions(-) create mode 100644 bon-macros/src/builder/builder_gen/member/into_conversion.rs rename bon-macros/src/builder/builder_gen/{member.rs => member/mod.rs} (54%) create mode 100644 bon/tests/integration/builder/positional_members.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a450d279..ff98bb19 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -98,10 +98,6 @@ jobs: components: clippy toolchain: 1.59.0 - # We just test that `bon` crate compile fine. This is the main crate - # that needs to compile fine. We also make sure its tests compile - # fine as well. Therefore right now we use the MSRV specifically such - # that `trybuild` compiles as well (it's trybuild's MSRV). - run: ./scripts/test-msrv.sh test-unstable: diff --git a/bon-macros/src/builder/builder_gen/builder_derives.rs b/bon-macros/src/builder/builder_gen/builder_derives.rs index bbba7760..98e57400 100644 --- a/bon-macros/src/builder/builder_gen/builder_derives.rs +++ b/bon-macros/src/builder/builder_gen/builder_derives.rs @@ -34,7 +34,7 @@ impl BuilderGenCtx { .receiver() .map(|receiver| &receiver.without_self_keyword); - let member_types = self.regular_members().map(|member| &member.norm_ty); + let member_types = self.named_members().map(|member| &member.norm_ty); std::iter::empty() .chain(receiver_ty) @@ -81,7 +81,7 @@ impl BuilderGenCtx { Self { __private_phantom: ::core::marker::PhantomData, #clone_receiver - __private_members: self.__private_members.clone(), + __private_named_members: self.__private_named_members.clone(), } } } @@ -107,18 +107,18 @@ impl BuilderGenCtx { let builder_ident_str = builder_ident.to_string(); let state_type_vars = self - .regular_members() + .named_members() .map(|member| &member.generic_var_ident) .collect::>(); - let format_members = self.regular_members().map(|member| { + let format_members = self.named_members().map(|member| { let member_index = &member.index; let member_ident_str = member.orig_ident.to_string(); quote! { // Skip members that are not set to reduce noise - if self.__private_members.#member_index.is_set() { - output.field(#member_ident_str, &self.__private_members.#member_index); + if self.__private_named_members.#member_index.is_set() { + output.field(#member_ident_str, &self.__private_named_members.#member_index); } } }); diff --git a/bon-macros/src/builder/builder_gen/input_func.rs b/bon-macros/src/builder/builder_gen/input_func.rs index a4cb69f0..ad7ed2fc 100644 --- a/bon-macros/src/builder/builder_gen/input_func.rs +++ b/bon-macros/src/builder/builder_gen/input_func.rs @@ -422,7 +422,7 @@ impl FinishFuncBody for FnCallBody { let func_ident = &self.sig.ident; // The variables with values of members are in scope for this expression. - let member_vars = members.iter().map(Member::ident); + let member_vars = members.iter().map(Member::orig_ident); quote! { #prefix #func_ident::<#(#generic_args,)*>( diff --git a/bon-macros/src/builder/builder_gen/input_struct.rs b/bon-macros/src/builder/builder_gen/input_struct.rs index 2d8bbd14..739540f3 100644 --- a/bon-macros/src/builder/builder_gen/input_struct.rs +++ b/bon-macros/src/builder/builder_gen/input_struct.rs @@ -211,7 +211,7 @@ impl FinishFuncBody for StructLiteralBody { let Self { struct_ident } = self; // The variables with values of members are in scope for this expression. - let member_vars = member_exprs.iter().map(Member::ident); + let member_vars = member_exprs.iter().map(Member::orig_ident); quote! { #struct_ident { diff --git a/bon-macros/src/builder/builder_gen/member/into_conversion.rs b/bon-macros/src/builder/builder_gen/member/into_conversion.rs new file mode 100644 index 00000000..2d4c6e95 --- /dev/null +++ b/bon-macros/src/builder/builder_gen/member/into_conversion.rs @@ -0,0 +1,96 @@ +use super::{MemberOrigin, MemberParams, NamedMember, PositionalFnArgMember}; +use crate::builder::params::ConditionalParams; +use crate::util::prelude::*; +use quote::{quote, ToTokens}; + +impl NamedMember { + pub(crate) fn param_into(&self, conditional_params: &[ConditionalParams]) -> Result { + // For optional named members the target of the `Into` conversion is the type + // inside of the `Option`, not the `Option` itself because we generate + // a setter that accepts `T` itself. It also makes this logic stable regardless + // if `Option` is used or the member of type `T` has `#[builder(default)]` on it. + let scrutinee = self + .as_optional_with_ty(&self.orig_ty) + .unwrap_or(&self.orig_ty); + + is_into_enabled(self.origin, &self.params, scrutinee, conditional_params) + } + + pub(crate) fn setter_method_core_name(&self) -> &syn::Ident { + self.params.name.as_ref().unwrap_or(&self.norm_ident) + } +} + +impl PositionalFnArgMember { + pub(crate) fn param_into(&self, conditional_params: &[ConditionalParams]) -> Result { + // Positional members are never optional. Users must always specify them, so there + // is no need to us to look into the `Option` generic parameter, because the + // `Option` itself is the target of the into conversion, not the `T` inside it. + let scrutinee = self.orig_ty.as_ref(); + + is_into_enabled(self.origin, &self.params, scrutinee, conditional_params) + } + + pub(crate) fn fn_input_type( + &self, + conditional_params: &[ConditionalParams], + ) -> Result { + let has_into = self.param_into(conditional_params)?; + let norm_ty = &self.norm_ty; + + let ty = if has_into { + quote! { impl Into<#norm_ty> } + } else { + quote! { #norm_ty } + }; + + Ok(ty) + } + + pub(crate) fn maybe_into_ident_expr( + &self, + conditional_params: &[ConditionalParams], + ) -> Result { + let has_into = self.param_into(conditional_params)?; + let ident = &self.ident; + + let expr = if has_into { + quote! { + ::core::convert::Into::into(#ident) + } + } else { + ident.to_token_stream() + }; + + Ok(expr) + } +} + +fn is_into_enabled( + origin: MemberOrigin, + member_params: &MemberParams, + scrutinee: &syn::Type, + conditional_params: &[ConditionalParams], +) -> Result { + let verdict_from_defaults = conditional_params + .iter() + .map(|params| Ok((params, scrutinee.matches(¶ms.type_pattern)?))) + .collect::>>()? + .into_iter() + .filter(|(_, matched)| *matched) + .any(|(params, _)| params.into.is_present()); + + let verdict_from_override = member_params.into.is_present(); + + if verdict_from_defaults && verdict_from_override { + bail!( + &member_params.into.span(), + "this `#[builder(into)]` attribute is redundant, because `into` \ + is already implied for this member via the `#[builder(on(...))]` \ + at the top of the {}", + origin.parent_construct(), + ); + } + + Ok(verdict_from_override || verdict_from_defaults) +} diff --git a/bon-macros/src/builder/builder_gen/member.rs b/bon-macros/src/builder/builder_gen/member/mod.rs similarity index 54% rename from bon-macros/src/builder/builder_gen/member.rs rename to bon-macros/src/builder/builder_gen/member/mod.rs index 2ae9cf85..b8e6f5c2 100644 --- a/bon-macros/src/builder/builder_gen/member.rs +++ b/bon-macros/src/builder/builder_gen/member/mod.rs @@ -1,7 +1,8 @@ -use crate::builder::params::ConditionalParams; +mod into_conversion; + use crate::util::prelude::*; use darling::util::SpannedValue; -use darling::FromAttributes; +use darling::{FromAttributes, FromMeta}; use quote::quote; use std::fmt; use syn::spanned::Spanned; @@ -32,13 +33,20 @@ impl MemberOrigin { #[derive(Debug)] pub(crate) enum Member { - Regular(RegularMember), + Named(NamedMember), + + /// Member that was marked with `#[builder(pos = start_fn)]` + StartFnArg(StartFnArgMember), + + /// Member that was marked with `#[builder(pos = finish_fn)]` + FinishFnArg(PositionalFnArgMember), + Skipped(SkippedMember), } /// Regular member for which the builder should have setter methods #[derive(Debug, Clone)] -pub(crate) struct RegularMember { +pub(crate) struct NamedMember { /// Specifies what syntax the member comes from. pub(crate) origin: MemberOrigin, @@ -76,6 +84,33 @@ pub(crate) struct RegularMember { pub(crate) params: MemberParams, } +/// Member that was marked with `#[builder(pos = start_fn)]` +#[derive(Debug)] +pub(crate) struct StartFnArgMember { + pub(crate) base: PositionalFnArgMember, + + /// Index of the member relative to other positional members. The index is 0-based. + pub(crate) index: syn::Index, +} + +#[derive(Debug)] +pub(crate) struct PositionalFnArgMember { + /// Specifies what syntax the member comes from. + pub(crate) origin: MemberOrigin, + + /// Original identifier of the member + pub(crate) ident: syn::Ident, + + /// Normalized type of the member + pub(crate) norm_ty: Box, + + /// Original type of the member (not normalized) + pub(crate) orig_ty: Box, + + /// Parameters configured by the user explicitly via attributes + pub(crate) params: MemberParams, +} + /// Member that was skipped by the user with `#[builder(skip)]` #[derive(Debug)] pub(crate) struct SkippedMember { @@ -91,35 +126,115 @@ pub(crate) struct SkippedMember { #[darling(attributes(builder))] pub(crate) struct MemberParams { /// Enables an `Into` conversion for the setter method. - pub(crate) into: darling::util::Flag, + into: darling::util::Flag, /// Assign a default value to the member it it's not specified. /// /// An optional expression can be provided to set the value for the member, /// otherwise its [`Default`] trait impl will be used. #[darling(with = parse_optional_expression, map = Some)] - pub(crate) default: Option>>, + default: Option>>, /// Skip generating a setter method for this member. /// /// An optional expression can be provided to set the value for the member, /// otherwise its [`Default`] trait impl will be used. #[darling(with = parse_optional_expression, map = Some)] - pub(crate) skip: Option>>, + skip: Option>>, /// Rename the name exposed in the builder API. - pub(crate) name: Option, + name: Option, + + /// Where to place the member in the generate builder methods API. + /// By default the member is treated like a named parameter that + /// gets its own setter methods. + pos: Option, +} + +#[derive(PartialEq, Eq, Clone, Copy)] +enum MemberParamName { + Default, + Into, + Name, + Pos, + Skip, +} + +impl fmt::Display for MemberParamName { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let str = match self { + Self::Default => "default", + Self::Into => "into", + Self::Name => "name", + Self::Pos => "pos", + Self::Skip => "skip", + }; + f.write_str(str) + } } impl MemberParams { - fn validate(&self, origin: MemberOrigin) -> Result { - if let Self { - skip: Some(skip), + fn validate_mutually_allowed( + &self, + attr_name: MemberParamName, + attr_span: Span, + allowed: &[MemberParamName], + ) -> Result<()> { + let conflicting: Vec<_> = self + .specified_param_names() + .filter(|name| *name != attr_name && !allowed.contains(name)) + .collect(); + + if conflicting.is_empty() { + return Ok(()); + } + + let conflicting = conflicting + .iter() + .map(|name| format!("`{name}`")) + .join(", "); + + bail!( + &attr_span, + "`{attr_name}` attribute can't be specified with other \ + attributes like {}", + conflicting, + ); + } + + fn specified_param_names(&self) -> impl Iterator { + let Self { into, default, + skip, name, - } = self - { + pos, + } = self; + + let attrs = [ + (default.is_some(), MemberParamName::Default), + (name.is_some(), MemberParamName::Name), + (into.is_present(), MemberParamName::Into), + (pos.is_some(), MemberParamName::Pos), + (skip.is_some(), MemberParamName::Skip), + ]; + + attrs + .into_iter() + .filter(|(is_present, _)| *is_present) + .map(|(_, name)| name) + } + + fn validate(&self, origin: MemberOrigin) -> Result { + if let Some(pos) = self.pos { + self.validate_mutually_allowed( + MemberParamName::Pos, + pos.span(), + &[MemberParamName::Into], + )?; + } + + if let Some(skip) = &self.skip { match origin { MemberOrigin::FnArg => { bail!( @@ -131,29 +246,16 @@ impl MemberParams { MemberOrigin::StructField => {} } - let other_attr = [ - default.as_ref().map(|attr| ("default", attr.span())), - name.as_ref().map(|attr| ("name", attr.span())), - into.is_present().then(|| ("into", into.span())), - ] - .into_iter() - .flatten() - .next(); - - if let Some((attr_name, span)) = other_attr { - let default_hint = if let Some(Some(_expr)) = default.as_deref() { - ". If you wanted to specify a value for the member, then use \ - the following syntax instead `#[builder(skip = value)]`" - } else { - "" - }; - + if let Some(Some(_expr)) = self.default.as_deref() { bail!( - &span, - "`skip` attribute can't be specified with other attributes like `{attr_name}` \ - because there will be no setter generated for this member to configure{default_hint}", + &skip.span(), + "`skip` attribute can't be specified with `default` attribute; \ + if you wanted to specify a value for the member, then use \ + the following syntax instead `#[builder(skip = value)]`", ); } + + self.validate_mutually_allowed(MemberParamName::Skip, skip.span(), &[])?; } Ok(()) @@ -168,7 +270,7 @@ fn parse_optional_expression(meta: &syn::Meta) -> Result Result { super::reject_self_references_in_docs(&self.docs)?; @@ -215,38 +317,6 @@ impl RegularMember { .as_ref() .map(|default| default.as_ref().as_ref()) } - - pub(crate) fn param_into(&self, conditional_params: &[ConditionalParams]) -> Result { - let scrutinee = self - .as_optional_with_ty(&self.orig_ty) - .unwrap_or(&self.orig_ty); - - let verdict_from_defaults = conditional_params - .iter() - .map(|params| Ok((params, scrutinee.matches(¶ms.type_pattern)?))) - .collect::>>()? - .into_iter() - .filter(|(_, matched)| *matched) - .any(|(params, _)| params.into.is_present()); - - let verdict_from_override = self.params.into.is_present(); - - if verdict_from_defaults && verdict_from_override { - bail!( - &self.params.into.span(), - "this `#[builder(into)]` attribute is redundant, because `into` \ - is already implied for this member via the `#[builder(on(...))]` \ - at the top of the {}", - self.origin.parent_construct(), - ); - } - - Ok(verdict_from_override || verdict_from_defaults) - } - - pub(crate) fn setter_method_core_name(&self) -> &syn::Ident { - self.params.name.as_ref().unwrap_or(&self.norm_ident) - } } pub(crate) struct RawMember<'a> { @@ -266,7 +336,8 @@ impl Member { origin: MemberOrigin, members: impl IntoIterator>, ) -> Result> { - let mut regular_members_count = 0; + let mut named_count = 0; + let mut start_fn_arg_count = 0; members .into_iter() @@ -289,6 +360,33 @@ impl Member { })); } + if let Some(pos) = params.pos { + let base = PositionalFnArgMember { + origin, + ident: orig_ident, + norm_ty, + orig_ty, + params, + }; + match pos { + PosMemberLocation::StartFn(_) => { + let index = start_fn_arg_count.into(); + start_fn_arg_count += 1; + return Ok(Self::StartFnArg(StartFnArgMember { base, index })); + } + PosMemberLocation::FinishFn(_) => { + return Ok(Self::FinishFnArg(base)); + } + } + } + + // XXX: docs are collected only for named members. There is obvious + // place where to put the docs for positional and skipped members. + // + // Even if there are some docs on them and the function syntax is used + // then these docs will just be removed from the output function. + // It's probably fine since the doc comments are there in the code + // itself which is also useful for people reading the source code. let docs = attrs.iter().filter(|attr| attr.is_doc()).cloned().collect(); let orig_ident_str = orig_ident.to_string(); @@ -304,8 +402,8 @@ impl Member { let norm_ident = syn::Ident::new_maybe_raw(norm_ident, orig_ident.span()); let norm_ident_pascal = norm_ident.snake_to_pascal_case(); - let me = RegularMember { - index: regular_members_count.into(), + let me = NamedMember { + index: named_count.into(), origin, generic_var_ident: quote::format_ident!("__{}", norm_ident_pascal), norm_ident_pascal, @@ -317,11 +415,11 @@ impl Member { docs, }; - regular_members_count += 1; + named_count += 1; me.validate()?; - Ok(Self::Regular(me)) + Ok(Self::Named(me)) }) .collect() } @@ -330,22 +428,77 @@ impl Member { impl Member { pub(crate) fn norm_ty(&self) -> &syn::Type { match self { - Self::Regular(me) => &me.norm_ty, + Self::Named(me) => &me.norm_ty, + Self::StartFnArg(me) => &me.base.norm_ty, + Self::FinishFnArg(me) => &me.norm_ty, Self::Skipped(me) => &me.norm_ty, } } - pub(crate) fn ident(&self) -> &syn::Ident { + pub(crate) fn orig_ident(&self) -> &syn::Ident { match self { - Self::Regular(me) => &me.orig_ident, + Self::Named(me) => &me.orig_ident, + Self::StartFnArg(me) => &me.base.ident, + Self::FinishFnArg(me) => &me.ident, Self::Skipped(me) => &me.ident, } } - pub(crate) fn as_regular(&self) -> Option<&RegularMember> { + pub(crate) fn as_named(&self) -> Option<&NamedMember> { + match self { + Self::Named(me) => Some(me), + _ => None, + } + } + + pub(crate) fn as_start_fn_arg(&self) -> Option<&StartFnArgMember> { + match self { + Self::StartFnArg(me) => Some(me), + _ => None, + } + } + + pub(crate) fn as_finish_fn_arg(&self) -> Option<&PositionalFnArgMember> { + match self { + Self::FinishFnArg(me) => Some(me), + _ => None, + } + } +} + +#[derive(Debug, Copy, Clone)] +enum PosMemberLocation { + /// The member must be a positional argument in the start function + StartFn(Span), + + /// The member must be a positional argument in the finish function + FinishFn(Span), +} + +impl FromMeta for PosMemberLocation { + fn from_expr(expr: &syn::Expr) -> Result { + let err = || err!(expr, "expected one of `start_fn` or `finish_fn`"); + let path = match expr { + syn::Expr::Path(path) if path.attrs.is_empty() && path.qself.is_none() => path, + _ => return Err(err()), + }; + + let ident = path.path.get_ident().ok_or_else(err)?.to_string(); + + let kind = match ident.as_str() { + "start_fn" => Self::StartFn, + "finish_fn" => Self::FinishFn, + _ => return Err(err()), + }; + + Ok(kind(path.span())) + } +} + +impl PosMemberLocation { + fn span(self) -> Span { match self { - Self::Regular(me) => Some(me), - Self::Skipped(_) => None, + Self::StartFn(span) | Self::FinishFn(span) => span, } } } diff --git a/bon-macros/src/builder/builder_gen/mod.rs b/bon-macros/src/builder/builder_gen/mod.rs index 57808752..b5efa3c1 100644 --- a/bon-macros/src/builder/builder_gen/mod.rs +++ b/bon-macros/src/builder/builder_gen/mod.rs @@ -173,12 +173,16 @@ impl BuilderGenCtx { self.assoc_method_ctx.as_ref()?.receiver.as_ref() } - fn regular_members(&self) -> impl Iterator { - self.members.iter().filter_map(Member::as_regular) + fn named_members(&self) -> impl Iterator { + self.members.iter().filter_map(Member::as_named) + } + + fn start_fn_args(&self) -> impl Iterator { + self.members.iter().filter_map(Member::as_start_fn_arg) } pub(crate) fn output(self) -> Result { - let start_func = self.start_func(); + let start_func = self.start_func()?; let builder_decl = self.builder_decl(); let builder_impl = self.builder_impl()?; let builder_derives = self.builder_derives(); @@ -203,7 +207,7 @@ impl BuilderGenCtx { let generic_args = &self.generics.args; let where_clause = &self.generics.where_clause; let state_type_vars = self - .regular_members() + .named_members() .map(|member| &member.generic_var_ident) .collect::>(); @@ -212,7 +216,7 @@ impl BuilderGenCtx { let allows = allow_warnings_on_member_types(); let regular_members_labels = self - .regular_members() + .named_members() .map(|member| self.members_label(member)); let vis = &self.vis; @@ -283,7 +287,7 @@ impl BuilderGenCtx { } } - fn start_func(&self) -> syn::ItemFn { + fn start_func(&self) -> Result { let builder_ident = &self.builder_ident; let docs = &self.start_func.attrs; @@ -315,7 +319,7 @@ impl BuilderGenCtx { let receiver = receiver.map(|receiver| &receiver.with_self_keyword); - let unset_state_literals = self.regular_members().map(|member| { + let unset_state_literals = self.named_members().map(|member| { if member.is_optional() { quote!(::bon::private::Unset(::bon::private::Optional)) } else { @@ -323,6 +327,22 @@ impl BuilderGenCtx { } }); + let start_fn_args = self + .start_fn_args() + .map(|member| member.base.fn_input_type(&self.conditional_params)) + .collect::>>()?; + + let start_fn_arg_exprs = self + .start_fn_args() + .map(|member| member.base.maybe_into_ident_expr(&self.conditional_params)) + .collect::>>()?; + + let start_fn_args_field_init = (!start_fn_arg_exprs.is_empty()).then(|| { + quote! { + __private_start_fn_args: (#(#start_fn_arg_exprs,)*), + } + }); + let ide_hints = self.ide_hints(); let func = quote! { @@ -339,6 +359,7 @@ impl BuilderGenCtx { )] #vis fn #start_func_ident<#(#generics_decl),*>( #receiver + #(#start_fn_args,)* ) -> #builder_ident<#(#generic_args,)*> #where_clause { @@ -347,12 +368,13 @@ impl BuilderGenCtx { #builder_ident { __private_phantom: ::core::marker::PhantomData, #receiver_field_init - __private_members: (#( #unset_state_literals, )*) + #start_fn_args_field_init + __private_named_members: (#( #unset_state_literals, )*) } } }; - syn::parse_quote!(#func) + Ok(syn::parse_quote!(#func)) } fn phantom_data(&self) -> TokenStream2 { @@ -450,7 +472,7 @@ impl BuilderGenCtx { let initial_state_type_alias_ident = quote::format_ident!("__{}InitialState", builder_ident.raw_name()); - let unset_state_types = self.regular_members().map(|member| { + let unset_state_types = self.named_members().map(|member| { if member.is_optional() { quote!(::bon::private::Unset<::bon::private::Optional>) } else { @@ -458,6 +480,18 @@ impl BuilderGenCtx { } }); + let mut start_fn_arg_types = self + .start_fn_args() + .map(|member| &member.base.norm_ty) + .peekable(); + + let start_fn_arg_types_field = start_fn_arg_types.peek().is_some().then(|| { + quote! { + #[doc = #private_field_doc] + __private_start_fn_args: (#(#start_fn_arg_types,)*), + } + }); + quote! { // This type alias exists just to shorten the type signature of // the default generic argument of the builder struct. It's not @@ -495,16 +529,17 @@ impl BuilderGenCtx { __private_phantom: #phantom_data, #receiver_field + #start_fn_arg_types_field #[doc = #private_field_doc] - __private_members: ___State + __private_named_members: ___State } } } fn member_expr(&self, member: &Member) -> Result { let member = match member { - Member::Regular(member) => member, + Member::Named(member) => member, Member::Skipped(member) => { let expr = member .value @@ -515,6 +550,13 @@ impl BuilderGenCtx { return Ok(expr); } + Member::StartFnArg(member) => { + let index = &member.index; + return Ok(quote! { self.__private_start_fn_args.#index }); + } + Member::FinishFnArg(member) => { + return member.maybe_into_ident_expr(&self.conditional_params); + } }; let maybe_default = member @@ -549,7 +591,7 @@ impl BuilderGenCtx { ::bon::private::IntoSet::< #set_state_type_param, #member_label - >::into_set(self.__private_members.#index) + >::into_set(self.__private_named_members.#index) #maybe_default }; @@ -558,7 +600,7 @@ impl BuilderGenCtx { /// Name of the dummy struct that is generated just to give a name for /// the member in the error message when `IntoSet` trait is not implemented. - fn members_label(&self, member: &RegularMember) -> syn::Ident { + fn members_label(&self, member: &NamedMember) -> syn::Ident { quote::format_ident!( "{}__{}", self.builder_ident.raw_name(), @@ -572,7 +614,7 @@ impl BuilderGenCtx { .iter() .map(|member| { let expr = self.member_expr(member)?; - let var_ident = member.ident(); + let var_ident = member.orig_ident(); // The type hint is necessary in some cases to assist the compiler // in type inference. @@ -599,7 +641,7 @@ impl BuilderGenCtx { let finish_func_ident = &self.finish_func.ident; let output = &self.finish_func.output; - let where_bounds = self.regular_members().map(|member| { + let where_bounds = self.named_members().map(|member| { let member_type_var = &member.generic_var_ident; let set_state_type_param = member.set_state_type_param(); let member_label = self.members_label(member); @@ -611,13 +653,31 @@ impl BuilderGenCtx { } }); + let finish_fn_args = self + .members + .iter() + .filter_map(Member::as_finish_fn_arg) + .map(|member| member.fn_input_type(&self.conditional_params)) + .collect::>>()?; + Ok(quote! { #[doc = #docs] #[inline(always)] - // This is intentional. We want the builder syntax to compile away - #[allow(clippy::inline_always)] + #[allow( + // This is intentional. We want the builder syntax to compile away + clippy::inline_always, + + // This lint flags any function that returns a possibly `!Send` future. + // However, it doesn't apply in the generic context where the future is + // `Send` if the generic parameters are `Send` as well, so we just suppress + // this lint. See the issue: https://github.com/rust-lang/rust-clippy/issues/6947 + clippy::future_not_send, + )] #must_use - #vis #asyncness #unsafety fn #finish_func_ident(self) #output + #vis #asyncness #unsafety fn #finish_func_ident( + self, + #(#finish_fn_args,)* + ) #output where #(#where_bounds,)* { @@ -634,7 +694,7 @@ impl BuilderGenCtx { let where_clause = &self.generics.where_clause; let state_type_vars = self - .regular_members() + .named_members() .map(|member| &member.generic_var_ident) .collect::>(); @@ -643,7 +703,7 @@ impl BuilderGenCtx { let next_state_trait_ident = quote::format_ident!("__{}SetMember", builder_ident.raw_name()); - let next_states_decls = self.regular_members().map(|member| { + let next_states_decls = self.named_members().map(|member| { let member_pascal = &member.norm_ident_pascal; quote! { type #member_pascal; @@ -651,9 +711,9 @@ impl BuilderGenCtx { }); let setters = self - .regular_members() + .named_members() .map(|member| { - let state_types = self.regular_members().map(|other_member| { + let state_types = self.named_members().map(|other_member| { if other_member.orig_ident == member.orig_ident { let ty = member.set_state_type_param(); quote!(::bon::private::Set<#ty>) diff --git a/bon-macros/src/builder/builder_gen/setter_methods.rs b/bon-macros/src/builder/builder_gen/setter_methods.rs index b41a770f..fd44a87e 100644 --- a/bon-macros/src/builder/builder_gen/setter_methods.rs +++ b/bon-macros/src/builder/builder_gen/setter_methods.rs @@ -1,4 +1,4 @@ -use super::{BuilderGenCtx, RegularMember}; +use super::{BuilderGenCtx, NamedMember}; use crate::util::prelude::*; use quote::quote; @@ -18,14 +18,14 @@ pub(crate) struct SettersReturnType { pub(crate) struct MemberSettersCtx<'a> { builder_gen: &'a BuilderGenCtx, - member: &'a RegularMember, + member: &'a NamedMember, return_type: SettersReturnType, } impl<'a> MemberSettersCtx<'a> { pub(crate) fn new( builder_gen: &'a BuilderGenCtx, - member: &'a RegularMember, + member: &'a NamedMember, return_type: SettersReturnType, ) -> Self { Self { @@ -144,19 +144,19 @@ impl<'a> MemberSettersCtx<'a> { let builder_ident = &self.builder_gen.builder_ident; - let member_exprs = self.builder_gen.regular_members().map(|other_member| { + let member_exprs = self.builder_gen.named_members().map(|other_member| { if other_member.norm_ident == self.member.norm_ident { return member_init.clone(); } let index = &other_member.index; - quote!(self.__private_members.#index) + quote!(self.__private_named_members.#index) }); quote! { #builder_ident { __private_phantom: ::core::marker::PhantomData, #maybe_receiver_field - __private_members: (#( #member_exprs, )*) + __private_named_members: (#( #member_exprs, )*) } } } diff --git a/bon-macros/src/builder/item_impl.rs b/bon-macros/src/builder/item_impl.rs index 3a73ed1a..d33be034 100644 --- a/bon-macros/src/builder/item_impl.rs +++ b/bon-macros/src/builder/item_impl.rs @@ -63,7 +63,7 @@ pub(crate) fn generate(mut orig_impl_block: syn::ItemImpl) -> Result = orig_impl_block + let outputs = orig_impl_block .items .into_iter() .zip(norm_impl_block.items) @@ -104,7 +104,7 @@ pub(crate) fn generate(mut orig_impl_block: syn::ItemImpl) -> Result::Ok((ctx.adapted_func()?, ctx.into_builder_gen_ctx()?.output()?)) }) - .try_collect()?; + .collect::>>()?; let new_impl_items = outputs.iter().flat_map(|(adapted_func, output)| { let start_func = &output.start_func; diff --git a/bon-macros/src/util/iterator.rs b/bon-macros/src/util/iterator.rs index 253b4e5b..aafcfcc7 100644 --- a/bon-macros/src/util/iterator.rs +++ b/bon-macros/src/util/iterator.rs @@ -2,13 +2,6 @@ use crate::util::prelude::*; use std::fmt::Write; pub(crate) trait IteratorExt: Iterator + Sized { - fn try_collect(self) -> Result - where - Result: FromIterator, - { - self.collect() - } - /// Based on itertools: /// fn join(mut self, sep: &str) -> String diff --git a/bon-macros/src/util/ty/mod.rs b/bon-macros/src/util/ty/mod.rs index 7cf5fb12..772b643b 100644 --- a/bon-macros/src/util/ty/mod.rs +++ b/bon-macros/src/util/ty/mod.rs @@ -65,13 +65,13 @@ impl TypeExt for syn::Type { fn type_param(&self, desired_type: &str) -> Option<&syn::Type> { let path = self.as_path()?; - let vec_segment = path + let segment = path .path .segments .iter() .find(|&segment| segment.ident == desired_type)?; - let args = match &vec_segment.arguments { + let args = match &segment.arguments { syn::PathArguments::AngleBracketed(args) => args, _ => return None, }; diff --git a/bon/src/private/mod.rs b/bon/src/private/mod.rs index 4c834f98..41b5eef0 100644 --- a/bon/src/private/mod.rs +++ b/bon/src/private/mod.rs @@ -106,7 +106,7 @@ impl MemberState for Unset { /// removed by the time the `#[builder]`'s macro expansion is invoked. /// /// It is a problem because the `#[builder]` macro needs to know the exact list -/// of members it has to generate setters for. It doesn't know whether the +/// of members it has to generate setters for. It doesn't know whether /// the `windows` predicate evaluates to `true` or `false`, especially if this was /// a more complex predicate. So it can't decide whether to generate a setter for /// the `windows_only_param` or not. diff --git a/bon/tests/integration/builder/mod.rs b/bon/tests/integration/builder/mod.rs index 11b0f6c9..ff16dda5 100644 --- a/bon/tests/integration/builder/mod.rs +++ b/bon/tests/integration/builder/mod.rs @@ -11,6 +11,7 @@ mod lints; mod many_params; mod name_conflicts; mod raw_idents; +mod positional_members; mod smoke; /// Tests for the deprecated features that we still support, but that we'll @@ -43,14 +44,29 @@ fn lifetime_elision() { #[cfg(feature = "std")] #[tokio::test] async fn async_func() { - // FIXME: https://github.com/elastio/bon/issues/100 - #![allow(clippy::future_not_send)] + #[builder] + async fn sut(arg: u32) -> u32 { + std::future::ready(arg).await + } + + let actual = sut().arg(42).call().await; + assert_eq!(actual, 42); +} +#[cfg(feature = "std")] +#[tokio::test] +async fn async_func_with_future_arg() { #[builder] - async fn sut(fut: Fut) -> Fut::Output { + async fn sut(fut: Fut) -> Fut::Output { fut.await } + fn is_send(_val: impl Send + Sync) {} + + let fut = sut().fut(std::future::ready(42)).call(); + + is_send(fut); + let actual = sut().fut(async { 42 }).call().await; assert_eq!(actual, 42); } diff --git a/bon/tests/integration/builder/positional_members.rs b/bon/tests/integration/builder/positional_members.rs new file mode 100644 index 00000000..dbf84366 --- /dev/null +++ b/bon/tests/integration/builder/positional_members.rs @@ -0,0 +1,23 @@ +use crate::prelude::*; + +#[test] +fn smoke() { + #[derive(Builder)] + struct Sut { + #[builder(pos = start_fn)] + starter_1: bool, + starter_2: char, + + named: u32, + + #[builder(pos = finish_fn)] + finisher_1: &'static str, + + #[builder(pos = finish_fn)] + finisher_2: &'static str, + } + + Sut::builder(true, 'c') + .named(99) + .build("1", "2"); +} From 1eea830803dabb220ff1988cb9e48d7bd98a0aa6 Mon Sep 17 00:00:00 2001 From: Veetaha Date: Tue, 10 Sep 2024 23:39:47 +0000 Subject: [PATCH 02/21] Add support for positional parameters --- .../builder_gen/member/into_conversion.rs | 11 +- bon-macros/src/builder/builder_gen/mod.rs | 17 ++- .../src/builder/builder_gen/setter_methods.rs | 7 + .../integration/builder/positional_members.rs | 134 +++++++++++++++++- .../integration/ui/compile_fail/errors.stderr | 18 +-- 5 files changed, 160 insertions(+), 27 deletions(-) diff --git a/bon-macros/src/builder/builder_gen/member/into_conversion.rs b/bon-macros/src/builder/builder_gen/member/into_conversion.rs index 2d4c6e95..7705d9e0 100644 --- a/bon-macros/src/builder/builder_gen/member/into_conversion.rs +++ b/bon-macros/src/builder/builder_gen/member/into_conversion.rs @@ -31,20 +31,21 @@ impl PositionalFnArgMember { is_into_enabled(self.origin, &self.params, scrutinee, conditional_params) } - pub(crate) fn fn_input_type( + pub(crate) fn fn_input_param( &self, conditional_params: &[ConditionalParams], ) -> Result { let has_into = self.param_into(conditional_params)?; let norm_ty = &self.norm_ty; + let ident = &self.ident; - let ty = if has_into { - quote! { impl Into<#norm_ty> } + let input = if has_into { + quote! { #ident: impl Into<#norm_ty> } } else { - quote! { #norm_ty } + quote! { #ident: #norm_ty } }; - Ok(ty) + Ok(input) } pub(crate) fn maybe_into_ident_expr( diff --git a/bon-macros/src/builder/builder_gen/mod.rs b/bon-macros/src/builder/builder_gen/mod.rs index b5efa3c1..331ee5d8 100644 --- a/bon-macros/src/builder/builder_gen/mod.rs +++ b/bon-macros/src/builder/builder_gen/mod.rs @@ -317,7 +317,10 @@ impl BuilderGenCtx { } }); - let receiver = receiver.map(|receiver| &receiver.with_self_keyword); + let receiver = receiver.map(|receiver| { + let receiver = &receiver.with_self_keyword; + quote! { #receiver, } + }); let unset_state_literals = self.named_members().map(|member| { if member.is_optional() { @@ -327,9 +330,9 @@ impl BuilderGenCtx { } }); - let start_fn_args = self + let start_fn_params = self .start_fn_args() - .map(|member| member.base.fn_input_type(&self.conditional_params)) + .map(|member| member.base.fn_input_param(&self.conditional_params)) .collect::>>()?; let start_fn_arg_exprs = self @@ -359,7 +362,7 @@ impl BuilderGenCtx { )] #vis fn #start_func_ident<#(#generics_decl),*>( #receiver - #(#start_fn_args,)* + #(#start_fn_params,)* ) -> #builder_ident<#(#generic_args,)*> #where_clause { @@ -653,11 +656,11 @@ impl BuilderGenCtx { } }); - let finish_fn_args = self + let finish_fn_params = self .members .iter() .filter_map(Member::as_finish_fn_arg) - .map(|member| member.fn_input_type(&self.conditional_params)) + .map(|member| member.fn_input_param(&self.conditional_params)) .collect::>>()?; Ok(quote! { @@ -676,7 +679,7 @@ impl BuilderGenCtx { #must_use #vis #asyncness #unsafety fn #finish_func_ident( self, - #(#finish_fn_args,)* + #(#finish_fn_params,)* ) #output where #(#where_bounds,)* diff --git a/bon-macros/src/builder/builder_gen/setter_methods.rs b/bon-macros/src/builder/builder_gen/setter_methods.rs index fd44a87e..bafe9bb9 100644 --- a/bon-macros/src/builder/builder_gen/setter_methods.rs +++ b/bon-macros/src/builder/builder_gen/setter_methods.rs @@ -142,6 +142,12 @@ impl<'a> MemberSettersCtx<'a> { .receiver() .map(|_| quote!(__private_receiver: self.__private_receiver,)); + let maybe_start_fn_args_field = self + .builder_gen + .start_fn_args() + .next() + .map(|_| quote!(__private_start_fn_args: self.__private_start_fn_args,)); + let builder_ident = &self.builder_gen.builder_ident; let member_exprs = self.builder_gen.named_members().map(|other_member| { @@ -156,6 +162,7 @@ impl<'a> MemberSettersCtx<'a> { #builder_ident { __private_phantom: ::core::marker::PhantomData, #maybe_receiver_field + #maybe_start_fn_args_field __private_named_members: (#( #member_exprs, )*) } } diff --git a/bon/tests/integration/builder/positional_members.rs b/bon/tests/integration/builder/positional_members.rs index dbf84366..ed540da7 100644 --- a/bon/tests/integration/builder/positional_members.rs +++ b/bon/tests/integration/builder/positional_members.rs @@ -1,23 +1,145 @@ use crate::prelude::*; +struct IntoStrRef<'a>(&'a str); + +impl<'a> From> for &'a str { + fn from(val: IntoStrRef<'a>) -> Self { + val.0 + } +} + +struct IntoChar(char); + +impl From for char { + fn from(val: IntoChar) -> Self { + val.0 + } +} + #[test] -fn smoke() { - #[derive(Builder)] +fn smoke_struct() { + #[derive(Debug, Builder)] + #[allow(dead_code)] struct Sut { #[builder(pos = start_fn)] starter_1: bool, + + #[builder(pos = start_fn, into)] starter_2: char, + #[builder(pos = start_fn, into)] + starter_3: Option<&'static str>, + named: u32, #[builder(pos = finish_fn)] finisher_1: &'static str, - #[builder(pos = finish_fn)] + #[builder(pos = finish_fn, into)] finisher_2: &'static str, } - Sut::builder(true, 'c') - .named(99) - .build("1", "2"); + assert_debug_eq( + Sut::builder(true, IntoChar('c'), None) + .named(99) + .build("1", IntoStrRef("2")), + expect![[r#" + Sut { + starter_1: true, + starter_2: 'c', + starter_3: None, + named: 99, + finisher_1: "1", + finisher_2: "2", + }"#]], + ); + + let _ = Sut::builder(true, 'c', "str"); +} + +#[test] +fn smoke_fn() { + #[builder] + fn sut( + #[builder(pos = start_fn)] starter_1: bool, + #[builder(pos = start_fn, into)] starter_2: char, + #[builder(pos = start_fn, into)] starter_3: Option<&'static str>, + named: u32, + #[builder(pos = finish_fn)] finisher_1: &'static str, + #[builder(pos = finish_fn, into)] finisher_2: &'static str, + ) -> ( + bool, + char, + Option<&'static str>, + u32, + &'static str, + &'static str, + ) { + ( + starter_1, starter_2, starter_3, named, finisher_1, finisher_2, + ) + } + + assert_debug_eq( + sut(true, IntoChar('c'), None) + .named(99) + .call("1", IntoStrRef("2")), + expect![[r#"(true, 'c', None, 99, "1", "2")"#]], + ); + + let _ = sut(true, 'c', "str"); +} + +#[test] +fn smoke_impl_block() { + struct Sut; + + #[bon] + impl Sut { + #[builder] + fn sut( + #[builder(pos = start_fn)] starter_1: bool, + #[builder(pos = start_fn, into)] starter_2: char, + #[builder(pos = start_fn, into)] starter_3: Option<&'static str>, + named: u32, + #[builder(pos = finish_fn)] finisher_1: &'static str, + #[builder(pos = finish_fn, into)] finisher_2: &'static str, + ) -> ( + bool, + char, + Option<&'static str>, + u32, + &'static str, + &'static str, + ) { + ( + starter_1, starter_2, starter_3, named, finisher_1, finisher_2, + ) + } + + #[builder] + fn with_self( + &self, + #[builder(pos = start_fn)] starter_1: bool, + named: u32, + #[builder(pos = finish_fn)] finisher_1: &'static str, + ) -> (bool, u32, &'static str) { + let _ = self; + (starter_1, named, finisher_1) + } + } + + assert_debug_eq( + Sut::sut(true, IntoChar('c'), None) + .named(99) + .call("1", IntoStrRef("2")), + expect![[r#"(true, 'c', None, 99, "1", "2")"#]], + ); + + let _ = Sut::sut(true, 'c', "str"); + + assert_debug_eq( + Sut.with_self(true).named(99).call("1"), + expect![[r#"(true, 99, "1")"#]], + ); } diff --git a/bon/tests/integration/ui/compile_fail/errors.stderr b/bon/tests/integration/ui/compile_fail/errors.stderr index af30da0f..00e2a942 100644 --- a/bon/tests/integration/ui/compile_fail/errors.stderr +++ b/bon/tests/integration/ui/compile_fail/errors.stderr @@ -94,23 +94,23 @@ error: expected at least one parameter in parentheses 85 | #[builder(start_fn())] | ^^^^^^^^ -error: `skip` attribute can't be specified with other attributes like `into` because there will be no setter generated for this member to configure - --> tests/integration/ui/compile_fail/errors.rs:90:21 +error: `skip` attribute can't be specified with other attributes like `into` + --> tests/integration/ui/compile_fail/errors.rs:90:15 | 90 | #[builder(skip, into)] - | ^^^^ + | ^^^^ -error: `skip` attribute can't be specified with other attributes like `name` because there will be no setter generated for this member to configure - --> tests/integration/ui/compile_fail/errors.rs:96:28 +error: `skip` attribute can't be specified with other attributes like `name` + --> tests/integration/ui/compile_fail/errors.rs:96:15 | 96 | #[builder(skip, name = bar)] - | ^^^ + | ^^^^ -error: `skip` attribute can't be specified with other attributes like `default` because there will be no setter generated for this member to configure. If you wanted to specify a value for the member, then use the following syntax instead `#[builder(skip = value)]` - --> tests/integration/ui/compile_fail/errors.rs:102:21 +error: `skip` attribute can't be specified with `default` attribute; if you wanted to specify a value for the member, then use the following syntax instead `#[builder(skip = value)]` + --> tests/integration/ui/compile_fail/errors.rs:102:15 | 102 | #[builder(skip, default = 42)] - | ^^^^^^^ + | ^^^^ error: `skip` attribute is not supported on function arguments. Use a local variable instead. --> tests/integration/ui/compile_fail/errors.rs:108:15 From d49c60fd77ac554cbb5848b2cd622a7d430e5a08 Mon Sep 17 00:00:00 2001 From: Veetaha Date: Wed, 11 Sep 2024 00:11:54 +0000 Subject: [PATCH 03/21] cargo fmt --- bon/tests/integration/builder/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bon/tests/integration/builder/mod.rs b/bon/tests/integration/builder/mod.rs index ff16dda5..f5b56ce5 100644 --- a/bon/tests/integration/builder/mod.rs +++ b/bon/tests/integration/builder/mod.rs @@ -10,8 +10,8 @@ mod init_order; mod lints; mod many_params; mod name_conflicts; -mod raw_idents; mod positional_members; +mod raw_idents; mod smoke; /// Tests for the deprecated features that we still support, but that we'll From 65bdc95bb2cd4f91ebe81e02f6a21871265a63f2 Mon Sep 17 00:00:00 2001 From: Veetaha Date: Wed, 11 Sep 2024 00:14:28 +0000 Subject: [PATCH 04/21] Fix grammar --- bon-macros/src/builder/builder_gen/member/into_conversion.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bon-macros/src/builder/builder_gen/member/into_conversion.rs b/bon-macros/src/builder/builder_gen/member/into_conversion.rs index 7705d9e0..177122f0 100644 --- a/bon-macros/src/builder/builder_gen/member/into_conversion.rs +++ b/bon-macros/src/builder/builder_gen/member/into_conversion.rs @@ -24,7 +24,7 @@ impl NamedMember { impl PositionalFnArgMember { pub(crate) fn param_into(&self, conditional_params: &[ConditionalParams]) -> Result { // Positional members are never optional. Users must always specify them, so there - // is no need to us to look into the `Option` generic parameter, because the + // is no need for us to look into the `Option` generic parameter, because the // `Option` itself is the target of the into conversion, not the `T` inside it. let scrutinee = self.orig_ty.as_ref(); From 60f7cb5509c6abe4fecf43dcc3083699e1a33e13 Mon Sep 17 00:00:00 2001 From: Veetaha Date: Wed, 11 Sep 2024 20:27:23 +0000 Subject: [PATCH 05/21] Rename `pos` to `source` --- .../src/builder/builder_gen/member/mod.rs | 196 +----------------- .../src/builder/builder_gen/member/params.rs | 187 +++++++++++++++++ bon-macros/src/util/mod.rs | 3 +- .../integration/builder/positional_members.rs | 34 +-- 4 files changed, 211 insertions(+), 209 deletions(-) create mode 100644 bon-macros/src/builder/builder_gen/member/params.rs diff --git a/bon-macros/src/builder/builder_gen/member/mod.rs b/bon-macros/src/builder/builder_gen/member/mod.rs index b8e6f5c2..40b9b4d0 100644 --- a/bon-macros/src/builder/builder_gen/member/mod.rs +++ b/bon-macros/src/builder/builder_gen/member/mod.rs @@ -1,11 +1,12 @@ mod into_conversion; +mod params; use crate::util::prelude::*; use darling::util::SpannedValue; -use darling::{FromAttributes, FromMeta}; +use darling::FromAttributes; +use params::{MemberInputSource, MemberParams}; use quote::quote; use std::fmt; -use syn::spanned::Spanned; #[derive(Debug, Clone, Copy)] pub(crate) enum MemberOrigin { @@ -122,154 +123,6 @@ pub(crate) struct SkippedMember { pub(crate) value: SpannedValue>, } -#[derive(Debug, Clone, darling::FromAttributes)] -#[darling(attributes(builder))] -pub(crate) struct MemberParams { - /// Enables an `Into` conversion for the setter method. - into: darling::util::Flag, - - /// Assign a default value to the member it it's not specified. - /// - /// An optional expression can be provided to set the value for the member, - /// otherwise its [`Default`] trait impl will be used. - #[darling(with = parse_optional_expression, map = Some)] - default: Option>>, - - /// Skip generating a setter method for this member. - /// - /// An optional expression can be provided to set the value for the member, - /// otherwise its [`Default`] trait impl will be used. - #[darling(with = parse_optional_expression, map = Some)] - skip: Option>>, - - /// Rename the name exposed in the builder API. - name: Option, - - /// Where to place the member in the generate builder methods API. - /// By default the member is treated like a named parameter that - /// gets its own setter methods. - pos: Option, -} - -#[derive(PartialEq, Eq, Clone, Copy)] -enum MemberParamName { - Default, - Into, - Name, - Pos, - Skip, -} - -impl fmt::Display for MemberParamName { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let str = match self { - Self::Default => "default", - Self::Into => "into", - Self::Name => "name", - Self::Pos => "pos", - Self::Skip => "skip", - }; - f.write_str(str) - } -} - -impl MemberParams { - fn validate_mutually_allowed( - &self, - attr_name: MemberParamName, - attr_span: Span, - allowed: &[MemberParamName], - ) -> Result<()> { - let conflicting: Vec<_> = self - .specified_param_names() - .filter(|name| *name != attr_name && !allowed.contains(name)) - .collect(); - - if conflicting.is_empty() { - return Ok(()); - } - - let conflicting = conflicting - .iter() - .map(|name| format!("`{name}`")) - .join(", "); - - bail!( - &attr_span, - "`{attr_name}` attribute can't be specified with other \ - attributes like {}", - conflicting, - ); - } - - fn specified_param_names(&self) -> impl Iterator { - let Self { - into, - default, - skip, - name, - pos, - } = self; - - let attrs = [ - (default.is_some(), MemberParamName::Default), - (name.is_some(), MemberParamName::Name), - (into.is_present(), MemberParamName::Into), - (pos.is_some(), MemberParamName::Pos), - (skip.is_some(), MemberParamName::Skip), - ]; - - attrs - .into_iter() - .filter(|(is_present, _)| *is_present) - .map(|(_, name)| name) - } - - fn validate(&self, origin: MemberOrigin) -> Result { - if let Some(pos) = self.pos { - self.validate_mutually_allowed( - MemberParamName::Pos, - pos.span(), - &[MemberParamName::Into], - )?; - } - - if let Some(skip) = &self.skip { - match origin { - MemberOrigin::FnArg => { - bail!( - &skip.span(), - "`skip` attribute is not supported on function arguments. \ - Use a local variable instead.", - ); - } - MemberOrigin::StructField => {} - } - - if let Some(Some(_expr)) = self.default.as_deref() { - bail!( - &skip.span(), - "`skip` attribute can't be specified with `default` attribute; \ - if you wanted to specify a value for the member, then use \ - the following syntax instead `#[builder(skip = value)]`", - ); - } - - self.validate_mutually_allowed(MemberParamName::Skip, skip.span(), &[])?; - } - - Ok(()) - } -} - -fn parse_optional_expression(meta: &syn::Meta) -> Result>> { - match meta { - syn::Meta::Path(_) => Ok(SpannedValue::new(None, meta.span())), - syn::Meta::List(_) => Err(Error::unsupported_format("list").with_span(meta)), - syn::Meta::NameValue(nv) => Ok(SpannedValue::new(Some(nv.value.clone()), nv.span())), - } -} - impl NamedMember { fn validate(&self) -> Result { super::reject_self_references_in_docs(&self.docs)?; @@ -360,7 +213,7 @@ impl Member { })); } - if let Some(pos) = params.pos { + if let Some(pos) = params.source { let base = PositionalFnArgMember { origin, ident: orig_ident, @@ -369,12 +222,12 @@ impl Member { params, }; match pos { - PosMemberLocation::StartFn(_) => { + MemberInputSource::StartFn(_) => { let index = start_fn_arg_count.into(); start_fn_arg_count += 1; return Ok(Self::StartFnArg(StartFnArgMember { base, index })); } - PosMemberLocation::FinishFn(_) => { + MemberInputSource::FinishFn(_) => { return Ok(Self::FinishFnArg(base)); } } @@ -465,40 +318,3 @@ impl Member { } } } - -#[derive(Debug, Copy, Clone)] -enum PosMemberLocation { - /// The member must be a positional argument in the start function - StartFn(Span), - - /// The member must be a positional argument in the finish function - FinishFn(Span), -} - -impl FromMeta for PosMemberLocation { - fn from_expr(expr: &syn::Expr) -> Result { - let err = || err!(expr, "expected one of `start_fn` or `finish_fn`"); - let path = match expr { - syn::Expr::Path(path) if path.attrs.is_empty() && path.qself.is_none() => path, - _ => return Err(err()), - }; - - let ident = path.path.get_ident().ok_or_else(err)?.to_string(); - - let kind = match ident.as_str() { - "start_fn" => Self::StartFn, - "finish_fn" => Self::FinishFn, - _ => return Err(err()), - }; - - Ok(kind(path.span())) - } -} - -impl PosMemberLocation { - fn span(self) -> Span { - match self { - Self::StartFn(span) | Self::FinishFn(span) => span, - } - } -} diff --git a/bon-macros/src/builder/builder_gen/member/params.rs b/bon-macros/src/builder/builder_gen/member/params.rs new file mode 100644 index 00000000..98f70bad --- /dev/null +++ b/bon-macros/src/builder/builder_gen/member/params.rs @@ -0,0 +1,187 @@ +use super::MemberOrigin; +use crate::util::prelude::*; +use darling::util::SpannedValue; +use darling::FromMeta; +use std::fmt; +use syn::spanned::Spanned; + +#[derive(Debug, Clone, darling::FromAttributes)] +#[darling(attributes(builder))] +pub(crate) struct MemberParams { + /// Enables an `Into` conversion for the setter method. + pub(crate) into: darling::util::Flag, + + /// Assign a default value to the member it it's not specified. + /// + /// An optional expression can be provided to set the value for the member, + /// otherwise its [`Default`] trait impl will be used. + #[darling(with = parse_optional_expression, map = Some)] + pub(crate) default: Option>>, + + /// Skip generating a setter method for this member. + /// + /// An optional expression can be provided to set the value for the member, + /// otherwise its [`Default`] trait impl will be used. + #[darling(with = parse_optional_expression, map = Some)] + pub(crate) skip: Option>>, + + /// Rename the name exposed in the builder API. + pub(crate) name: Option, + + /// Where to place the member in the generate builder methods API. + /// By default the member is treated like a named parameter that + /// gets its own setter methods. + pub(crate) source: Option, +} + +#[derive(PartialEq, Eq, Clone, Copy)] +enum ParamName { + Default, + Into, + Name, + Source, + Skip, +} + +impl fmt::Display for ParamName { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let str = match self { + Self::Default => "default", + Self::Into => "into", + Self::Name => "name", + Self::Source => "source", + Self::Skip => "skip", + }; + f.write_str(str) + } +} + +impl MemberParams { + fn validate_mutually_allowed( + &self, + attr_name: ParamName, + attr_span: Span, + allowed: &[ParamName], + ) -> Result<()> { + let conflicting: Vec<_> = self + .specified_param_names() + .filter(|name| *name != attr_name && !allowed.contains(name)) + .collect(); + + if conflicting.is_empty() { + return Ok(()); + } + + let conflicting = conflicting + .iter() + .map(|name| format!("`{name}`")) + .join(", "); + + bail!( + &attr_span, + "`{attr_name}` attribute can't be specified with other \ + attributes like {}", + conflicting, + ); + } + + fn specified_param_names(&self) -> impl Iterator { + let Self { + into, + default, + skip, + name, + source: pos, + } = self; + + let attrs = [ + (default.is_some(), ParamName::Default), + (name.is_some(), ParamName::Name), + (into.is_present(), ParamName::Into), + (pos.is_some(), ParamName::Source), + (skip.is_some(), ParamName::Skip), + ]; + + attrs + .into_iter() + .filter(|(is_present, _)| *is_present) + .map(|(_, name)| name) + } + + pub(crate) fn validate(&self, origin: MemberOrigin) -> Result { + if let Some(pos) = self.source { + self.validate_mutually_allowed(ParamName::Source, pos.span(), &[ParamName::Into])?; + } + + if let Some(skip) = &self.skip { + match origin { + MemberOrigin::FnArg => { + bail!( + &skip.span(), + "`skip` attribute is not supported on function arguments. \ + Use a local variable instead.", + ); + } + MemberOrigin::StructField => {} + } + + if let Some(Some(_expr)) = self.default.as_deref() { + bail!( + &skip.span(), + "`skip` attribute can't be specified with `default` attribute; \ + if you wanted to specify a value for the member, then use \ + the following syntax instead `#[builder(skip = value)]`", + ); + } + + self.validate_mutually_allowed(ParamName::Skip, skip.span(), &[])?; + } + + Ok(()) + } +} + +fn parse_optional_expression(meta: &syn::Meta) -> Result>> { + match meta { + syn::Meta::Path(_) => Ok(SpannedValue::new(None, meta.span())), + syn::Meta::List(_) => Err(Error::unsupported_format("list").with_span(meta)), + syn::Meta::NameValue(nv) => Ok(SpannedValue::new(Some(nv.value.clone()), nv.span())), + } +} + +#[derive(Debug, Copy, Clone)] +pub(crate) enum MemberInputSource { + /// The member must be a positional argument in the start function + StartFn(Span), + + /// The member must be a positional argument in the finish function + FinishFn(Span), +} + +impl FromMeta for MemberInputSource { + fn from_expr(expr: &syn::Expr) -> Result { + let err = || err!(expr, "expected one of `start_fn` or `finish_fn`"); + let path = match expr { + syn::Expr::Path(path) if path.attrs.is_empty() && path.qself.is_none() => path, + _ => return Err(err()), + }; + + let ident = path.path.get_ident().ok_or_else(err)?.to_string(); + + let kind = match ident.as_str() { + "start_fn" => Self::StartFn, + "finish_fn" => Self::FinishFn, + _ => return Err(err()), + }; + + Ok(kind(path.span())) + } +} + +impl MemberInputSource { + fn span(self) -> Span { + match self { + Self::StartFn(span) | Self::FinishFn(span) => span, + } + } +} diff --git a/bon-macros/src/util/mod.rs b/bon-macros/src/util/mod.rs index efc464db..d212745e 100644 --- a/bon-macros/src/util/mod.rs +++ b/bon-macros/src/util/mod.rs @@ -28,8 +28,7 @@ pub(crate) mod prelude { pub(crate) use super::attrs::AttributeExt; pub(crate) use super::fn_arg::FnArgExt; pub(crate) use super::ident::IdentExt; - pub(crate) use super::iterator::IntoIteratorExt; - pub(crate) use super::iterator::IteratorExt; + pub(crate) use super::iterator::{IntoIteratorExt, IteratorExt}; pub(crate) use super::path::PathExt; pub(crate) use super::punctuated::PunctuatedExt; pub(crate) use super::ty::TypeExt; diff --git a/bon/tests/integration/builder/positional_members.rs b/bon/tests/integration/builder/positional_members.rs index ed540da7..0f371a86 100644 --- a/bon/tests/integration/builder/positional_members.rs +++ b/bon/tests/integration/builder/positional_members.rs @@ -21,21 +21,21 @@ fn smoke_struct() { #[derive(Debug, Builder)] #[allow(dead_code)] struct Sut { - #[builder(pos = start_fn)] + #[builder(source = start_fn)] starter_1: bool, - #[builder(pos = start_fn, into)] + #[builder(source = start_fn, into)] starter_2: char, - #[builder(pos = start_fn, into)] + #[builder(source = start_fn, into)] starter_3: Option<&'static str>, named: u32, - #[builder(pos = finish_fn)] + #[builder(source = finish_fn)] finisher_1: &'static str, - #[builder(pos = finish_fn, into)] + #[builder(source = finish_fn, into)] finisher_2: &'static str, } @@ -61,12 +61,12 @@ fn smoke_struct() { fn smoke_fn() { #[builder] fn sut( - #[builder(pos = start_fn)] starter_1: bool, - #[builder(pos = start_fn, into)] starter_2: char, - #[builder(pos = start_fn, into)] starter_3: Option<&'static str>, + #[builder(source = start_fn)] starter_1: bool, + #[builder(source = start_fn, into)] starter_2: char, + #[builder(source = start_fn, into)] starter_3: Option<&'static str>, named: u32, - #[builder(pos = finish_fn)] finisher_1: &'static str, - #[builder(pos = finish_fn, into)] finisher_2: &'static str, + #[builder(source = finish_fn)] finisher_1: &'static str, + #[builder(source = finish_fn, into)] finisher_2: &'static str, ) -> ( bool, char, @@ -98,12 +98,12 @@ fn smoke_impl_block() { impl Sut { #[builder] fn sut( - #[builder(pos = start_fn)] starter_1: bool, - #[builder(pos = start_fn, into)] starter_2: char, - #[builder(pos = start_fn, into)] starter_3: Option<&'static str>, + #[builder(source = start_fn)] starter_1: bool, + #[builder(source = start_fn, into)] starter_2: char, + #[builder(source = start_fn, into)] starter_3: Option<&'static str>, named: u32, - #[builder(pos = finish_fn)] finisher_1: &'static str, - #[builder(pos = finish_fn, into)] finisher_2: &'static str, + #[builder(source = finish_fn)] finisher_1: &'static str, + #[builder(source = finish_fn, into)] finisher_2: &'static str, ) -> ( bool, char, @@ -120,9 +120,9 @@ fn smoke_impl_block() { #[builder] fn with_self( &self, - #[builder(pos = start_fn)] starter_1: bool, + #[builder(source = start_fn)] starter_1: bool, named: u32, - #[builder(pos = finish_fn)] finisher_1: &'static str, + #[builder(source = finish_fn)] finisher_1: &'static str, ) -> (bool, u32, &'static str) { let _ = self; (starter_1, named, finisher_1) From 66b72b6a98450f579fe0bb4ade95fcd52639529f Mon Sep 17 00:00:00 2001 From: Veetaha Date: Wed, 11 Sep 2024 22:44:56 +0000 Subject: [PATCH 06/21] Use `#[builder(finish_fn/start_fn)]` syntax --- .../src/builder/builder_gen/member/mod.rs | 22 +++--- .../src/builder/builder_gen/member/params.rs | 71 +++++++------------ .../integration/builder/positional_members.rs | 34 ++++----- 3 files changed, 53 insertions(+), 74 deletions(-) diff --git a/bon-macros/src/builder/builder_gen/member/mod.rs b/bon-macros/src/builder/builder_gen/member/mod.rs index 40b9b4d0..b29fd0ae 100644 --- a/bon-macros/src/builder/builder_gen/member/mod.rs +++ b/bon-macros/src/builder/builder_gen/member/mod.rs @@ -4,7 +4,7 @@ mod params; use crate::util::prelude::*; use darling::util::SpannedValue; use darling::FromAttributes; -use params::{MemberInputSource, MemberParams}; +use params::MemberParams; use quote::quote; use std::fmt; @@ -213,7 +213,9 @@ impl Member { })); } - if let Some(pos) = params.source { + if params.finish_fn.is_present() || params.start_fn.is_present() { + let is_finish_fn = params.finish_fn.is_present(); + let base = PositionalFnArgMember { origin, ident: orig_ident, @@ -221,16 +223,14 @@ impl Member { orig_ty, params, }; - match pos { - MemberInputSource::StartFn(_) => { - let index = start_fn_arg_count.into(); - start_fn_arg_count += 1; - return Ok(Self::StartFnArg(StartFnArgMember { base, index })); - } - MemberInputSource::FinishFn(_) => { - return Ok(Self::FinishFnArg(base)); - } + + if is_finish_fn { + return Ok(Self::FinishFnArg(base)); } + + let index = start_fn_arg_count.into(); + start_fn_arg_count += 1; + return Ok(Self::StartFnArg(StartFnArgMember { base, index })); } // XXX: docs are collected only for named members. There is obvious diff --git a/bon-macros/src/builder/builder_gen/member/params.rs b/bon-macros/src/builder/builder_gen/member/params.rs index 98f70bad..eeec0874 100644 --- a/bon-macros/src/builder/builder_gen/member/params.rs +++ b/bon-macros/src/builder/builder_gen/member/params.rs @@ -1,7 +1,6 @@ use super::MemberOrigin; use crate::util::prelude::*; use darling::util::SpannedValue; -use darling::FromMeta; use std::fmt; use syn::spanned::Spanned; @@ -28,10 +27,11 @@ pub(crate) struct MemberParams { /// Rename the name exposed in the builder API. pub(crate) name: Option, - /// Where to place the member in the generate builder methods API. + /// Where to place the member in the generated builder methods API. /// By default the member is treated like a named parameter that /// gets its own setter methods. - pub(crate) source: Option, + pub(crate) start_fn: darling::util::Flag, + pub(crate) finish_fn: darling::util::Flag, } #[derive(PartialEq, Eq, Clone, Copy)] @@ -39,8 +39,9 @@ enum ParamName { Default, Into, Name, - Source, Skip, + StartFn, + FinishFn, } impl fmt::Display for ParamName { @@ -49,8 +50,9 @@ impl fmt::Display for ParamName { Self::Default => "default", Self::Into => "into", Self::Name => "name", - Self::Source => "source", Self::Skip => "skip", + Self::StartFn => "start_fn", + Self::FinishFn => "finish_fn", }; f.write_str(str) } @@ -91,15 +93,17 @@ impl MemberParams { default, skip, name, - source: pos, + finish_fn, + start_fn, } = self; let attrs = [ (default.is_some(), ParamName::Default), (name.is_some(), ParamName::Name), (into.is_present(), ParamName::Into), - (pos.is_some(), ParamName::Source), (skip.is_some(), ParamName::Skip), + (start_fn.is_present(), ParamName::StartFn), + (finish_fn.is_present(), ParamName::FinishFn), ]; attrs @@ -109,8 +113,20 @@ impl MemberParams { } pub(crate) fn validate(&self, origin: MemberOrigin) -> Result { - if let Some(pos) = self.source { - self.validate_mutually_allowed(ParamName::Source, pos.span(), &[ParamName::Into])?; + if self.start_fn.is_present() { + self.validate_mutually_allowed( + ParamName::StartFn, + self.start_fn.span(), + &[ParamName::Into], + )?; + } + + if self.finish_fn.is_present() { + self.validate_mutually_allowed( + ParamName::FinishFn, + self.finish_fn.span(), + &[ParamName::Into], + )?; } if let Some(skip) = &self.skip { @@ -148,40 +164,3 @@ fn parse_optional_expression(meta: &syn::Meta) -> Result Ok(SpannedValue::new(Some(nv.value.clone()), nv.span())), } } - -#[derive(Debug, Copy, Clone)] -pub(crate) enum MemberInputSource { - /// The member must be a positional argument in the start function - StartFn(Span), - - /// The member must be a positional argument in the finish function - FinishFn(Span), -} - -impl FromMeta for MemberInputSource { - fn from_expr(expr: &syn::Expr) -> Result { - let err = || err!(expr, "expected one of `start_fn` or `finish_fn`"); - let path = match expr { - syn::Expr::Path(path) if path.attrs.is_empty() && path.qself.is_none() => path, - _ => return Err(err()), - }; - - let ident = path.path.get_ident().ok_or_else(err)?.to_string(); - - let kind = match ident.as_str() { - "start_fn" => Self::StartFn, - "finish_fn" => Self::FinishFn, - _ => return Err(err()), - }; - - Ok(kind(path.span())) - } -} - -impl MemberInputSource { - fn span(self) -> Span { - match self { - Self::StartFn(span) | Self::FinishFn(span) => span, - } - } -} diff --git a/bon/tests/integration/builder/positional_members.rs b/bon/tests/integration/builder/positional_members.rs index 0f371a86..1912093a 100644 --- a/bon/tests/integration/builder/positional_members.rs +++ b/bon/tests/integration/builder/positional_members.rs @@ -21,21 +21,21 @@ fn smoke_struct() { #[derive(Debug, Builder)] #[allow(dead_code)] struct Sut { - #[builder(source = start_fn)] + #[builder(start_fn)] starter_1: bool, - #[builder(source = start_fn, into)] + #[builder(start_fn, into)] starter_2: char, - #[builder(source = start_fn, into)] + #[builder(start_fn, into)] starter_3: Option<&'static str>, named: u32, - #[builder(source = finish_fn)] + #[builder(finish_fn)] finisher_1: &'static str, - #[builder(source = finish_fn, into)] + #[builder(finish_fn, into)] finisher_2: &'static str, } @@ -61,12 +61,12 @@ fn smoke_struct() { fn smoke_fn() { #[builder] fn sut( - #[builder(source = start_fn)] starter_1: bool, - #[builder(source = start_fn, into)] starter_2: char, - #[builder(source = start_fn, into)] starter_3: Option<&'static str>, + #[builder(start_fn)] starter_1: bool, + #[builder(start_fn, into)] starter_2: char, + #[builder(start_fn, into)] starter_3: Option<&'static str>, named: u32, - #[builder(source = finish_fn)] finisher_1: &'static str, - #[builder(source = finish_fn, into)] finisher_2: &'static str, + #[builder(finish_fn)] finisher_1: &'static str, + #[builder(finish_fn, into)] finisher_2: &'static str, ) -> ( bool, char, @@ -98,12 +98,12 @@ fn smoke_impl_block() { impl Sut { #[builder] fn sut( - #[builder(source = start_fn)] starter_1: bool, - #[builder(source = start_fn, into)] starter_2: char, - #[builder(source = start_fn, into)] starter_3: Option<&'static str>, + #[builder(start_fn)] starter_1: bool, + #[builder(start_fn, into)] starter_2: char, + #[builder(start_fn, into)] starter_3: Option<&'static str>, named: u32, - #[builder(source = finish_fn)] finisher_1: &'static str, - #[builder(source = finish_fn, into)] finisher_2: &'static str, + #[builder(finish_fn)] finisher_1: &'static str, + #[builder(finish_fn, into)] finisher_2: &'static str, ) -> ( bool, char, @@ -120,9 +120,9 @@ fn smoke_impl_block() { #[builder] fn with_self( &self, - #[builder(source = start_fn)] starter_1: bool, + #[builder(start_fn)] starter_1: bool, named: u32, - #[builder(source = finish_fn)] finisher_1: &'static str, + #[builder(finish_fn)] finisher_1: &'static str, ) -> (bool, u32, &'static str) { let _ = self; (starter_1, named, finisher_1) From fb1d77e8190aab04fb55751d22e56a73952470b5 Mon Sep 17 00:00:00 2001 From: Veetaha Date: Thu, 12 Sep 2024 01:04:09 +0000 Subject: [PATCH 07/21] Update Clone/Debug impls --- .../builder/builder_gen/builder_derives.rs | 70 +++++++----- bon/src/private/mod.rs | 3 + .../integration/builder/builder_derives.rs | 106 +++++++++++++++++- bon/tests/integration/main.rs | 2 + .../ui/compile_fail/builder_derives.stderr | 90 +++++++++------ 5 files changed, 209 insertions(+), 62 deletions(-) diff --git a/bon-macros/src/builder/builder_gen/builder_derives.rs b/bon-macros/src/builder/builder_gen/builder_derives.rs index 98e57400..e0c6be36 100644 --- a/bon-macros/src/builder/builder_gen/builder_derives.rs +++ b/bon-macros/src/builder/builder_gen/builder_derives.rs @@ -1,4 +1,5 @@ use super::BuilderGenCtx; +use crate::builder::builder_gen::Member; use crate::util::prelude::*; use quote::quote; @@ -22,14 +23,7 @@ impl BuilderGenCtx { tokens } - /// These bounds are required to ensure that all members of the, - /// builder (including the receiver) implement the target trait, - /// so that there is no possible state of the builder that cannot - /// implement the target trait. - fn builder_components_trait_bounds<'a>( - &'a self, - trait_path: &'a TokenStream2, - ) -> impl Iterator + 'a { + fn builder_component_types(&self) -> impl Iterator { let receiver_ty = self .receiver() .map(|receiver| &receiver.without_self_keyword); @@ -39,11 +33,7 @@ impl BuilderGenCtx { std::iter::empty() .chain(receiver_ty) .chain(member_types) - .map(move |ty| { - quote! { - #ty: #trait_path - } - }) + .map(Box::as_ref) } fn derive_clone(&self) -> TokenStream2 { @@ -59,8 +49,15 @@ impl BuilderGenCtx { } }); + let clone_start_fn_args = self.start_fn_args().next().map(|_| { + quote! { + __private_start_fn_args: #clone::clone(&self.__private_start_fn_args), + } + }); + let builder_where_clause_predicates = self.generics.where_clause_predicates(); - let components_where_clause_predicates = self.builder_components_trait_bounds(&clone); + + let builder_component_types = self.builder_component_types(); quote! { #[automatically_derived] @@ -75,12 +72,13 @@ impl BuilderGenCtx { where #(#builder_where_clause_predicates,)* ___State: #clone, - #(#components_where_clause_predicates,)* { fn clone(&self) -> Self { + #(::bon::private::assert_clone::<#builder_component_types>();)* Self { __private_phantom: ::core::marker::PhantomData, #clone_receiver + #clone_start_fn_args __private_named_members: self.__private_named_members.clone(), } } @@ -102,7 +100,7 @@ impl BuilderGenCtx { }); let builder_where_clause_predicates = self.generics.where_clause_predicates(); - let components_where_clause_predicates = self.builder_components_trait_bounds(&debug); + let builder_component_types = self.builder_component_types(); let builder_ident_str = builder_ident.to_string(); @@ -111,15 +109,36 @@ impl BuilderGenCtx { .map(|member| &member.generic_var_ident) .collect::>(); - let format_members = self.named_members().map(|member| { - let member_index = &member.index; - let member_ident_str = member.orig_ident.to_string(); - - quote! { - // Skip members that are not set to reduce noise - if self.__private_named_members.#member_index.is_set() { - output.field(#member_ident_str, &self.__private_named_members.#member_index); + let format_members = self.members.iter().filter_map(|member| { + match member { + Member::Named(member) => { + let member_index = &member.index; + let member_ident_str = member.orig_ident.to_string(); + Some(quote! { + // Skip members that are not set to reduce noise + if self.__private_named_members.#member_index.is_set() { + output.field( + #member_ident_str, + &self.__private_named_members.#member_index + ); + } + }) + } + Member::StartFnArg(member) => { + let member_index = &member.index; + let member_ident_str = member.base.ident.to_string(); + Some(quote! { + output.field( + #member_ident_str, + &self.__private_start_fn_args.#member_index + ); + }) } + + // The values for these members are computed only in the finishing + // function where the builder is consumed, and they aren't stored + // in the builder itself. + Member::FinishFnArg(_) | Member::Skipped(_) => None, } }); @@ -135,10 +154,11 @@ impl BuilderGenCtx { > where #(#builder_where_clause_predicates,)* - #(#components_where_clause_predicates,)* #(#state_type_vars: ::bon::private::MemberState + ::core::fmt::Debug,)* { fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + #(::bon::private::assert_debug::<#builder_component_types>();)* + let mut output = f.debug_struct(#builder_ident_str); #format_receiver diff --git a/bon/src/private/mod.rs b/bon/src/private/mod.rs index 41b5eef0..0d5ea6a0 100644 --- a/bon/src/private/mod.rs +++ b/bon/src/private/mod.rs @@ -9,6 +9,9 @@ pub mod ide; #[cfg(feature = "alloc")] pub extern crate alloc; +pub const fn assert_clone() {} +pub const fn assert_debug() {} + /// Marker trait to denote the state of the member that is not set yet. #[rustversion::attr( since(1.78.0), diff --git a/bon/tests/integration/builder/builder_derives.rs b/bon/tests/integration/builder/builder_derives.rs index 1d745cf3..491df33b 100644 --- a/bon/tests/integration/builder/builder_derives.rs +++ b/bon/tests/integration/builder/builder_derives.rs @@ -1,3 +1,7 @@ +// We intentionally exercise cloning from `#[builder(derive(Clone))]` here +// to make sure that it works. +#![allow(clippy::redundant_clone)] + use crate::prelude::*; #[test] @@ -113,7 +117,6 @@ fn skipped_members() { _arg2: NoDebug, } - #[allow(clippy::redundant_clone)] let actual = Sut::builder().arg1(true).clone(); assert_debug_eq(actual, expect!["SutBuilder { _arg1: true }"]); @@ -130,3 +133,104 @@ fn empty_builder() { assert_debug_eq(actual, expect!["SutBuilder"]); } + +#[test] +fn positional_members_struct() { + #[derive(Builder)] + #[builder(derive(Clone, Debug))] + #[allow(dead_code)] + struct Sut { + #[builder(start_fn)] + start_fn_arg: bool, + + named: u32, + + #[builder(finish_fn)] + finish_fn_arg: &'static str, + } + + let actual = Sut::builder(true); + + assert_debug_eq(actual.clone(), expect!["SutBuilder { start_fn_arg: true }"]); + + assert_debug_eq( + actual.named(42).clone(), + expect!["SutBuilder { start_fn_arg: true, named: 42 }"], + ); +} + +#[test] +fn positional_members_fn() { + #[builder(derive(Clone, Debug))] + #[allow(unused_variables)] + fn sut( + #[builder(start_fn)] start_fn_arg: bool, + named: u32, + #[builder(finish_fn)] finish_fn_arg: &'static str, + ) { + } + + let actual = sut(true); + + assert_debug_eq(actual.clone(), expect!["SutBuilder { start_fn_arg: true }"]); + + assert_debug_eq( + actual.named(42).clone(), + expect!["SutBuilder { start_fn_arg: true, named: 42 }"], + ); +} + +#[test] +fn positional_members_impl_block() { + #[derive(Debug)] + struct Sut; + + #[bon] + #[allow(unused_variables)] + impl Sut { + #[builder(derive(Clone, Debug))] + fn sut( + #[builder(start_fn)] start_fn_arg: bool, + named: u32, + #[builder(finish_fn)] finish_fn_arg: &'static str, + ) { + } + + #[builder(derive(Clone, Debug))] + fn with_self( + &self, + #[builder(start_fn)] start_fn_arg: bool, + named: u32, + #[builder(finish_fn)] finish_fn_arg: &'static str, + ) { + let _ = self; + } + } + + let actual = Sut::sut(true); + + assert_debug_eq( + actual.clone(), + expect!["SutSutBuilder { start_fn_arg: true }"], + ); + assert_debug_eq( + actual.named(42).clone(), + expect!["SutSutBuilder { start_fn_arg: true, named: 42 }"], + ); + + let actual = Sut.with_self(true); + + assert_debug_eq( + actual.clone(), + expect!["SutWithSelfBuilder { self: Sut, start_fn_arg: true }"], + ); + assert_debug_eq( + actual.named(42).clone(), + expect![[r#" + SutWithSelfBuilder { + self: Sut, + start_fn_arg: true, + named: 42, + }"#]], + ); +} diff --git a/bon/tests/integration/main.rs b/bon/tests/integration/main.rs index 1bcc4419..faf3b05d 100644 --- a/bon/tests/integration/main.rs +++ b/bon/tests/integration/main.rs @@ -4,6 +4,8 @@ clippy::missing_const_for_fn, clippy::needless_pass_by_value, clippy::too_many_lines, + // `expect_test` sometimes adds redundant hashes, we just have to live with that + clippy::needless_raw_string_hashes, non_local_definitions )] diff --git a/bon/tests/integration/ui/compile_fail/builder_derives.stderr b/bon/tests/integration/ui/compile_fail/builder_derives.stderr index 3794a03f..8b75f339 100644 --- a/bon/tests/integration/ui/compile_fail/builder_derives.stderr +++ b/bon/tests/integration/ui/compile_fail/builder_derives.stderr @@ -1,41 +1,50 @@ error[E0277]: the trait bound `NoTraitImpls: Clone` is not satisfied - --> tests/integration/ui/compile_fail/builder_derives.rs:5:10 + --> tests/integration/ui/compile_fail/builder_derives.rs:8:16 | -5 | #[derive(Builder)] - | ^^^^^^^ the trait `Clone` is not implemented for `NoTraitImpls` +8 | non_debug: NoTraitImpls, + | ^^^^^^^^^^^^ the trait `Clone` is not implemented for `NoTraitImpls` | - = help: see issue #48214 - = note: this error originates in the derive macro `Builder` (in Nightly builds, run with -Z macro-backtrace for more info) -help: consider annotating `NoTraitImpls` with `#[derive(Clone)]` +note: required by a bound in `assert_clone` + --> src/private/mod.rs | -3 + #[derive(Clone)] -4 | struct NoTraitImpls; + | pub const fn assert_clone() {} + | ^^^^^ required by this bound in `assert_clone` +help: consider annotating `NoTraitImpls` with `#[derive(Clone)]` | +3 + #[derive(Clone)] +4 | struct NoTraitImpls; + | error[E0277]: `NoTraitImpls` doesn't implement `Debug` - --> tests/integration/ui/compile_fail/builder_derives.rs:5:10 + --> tests/integration/ui/compile_fail/builder_derives.rs:8:16 | -5 | #[derive(Builder)] - | ^^^^^^^ `NoTraitImpls` cannot be formatted using `{:?}` +8 | non_debug: NoTraitImpls, + | ^^^^^^^^^^^^ `NoTraitImpls` cannot be formatted using `{:?}` | = help: the trait `Debug` is not implemented for `NoTraitImpls` = note: add `#[derive(Debug)]` to `NoTraitImpls` or manually `impl Debug for NoTraitImpls` - = help: see issue #48214 - = note: this error originates in the derive macro `Builder` (in Nightly builds, run with -Z macro-backtrace for more info) -help: consider annotating `NoTraitImpls` with `#[derive(Debug)]` +note: required by a bound in `assert_debug` + --> src/private/mod.rs | -3 + #[derive(Debug)] -4 | struct NoTraitImpls; + | pub const fn assert_debug() {} + | ^^^^^^^^^^^^^^^^ required by this bound in `assert_debug` +help: consider annotating `NoTraitImpls` with `#[derive(Debug)]` | +3 + #[derive(Debug)] +4 | struct NoTraitImpls; + | error[E0277]: the trait bound `NoTraitImpls: Clone` is not satisfied - --> tests/integration/ui/compile_fail/builder_derives.rs:12:1 + --> tests/integration/ui/compile_fail/builder_derives.rs:13:38 + | +13 | fn fn_contains_non_trait(_non_debug: NoTraitImpls, _x: u32) {} + | ^^^^^^^^^^^^ the trait `Clone` is not implemented for `NoTraitImpls` | -12 | #[builder(derive(Clone, Debug))] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Clone` is not implemented for `NoTraitImpls` +note: required by a bound in `assert_clone` + --> src/private/mod.rs | - = help: see issue #48214 - = note: this error originates in the attribute macro `builder` (in Nightly builds, run with -Z macro-backtrace for more info) + | pub const fn assert_clone() {} + | ^^^^^ required by this bound in `assert_clone` help: consider annotating `NoTraitImpls` with `#[derive(Clone)]` | 3 + #[derive(Clone)] @@ -43,15 +52,18 @@ help: consider annotating `NoTraitImpls` with `#[derive(Clone)]` | error[E0277]: `NoTraitImpls` doesn't implement `Debug` - --> tests/integration/ui/compile_fail/builder_derives.rs:12:1 + --> tests/integration/ui/compile_fail/builder_derives.rs:13:38 | -12 | #[builder(derive(Clone, Debug))] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `NoTraitImpls` cannot be formatted using `{:?}` +13 | fn fn_contains_non_trait(_non_debug: NoTraitImpls, _x: u32) {} + | ^^^^^^^^^^^^ `NoTraitImpls` cannot be formatted using `{:?}` | = help: the trait `Debug` is not implemented for `NoTraitImpls` = note: add `#[derive(Debug)]` to `NoTraitImpls` or manually `impl Debug for NoTraitImpls` - = help: see issue #48214 - = note: this error originates in the attribute macro `builder` (in Nightly builds, run with -Z macro-backtrace for more info) +note: required by a bound in `assert_debug` + --> src/private/mod.rs + | + | pub const fn assert_debug() {} + | ^^^^^^^^^^^^^^^^ required by this bound in `assert_debug` help: consider annotating `NoTraitImpls` with `#[derive(Debug)]` | 3 + #[derive(Debug)] @@ -59,13 +71,16 @@ help: consider annotating `NoTraitImpls` with `#[derive(Debug)]` | error[E0277]: the trait bound `NoTraitImpls: Clone` is not satisfied - --> tests/integration/ui/compile_fail/builder_derives.rs:15:1 + --> tests/integration/ui/compile_fail/builder_derives.rs:18:46 + | +18 | fn method_contains_non_trait(_non_debug: NoTraitImpls, _x: u32) {} + | ^^^^^^^^^^^^ the trait `Clone` is not implemented for `NoTraitImpls` | -15 | #[bon] - | ^^^^^^ the trait `Clone` is not implemented for `NoTraitImpls` +note: required by a bound in `assert_clone` + --> src/private/mod.rs | - = help: see issue #48214 - = note: this error originates in the attribute macro `bon` (in Nightly builds, run with -Z macro-backtrace for more info) + | pub const fn assert_clone() {} + | ^^^^^ required by this bound in `assert_clone` help: consider annotating `NoTraitImpls` with `#[derive(Clone)]` | 3 + #[derive(Clone)] @@ -73,15 +88,18 @@ help: consider annotating `NoTraitImpls` with `#[derive(Clone)]` | error[E0277]: `NoTraitImpls` doesn't implement `Debug` - --> tests/integration/ui/compile_fail/builder_derives.rs:15:1 + --> tests/integration/ui/compile_fail/builder_derives.rs:18:46 | -15 | #[bon] - | ^^^^^^ `NoTraitImpls` cannot be formatted using `{:?}` +18 | fn method_contains_non_trait(_non_debug: NoTraitImpls, _x: u32) {} + | ^^^^^^^^^^^^ `NoTraitImpls` cannot be formatted using `{:?}` | = help: the trait `Debug` is not implemented for `NoTraitImpls` = note: add `#[derive(Debug)]` to `NoTraitImpls` or manually `impl Debug for NoTraitImpls` - = help: see issue #48214 - = note: this error originates in the attribute macro `bon` (in Nightly builds, run with -Z macro-backtrace for more info) +note: required by a bound in `assert_debug` + --> src/private/mod.rs + | + | pub const fn assert_debug() {} + | ^^^^^^^^^^^^^^^^ required by this bound in `assert_debug` help: consider annotating `NoTraitImpls` with `#[derive(Debug)]` | 3 + #[derive(Debug)] From c32a7bf27413cacbaaacf1bbdb6a4aa2345279bc Mon Sep 17 00:00:00 2001 From: Veetaha Date: Thu, 12 Sep 2024 23:48:09 +0000 Subject: [PATCH 08/21] Enforce the ordering of members --- bon-cli/src/main.rs | 2 +- .../src/builder/builder_gen/member/mod.rs | 195 +++++++++++------- bon-macros/src/util/iterator.rs | 23 +++ bon-macros/src/util/mod.rs | 2 +- .../integration/builder/builder_derives.rs | 10 +- .../integration/builder/positional_members.rs | 12 +- .../ui/compile_fail/positional_members.rs | 70 +++++++ .../ui/compile_fail/positional_members.stderr | 82 ++++++++ 8 files changed, 306 insertions(+), 90 deletions(-) create mode 100644 bon/tests/integration/ui/compile_fail/positional_members.rs create mode 100644 bon/tests/integration/ui/compile_fail/positional_members.stderr diff --git a/bon-cli/src/main.rs b/bon-cli/src/main.rs index 8de5f956..732196bd 100644 --- a/bon-cli/src/main.rs +++ b/bon-cli/src/main.rs @@ -1,4 +1,4 @@ -//! Simple CLI initially created to assits with upgrading to the new versions of +//! Simple CLI initially created to assist with upgrading to the new versions of //! the `bon` crate. #![allow(missing_docs)] diff --git a/bon-macros/src/builder/builder_gen/member/mod.rs b/bon-macros/src/builder/builder_gen/member/mod.rs index b29fd0ae..5328e927 100644 --- a/bon-macros/src/builder/builder_gen/member/mod.rs +++ b/bon-macros/src/builder/builder_gen/member/mod.rs @@ -189,92 +189,114 @@ impl Member { origin: MemberOrigin, members: impl IntoIterator>, ) -> Result> { - let mut named_count = 0; - let mut start_fn_arg_count = 0; - - members + let mut members = members .into_iter() .map(|member| { - let RawMember { - attrs, + let params = MemberParams::from_attributes(member.attrs)?; + params.validate(origin)?; + Ok((member, params)) + }) + .collect::>>()? + .into_iter() + .peekable(); + + let mut output = vec![]; + + let start_fn_args = (0..).map_while(|index| { + let (member, params) = members.next_if(|(_, params)| params.start_fn.is_present())?; + let base = PositionalFnArgMember::new(origin, member, params); + Some(Self::StartFnArg(StartFnArgMember { + base, + index: index.into(), + })) + }); + + output.extend(start_fn_args); + + let finish_fn_args = members + .peeking_take_while(|(_, params)| params.finish_fn.is_present()) + .map(|(member, params)| { + Self::FinishFnArg(PositionalFnArgMember::new(origin, member, params)) + }); + + output.extend(finish_fn_args); + + let mut named_count = 0; + + for (member, params) in members { + let RawMember { + attrs, + ident: orig_ident, + norm_ty, + orig_ty, + } = member; + + if let Some(value) = params.skip { + output.push(Self::Skipped(SkippedMember { ident: orig_ident, norm_ty, - orig_ty, - } = member; + value, + })); + continue; + } - let params = MemberParams::from_attributes(attrs)?; - params.validate(origin)?; + let active_flag = |flag: darling::util::Flag| flag.is_present().then(|| flag); - if let Some(value) = params.skip { - return Ok(Self::Skipped(SkippedMember { - ident: orig_ident, - norm_ty, - value, - })); - } - - if params.finish_fn.is_present() || params.start_fn.is_present() { - let is_finish_fn = params.finish_fn.is_present(); - - let base = PositionalFnArgMember { - origin, - ident: orig_ident, - norm_ty, - orig_ty, - params, - }; - - if is_finish_fn { - return Ok(Self::FinishFnArg(base)); - } - - let index = start_fn_arg_count.into(); - start_fn_arg_count += 1; - return Ok(Self::StartFnArg(StartFnArgMember { base, index })); - } - - // XXX: docs are collected only for named members. There is obvious - // place where to put the docs for positional and skipped members. - // - // Even if there are some docs on them and the function syntax is used - // then these docs will just be removed from the output function. - // It's probably fine since the doc comments are there in the code - // itself which is also useful for people reading the source code. - let docs = attrs.iter().filter(|attr| attr.is_doc()).cloned().collect(); - - let orig_ident_str = orig_ident.to_string(); - let norm_ident = orig_ident_str - // Remove the leading underscore from the member name since it's used - // to denote unused symbols in Rust. That doesn't mean the builder - // API should expose that knowledge to the caller. - .strip_prefix('_') - .unwrap_or(&orig_ident_str); - - // Preserve the original identifier span to make IDE go to definition correctly - // and make error messages point to the correct place. - let norm_ident = syn::Ident::new_maybe_raw(norm_ident, orig_ident.span()); - let norm_ident_pascal = norm_ident.snake_to_pascal_case(); - - let me = NamedMember { - index: named_count.into(), - origin, - generic_var_ident: quote::format_ident!("__{}", norm_ident_pascal), - norm_ident_pascal, - orig_ident, - norm_ident, - norm_ty, - orig_ty, - params, - docs, - }; + let incorrect_order = + active_flag(params.finish_fn).or_else(|| active_flag(params.start_fn)); - named_count += 1; + if let Some(attr) = incorrect_order { + bail!( + &attr.span(), + "incorrect members oredering; the order of members must be the following:\n\ + (1) members annotated with #[builder(start_fn)]\n\ + (2) members annotated with #[builder(finish_fn)]\n\ + (3) all other members in any order", + ); + } - me.validate()?; + // XXX: docs are collected only for named members. There is no obvious + // place where to put the docs for positional and skipped members. + // + // Even if there are some docs on them and the function syntax is used + // then these docs will just be removed from the output function. + // It's probably fine since the doc comments are there in the code + // itself which is also useful for people reading the source code. + let docs = attrs.iter().filter(|attr| attr.is_doc()).cloned().collect(); + + let orig_ident_str = orig_ident.to_string(); + let norm_ident = orig_ident_str + // Remove the leading underscore from the member name since it's used + // to denote unused symbols in Rust. That doesn't mean the builder + // API should expose that knowledge to the caller. + .strip_prefix('_') + .unwrap_or(&orig_ident_str); + + // Preserve the original identifier span to make IDE go to definition correctly + // and make error messages point to the correct place. + let norm_ident = syn::Ident::new_maybe_raw(norm_ident, orig_ident.span()); + let norm_ident_pascal = norm_ident.snake_to_pascal_case(); + + let me = NamedMember { + index: named_count.into(), + origin, + generic_var_ident: quote::format_ident!("__{}", norm_ident_pascal), + norm_ident_pascal, + orig_ident, + norm_ident, + norm_ty, + orig_ty, + params, + docs, + }; + + me.validate()?; + + output.push(Self::Named(me)); + named_count += 1; + } - Ok(Self::Named(me)) - }) - .collect() + Ok(output) } } @@ -318,3 +340,22 @@ impl Member { } } } + +impl PositionalFnArgMember { + fn new(origin: MemberOrigin, member: RawMember<'_>, params: MemberParams) -> Self { + let RawMember { + attrs: _, + ident, + norm_ty, + orig_ty, + } = member; + + Self { + origin, + ident, + norm_ty, + orig_ty, + params, + } + } +} diff --git a/bon-macros/src/util/iterator.rs b/bon-macros/src/util/iterator.rs index aafcfcc7..b2719bc0 100644 --- a/bon-macros/src/util/iterator.rs +++ b/bon-macros/src/util/iterator.rs @@ -1,5 +1,6 @@ use crate::util::prelude::*; use std::fmt::Write; +use std::iter::Peekable; pub(crate) trait IteratorExt: Iterator + Sized { /// Based on itertools: @@ -71,3 +72,25 @@ pub(crate) trait IntoIteratorExt: IntoIterator + Sized { } impl IntoIteratorExt for I {} + +pub(crate) trait PeekableExt: Iterator { + fn peeking_take_while( + &mut self, + predicate: impl FnMut(&Self::Item) -> bool, + ) -> impl Iterator; +} + +impl PeekableExt for Peekable { + fn peeking_take_while( + &mut self, + mut predicate: impl FnMut(&Self::Item) -> bool, + ) -> impl Iterator { + std::iter::from_fn(move || { + if !predicate(self.peek()?) { + return None; + } + + self.next() + }) + } +} diff --git a/bon-macros/src/util/mod.rs b/bon-macros/src/util/mod.rs index d212745e..3cace786 100644 --- a/bon-macros/src/util/mod.rs +++ b/bon-macros/src/util/mod.rs @@ -28,7 +28,7 @@ pub(crate) mod prelude { pub(crate) use super::attrs::AttributeExt; pub(crate) use super::fn_arg::FnArgExt; pub(crate) use super::ident::IdentExt; - pub(crate) use super::iterator::{IntoIteratorExt, IteratorExt}; + pub(crate) use super::iterator::{IntoIteratorExt, IteratorExt, PeekableExt}; pub(crate) use super::path::PathExt; pub(crate) use super::punctuated::PunctuatedExt; pub(crate) use super::ty::TypeExt; diff --git a/bon/tests/integration/builder/builder_derives.rs b/bon/tests/integration/builder/builder_derives.rs index 491df33b..a071bb66 100644 --- a/bon/tests/integration/builder/builder_derives.rs +++ b/bon/tests/integration/builder/builder_derives.rs @@ -143,10 +143,10 @@ fn positional_members_struct() { #[builder(start_fn)] start_fn_arg: bool, - named: u32, - #[builder(finish_fn)] finish_fn_arg: &'static str, + + named: u32, } let actual = Sut::builder(true); @@ -165,8 +165,8 @@ fn positional_members_fn() { #[allow(unused_variables)] fn sut( #[builder(start_fn)] start_fn_arg: bool, - named: u32, #[builder(finish_fn)] finish_fn_arg: &'static str, + named: u32, ) { } @@ -191,8 +191,8 @@ fn positional_members_impl_block() { #[builder(derive(Clone, Debug))] fn sut( #[builder(start_fn)] start_fn_arg: bool, - named: u32, #[builder(finish_fn)] finish_fn_arg: &'static str, + named: u32, ) { } @@ -200,8 +200,8 @@ fn positional_members_impl_block() { fn with_self( &self, #[builder(start_fn)] start_fn_arg: bool, - named: u32, #[builder(finish_fn)] finish_fn_arg: &'static str, + named: u32, ) { let _ = self; } diff --git a/bon/tests/integration/builder/positional_members.rs b/bon/tests/integration/builder/positional_members.rs index 1912093a..25c702d9 100644 --- a/bon/tests/integration/builder/positional_members.rs +++ b/bon/tests/integration/builder/positional_members.rs @@ -30,13 +30,13 @@ fn smoke_struct() { #[builder(start_fn, into)] starter_3: Option<&'static str>, - named: u32, - #[builder(finish_fn)] finisher_1: &'static str, #[builder(finish_fn, into)] finisher_2: &'static str, + + named: u32, } assert_debug_eq( @@ -48,9 +48,9 @@ fn smoke_struct() { starter_1: true, starter_2: 'c', starter_3: None, - named: 99, finisher_1: "1", finisher_2: "2", + named: 99, }"#]], ); @@ -64,9 +64,9 @@ fn smoke_fn() { #[builder(start_fn)] starter_1: bool, #[builder(start_fn, into)] starter_2: char, #[builder(start_fn, into)] starter_3: Option<&'static str>, - named: u32, #[builder(finish_fn)] finisher_1: &'static str, #[builder(finish_fn, into)] finisher_2: &'static str, + named: u32, ) -> ( bool, char, @@ -101,9 +101,9 @@ fn smoke_impl_block() { #[builder(start_fn)] starter_1: bool, #[builder(start_fn, into)] starter_2: char, #[builder(start_fn, into)] starter_3: Option<&'static str>, - named: u32, #[builder(finish_fn)] finisher_1: &'static str, #[builder(finish_fn, into)] finisher_2: &'static str, + named: u32, ) -> ( bool, char, @@ -121,8 +121,8 @@ fn smoke_impl_block() { fn with_self( &self, #[builder(start_fn)] starter_1: bool, - named: u32, #[builder(finish_fn)] finisher_1: &'static str, + named: u32, ) -> (bool, u32, &'static str) { let _ = self; (starter_1, named, finisher_1) diff --git a/bon/tests/integration/ui/compile_fail/positional_members.rs b/bon/tests/integration/ui/compile_fail/positional_members.rs new file mode 100644 index 00000000..a1c182eb --- /dev/null +++ b/bon/tests/integration/ui/compile_fail/positional_members.rs @@ -0,0 +1,70 @@ +use bon::Builder; + +#[derive(Builder)] +struct IncorrectOrder1 { + #[builder(start_fn)] + _a: (), + _b: (), + #[builder(start_fn)] + _c: (), +} + +#[derive(Builder)] +struct IncorrectOrder2 { + #[builder(finish_fn)] + _a: (), + _b: (), + #[builder(start_fn)] + _c: (), +} + +#[derive(Builder)] +struct IncorrectOrder3 { + _a: (), + #[builder(start_fn)] + _b: (), +} + +#[derive(Builder)] +struct IncorrectOrder4 { + _a: (), + #[builder(finish_fn)] + _b: (), +} + +#[derive(Builder)] +struct IncorrectOrder5 { + #[builder(skip)] + _a: (), + #[builder(start_fn)] + _b: (), +} + +#[derive(Builder)] +struct IncorrectOrder6 { + #[builder(skip)] + _a: (), + #[builder(finish_fn)] + _b: (), +} + +#[derive(Builder)] +struct IncorrectOrder7 { + #[builder(finish_fn)] + _a: (), + #[builder(start_fn)] + _b: (), +} + +#[derive(Builder)] +struct IncorrectOrder7 { + #[builder(start_fn)] + _a: (), + #[builder(finish_fn)] + _b: (), + #[builder(start_fn)] + _c: (), +} + + +fn main() {} diff --git a/bon/tests/integration/ui/compile_fail/positional_members.stderr b/bon/tests/integration/ui/compile_fail/positional_members.stderr new file mode 100644 index 00000000..c7a4190c --- /dev/null +++ b/bon/tests/integration/ui/compile_fail/positional_members.stderr @@ -0,0 +1,82 @@ +error: incorrect members oredering; the order of members must be the following: + (1) members annotated with #[builder(start_fn)] + (2) members annotated with #[builder(finish_fn)] + (3) all other members in any order + --> tests/integration/ui/compile_fail/positional_members.rs:8:15 + | +8 | #[builder(start_fn)] + | ^^^^^^^^ + +error: incorrect members oredering; the order of members must be the following: + (1) members annotated with #[builder(start_fn)] + (2) members annotated with #[builder(finish_fn)] + (3) all other members in any order + --> tests/integration/ui/compile_fail/positional_members.rs:17:15 + | +17 | #[builder(start_fn)] + | ^^^^^^^^ + +error: incorrect members oredering; the order of members must be the following: + (1) members annotated with #[builder(start_fn)] + (2) members annotated with #[builder(finish_fn)] + (3) all other members in any order + --> tests/integration/ui/compile_fail/positional_members.rs:24:15 + | +24 | #[builder(start_fn)] + | ^^^^^^^^ + +error: incorrect members oredering; the order of members must be the following: + (1) members annotated with #[builder(start_fn)] + (2) members annotated with #[builder(finish_fn)] + (3) all other members in any order + --> tests/integration/ui/compile_fail/positional_members.rs:31:15 + | +31 | #[builder(finish_fn)] + | ^^^^^^^^^ + +error: incorrect members oredering; the order of members must be the following: + (1) members annotated with #[builder(start_fn)] + (2) members annotated with #[builder(finish_fn)] + (3) all other members in any order + --> tests/integration/ui/compile_fail/positional_members.rs:39:15 + | +39 | #[builder(start_fn)] + | ^^^^^^^^ + +error: incorrect members oredering; the order of members must be the following: + (1) members annotated with #[builder(start_fn)] + (2) members annotated with #[builder(finish_fn)] + (3) all other members in any order + --> tests/integration/ui/compile_fail/positional_members.rs:47:15 + | +47 | #[builder(finish_fn)] + | ^^^^^^^^^ + +error: incorrect members oredering; the order of members must be the following: + (1) members annotated with #[builder(start_fn)] + (2) members annotated with #[builder(finish_fn)] + (3) all other members in any order + --> tests/integration/ui/compile_fail/positional_members.rs:55:15 + | +55 | #[builder(start_fn)] + | ^^^^^^^^ + +error[E0428]: the name `IncorrectOrder7` is defined multiple times + --> tests/integration/ui/compile_fail/positional_members.rs:60:1 + | +52 | struct IncorrectOrder7 { + | ---------------------- previous definition of the type `IncorrectOrder7` here +... +60 | struct IncorrectOrder7 { + | ^^^^^^^^^^^^^^^^^^^^^^ `IncorrectOrder7` redefined here + | + = note: `IncorrectOrder7` must be defined only once in the type namespace of this module + +error: incorrect members oredering; the order of members must be the following: + (1) members annotated with #[builder(start_fn)] + (2) members annotated with #[builder(finish_fn)] + (3) all other members in any order + --> tests/integration/ui/compile_fail/positional_members.rs:65:15 + | +65 | #[builder(start_fn)] + | ^^^^^^^^ From 63e991987e36c12f89a40523cac7b3726a9975e6 Mon Sep 17 00:00:00 2001 From: Veetaha Date: Thu, 12 Sep 2024 23:57:26 +0000 Subject: [PATCH 09/21] Fix msrv --- .../src/builder/builder_gen/member/mod.rs | 13 +++++------ bon-macros/src/util/iterator.rs | 23 ------------------- bon-macros/src/util/mod.rs | 2 +- bon/src/private/mod.rs | 17 ++++++++++---- 4 files changed, 19 insertions(+), 36 deletions(-) diff --git a/bon-macros/src/builder/builder_gen/member/mod.rs b/bon-macros/src/builder/builder_gen/member/mod.rs index 5328e927..40ced26a 100644 --- a/bon-macros/src/builder/builder_gen/member/mod.rs +++ b/bon-macros/src/builder/builder_gen/member/mod.rs @@ -213,13 +213,12 @@ impl Member { output.extend(start_fn_args); - let finish_fn_args = members - .peeking_take_while(|(_, params)| params.finish_fn.is_present()) - .map(|(member, params)| { - Self::FinishFnArg(PositionalFnArgMember::new(origin, member, params)) - }); - - output.extend(finish_fn_args); + while let Some((member, params)) = + members.next_if(|(_, params)| params.finish_fn.is_present()) + { + let member = PositionalFnArgMember::new(origin, member, params); + output.push(Self::FinishFnArg(member)); + } let mut named_count = 0; diff --git a/bon-macros/src/util/iterator.rs b/bon-macros/src/util/iterator.rs index b2719bc0..aafcfcc7 100644 --- a/bon-macros/src/util/iterator.rs +++ b/bon-macros/src/util/iterator.rs @@ -1,6 +1,5 @@ use crate::util::prelude::*; use std::fmt::Write; -use std::iter::Peekable; pub(crate) trait IteratorExt: Iterator + Sized { /// Based on itertools: @@ -72,25 +71,3 @@ pub(crate) trait IntoIteratorExt: IntoIterator + Sized { } impl IntoIteratorExt for I {} - -pub(crate) trait PeekableExt: Iterator { - fn peeking_take_while( - &mut self, - predicate: impl FnMut(&Self::Item) -> bool, - ) -> impl Iterator; -} - -impl PeekableExt for Peekable { - fn peeking_take_while( - &mut self, - mut predicate: impl FnMut(&Self::Item) -> bool, - ) -> impl Iterator { - std::iter::from_fn(move || { - if !predicate(self.peek()?) { - return None; - } - - self.next() - }) - } -} diff --git a/bon-macros/src/util/mod.rs b/bon-macros/src/util/mod.rs index 3cace786..d212745e 100644 --- a/bon-macros/src/util/mod.rs +++ b/bon-macros/src/util/mod.rs @@ -28,7 +28,7 @@ pub(crate) mod prelude { pub(crate) use super::attrs::AttributeExt; pub(crate) use super::fn_arg::FnArgExt; pub(crate) use super::ident::IdentExt; - pub(crate) use super::iterator::{IntoIteratorExt, IteratorExt, PeekableExt}; + pub(crate) use super::iterator::{IntoIteratorExt, IteratorExt}; pub(crate) use super::path::PathExt; pub(crate) use super::punctuated::PunctuatedExt; pub(crate) use super::ty::TypeExt; diff --git a/bon/src/private/mod.rs b/bon/src/private/mod.rs index 0d5ea6a0..0d37ea8c 100644 --- a/bon/src/private/mod.rs +++ b/bon/src/private/mod.rs @@ -1,6 +1,13 @@ -// We place `#[inline(always)]` only on very small methods where we'd event want -// a guarantee of them being inlined. -#![allow(clippy::inline_always)] +#![allow( + // We place `#[inline(always)]` only on very small methods where we'd event want + // a guarantee of them being inlined. + clippy::inline_always, + + // Marking every potential function as `const` is a bit too much. + // Especially, this doesn't play well with our MSRV trait bounds + // aren't allowed on const functions. + clippy::missing_const_for_fn +)] /// Used for providing better IDE hints (completions and syntax highlighting). pub mod ide; @@ -9,8 +16,8 @@ pub mod ide; #[cfg(feature = "alloc")] pub extern crate alloc; -pub const fn assert_clone() {} -pub const fn assert_debug() {} +pub fn assert_clone() {} +pub fn assert_debug() {} /// Marker trait to denote the state of the member that is not set yet. #[rustversion::attr( From e19b3e7249ac6367906e7b4e905a865594e19831 Mon Sep 17 00:00:00 2001 From: Veetaha Date: Fri, 13 Sep 2024 00:01:52 +0000 Subject: [PATCH 10/21] Update trybuild --- .../ui/compile_fail/builder_derives.stderr | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/bon/tests/integration/ui/compile_fail/builder_derives.stderr b/bon/tests/integration/ui/compile_fail/builder_derives.stderr index 8b75f339..cd7d0b9d 100644 --- a/bon/tests/integration/ui/compile_fail/builder_derives.stderr +++ b/bon/tests/integration/ui/compile_fail/builder_derives.stderr @@ -7,8 +7,8 @@ error[E0277]: the trait bound `NoTraitImpls: Clone` is not satisfied note: required by a bound in `assert_clone` --> src/private/mod.rs | - | pub const fn assert_clone() {} - | ^^^^^ required by this bound in `assert_clone` + | pub fn assert_clone() {} + | ^^^^^ required by this bound in `assert_clone` help: consider annotating `NoTraitImpls` with `#[derive(Clone)]` | 3 + #[derive(Clone)] @@ -26,8 +26,8 @@ error[E0277]: `NoTraitImpls` doesn't implement `Debug` note: required by a bound in `assert_debug` --> src/private/mod.rs | - | pub const fn assert_debug() {} - | ^^^^^^^^^^^^^^^^ required by this bound in `assert_debug` + | pub fn assert_debug() {} + | ^^^^^^^^^^^^^^^^ required by this bound in `assert_debug` help: consider annotating `NoTraitImpls` with `#[derive(Debug)]` | 3 + #[derive(Debug)] @@ -43,8 +43,8 @@ error[E0277]: the trait bound `NoTraitImpls: Clone` is not satisfied note: required by a bound in `assert_clone` --> src/private/mod.rs | - | pub const fn assert_clone() {} - | ^^^^^ required by this bound in `assert_clone` + | pub fn assert_clone() {} + | ^^^^^ required by this bound in `assert_clone` help: consider annotating `NoTraitImpls` with `#[derive(Clone)]` | 3 + #[derive(Clone)] @@ -62,8 +62,8 @@ error[E0277]: `NoTraitImpls` doesn't implement `Debug` note: required by a bound in `assert_debug` --> src/private/mod.rs | - | pub const fn assert_debug() {} - | ^^^^^^^^^^^^^^^^ required by this bound in `assert_debug` + | pub fn assert_debug() {} + | ^^^^^^^^^^^^^^^^ required by this bound in `assert_debug` help: consider annotating `NoTraitImpls` with `#[derive(Debug)]` | 3 + #[derive(Debug)] @@ -79,8 +79,8 @@ error[E0277]: the trait bound `NoTraitImpls: Clone` is not satisfied note: required by a bound in `assert_clone` --> src/private/mod.rs | - | pub const fn assert_clone() {} - | ^^^^^ required by this bound in `assert_clone` + | pub fn assert_clone() {} + | ^^^^^ required by this bound in `assert_clone` help: consider annotating `NoTraitImpls` with `#[derive(Clone)]` | 3 + #[derive(Clone)] @@ -98,8 +98,8 @@ error[E0277]: `NoTraitImpls` doesn't implement `Debug` note: required by a bound in `assert_debug` --> src/private/mod.rs | - | pub const fn assert_debug() {} - | ^^^^^^^^^^^^^^^^ required by this bound in `assert_debug` + | pub fn assert_debug() {} + | ^^^^^^^^^^^^^^^^ required by this bound in `assert_debug` help: consider annotating `NoTraitImpls` with `#[derive(Debug)]` | 3 + #[derive(Debug)] From be3c9823e4181cfd7db2fde82f04f868a89a687b Mon Sep 17 00:00:00 2001 From: Veetaha Date: Fri, 13 Sep 2024 01:08:26 +0000 Subject: [PATCH 11/21] Inherit all allow/expect attributes. More tests --- .../src/builder/builder_gen/input_func.rs | 26 +++- .../src/builder/builder_gen/input_struct.rs | 14 +- bon-macros/src/builder/builder_gen/mod.rs | 54 +++---- bon-macros/src/builder/item_impl.rs | 5 + bon-macros/src/util/attrs.rs | 27 ++++ bon-macros/src/util/item.rs | 28 ++++ bon-macros/src/util/mod.rs | 2 + bon/tests/integration/builder/attr_default.rs | 144 ++++++++++++++++++ 8 files changed, 260 insertions(+), 40 deletions(-) create mode 100644 bon-macros/src/util/item.rs diff --git a/bon-macros/src/builder/builder_gen/input_func.rs b/bon-macros/src/builder/builder_gen/input_func.rs index ad7ed2fc..1e70ec16 100644 --- a/bon-macros/src/builder/builder_gen/input_func.rs +++ b/bon-macros/src/builder/builder_gen/input_func.rs @@ -2,7 +2,6 @@ use super::{ generic_param_to_arg, AssocMethodCtx, AssocMethodReceiverCtx, BuilderGenCtx, FinishFunc, FinishFuncBody, Generics, Member, MemberOrigin, RawMember, StartFunc, }; -use crate::builder::builder_gen::BuilderGenParams; use crate::builder::params::BuilderParams; use crate::normalization::NormalizeSelfTy; use crate::util::prelude::*; @@ -71,6 +70,11 @@ pub(crate) struct FuncInputCtx { pub(crate) struct ImplCtx { pub(crate) self_ty: Box, pub(crate) generics: syn::Generics, + + /// Lint suppressions from the original item that will be inherited by all items + /// generated by the macro. If the original syntax used `#[expect(...)]`, + /// then it must be represented as `#[allow(...)]` here. + pub(crate) allow_attrs: Vec, } impl FuncInputCtx { @@ -341,6 +345,20 @@ impl FuncInputCtx { docs: "Finishes building and performs the requested action.".to_owned(), }; + let fn_allows = self + .norm_func + .attrs + .iter() + .filter_map(syn::Attribute::to_allow); + + let allow_attrs = self + .impl_ctx + .as_ref() + .into_iter() + .flat_map(|impl_ctx| impl_ctx.allow_attrs.iter().cloned()) + .chain(fn_allows) + .collect(); + let start_func = StartFunc { ident: start_func_ident, @@ -364,9 +382,11 @@ impl FuncInputCtx { )), }; - let ctx = BuilderGenCtx::new(BuilderGenParams { + let ctx = BuilderGenCtx { members, + allow_attrs, + conditional_params: self.params.base.on, builder_derives: self.params.base.derive, @@ -378,7 +398,7 @@ impl FuncInputCtx { start_func, finish_func, - }); + }; Ok(ctx) } diff --git a/bon-macros/src/builder/builder_gen/input_struct.rs b/bon-macros/src/builder/builder_gen/input_struct.rs index 739540f3..8d2fa229 100644 --- a/bon-macros/src/builder/builder_gen/input_struct.rs +++ b/bon-macros/src/builder/builder_gen/input_struct.rs @@ -2,7 +2,6 @@ use super::{ AssocMethodCtx, BuilderGenCtx, FinishFunc, FinishFuncBody, Generics, Member, MemberOrigin, RawMember, StartFunc, }; -use crate::builder::builder_gen::BuilderGenParams; use crate::builder::params::{BuilderParams, ItemParams}; use crate::util::prelude::*; use darling::FromMeta; @@ -182,9 +181,18 @@ impl StructInputCtx { receiver: None, }); - let ctx = BuilderGenCtx::new(BuilderGenParams { + let allow_attrs = self + .norm_struct + .attrs + .iter() + .filter_map(syn::Attribute::to_allow) + .collect(); + + let ctx = BuilderGenCtx { members, + allow_attrs, + conditional_params: self.params.base.on, builder_derives: self.params.base.derive, @@ -196,7 +204,7 @@ impl StructInputCtx { start_func, finish_func, - }); + }; Ok(ctx) } diff --git a/bon-macros/src/builder/builder_gen/mod.rs b/bon-macros/src/builder/builder_gen/mod.rs index 331ee5d8..77fd0952 100644 --- a/bon-macros/src/builder/builder_gen/mod.rs +++ b/bon-macros/src/builder/builder_gen/mod.rs @@ -30,6 +30,11 @@ struct AssocMethodCtx { pub(crate) struct BuilderGenCtx { members: Vec, + /// Lint suppressions from the original item that will be inherited by all items + /// generated by the macro. If the original syntax used `#[expect(...)]`, + /// then it must be represented as `#[allow(...)]` here. + allow_attrs: Vec, + conditional_params: Vec, builder_derives: Option, @@ -137,38 +142,7 @@ pub(crate) struct MacroOutput { pub(crate) other_items: TokenStream2, } -struct BuilderGenParams { - members: Vec, - - conditional_params: Vec, - builder_derives: Option, - - generics: Generics, - - vis: syn::Visibility, - assoc_method_ctx: Option, - - start_func: StartFunc, - finish_func: FinishFunc, - - builder_ident: syn::Ident, -} - impl BuilderGenCtx { - fn new(params: BuilderGenParams) -> Self { - Self { - members: params.members, - conditional_params: params.conditional_params, - builder_derives: params.builder_derives, - generics: params.generics, - vis: params.vis, - assoc_method_ctx: params.assoc_method_ctx, - start_func: params.start_func, - finish_func: params.finish_func, - builder_ident: params.builder_ident, - } - } - fn receiver(&self) -> Option<&AssocMethodReceiverCtx> { self.assoc_method_ctx.as_ref()?.receiver.as_ref() } @@ -182,20 +156,32 @@ impl BuilderGenCtx { } pub(crate) fn output(self) -> Result { - let start_func = self.start_func()?; + let mut start_func = self.start_func()?; let builder_decl = self.builder_decl(); let builder_impl = self.builder_impl()?; let builder_derives = self.builder_derives(); - let other_items = quote! { + // -- Postprocessing -- + // Here we parse all items back and add the `allow` attributes to them. + let other_items: syn::File = syn::parse_quote! { #builder_decl #builder_derives #builder_impl }; + let mut other_items = other_items.items; + + for item in &mut other_items { + if let Some(attrs) = item.attrs_mut() { + attrs.extend(self.allow_attrs.iter().cloned()); + } + } + + start_func.attrs.extend(self.allow_attrs); + Ok(MacroOutput { start_func, - other_items, + other_items: quote!(#(#other_items)*), }) } diff --git a/bon-macros/src/builder/item_impl.rs b/bon-macros/src/builder/item_impl.rs index d33be034..7b0e5ef3 100644 --- a/bon-macros/src/builder/item_impl.rs +++ b/bon-macros/src/builder/item_impl.rs @@ -61,6 +61,11 @@ pub(crate) fn generate(mut orig_impl_block: syn::ItemImpl) -> Result bool; fn as_doc(&self) -> Option<&syn::Expr>; + fn to_allow(&self) -> Option; } impl AttributeExt for syn::Attribute { @@ -20,4 +21,30 @@ impl AttributeExt for syn::Attribute { Some(&attr.value) } + + /// Returns `Some` if this is an `#[allow(...)]` or `#[expect(...)]` attribute. + /// Turns an `#[expect(...)]` into `#[allow(...)]`, which is useful to make sure + /// that macro doesn't trigger another warning that there is actually no + /// instance of a lint warning under the `#[expect(...)]`. + fn to_allow(&self) -> Option { + if self.path().is_ident("allow") { + return Some(self.clone()); + } + + if !self.path().is_ident("expect") { + return None; + } + + // Turn an `expect` into allow + let mut attr = self.clone(); + let path = match &mut attr.meta { + syn::Meta::Path(path) => path, + syn::Meta::List(meta) => &mut meta.path, + syn::Meta::NameValue(meta) => &mut meta.path, + }; + + *path = syn::parse_quote!(allow); + + Some(attr) + } } diff --git a/bon-macros/src/util/item.rs b/bon-macros/src/util/item.rs new file mode 100644 index 00000000..d8575a03 --- /dev/null +++ b/bon-macros/src/util/item.rs @@ -0,0 +1,28 @@ +pub(crate) trait ItemExt { + fn attrs_mut(&mut self) -> Option<&mut Vec>; +} + +impl ItemExt for syn::Item { + fn attrs_mut(&mut self) -> Option<&mut Vec> { + let attrs = match self { + Self::Const(item) => &mut item.attrs, + Self::Enum(item) => &mut item.attrs, + Self::ExternCrate(item) => &mut item.attrs, + Self::Fn(item) => &mut item.attrs, + Self::ForeignMod(item) => &mut item.attrs, + Self::Impl(item) => &mut item.attrs, + Self::Macro(item) => &mut item.attrs, + Self::Mod(item) => &mut item.attrs, + Self::Static(item) => &mut item.attrs, + Self::Struct(item) => &mut item.attrs, + Self::Trait(item) => &mut item.attrs, + Self::TraitAlias(item) => &mut item.attrs, + Self::Type(item) => &mut item.attrs, + Self::Union(item) => &mut item.attrs, + Self::Use(item) => &mut item.attrs, + _ => return None, + }; + + Some(attrs) + } +} diff --git a/bon-macros/src/util/mod.rs b/bon-macros/src/util/mod.rs index d212745e..7c3a1182 100644 --- a/bon-macros/src/util/mod.rs +++ b/bon-macros/src/util/mod.rs @@ -1,6 +1,7 @@ mod attrs; mod fn_arg; mod ident; +mod item; mod iterator; mod path; mod punctuated; @@ -28,6 +29,7 @@ pub(crate) mod prelude { pub(crate) use super::attrs::AttributeExt; pub(crate) use super::fn_arg::FnArgExt; pub(crate) use super::ident::IdentExt; + pub(crate) use super::item::ItemExt; pub(crate) use super::iterator::{IntoIteratorExt, IteratorExt}; pub(crate) use super::path::PathExt; pub(crate) use super::punctuated::PunctuatedExt; diff --git a/bon/tests/integration/builder/attr_default.rs b/bon/tests/integration/builder/attr_default.rs index d5753c50..2bc72136 100644 --- a/bon/tests/integration/builder/attr_default.rs +++ b/bon/tests/integration/builder/attr_default.rs @@ -209,3 +209,147 @@ fn fn_generic_default() { sut::<(), ()>().call(); } + +mod interaction_with_positional_members { + use crate::prelude::*; + + #[test] + fn test_struct() { + #[derive(Builder, Debug)] + #[allow(dead_code)] + struct Sut { + #[builder(start_fn)] + starter_1: u32, + + #[builder(start_fn)] + starter_2: u32, + + #[builder(finish_fn)] + finisher_1: u32, + + #[builder(finish_fn)] + finisher_2: u32, + + #[builder(default = [starter_1, starter_2, finisher_1, finisher_2])] + named_1: [u32; 4], + + #[builder(default = (32, named_1))] + named_2: (u32, [u32; 4]), + } + + assert_debug_eq( + Sut::builder(1, 2).build(3, 4), + expect![[r#" + Sut { + starter_1: 1, + starter_2: 2, + finisher_1: 3, + finisher_2: 4, + named_1: [ + 1, + 2, + 3, + 4, + ], + named_2: ( + 32, + [ + 1, + 2, + 3, + 4, + ], + ), + }"#]], + ); + + assert_debug_eq( + Sut::builder(1, 2).named_1([5, 6, 7, 8]).build(3, 4), + expect![[r#" + Sut { + starter_1: 1, + starter_2: 2, + finisher_1: 3, + finisher_2: 4, + named_1: [ + 5, + 6, + 7, + 8, + ], + named_2: ( + 32, + [ + 5, + 6, + 7, + 8, + ], + ), + }"#]], + ); + } + + #[test] + fn test_free_fn() { + #[builder] + #[allow(clippy::type_complexity)] + fn sut( + #[builder(start_fn)] starter_1: u32, + #[builder(start_fn)] starter_2: u32, + #[builder(finish_fn)] finisher_1: u32, + #[builder(finish_fn)] finisher_2: u32, + #[builder(default = [starter_1, starter_2, finisher_1, finisher_2])] // + named_1: [u32; 4], + #[builder(default = (32, named_1))] named_2: (u32, [u32; 4]), + ) -> (u32, u32, u32, u32, [u32; 4], (u32, [u32; 4])) { + ( + starter_1, starter_2, finisher_1, finisher_2, named_1, named_2, + ) + } + + assert_debug_eq( + sut(1, 2).call(3, 4), + expect!["(1, 2, 3, 4, [1, 2, 3, 4], (32, [1, 2, 3, 4]))"], + ); + + assert_debug_eq( + sut(1, 2).named_1([5, 6, 7, 8]).call(3, 4), + expect!["(1, 2, 3, 4, [5, 6, 7, 8], (32, [5, 6, 7, 8]))"], + ); + } + + #[test] + fn test_assoc_method() { + struct Sut; + + #[bon] + impl Sut { + #[builder] + #[allow(clippy::type_complexity)] + fn sut( + #[builder(start_fn)] starter_1: u32, + #[builder(start_fn)] starter_2: u32, + #[builder(finish_fn)] finisher_1: u32, + #[builder(finish_fn)] finisher_2: u32, + #[builder(default = [starter_1, starter_2, finisher_1, finisher_2])] // + named_1: [u32; 4], + #[builder(default = (32, named_1))] named_2: (u32, [u32; 4]), + ) -> (u32, u32, u32, u32, [u32; 4], (u32, [u32; 4])) { + ( + starter_1, starter_2, finisher_1, finisher_2, named_1, named_2, + ) + } + } + + assert_debug_eq( + Sut::sut(1, 2).call(3, 4), + expect!["(1, 2, 3, 4, [1, 2, 3, 4], (32, [1, 2, 3, 4]))"], + ); + + assert_debug_eq( + Sut::sut(1, 2).named_1([5, 6, 7, 8]).call(3, 4), + expect!["(1, 2, 3, 4, [5, 6, 7, 8], (32, [5, 6, 7, 8]))"], + ); + } +} From 6a0393ae3607105ece479208c28a04a09a7291f0 Mon Sep 17 00:00:00 2001 From: Veetaha Date: Fri, 13 Sep 2024 19:38:41 +0000 Subject: [PATCH 12/21] Moar tests --- bon/tests/integration/builder/attr_skip.rs | 51 +++ .../integration/builder/positional_members.rs | 411 ++++++++++++++---- 2 files changed, 382 insertions(+), 80 deletions(-) diff --git a/bon/tests/integration/builder/attr_skip.rs b/bon/tests/integration/builder/attr_skip.rs index 886c8553..98e5a9f2 100644 --- a/bon/tests/integration/builder/attr_skip.rs +++ b/bon/tests/integration/builder/attr_skip.rs @@ -76,3 +76,54 @@ fn struct_generic_skipped() { let _: Sut<(), ()> = Sut::<(), ()>::builder().build(); } + +#[test] +fn interaction_with_positional_members() { + #[derive(Builder, Debug)] + #[allow(dead_code)] + struct Sut { + #[builder(start_fn)] + starter_1: u32, + + #[builder(start_fn)] + starter_2: u32, + + #[builder(finish_fn)] + finisher_1: u32, + + #[builder(finish_fn)] + finisher_2: u32, + + #[builder(skip = [starter_1, starter_2, finisher_1, finisher_2])] + named_1: [u32; 4], + + #[builder(skip = (32, named_1))] + named_2: (u32, [u32; 4]), + } + + assert_debug_eq( + Sut::builder(1, 2).build(3, 4), + expect![[r#" + Sut { + starter_1: 1, + starter_2: 2, + finisher_1: 3, + finisher_2: 4, + named_1: [ + 1, + 2, + 3, + 4, + ], + named_2: ( + 32, + [ + 1, + 2, + 3, + 4, + ], + ), + }"#]], + ); +} diff --git a/bon/tests/integration/builder/positional_members.rs b/bon/tests/integration/builder/positional_members.rs index 25c702d9..02e3f70f 100644 --- a/bon/tests/integration/builder/positional_members.rs +++ b/bon/tests/integration/builder/positional_members.rs @@ -16,34 +16,37 @@ impl From for char { } } -#[test] -fn smoke_struct() { - #[derive(Debug, Builder)] - #[allow(dead_code)] - struct Sut { - #[builder(start_fn)] - starter_1: bool, +mod smoke { + use super::*; - #[builder(start_fn, into)] - starter_2: char, + #[test] + fn test_struct() { + #[derive(Debug, Builder)] + #[allow(dead_code)] + struct Sut { + #[builder(start_fn)] + starter_1: bool, - #[builder(start_fn, into)] - starter_3: Option<&'static str>, + #[builder(start_fn, into)] + starter_2: char, - #[builder(finish_fn)] - finisher_1: &'static str, + #[builder(start_fn, into)] + starter_3: Option<&'static str>, - #[builder(finish_fn, into)] - finisher_2: &'static str, + #[builder(finish_fn)] + finisher_1: &'static str, - named: u32, - } + #[builder(finish_fn, into)] + finisher_2: &'static str, - assert_debug_eq( - Sut::builder(true, IntoChar('c'), None) - .named(99) - .build("1", IntoStrRef("2")), - expect![[r#" + named: u32, + } + + assert_debug_eq( + Sut::builder(true, IntoChar('c'), None) + .named(99) + .build("1", IntoStrRef("2")), + expect![[r#" Sut { starter_1: true, starter_2: 'c', @@ -52,50 +55,13 @@ fn smoke_struct() { finisher_2: "2", named: 99, }"#]], - ); - - let _ = Sut::builder(true, 'c', "str"); -} + ); -#[test] -fn smoke_fn() { - #[builder] - fn sut( - #[builder(start_fn)] starter_1: bool, - #[builder(start_fn, into)] starter_2: char, - #[builder(start_fn, into)] starter_3: Option<&'static str>, - #[builder(finish_fn)] finisher_1: &'static str, - #[builder(finish_fn, into)] finisher_2: &'static str, - named: u32, - ) -> ( - bool, - char, - Option<&'static str>, - u32, - &'static str, - &'static str, - ) { - ( - starter_1, starter_2, starter_3, named, finisher_1, finisher_2, - ) + let _ = Sut::builder(true, 'c', "str"); } - assert_debug_eq( - sut(true, IntoChar('c'), None) - .named(99) - .call("1", IntoStrRef("2")), - expect![[r#"(true, 'c', None, 99, "1", "2")"#]], - ); - - let _ = sut(true, 'c', "str"); -} - -#[test] -fn smoke_impl_block() { - struct Sut; - - #[bon] - impl Sut { + #[test] + fn test_free_fn() { #[builder] fn sut( #[builder(start_fn)] starter_1: bool, @@ -117,29 +83,314 @@ fn smoke_impl_block() { ) } - #[builder] - fn with_self( - &self, + assert_debug_eq( + sut(true, IntoChar('c'), None) + .named(99) + .call("1", IntoStrRef("2")), + expect![[r#"(true, 'c', None, 99, "1", "2")"#]], + ); + + let _ = sut(true, 'c', "str"); + } + + #[test] + fn test_assoc_method() { + struct Sut; + + #[bon] + impl Sut { + #[builder] + fn sut( + #[builder(start_fn)] starter_1: bool, + #[builder(start_fn, into)] starter_2: char, + #[builder(start_fn, into)] starter_3: Option<&'static str>, + #[builder(finish_fn)] finisher_1: &'static str, + #[builder(finish_fn, into)] finisher_2: &'static str, + named: u32, + ) -> ( + bool, + char, + Option<&'static str>, + u32, + &'static str, + &'static str, + ) { + ( + starter_1, starter_2, starter_3, named, finisher_1, finisher_2, + ) + } + + #[builder] + fn with_self( + &self, + #[builder(start_fn)] starter_1: bool, + #[builder(finish_fn)] finisher_1: &'static str, + named: u32, + ) -> (bool, u32, &'static str) { + let _ = self; + (starter_1, named, finisher_1) + } + } + + assert_debug_eq( + Sut::sut(true, IntoChar('c'), None) + .named(99) + .call("1", IntoStrRef("2")), + expect![[r#"(true, 'c', None, 99, "1", "2")"#]], + ); + + let _ = Sut::sut(true, 'c', "str"); + + assert_debug_eq( + Sut.with_self(true).named(99).call("1"), + expect![[r#"(true, 99, "1")"#]], + ); + } +} + +mod attr_on { + use super::*; + + #[test] + fn test_struct() { + #[derive(Debug, Builder)] + #[builder(on(&str, into))] + #[allow(dead_code)] + struct Sut { + #[builder(start_fn)] + starter_1: bool, + + #[builder(start_fn, into)] + starter_3: Option<&'static str>, + + #[builder(finish_fn)] + finisher_1: &'static str, + + named: u32, + } + + assert_debug_eq( + Sut::builder(true, "Roseluck") + .named(99) + .build(IntoStrRef("Daisy")), + expect![[r#" + Sut { + starter_1: true, + starter_3: Some( + "Roseluck", + ), + finisher_1: "Daisy", + named: 99, + }"#]], + ); + } + + #[test] + fn test_free_fn() { + #[builder(on(&str, into))] + fn sut( #[builder(start_fn)] starter_1: bool, + + #[builder(start_fn, into)] starter_3: Option<&'static str>, + #[builder(finish_fn)] finisher_1: &'static str, + named: u32, - ) -> (bool, u32, &'static str) { - let _ = self; - (starter_1, named, finisher_1) + ) -> (bool, Option<&'static str>, &'static str, u32) { + (starter_1, starter_3, finisher_1, named) } + + assert_debug_eq( + sut(true, "Roseluck").named(99).call(IntoStrRef("Daisy")), + expect![[r#"(true, Some("Roseluck"), "Daisy", 99)"#]], + ); } - assert_debug_eq( - Sut::sut(true, IntoChar('c'), None) - .named(99) - .call("1", IntoStrRef("2")), - expect![[r#"(true, 'c', None, 99, "1", "2")"#]], - ); + #[test] + fn test_assoc_method() { + struct Sut; + + #[bon] + impl Sut { + #[builder(on(&str, into))] + fn sut( + #[builder(start_fn)] starter_1: bool, - let _ = Sut::sut(true, 'c', "str"); + #[builder(start_fn, into)] starter_3: Option<&'static str>, - assert_debug_eq( - Sut.with_self(true).named(99).call("1"), - expect![[r#"(true, 99, "1")"#]], - ); + #[builder(finish_fn)] finisher_1: &'static str, + + named: u32, + ) -> (bool, Option<&'static str>, &'static str, u32) { + (starter_1, starter_3, finisher_1, named) + } + + #[builder(on(&str, into))] + fn with_self( + &self, + #[builder(start_fn)] starter_1: bool, + #[builder(finish_fn)] finisher_1: &'static str, + named: u32, + ) -> (bool, &'static str, u32) { + let _ = self; + (starter_1, finisher_1, named) + } + } + + assert_debug_eq( + Sut::sut(true, "Roseluck") + .named(99) + .call(IntoStrRef("Daisy")), + expect![[r#"(true, Some("Roseluck"), "Daisy", 99)"#]], + ); + + assert_debug_eq( + Sut.with_self(true).named(99).call("Daisy"), + expect![[r#"(true, "Daisy", 99)"#]], + ); + } +} + +mod generics { + use super::*; + use std::fmt::Debug; + + #[test] + fn test_struct() { + #[derive(Debug, Builder)] + #[allow(dead_code)] + #[builder(derive(Debug, Clone))] + struct Sut<'a, 'b, T, U, A, const N1: usize, const N2: usize> + where + T: PartialEq + Debug + Clone, + Self: Sized + 'b, + U: Debug + Clone, + A: Debug + Clone, + { + #[builder(start_fn)] + starter: &'a [T; N1], + + #[builder(finish_fn, into)] + finisher: [&'b U; N2], + + named: A, + } + + let builder = Sut::builder(&[1_u32, 2, 3]); + + assert_debug_eq( + builder.clone(), + expect!["SutBuilder { starter: [1, 2, 3] }"], + ); + + assert_debug_eq( + builder.named(99).build([&false]), + expect!["Sut { starter: [1, 2, 3], finisher: [false], named: 99 }"], + ); + } + + #[test] + fn test_free_fn() { + #[builder(derive(Debug, Clone))] + fn sut<'a, 'b, T, U, A, const N1: usize, const N2: usize>( + #[builder(start_fn)] starter: &'a [T; N1], + #[builder(finish_fn, into)] finisher: [&'b U; N2], + named: A, + ) -> (&'a [T; N1], [&'b U; N2], A) + where + T: PartialEq + Debug + Clone, + U: Debug + Clone, + A: Debug + Clone, + { + (starter, finisher, named) + } + + let builder = sut(&[1_u32, 2, 3]); + + assert_debug_eq( + builder.clone(), + expect!["SutBuilder { starter: [1, 2, 3] }"], + ); + + assert_debug_eq( + builder.named(99).call([&false]), + expect!["([1, 2, 3], [false], 99)"], + ); + } + + #[test] + fn test_impl_trait_free_fn() { + #[builder(derive(Clone, Debug))] + fn sut( + #[builder(start_fn)] starter: impl IntoIterator + Clone + Debug, + #[builder(finish_fn)] _finisher: impl Debug + Clone + 'static, + ) -> u32 { + starter.into_iter().sum() + } + + let builder = sut([1, 2, 3]); + + assert_debug_eq( + builder.clone(), + expect!["SutBuilder { starter: [1, 2, 3] }"], + ); + + assert_eq!(builder.call(()), 6); + } + + #[test] + fn test_assoc_method() { + #[derive(Debug, Clone)] + struct Sut; + + #[bon] + impl Sut { + #[builder(derive(Debug, Clone))] + fn sut<'a, 'b, T, U, A, const N1: usize, const N2: usize>( + #[builder(start_fn)] starter_1: &'a [T; N1], + #[builder(finish_fn, into)] starter_2: [&'b U; N2], + named: A, + ) -> (&'a [T; N1], [&'b U; N2], A) + where + T: PartialEq + Debug + Clone, + U: Debug + Clone, + A: Debug + Clone, + { + (starter_1, starter_2, named) + } + + #[builder(derive(Debug, Clone))] + fn with_self<'a, 'b, T, U, A, const N1: usize, const N2: usize>( + &self, + #[builder(start_fn)] starter_1: &'a [T; N1], + #[builder(finish_fn, into)] starter_2: [&'b U; N2], + named: A, + ) -> (&'a [T; N1], [&'b U; N2], A) + where + T: PartialEq + Debug + Clone, + U: Debug + Clone, + A: Debug + Clone, + { + let _ = self; + (starter_1, starter_2, named) + } + } + + let builder = Sut::sut(&[1_u32, 2, 3]); + + assert_debug_eq( + builder.clone(), + expect!["SutSutBuilder { starter_1: [1, 2, 3] }"], + ); + + assert_debug_eq( + builder.named(99).call([&false]), + expect!["([1, 2, 3], [false], 99)"], + ); + + assert_debug_eq( + Sut.with_self(&[1_u32, 2, 3]).named(99).call([&false]), + expect!["([1, 2, 3], [false], 99)"], + ); + } } From b0b5475b981aae22355785c6b3ea10b2fa79d2ff Mon Sep 17 00:00:00 2001 From: Veetaha Date: Fri, 13 Sep 2024 19:40:26 +0000 Subject: [PATCH 13/21] Fix --- bon/tests/integration/builder/positional_members.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bon/tests/integration/builder/positional_members.rs b/bon/tests/integration/builder/positional_members.rs index 02e3f70f..0f554a1b 100644 --- a/bon/tests/integration/builder/positional_members.rs +++ b/bon/tests/integration/builder/positional_members.rs @@ -253,7 +253,7 @@ mod attr_on { mod generics { use super::*; - use std::fmt::Debug; + use core::fmt::Debug; #[test] fn test_struct() { From 7451477915ff32d12f7d375deab52f169219db0c Mon Sep 17 00:00:00 2001 From: Veetaha Date: Fri, 13 Sep 2024 19:45:01 +0000 Subject: [PATCH 14/21] Fix nightly job --- bon/tests/integration/main.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bon/tests/integration/main.rs b/bon/tests/integration/main.rs index faf3b05d..0a2a323d 100644 --- a/bon/tests/integration/main.rs +++ b/bon/tests/integration/main.rs @@ -6,7 +6,8 @@ clippy::too_many_lines, // `expect_test` sometimes adds redundant hashes, we just have to live with that clippy::needless_raw_string_hashes, - non_local_definitions + non_local_definitions, + missing_docs, )] #[cfg(feature = "alloc")] From 93e3843483334eebb5c4ecc08921849525a81ba6 Mon Sep 17 00:00:00 2001 From: Veetaha Date: Fri, 13 Sep 2024 21:56:52 +0000 Subject: [PATCH 15/21] Add reference sections in the docs --- website/.vitepress/config.mts | 9 ++ website/reference/builder.md | 221 +++++++++++++++++++++++++++++++++- 2 files changed, 227 insertions(+), 3 deletions(-) diff --git a/website/.vitepress/config.mts b/website/.vitepress/config.mts index 3c71f1a6..141c7203 100644 --- a/website/.vitepress/config.mts +++ b/website/.vitepress/config.mts @@ -197,6 +197,15 @@ export default defineConfig({ text: "skip", link: "/reference/builder#skip", }, + { + text: "start_fn", + link: "/reference/builder#start-fn-1", + }, + { + text: "finish_fn", + link: "/reference/builder#finish-fn-1", + }, + ], }, ], diff --git a/website/reference/builder.md b/website/reference/builder.md index 785662e4..23a07410 100644 --- a/website/reference/builder.md +++ b/website/reference/builder.md @@ -237,7 +237,6 @@ assert_eq!( builder.is_admin(true).call(); ``` - ```rust [Associated method] use bon::bon; @@ -599,9 +598,9 @@ Example::example() This attribute must be of form `on(type_pattern, attributes)`. -- `type_pattern` - type that will be compared with the types of the members. The types are compared textually. For example, `String` doesn't match `std::string::String`. You can use `_` to mark parts of the type to ignore when matching. For example, `Vec<_>` matches `Vec` or `Vec`. Lifetimes are ignored during matching. +- `type_pattern` - type that will be compared with the types of the members. The types are compared textually. For example, `String` doesn't match `std::string::String`. You can use `_` to mark parts of the type to ignore when matching. For example, `Vec<_>` matches `Vec` or `Vec`. Lifetimes are ignored during matching. -- `attributes` - for now, the only attribute supported in the `attributes` position is [`into`](#into). It sets `#[builder(into)]` for members that match the `type_pattern`. +- `attributes` - for now, the only attribute supported in the `attributes` position is [`into`](#into). It sets `#[builder(into)]` for members that match the `type_pattern`. If you want to apply the `attributes` to all members, you can use the `_` type pattern that matches any type. For example, `#[builder(on(_, into))]`. @@ -1102,6 +1101,222 @@ assert_eq!(example.member_3, 9); This attribute is not supported with free function arguments or associated method arguments because it's simply unnecessary there and can easier be expressed with local variables. +### `start_fn` + +**Applies to:** + +Makes the member a positional argument on the starting function that creates the builder. + +The ordering of members annotated with `#[builder(start_fn)]` matters! They will appear in the same order relative to each other in the starting function signature. They must also be declared at the top of the members' list. + +This ensures a consistent initialization order, and it makes these members available for expressions in `#[builder(default/skip = ...)]` for regular members that follow them. + +::: tip + +Don't confuse this with the top-level [`#[builder(start_fn = ...)]`](#start-fn) attribute that can be used to configure the name and visibility of the starting function. You'll likely want to use it in combination with this member-level attribute to define a better name for the starting function. + +::: + +**Example**: + +::: code-group + +```rust [Struct field] +use bon::Builder; + +#[derive(Builder)] +// Top-level attribute to give a better name for the starting function // [!code highlight] +#[builder(start_fn = with_coordinates)] // [!code highlight] +struct Treasure { + // Member-level attribute to mark the member as // [!code highlight] + // a parameter of `with_coordinates()` // [!code highlight] + #[builder(start_fn)] // [!code highlight] + x: u32, + + #[builder(start_fn)] // [!code highlight] + y: u32, + + label: Option, +} + +let treasure = Treasure::with_coordinates(2, 9) // [!code highlight] + .label("knowledge".to_owned()) + .build(); + +assert_eq!(treasure.x, 2); +assert_eq!(treasure.y, 9); +assert_eq!(treasure.label.as_deref(), Some("knowledge")); +``` + +```rust [Free function] +use bon::builder; + +#[builder] +fn mark_treasure_at( + #[builder(start_fn)] // [!code highlight] + x: u32, + + #[builder(start_fn)] // [!code highlight] + y: u32, + + label: Option, +) {} + +mark_treasure_at(2, 9) + .label("knowledge".to_owned()) + .call(); +``` + +```rust [Associated method] +use bon::bon; + +struct Navigator {} + +#[bon] +impl Navigator { + #[builder] + fn mark_treasure_at( + &self, + + #[builder(start_fn)] // [!code highlight] + x: u32, + + #[builder(start_fn)] // [!code highlight] + y: u32, + + label: String, + ) {} +} + +let navigator = Navigator {}; + +navigator + .mark_treasure_at(2, 9) + .label("knowledge".to_owned()) + .call(); +``` + +::: + +You can also combine this attribute with [`#[builder(into)]`](#into) or [`#[builder(on(..., into))]`](#on) to add an into conversion for the parameter. + +Importantly, `Into` conversions for such members work slightly differently from the regular (named) members in regard to the `Option` type. The `Option` type gives no additional meaning to the member annotated with `#[builder(start_fn)]`. Thus, it is matched by the type pattern of `on(..., into)` and wrapped with `impl Into>` as any other type. + +::: tip + +In general, it's not recommended to annotate optional members with `#[builder(start_fn)]` because you can't omit setting them using the positional function call syntax. + +::: + +### `finish_fn` + +**Applies to:** + +Makes the member a positional argument on the finishing function that consumes the builder and returns the resulting object (for struct syntax) or performs the requested action (for function/method syntax). + +The ordering of members annotated with `#[builder(finish_fn)]` matters! They will appear in the same order relative to each other in the finishing function signature. They must also be declared at the top of the members list strictly after members annotated with [`#[builder(start_fn)]`](#start-fn-1) (if any). + +This ensures a consistent initialization order, and it makes these members available for expressions in `#[builder(default/skip = ...)]` for regular members that follow them. + +::: tip + +Don't confuse this with the top-level [`#[builder(finish_fn = ...)]`](#finish-fn) attribute that can be used to configure the name and visibility of the finishing function. You'll likely want to use it in combination with this member-level attribute to define a better name for the finishing function. + +::: + +**Example**: + +::: code-group + +```rust [Struct field] +use bon::Builder; + +#[derive(Builder)] +// Top-level attribute to give a better name for the finishing function // [!code highlight] +#[builder(finish_fn = sign)] // [!code highlight] +struct Message { + // Member-level attribute to mark the member as a parameter of `sign()` // [!code highlight] + #[builder(finish_fn)] // [!code highlight] + author_first_name: String, + + #[builder(finish_fn)] // [!code highlight] + author_last_name: String, + + payload: String, +} + +let message = Message::builder() + .payload("Bon is great! Give it a ⭐".to_owned()) + .sign("Sweetie".to_owned(), "Drops".to_owned()); + +assert_eq!(message.payload, "Bon is great! Give it a ⭐"); +assert_eq!(message.author_first_name, "Sweetie"); +assert_eq!(message.author_last_name, "Drops"); +``` + +```rust [Free function] +use bon::builder; + +// Top-level attribute to give a better name for the finishing function // [!code highlight] +#[builder(finish_fn = send)] // [!code highlight] +fn message( + // Member-level attribute to mark the member as a parameter of `sign()` // [!code highlight] + #[builder(finish_fn)] // [!code highlight] + receiver_first_name: String, + + #[builder(finish_fn)] // [!code highlight] + receiver_last_name: String, + + payload: String, +) {} + +message() + .payload("Bon is great! Give it a ⭐".to_owned()) + .send("Sweetie".to_owned(), "Drops".to_owned()); +``` + +```rust [Associated method] +use bon::bon; + +struct Chat {} + +#[bon] +impl Chat { + // Top-level attribute to give a better name for the finishing function // [!code highlight] + #[builder(finish_fn = send)] // [!code highlight] + fn message( + &self, + + // Member-level attribute to mark the member as a parameter of `sign()` // [!code highlight] + #[builder(finish_fn)] // [!code highlight] + receiver_first_name: String, + + #[builder(finish_fn)] // [!code highlight] + receiver_last_name: String, + + payload: String, + ) {} +} + +let chat = Chat {}; + +chat.message() + .payload("Bon is great! Give it a ⭐".to_owned()) + .send("Sweetie".to_owned(), "Drops".to_owned()); +``` + +::: + +You can also combine this attribute with [`#[builder(into)]`](#into) or [`#[builder(on(..., into))]`](#on) to add an into conversion for the parameter. + +Importantly, `Into` conversions for such members work slightly differently from the regular (named) members in regard to the `Option` type. The `Option` type gives no additional meaning to the member annotated with `#[builder(finish_fn)]`. Thus, it is matched by the type pattern of `on(..., into)` and wrapped with `impl Into>` as any other type. + +::: tip + +In general, it's not recommended to annotate optional members with `#[builder(finish_fn)]` because you can't omit setting them using the positional function call syntax. + +::: + *[Member]: Struct field or a function argument *[member]: Struct field or a function argument *[members]: Struct fields or function arguments From 770754aa519425775139cc6306cf4dd152afa834 Mon Sep 17 00:00:00 2001 From: Veetaha Date: Fri, 13 Sep 2024 22:01:21 +0000 Subject: [PATCH 16/21] Tests for `on(...)` type pattern matching --- .../ui/compile_fail/positional_members.rs | 32 +++++++++++++++- .../ui/compile_fail/positional_members.stderr | 38 +++++++++++++++++++ website/reference/builder.md | 4 +- 3 files changed, 71 insertions(+), 3 deletions(-) diff --git a/bon/tests/integration/ui/compile_fail/positional_members.rs b/bon/tests/integration/ui/compile_fail/positional_members.rs index a1c182eb..6ed78088 100644 --- a/bon/tests/integration/ui/compile_fail/positional_members.rs +++ b/bon/tests/integration/ui/compile_fail/positional_members.rs @@ -66,5 +66,35 @@ struct IncorrectOrder7 { _c: (), } +struct IntoUnit; -fn main() {} +impl From for () { + fn from(_: IntoUnit) -> Self { + () + } +} + +pub fn test_type_pattern_matching() { + #[derive(Builder)] + #[builder(on((), into))] + struct TypePatternMatching { + #[builder(start_fn)] + _a: (), + + #[builder(start_fn)] + _b: Option<()>, + + #[builder(finish_fn)] + _c: (), + + #[builder(finish_fn)] + _d: Option<()>, + } + + TypePatternMatching::builder(IntoUnit, IntoUnit) + .build(IntoUnit, IntoUnit); +} + +fn main() { + +} diff --git a/bon/tests/integration/ui/compile_fail/positional_members.stderr b/bon/tests/integration/ui/compile_fail/positional_members.stderr index c7a4190c..2a494e12 100644 --- a/bon/tests/integration/ui/compile_fail/positional_members.stderr +++ b/bon/tests/integration/ui/compile_fail/positional_members.stderr @@ -80,3 +80,41 @@ error: incorrect members oredering; the order of members must be the following: | 65 | #[builder(start_fn)] | ^^^^^^^^ + +error[E0308]: mismatched types + --> tests/integration/ui/compile_fail/positional_members.rs:94:44 + | +94 | TypePatternMatching::builder(IntoUnit, IntoUnit) + | ---------------------------- ^^^^^^^^ expected `Option<()>`, found `IntoUnit` + | | + | arguments to this function are incorrect + | + = note: expected enum `Option<()>` + found struct `IntoUnit` +note: associated function defined here + --> tests/integration/ui/compile_fail/positional_members.rs:80:12 + | +80 | struct TypePatternMatching { + | ^^^^^^^^^^^^^^^^^^^ +... +85 | _b: Option<()>, + | -------------- + +error[E0308]: mismatched types + --> tests/integration/ui/compile_fail/positional_members.rs:95:26 + | +95 | .build(IntoUnit, IntoUnit); + | ----- ^^^^^^^^ expected `Option<()>`, found `IntoUnit` + | | + | arguments to this method are incorrect + | + = note: expected enum `Option<()>` + found struct `IntoUnit` +note: method defined here + --> tests/integration/ui/compile_fail/positional_members.rs:80:12 + | +80 | struct TypePatternMatching { + | ^^^^^^^^^^^^^^^^^^^ +... +91 | _d: Option<()>, + | -------------- diff --git a/website/reference/builder.md b/website/reference/builder.md index 23a07410..fd4444c7 100644 --- a/website/reference/builder.md +++ b/website/reference/builder.md @@ -598,9 +598,9 @@ Example::example() This attribute must be of form `on(type_pattern, attributes)`. -- `type_pattern` - type that will be compared with the types of the members. The types are compared textually. For example, `String` doesn't match `std::string::String`. You can use `_` to mark parts of the type to ignore when matching. For example, `Vec<_>` matches `Vec` or `Vec`. Lifetimes are ignored during matching. +- `type_pattern` - type that will be compared with the types of the members. The types are compared textually. For example, `String` doesn't match `std::string::String`. You can use `_` to mark parts of the type to ignore when matching. For example, `Vec<_>` matches `Vec` or `Vec`. Lifetimes are ignored during matching. -- `attributes` - for now, the only attribute supported in the `attributes` position is [`into`](#into). It sets `#[builder(into)]` for members that match the `type_pattern`. +- `attributes` - for now, the only attribute supported in the `attributes` position is [`into`](#into). It sets `#[builder(into)]` for members that match the `type_pattern`. If you want to apply the `attributes` to all members, you can use the `_` type pattern that matches any type. For example, `#[builder(on(_, into))]`. From c39a105b45e4a9d7107bf13ec94b5585917320a2 Mon Sep 17 00:00:00 2001 From: Veetaha Date: Sat, 14 Sep 2024 12:08:13 +0000 Subject: [PATCH 17/21] Add a section in the guide --- website/.vitepress/config.mts | 4 + website/guide/positional-members.md | 142 ++++++++++++++++++++++++++++ 2 files changed, 146 insertions(+) create mode 100644 website/guide/positional-members.md diff --git a/website/.vitepress/config.mts b/website/.vitepress/config.mts index 141c7203..f238713a 100644 --- a/website/.vitepress/config.mts +++ b/website/.vitepress/config.mts @@ -86,6 +86,10 @@ export default defineConfig({ text: "Compatibility", link: "/guide/compatibility", }, + { + text: "Positional Members", + link: "/guide/positional-members", + }, { text: "Inspecting", link: "/guide/inspecting", diff --git a/website/guide/positional-members.md b/website/guide/positional-members.md new file mode 100644 index 00000000..b1e86a63 --- /dev/null +++ b/website/guide/positional-members.md @@ -0,0 +1,142 @@ +# Positional Members + +While having the ability to use separate setters for the members gives you a ton of flexibility and extensibility described on the ["Compatibility"](./compatibility) page, sometimes you don't need all of that. + +Maybe you'd like to pick out some specific members and let the user pass their values as positional parameters to the starting function that creates the builder or to the finishing function that consumes it. This reduces the syntax a bit at the cost of some extensibility loss ⚖️, but it may be worth it! + +## Starting function + +As an example, suppose we have a `Treasure` struct with `x` and `y` coordinates and a `label` that describes the payload of the treasure. Since all treasures are located somewhere, they all have coordinates, and it would be cool to specify them in a single starting function call. + +To do that we can use the `#[builder(start_fn)]` attribute. There are two contexts where we can place it, and they both have a different meaning: + +- [Top-level `#[builder(start_fn = ...)]`](../reference/builder#start-fn) - configures the name of the starting function and optionally its visibility +- [Member-level `#[builder(start_fn)]`](../reference/builder#start_fn-1) - configures the member to be a positional parameter on the starting function + +We'll want to use both of these attributes in our example to give a better name for the starting function that describes its inputs and configure `x` and `y` as positional parameters on the starting function as well. + +**Example:** + +```rust +use bon::Builder; + +#[derive(Builder)] +// Top-level attribute to give a better name for the starting function // [!code highlight] +#[builder(start_fn = with_coordinates)] // [!code highlight] +struct Treasure { + // Member-level attribute to mark the member as // [!code highlight] + // a parameter of `with_coordinates()` // [!code highlight] + #[builder(start_fn)] // [!code highlight] + x: u32, + + #[builder(start_fn)] // [!code highlight] + y: u32, + + label: Option, +} + +let treasure = Treasure::with_coordinates(2, 9) // [!code highlight] + .label("oats".to_owned()) + .build(); + +assert_eq!(treasure.x, 2); +assert_eq!(treasure.y, 9); +assert_eq!(treasure.label.as_deref(), Some("oats")); +``` + +Here, the generated `with_coordinates` method has the following signature: + +```rust ignore +impl Treasure { + fn with_coordinates(x: u32, y: u32) -> TreasureBuilder { /**/ } +} +``` + +## Finishing function + +Now let's say we need to know the person who claimed the `Treasure`. While describing the treasure using the current builder syntax we'd like the person who claimed it to specify their first name and last name at the end of the building process. + +We can use the similar combination of the [top-level](../reference/builder#finish-fn) `#[builder(finish_fn = ...)]` and the [member-level `#[builder(finish_fn)]`](../reference/builder#finish-fn-1) attributes to do that. + +**Example:** + +```rust +use bon::Builder; + +#[derive(Builder)] +#[builder(start_fn = with_coordinates)] +#[builder(finish_fn = claim)] // [!code highlight] +struct Treasure { + #[builder(start_fn)] + x: u32, + + #[builder(start_fn)] + y: u32, + + #[builder(finish_fn)] // [!code highlight] + claimed_by_first_name: String, // [!code highlight] + + #[builder(finish_fn)] // [!code highlight] + claimed_by_last_name: String, // [!code highlight] + + label: Option, +} + +let treasure = Treasure::with_coordinates(2, 9) + .label("oats".to_owned()) + .claim("Lyra".to_owned(), "Heartstrings".to_owned()); // [!code highlight] + +assert_eq!(treasure.x, 2); +assert_eq!(treasure.y, 9); +assert_eq!(treasure.label.as_deref(), Some("oats")); +assert_eq!(treasure.claimed_by_first_name, "Lyra"); // [!code highlight] +assert_eq!(treasure.claimed_by_last_name, "Heartstrings"); // [!code highlight] +``` + +## Into conversions + +You may also combine these attributes with [`#[builder(into)]`](../reference/builder#into) or [`#[builder(on(..., into))]`](../reference/builder#into) to reduce the number of `to_owned()` calls a bit. + +```rust +use bon::Builder; + +#[derive(Builder)] +#[builder(start_fn = with_coordinates)] +#[builder(finish_fn = claim)] +struct Treasure { + #[builder(start_fn)] + x: u32, + + #[builder(start_fn)] + y: u32, + + #[builder(finish_fn, into)] // [!code focus] + claimed_by_first_name: String, // [!code focus] + + #[builder(finish_fn, into)] // [!code focus] + claimed_by_last_name: String, // [!code focus] + + #[builder(into)] // [!code focus] + label: Option, // [!code focus] +} + +let treasure = Treasure::with_coordinates(2, 9) + .label("oats") // [!code focus] + .claim("Lyra", "Heartstrings"); // [!code focus] +``` + +However, keep in mind that positional members (ones annotated with `#[builder(start_fn/finish_fn)]`) are always required to pass. There is no special treatment of the `Option` type for such members. + +For example `#[builder(into)]` on a regular (named) member of the `Option` type generates two setters: +- One that accepts `impl Into`. +- The other that accepts `Option>`. + +For positional members, the story is completely different because there are no separate setters generated for them. There is just a single starting or finishing function. So if you enable an into conversion for a positional member of the `Option` type, it will be accepted as `impl Into>` in the starting or finishing function. + +Also, the type pattern of the `#[builder(on(..., into))]` attribute matches the `Option` fully. So, for example `on(String, into)` will not match the positional member of type `Option`, but `on(Option, into)` will. + +::: tip + +In general, it's not recommended to annotate optional members with `#[builder(start_fn/finish_fn)]` because you can't omit setting them using the positional function call syntax. + +::: From 07c181715bc74d7e4ae4049934aace2bda69374a Mon Sep 17 00:00:00 2001 From: Veetaha Date: Sat, 14 Sep 2024 12:09:38 +0000 Subject: [PATCH 18/21] Fix link --- website/guide/positional-members.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/guide/positional-members.md b/website/guide/positional-members.md index b1e86a63..da162b9e 100644 --- a/website/guide/positional-members.md +++ b/website/guide/positional-members.md @@ -11,7 +11,7 @@ As an example, suppose we have a `Treasure` struct with `x` and `y` coordinates To do that we can use the `#[builder(start_fn)]` attribute. There are two contexts where we can place it, and they both have a different meaning: - [Top-level `#[builder(start_fn = ...)]`](../reference/builder#start-fn) - configures the name of the starting function and optionally its visibility -- [Member-level `#[builder(start_fn)]`](../reference/builder#start_fn-1) - configures the member to be a positional parameter on the starting function +- [Member-level `#[builder(start_fn)]`](../reference/builder#start-fn-1) - configures the member to be a positional parameter on the starting function We'll want to use both of these attributes in our example to give a better name for the starting function that describes its inputs and configure `x` and `y` as positional parameters on the starting function as well. From 0351d8ff2dd30db1111b032d16a3c158519d9c73 Mon Sep 17 00:00:00 2001 From: Veetaha Date: Sat, 14 Sep 2024 12:50:25 +0000 Subject: [PATCH 19/21] 2.3 release blog post --- bon-macros/src/builder/mod.rs | 6 +- bon/src/private/deprecations.rs | 6 + bon/src/private/mod.rs | 9 +- bon/tests/integration/builder/legacy.rs | 3 +- website/blog/bon-builder-v2-3-release.md | 163 +++++++++++++++++++++++ website/changelog.md | 4 + website/guide/positional-members.md | 2 +- 7 files changed, 181 insertions(+), 12 deletions(-) create mode 100644 bon/src/private/deprecations.rs create mode 100644 website/blog/bon-builder-v2-3-release.md diff --git a/bon-macros/src/builder/mod.rs b/bon-macros/src/builder/mod.rs index 97fb3c21..e351d684 100644 --- a/bon-macros/src/builder/mod.rs +++ b/bon-macros/src/builder/mod.rs @@ -42,9 +42,9 @@ fn try_generate_from_attr(params: TokenStream2, item: TokenStream2) -> Result + + +## New features + +### Positional arguments in starting and finishing functions + +While having the ability to use separate setters for the members gives you a ton of flexibility and extensibility described on the ["Compatibility"](../guide/compatibility) page, sometimes you don't need all of that. + +Maybe you'd like to pick out some specific members and let the user pass their values as positional parameters to the starting function that creates the builder or to the finishing function that consumes it. This reduces the syntax a bit at the cost of some extensibility loss ⚖️, but it may be worth it! + +#### Starting function + +As an example, suppose we have a `Treasure` struct with `x` and `y` coordinates and a `label` that describes the payload of the treasure. Since all treasures are located somewhere, they all have coordinates, and it would be cool to specify them in a single starting function call. + +To do that we can use the `#[builder(start_fn)]` attribute. There are two contexts where we can place it, and they both have a different meaning: + +- [Top-level `#[builder(start_fn = ...)]`](../reference/builder#start-fn) - configures the name of the starting function and optionally its visibility +- [Member-level `#[builder(start_fn)]`](../reference/builder#start-fn-1) - configures the member to be a positional parameter on the starting function + +We'll want to use both of these attributes in our example to give a better name for the starting function that describes its inputs and configure `x` and `y` as positional parameters on the starting function as well. + +**Example:** + +```rust +use bon::Builder; + +#[derive(Builder)] +// Top-level attribute to give a better name for the starting function // [!code highlight] +#[builder(start_fn = with_coordinates)] // [!code highlight] +struct Treasure { + // Member-level attribute to mark the member as // [!code highlight] + // a parameter of `with_coordinates()` // [!code highlight] + #[builder(start_fn)] // [!code highlight] + x: u32, + + #[builder(start_fn)] // [!code highlight] + y: u32, + + label: Option, +} + +let treasure = Treasure::with_coordinates(2, 9) // [!code highlight] + .label("oats".to_owned()) + .build(); + +assert_eq!(treasure.x, 2); +assert_eq!(treasure.y, 9); +assert_eq!(treasure.label.as_deref(), Some("oats")); +``` + +Here, the generated `with_coordinates` method has the following signature: + +```rust ignore +impl Treasure { + fn with_coordinates(x: u32, y: u32) -> TreasureBuilder { /**/ } +} +``` + +#### Finishing function + +Now let's say we need to know the person who claimed the `Treasure`. While describing the treasure using the current builder syntax we'd like the person who claimed it to specify their first name and last name at the end of the building process. + +We can use a similar combination of the [top-level `#[builder(finish_fn = ...)]`](../reference/builder#finish-fn) and the [member-level `#[builder(finish_fn)]`](../reference/builder#finish-fn-1) attributes to do that. + +**Example:** + +```rust +use bon::Builder; + +#[derive(Builder)] +#[builder(start_fn = with_coordinates)] +#[builder(finish_fn = claim)] // [!code highlight] +struct Treasure { + #[builder(start_fn)] + x: u32, + + #[builder(start_fn)] + y: u32, + + #[builder(finish_fn)] // [!code highlight] + claimed_by_first_name: String, // [!code highlight] + + #[builder(finish_fn)] // [!code highlight] + claimed_by_last_name: String, // [!code highlight] + + label: Option, +} + +let treasure = Treasure::with_coordinates(2, 9) + .label("oats".to_owned()) + .claim("Lyra".to_owned(), "Heartstrings".to_owned()); // [!code highlight] + +assert_eq!(treasure.x, 2); +assert_eq!(treasure.y, 9); +assert_eq!(treasure.label.as_deref(), Some("oats")); +assert_eq!(treasure.claimed_by_first_name, "Lyra"); // [!code highlight] +assert_eq!(treasure.claimed_by_last_name, "Heartstrings"); // [!code highlight] +``` + +You may also combine these attributes with [`#[builder(into)]`](../reference/builder#into) or [`#[builder(on(..., into))]`](../reference/builder#into) to reduce the number of `to_owned()` calls a bit. See this described in detail on the new ["Positional members"](../guide/positional-members#into-conversions) page in the guide. + +### Guaranteed MSRV is 1.59.0 now + +On the previous week's update (2.2 release) [a promise was made](./bon-builder-v2-2-release#guaranteed-msrv) to reduce the MSRV (minimum supported Rust version) from the initial 1.70.0 even further, and this has been done 🎉! + +This is the lowest possible MSRV we can guarantee for now. The choice of this version was made based on our design requirements for const generics supports described in [the comment here](https://github.com/elastio/bon/blob/3217b4b0349f03f0b2a5853310f420c5b8b005a7/bon/Cargo.toml#L21-L28). + + +## Deprecation warnings + +As was [promised](./bon-builder-v2-2-release#derive-builder-syntax-for-structs) in the previous release we are enabling deprecation warnings for the usage of the bare `#[bon::builder]` attribute on structs in favour of the new `#[derive(bon::Builder)]` syntax. + +The `#[builder]` syntax is still supported on functions and associated methods, and it's the only way to generate builders for them. + +The reasons for this deprecation as well as the instruction to update your code are described in the [2.2. release blog post](./bon-builder-v2-2-release#derive-builder-syntax-for-structs). + + +::: warning + +This isn't a breaking change, and the code that uses `#[bon::builder]` on a struct will still compile albeit with a compiler warning. Once `bon` reaches a `3.0` release we'll remove support for `#[bon::builder]` on structs entirely. However, there are no particular reasons and plans for a new major release of `bon` yet. + +::: + +## Summary + +Huge thank you for 925 stars ⭐ [on Github](https://github.com/elastio/bon)! Consider giving [`bon`] a star if you haven't already. Your support and feedback are a big motivation and together we can build a better builder 🐱! + +Bon's goal is to empower everyone to build beautiful APIs with great flexibility and extensibility. If you have any feedback or ideas for improvement consider joining [our Discord server](https://discord.gg/QcBYSamw4c) to discuss them, or just [open an issue on Github](https://github.com/elastio/bon/issues). + + + + +[`bon`]: https://github.com/elastio/bon + +*[Member]: Struct field or a function argument +*[member]: Struct field or a function argument +*[members]: Struct fields or function arguments diff --git a/website/changelog.md b/website/changelog.md index 8a5f9680..9fe414c4 100644 --- a/website/changelog.md +++ b/website/changelog.md @@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Forward lint suppression from `#[allow()/expect()]` attributes written by the user on the top-level to the generated items. + ## [2.2.1](https://github.com/elastio/bon/compare/v2.2.0...v2.2.1) - 2024-09-09 ### Changed diff --git a/website/guide/positional-members.md b/website/guide/positional-members.md index da162b9e..3cf5e657 100644 --- a/website/guide/positional-members.md +++ b/website/guide/positional-members.md @@ -56,7 +56,7 @@ impl Treasure { Now let's say we need to know the person who claimed the `Treasure`. While describing the treasure using the current builder syntax we'd like the person who claimed it to specify their first name and last name at the end of the building process. -We can use the similar combination of the [top-level](../reference/builder#finish-fn) `#[builder(finish_fn = ...)]` and the [member-level `#[builder(finish_fn)]`](../reference/builder#finish-fn-1) attributes to do that. +We can use a similar combination of the [top-level `#[builder(finish_fn = ...)]`](../reference/builder#finish-fn) and the [member-level `#[builder(finish_fn)]`](../reference/builder#finish-fn-1) attributes to do that. **Example:** From ef8cfe3a5f68aaa3ab7f3c479a866171f47cc75e Mon Sep 17 00:00:00 2001 From: Veetaha Date: Sat, 14 Sep 2024 13:18:43 +0000 Subject: [PATCH 20/21] Self-review and post-refactoring --- .../src/builder/builder_gen/member/mod.rs | 39 ++++++++++++++++++- .../src/builder/builder_gen/member/params.rs | 4 +- bon-macros/src/builder/builder_gen/mod.rs | 39 +------------------ .../src/builder/builder_gen/setter_methods.rs | 2 +- bon/src/private/mod.rs | 4 +- .../integration/ui/compile_fail/errors.stderr | 13 ++++++- .../ui/compile_fail/positional_members.rs | 2 +- .../ui/compile_fail/positional_members.stderr | 11 ------ website/changelog.md | 3 +- website/guide/documenting.md | 4 ++ 10 files changed, 61 insertions(+), 60 deletions(-) diff --git a/bon-macros/src/builder/builder_gen/member/mod.rs b/bon-macros/src/builder/builder_gen/member/mod.rs index 40ced26a..f24f8839 100644 --- a/bon-macros/src/builder/builder_gen/member/mod.rs +++ b/bon-macros/src/builder/builder_gen/member/mod.rs @@ -124,8 +124,43 @@ pub(crate) struct SkippedMember { } impl NamedMember { + fn reject_self_references_in_docs(&self) -> Result { + for doc in &self.docs { + let doc = match doc.as_doc() { + Some(doc) => doc, + _ => continue, + }; + + let doc = match &doc { + syn::Expr::Lit(doc) => doc, + _ => continue, + }; + + let doc = match &doc.lit { + syn::Lit::Str(doc) => doc, + _ => continue, + }; + + let self_references = ["[`Self`]", "[Self]"]; + + if self_references + .iter() + .any(|self_ref| doc.value().contains(self_ref)) + { + bail!( + &doc.span(), + "The documentation for the member should not reference `Self` \ + because it will be moved to the builder struct context where \ + `Self` changes meaning. Use explicit type names instead.", + ); + } + } + + Ok(()) + } + fn validate(&self) -> Result { - super::reject_self_references_in_docs(&self.docs)?; + self.reject_self_references_in_docs()?; if let Some(default) = &self.params.default { if self.norm_ty.is_option() { @@ -271,7 +306,7 @@ impl Member { .strip_prefix('_') .unwrap_or(&orig_ident_str); - // Preserve the original identifier span to make IDE go to definition correctly + // Preserve the original identifier span to make IDE's "go to definition" work correctly // and make error messages point to the correct place. let norm_ident = syn::Ident::new_maybe_raw(norm_ident, orig_ident.span()); let norm_ident_pascal = norm_ident.snake_to_pascal_case(); diff --git a/bon-macros/src/builder/builder_gen/member/params.rs b/bon-macros/src/builder/builder_gen/member/params.rs index eeec0874..4c320991 100644 --- a/bon-macros/src/builder/builder_gen/member/params.rs +++ b/bon-macros/src/builder/builder_gen/member/params.rs @@ -81,9 +81,7 @@ impl MemberParams { bail!( &attr_span, - "`{attr_name}` attribute can't be specified with other \ - attributes like {}", - conflicting, + "`{attr_name}` attribute can't be specified together with {conflicting}", ); } diff --git a/bon-macros/src/builder/builder_gen/mod.rs b/bon-macros/src/builder/builder_gen/mod.rs index 77fd0952..4a301742 100644 --- a/bon-macros/src/builder/builder_gen/mod.rs +++ b/bon-macros/src/builder/builder_gen/mod.rs @@ -201,7 +201,7 @@ impl BuilderGenCtx { let allows = allow_warnings_on_member_types(); - let regular_members_labels = self + let named_members_labels = self .named_members() .map(|member| self.members_label(member)); @@ -213,7 +213,7 @@ impl BuilderGenCtx { #( #[allow(non_camel_case_types)] #[doc(hidden)] - #vis struct #regular_members_labels; + #vis struct #named_members_labels; )* #allows @@ -789,41 +789,6 @@ pub(crate) fn generic_param_to_arg(param: &syn::GenericParam) -> syn::GenericArg } } -fn reject_self_references_in_docs(docs: &[syn::Attribute]) -> Result { - for doc in docs { - let doc = match doc.as_doc() { - Some(doc) => doc, - _ => continue, - }; - - let doc = match &doc { - syn::Expr::Lit(doc) => doc, - _ => continue, - }; - - let doc = match &doc.lit { - syn::Lit::Str(doc) => doc, - _ => continue, - }; - - let self_references = ["[`Self`]", "[Self]"]; - - if self_references - .iter() - .any(|self_ref| doc.value().contains(self_ref)) - { - bail!( - &doc.span(), - "The documentation for the member should not reference `Self` \ - because it will be moved to the builder struct context where \ - `Self` changes meaning. Use explicit type names instead.", - ); - } - } - - Ok(()) -} - fn allow_warnings_on_member_types() -> TokenStream2 { quote! { // This warning may occur when the original unnormalized syntax was diff --git a/bon-macros/src/builder/builder_gen/setter_methods.rs b/bon-macros/src/builder/builder_gen/setter_methods.rs index bafe9bb9..b68fb8d5 100644 --- a/bon-macros/src/builder/builder_gen/setter_methods.rs +++ b/bon-macros/src/builder/builder_gen/setter_methods.rs @@ -74,7 +74,7 @@ impl<'a> MemberSettersCtx<'a> { let setter_method_name = self.member.setter_method_core_name().clone(); - // Preserve the original identifier span to make IDE go to definition correctly + // Preserve the original identifier span to make IDE's "go to definition" work correctly let option_method_name = syn::Ident::new( &format!("maybe_{}", setter_method_name.raw_name()), setter_method_name.span(), diff --git a/bon/src/private/mod.rs b/bon/src/private/mod.rs index e0f7772b..bb416f13 100644 --- a/bon/src/private/mod.rs +++ b/bon/src/private/mod.rs @@ -4,8 +4,8 @@ clippy::inline_always, // Marking every potential function as `const` is a bit too much. - // Especially, this doesn't play well with our MSRV trait bounds - // aren't allowed on const functions. + // Especially, this doesn't play well with our MSRV. Trait bounds + // aren't allowed on const functions in older Rust versions. clippy::missing_const_for_fn )] diff --git a/bon/tests/integration/ui/compile_fail/errors.stderr b/bon/tests/integration/ui/compile_fail/errors.stderr index 00e2a942..ecb9a4cc 100644 --- a/bon/tests/integration/ui/compile_fail/errors.stderr +++ b/bon/tests/integration/ui/compile_fail/errors.stderr @@ -94,13 +94,13 @@ error: expected at least one parameter in parentheses 85 | #[builder(start_fn())] | ^^^^^^^^ -error: `skip` attribute can't be specified with other attributes like `into` +error: `skip` attribute can't be specified together with `into` --> tests/integration/ui/compile_fail/errors.rs:90:15 | 90 | #[builder(skip, into)] | ^^^^ -error: `skip` attribute can't be specified with other attributes like `name` +error: `skip` attribute can't be specified together with `name` --> tests/integration/ui/compile_fail/errors.rs:96:15 | 96 | #[builder(skip, name = bar)] @@ -166,6 +166,15 @@ note: attribute also specified here = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: `#[warn(unused_attributes)]` on by default +warning: use of deprecated module `bon::private::deprecations::builder_attribute_on_a_struct`: #[bon::builder] on top of a struct is deprecated; use `#[derive(bon::Builder)]` instead; see more details at https://elastio.github.io/bon/blog/bon-builder-v2-2-release#derive-builder-syntax-for-structs + --> tests/integration/ui/compile_fail/errors.rs:131:1 + | +131 | #[builder] + | ^^^^^^^^^^ + | + = note: `#[warn(deprecated)]` on by default + = note: this warning originates in the attribute macro `builder` (in Nightly builds, run with -Z macro-backtrace for more info) + error[E0599]: no method named `x` found for struct `SkipGeneratesNoSetterBuilder` in the current scope --> tests/integration/ui/compile_fail/errors.rs:21:38 | diff --git a/bon/tests/integration/ui/compile_fail/positional_members.rs b/bon/tests/integration/ui/compile_fail/positional_members.rs index 6ed78088..37aa11a4 100644 --- a/bon/tests/integration/ui/compile_fail/positional_members.rs +++ b/bon/tests/integration/ui/compile_fail/positional_members.rs @@ -57,7 +57,7 @@ struct IncorrectOrder7 { } #[derive(Builder)] -struct IncorrectOrder7 { +struct IncorrectOrder8 { #[builder(start_fn)] _a: (), #[builder(finish_fn)] diff --git a/bon/tests/integration/ui/compile_fail/positional_members.stderr b/bon/tests/integration/ui/compile_fail/positional_members.stderr index 2a494e12..34c0a47e 100644 --- a/bon/tests/integration/ui/compile_fail/positional_members.stderr +++ b/bon/tests/integration/ui/compile_fail/positional_members.stderr @@ -61,17 +61,6 @@ error: incorrect members oredering; the order of members must be the following: 55 | #[builder(start_fn)] | ^^^^^^^^ -error[E0428]: the name `IncorrectOrder7` is defined multiple times - --> tests/integration/ui/compile_fail/positional_members.rs:60:1 - | -52 | struct IncorrectOrder7 { - | ---------------------- previous definition of the type `IncorrectOrder7` here -... -60 | struct IncorrectOrder7 { - | ^^^^^^^^^^^^^^^^^^^^^^ `IncorrectOrder7` redefined here - | - = note: `IncorrectOrder7` must be defined only once in the type namespace of this module - error: incorrect members oredering; the order of members must be the following: (1) members annotated with #[builder(start_fn)] (2) members annotated with #[builder(finish_fn)] diff --git a/website/changelog.md b/website/changelog.md index 9fe414c4..19770fdc 100644 --- a/website/changelog.md +++ b/website/changelog.md @@ -8,7 +8,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- Forward lint suppression from `#[allow()/expect()]` attributes written by the user on the top-level to the generated items. +- Forward lint suppression from `#[allow()/expect()]` attributes written by the user on the top-level to the generated items +- Suppress the false-positive ([clippy issue]https://github.com/rust-lang/rust-clippy/issues/6947)) `clippy::future_not_send` lint ## [2.2.1](https://github.com/elastio/bon/compare/v2.2.0...v2.2.1) - 2024-09-09 diff --git a/website/guide/documenting.md b/website/guide/documenting.md index 1e622f94..aec8207e 100644 --- a/website/guide/documenting.md +++ b/website/guide/documenting.md @@ -32,3 +32,7 @@ This works because Rust compiler checks for invalid placement of `#[doc = ...]` ::: When `#[derive(Builder)]` is placed on top of a struct, then documentation on the struct fields will be copied to the docs on the setter methods. + +## Positional members + +Documentation comments are allowed on [positional members](./positional-members). However, since there are no separate setter methods generated for them, the docs on these members will not be copied anywhere, and thus they won't appear in `rustdoc`. Instead, it's recommended to write documentation for these members on the top level of the struct or function. From 7882622b6456477aca2473730b5facb31b0d02ac Mon Sep 17 00:00:00 2001 From: Veetaha Date: Sat, 14 Sep 2024 13:24:04 +0000 Subject: [PATCH 21/21] Fix broken links --- .github/workflows/ci.yml | 9 +-------- scripts/validate-links.sh | 14 ++++++++++++++ website/blog/bon-builder-v2-3-release.md | 2 +- 3 files changed, 16 insertions(+), 9 deletions(-) create mode 100755 scripts/validate-links.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ff98bb19..9d609688 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -188,11 +188,4 @@ jobs: cache-dependency-path: website/package-lock.json - run: cd website && npm ci - - run: cd website && npm run build - - name: Validate for broken links (mostly broken anchors) - run: | - cd website && FORCE_COLOR=2 \ - node \ - --no-warnings=ExperimentalWarning \ - --loader ts-node/esm \ - ./validate-links.mts + - run: scripts/validate-links.sh diff --git a/scripts/validate-links.sh b/scripts/validate-links.sh new file mode 100755 index 00000000..3b8061c8 --- /dev/null +++ b/scripts/validate-links.sh @@ -0,0 +1,14 @@ +#!/usr/bin/env bash +# +# Validate for broken links (mostly broken anchors) + +set -euo pipefail + +cd website + +npm run build + +FORCE_COLOR=2 node \ + --no-warnings=ExperimentalWarning \ + --loader ts-node/esm \ + ./validate-links.mts diff --git a/website/blog/bon-builder-v2-3-release.md b/website/blog/bon-builder-v2-3-release.md index a723c9b3..c917032f 100644 --- a/website/blog/bon-builder-v2-3-release.md +++ b/website/blog/bon-builder-v2-3-release.md @@ -9,7 +9,7 @@ outline: deep If you don't know about [`bon`], then see the [motivational blog post](./how-to-do-named-function-arguments-in-rust) and [the crate overview](../guide/overview). -## Meme of this release 🐱 +## Meme of this release :cat: