Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 133 additions & 0 deletions bon-macros/src/builder/builder_gen/build_from.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
use crate::builder::builder_gen::{BuilderGenCtx, member::Member};

Check warning on line 1 in bon-macros/src/builder/builder_gen/build_from.rs

View workflow job for this annotation

GitHub Actions / cargo-fmt

Diff in /home/runner/work/bon/bon/bon-macros/src/builder/builder_gen/build_from.rs
use crate::util::prelude::*;
use proc_macro2::Span;
use quote::quote;
use syn::{Type, spanned::Spanned};

Check warning on line 5 in bon-macros/src/builder/builder_gen/build_from.rs

View workflow job for this annotation

GitHub Actions / cargo-fmt

Diff in /home/runner/work/bon/bon/bon-macros/src/builder/builder_gen/build_from.rs

pub(super) fn emit(ctx: &BuilderGenCtx, target_ty: &Type) -> Result<TokenStream> {
let mut tokens = TokenStream::new();

let field_vars: Vec<_> = ctx
.members
.iter()
.map(|member| {
let ident = member.orig_ident();
let ty = member.norm_ty();
let default_expr = quote! { ::core::default::Default::default() };

match member {
Member::Field(_) | Member::StartFn(_) => quote! {
let #ident: #ty = self.#ident;
},
Member::Named(member) => {
let index = &member.index;
quote! {
let #ident: #ty = match self.__unsafe_private_named.#index {
Some(value) => value,
None => from.#ident.clone(),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is that build_from accepts T by value and does not call clone() at all (that's why this method's name is a bit shorter since it is less expensive and should be the preferred thing), but build_from_clone accepts &T and calls .clone() if needed (that's why it has "clone" in its name).

};
}
}
Member::FinishFn(_) => quote! {
let #ident: #ty = from.#ident.clone();
},
Member::Skip(_) => quote! {
let #ident: #ty = #default_expr;
},
}
})
.collect();

let ctor_args: Vec<_> = ctx
.members
.iter()
.map(|m| {
let ident = m.orig_ident();
quote! { #ident }
})
.collect();

if ctx.build_from {
tokens.extend(emit_build_from_method(
false,
target_ty,
&field_vars,
&ctor_args,
));
}

if ctx.build_from_clone {
tokens.extend(emit_build_from_method(
true,
target_ty,
&field_vars,
&ctor_args,
)?);
}

Ok(tokens)
}

fn emit_build_from_method(
clone: bool,
target_ty: &Type,
field_vars: &[TokenStream],
ctor_args: &[TokenStream],
) -> Result<TokenStream> {
let doc = if clone {
"Fills unset builder fields from an owned value of the target type and builds it."
} else {
"Fills unset builder fields from a reference to the target type and builds it."
};

let method_name = if clone {
quote!(build_from_clone)
} else {
quote!(build_from)
};

let arg_type = if clone {
quote!(#target_ty)
} else {
quote!(&#target_ty)
};

let arg_pat = if clone {
quote!(mut from)
} else {
quote!(from)
};

let ctor_path = extract_ctor_ident_path(target_ty, target_ty.span())?;

Ok(quote! {
#[inline(always)]
#[doc = #doc]
pub fn #method_name(self, #arg_pat: #arg_type) -> #target_ty {
#( #field_vars )*
#ctor_path {
#( #ctor_args, )*
}
Comment on lines +82 to +84
Copy link
Collaborator

@Veetaha Veetaha Jul 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunatelly, this approach won't work in case if the builder is generated with the #[builder] macro on top of a function. We have the body.generate() in finish_fn.rs here that generates the proper body:

let body = &self.finish_fn.body.generate(self);

It's only requirement is that members are assigned to variables of the same name in scope. Plus we need to inherit the potential async / unsafe / const modifiers and maybe handle the case of the Result from the original function

}
})
}

pub(crate) fn extract_ctor_ident_path(ty: &Type, span: Span) -> Result<TokenStream> {
use quote::quote_spanned;

Check failure on line 116 in bon-macros/src/builder/builder_gen/build_from.rs

View workflow job for this annotation

GitHub Actions / test-msrv (windows, 1.61.0)

the item `quote_spanned` is imported redundantly

Check failure on line 116 in bon-macros/src/builder/builder_gen/build_from.rs

View workflow job for this annotation

GitHub Actions / test-msrv (ubuntu, 1.61.0)

the item `quote_spanned` is imported redundantly

Check failure on line 116 in bon-macros/src/builder/builder_gen/build_from.rs

View workflow job for this annotation

GitHub Actions / test-msrv (macos, 1.59.0)

the item `quote_spanned` is imported redundantly

Check failure on line 116 in bon-macros/src/builder/builder_gen/build_from.rs

View workflow job for this annotation

GitHub Actions / test-msrv (ubuntu, 1.59.0)

the item `quote_spanned` is imported redundantly

Check failure on line 116 in bon-macros/src/builder/builder_gen/build_from.rs

View workflow job for this annotation

GitHub Actions / test-msrv (macos, 1.61.0)

the item `quote_spanned` is imported redundantly

Check failure on line 116 in bon-macros/src/builder/builder_gen/build_from.rs

View workflow job for this annotation

GitHub Actions / test-msrv (windows, 1.59.0)

the item `quote_spanned` is imported redundantly

let path = ty.as_path_no_qself().ok_or_else(|| {
err!(
&span,
"expected a concrete type path (like `MyStruct`) for constructor"
)
})?;

let ident = path
.segments
.last()
.ok_or_else(|| err!(&span, "expected a named type, but found an empty path"))?
.ident
.clone();

Ok(quote_spanned! { span => #ident })
Copy link
Collaborator

@Veetaha Veetaha Jul 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a known footgun of quote_spanned!. It doesn't overwrite the span of interpolated tokens such as #ident - only the bare tokens created as part of that quasi-quotation syntax. But (1), otherwise, I don't see a need to override the span here. The span is preserved from the input by default, and that's desired. But (2) see my comment below about the body

}
2 changes: 2 additions & 0 deletions bon-macros/src/builder/builder_gen/input_fn/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,8 @@ impl<'a> FnInputCtx<'a> {
state_mod: self.config.state_mod,
start_fn: self.start_fn,
finish_fn,
build_from: self.config.build_from,
build_from_clone: self.config.build_from_clone,
})
}
}
Expand Down
2 changes: 2 additions & 0 deletions bon-macros/src/builder/builder_gen/input_struct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,8 @@ impl StructInputCtx {
state_mod: self.config.state_mod,
start_fn,
finish_fn,
build_from: self.config.build_from,
build_from_clone: self.config.build_from_clone,
})
}
}
Expand Down
7 changes: 7 additions & 0 deletions bon-macros/src/builder/builder_gen/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
mod build_from;
mod builder_decl;
mod builder_derives;
mod finish_fn;
Expand Down Expand Up @@ -134,6 +135,11 @@ impl BuilderGenCtx {

let allows = allow_warnings_on_member_types();

let build_froms = match &self.finish_fn.output {
syn::ReturnType::Type(_, ty) => build_from::emit(self, ty)?,
syn::ReturnType::Default => quote! {},
};

Ok(quote! {
#allows
#[automatically_derived]
Expand All @@ -145,6 +151,7 @@ impl BuilderGenCtx {
#where_clause
{
#finish_fn
#build_froms
#(#accessor_methods)*
}
})
Expand Down
8 changes: 8 additions & 0 deletions bon-macros/src/builder/builder_gen/models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,8 @@ pub(crate) struct BuilderGenCtx {
pub(super) state_mod: StateMod,
pub(super) start_fn: StartFn,
pub(super) finish_fn: FinishFn,
pub(super) build_from: bool,
pub(super) build_from_clone: bool,
}

pub(super) struct BuilderGenCtxParams<'a> {
Expand Down Expand Up @@ -201,6 +203,8 @@ pub(super) struct BuilderGenCtxParams<'a> {
pub(super) state_mod: ItemSigConfig,
pub(super) start_fn: StartFnParams,
pub(super) finish_fn: FinishFnParams,
pub(super) build_from: bool,
pub(super) build_from_clone: bool,
}

impl BuilderGenCtx {
Expand All @@ -219,6 +223,8 @@ impl BuilderGenCtx {
state_mod,
start_fn,
finish_fn,
build_from,
build_from_clone,
} = params;

let builder_type = BuilderType {
Expand Down Expand Up @@ -370,6 +376,8 @@ impl BuilderGenCtx {
state_mod,
start_fn,
finish_fn,
build_from,
build_from_clone,
})
}
}
Expand Down
10 changes: 8 additions & 2 deletions bon-macros/src/builder/builder_gen/top_level_config/mod.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
mod on;

pub(crate) use on::OnConfig;

Check warning on line 4 in bon-macros/src/builder/builder_gen/top_level_config/mod.rs

View workflow job for this annotation

GitHub Actions / cargo-fmt

Diff in /home/runner/work/bon/bon/bon-macros/src/builder/builder_gen/top_level_config/mod.rs
use crate::parsing::{BonCratePath, ItemSigConfig, ItemSigConfigParsing, SpannedKey};
use crate::util::prelude::*;
use darling::ast::NestedMeta;
use darling::FromMeta;
use darling::ast::NestedMeta;
use syn::ItemFn;
use syn::parse::Parser;
use syn::punctuated::Punctuated;
use syn::ItemFn;

fn parse_finish_fn(meta: &syn::Meta) -> Result<ItemSigConfig> {
ItemSigConfigParsing {
Expand Down Expand Up @@ -75,6 +75,12 @@
/// Specifies the derives to apply to the builder.
#[darling(default, with = crate::parsing::parse_non_empty_paren_meta_list)]
pub(crate) derive: DerivesConfig,

#[darling(default)]
pub(crate) build_from: bool,
Copy link
Collaborator

@Veetaha Veetaha Jul 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should use Option<Option<ItemSigConfig>> to make their names, visibility and docs configurable, plus inherit the visibility of the builder struct similar to how the regular finish_fn does that here:

vis: finish_fn.vis.unwrap_or_else(|| builder_type.vis.clone()),

(^ this is the place where a bunch of complex inter-field-dependent defaults are resolved)

Otherwise the docs would be incorrectly claiming that these attributes support vis, name, doc attributes.

I think this may require writing a custom with parser function that handles a syn::Meta::Path (in which case it returns None (i.e. Some(None))), and otherwise delegates to ItemSigConfig.


#[darling(default)]
pub(crate) build_from_clone: bool,
}

impl TopLevelConfig {
Expand Down
4 changes: 2 additions & 2 deletions bon-macros/src/builder/builder_gen/top_level_config/on.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::util::prelude::*;

Check warning on line 1 in bon-macros/src/builder/builder_gen/top_level_config/on.rs

View workflow job for this annotation

GitHub Actions / cargo-fmt

Diff in /home/runner/work/bon/bon/bon-macros/src/builder/builder_gen/top_level_config/on.rs
use darling::util::Flag;
use darling::FromMeta;
use darling::util::Flag;
use syn::parse::Parse;
use syn::spanned::Spanned;
use syn::visit::Visit;
Expand Down Expand Up @@ -79,7 +79,7 @@

impl Visit<'_> for FindAttr {
fn visit_attribute(&mut self, attr: &'_ syn::Attribute) {
self.attr.get_or_insert(attr.span());
self.attr.get_or_insert_with(|| attr.span());
}
}

Expand Down
34 changes: 34 additions & 0 deletions bon/src/__/ide.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,40 @@ pub mod builder_top_level {
pub use core::convert::Into;
}

/// See the docs at <https://bon-rs.com/reference/builder/top-level/build_from>
pub const build_from: Option<Identifier> = None;

/// See the docs at <https://bon-rs.com/reference/builder/top-level/build_from>
pub mod build_from {
use super::*;

/// See the docs at <https://bon-rs.com/reference/builder/top-level/build_from#name>
pub const name: Identifier = Identifier;

/// See the docs at <https://bon-rs.com/reference/builder/top-level/build_from#vis>
pub const vis: VisibilityString = VisibilityString;

/// See the docs at <https://bon-rs.com/reference/builder/top-level/build_from#doc>
pub const doc: DocComments = DocComments;
}

/// See the docs at <https://bon-rs.com/reference/builder/top-level/build_from_clone>
pub const build_from_clone: Option<Identifier> = None;

/// See the docs at <https://bon-rs.com/reference/builder/top-level/build_from_clone>
pub mod build_from_clone {
use super::*;

/// See the docs at <https://bon-rs.com/reference/builder/top-level/build_from_clone#name>
pub const name: Identifier = Identifier;

/// See the docs at <https://bon-rs.com/reference/builder/top-level/build_from_clone#vis>
pub const vis: VisibilityString = VisibilityString;

/// See the docs at <https://bon-rs.com/reference/builder/top-level/build_from_clone#doc>
pub const doc: DocComments = DocComments;
}

/// The real name of this parameter is `crate` (without the underscore).
/// It's hinted with an underscore due to the limitations of the current
/// completions limitation. This will be fixed in the future.
Expand Down
26 changes: 26 additions & 0 deletions bon/tests/integration/builder/build_from.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
use crate::prelude::*;

#[derive(Builder, Clone)]
#[builder(build_from, build_from_clone)]
Copy link
Collaborator

@Veetaha Veetaha Jul 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should support method/function syntax too, unless you'd like to keep it supported only on struct derives initially.

For that, we should have tests for methods/functions too. In fact, bon's tests are quite repetitive (unfortunately) - we test all 3 cases of the syntax - structs, methods and free functions. I recommend following the naming conventions and patterns like here. E.g. test functions are called test_{struct,method,function}. Different test suites are separated into different test modules (inline modules can be used if tests are small). You may notice that sometimes, not all three cases are covered (if as a white-box we know the code under test is agnostic to the kind of syntax - struct or function - doesn't matter to it).

struct User {
name: String,

Check failure on line 6 in bon/tests/integration/builder/build_from.rs

View workflow job for this annotation

GitHub Actions / test-stable (ubuntu, --locked)

cannot find type `String` in this scope

Check failure on line 6 in bon/tests/integration/builder/build_from.rs

View workflow job for this annotation

GitHub Actions / test-stable (windows, --locked)

cannot find type `String` in this scope

Check failure on line 6 in bon/tests/integration/builder/build_from.rs

View workflow job for this annotation

GitHub Actions / test-stable (macos, --locked)

cannot find type `String` in this scope

Check failure on line 6 in bon/tests/integration/builder/build_from.rs

View workflow job for this annotation

GitHub Actions / test-stable (windows)

cannot find type `String` in this scope

Check failure on line 6 in bon/tests/integration/builder/build_from.rs

View workflow job for this annotation

GitHub Actions / test-stable (ubuntu)

cannot find type `String` in this scope

Check failure on line 6 in bon/tests/integration/builder/build_from.rs

View workflow job for this annotation

GitHub Actions / test-stable (macos)

cannot find type `String` in this scope
age: u8,
}

#[test]
fn test_build_from_works() {
let jon = User::builder().name("Jon".into()).age(25).build();
let alice = User::builder().name("Alice".into()).build_from(&jon);
assert_eq!(alice.age, jon.age);
assert_eq!(alice.name, "Alice");
}

#[test]
fn test_build_from_clone_works() {
let jon = User::builder().name("Jon".into()).age(25).build();
let alice = User::builder()
.name("Alice".into())
.build_from_clone(jon.clone());
assert_eq!(alice.age, jon.age);
assert_eq!(alice.name, "Alice");
}
1 change: 1 addition & 0 deletions bon/tests/integration/builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ mod attr_setters;
mod attr_skip;
mod attr_top_level_start_fn;
mod attr_with;
mod build_from;
mod cfgs;
mod generics;
mod init_order;
Expand Down
Loading