Consider supporting updating of already set values e.g. extending a Vec

My usecase is the following, I have a component with multiple fields, one of them is special, the children.

struct Component {
    normal_field: String,
    children: Html
}

I would like to use the builder to extend the Html, I created a workaround manually implementing a child() function:

#[allow(dead_code, non_camel_case_types, missing_docs)]
impl<__normal_field>
    HtmlPageBuilder<(__normal_field, (Html,))>
{
    pub fn child(mut self, child: impl ToHtml) -> Self {
        child.write_to_html(&mut self.fields.1.0);
        self
    }
}

#[allow(dead_code, non_camel_case_types, missing_docs)]
impl<__normal_field>
    HtmlPageBuilder<(__normal_field, ())>
{
    pub fn child(
        self,
        child: impl ToHtml,
    ) -> HtmlPageBuilder<(__mobile, __title, __styles, __scripts, (Html,))> {
        let (normal_field, ()) = self.fields;
        HtmlPageBuilder {
            fields: (normal_field, (child.into_html(),)),
            phantom: self.phantom,
        }
    }
}

I would prefer being able to have this built-in, maybe just as a parameter where I could pass an Fn(&mut Html, impl IntoHtml) or Fn(Option<Html>, impl IntoHtml).

Something else I'd like would be to disable the default generated children function without removing the generic for children, i.e. something that was like #[builder(setter(skip))] but only for the setter and not for the generic.

Maybe something like this?

#[derive(TypedBuilder)]
struct Component {
    normal_field: String,
    #[builder(setter(skip), default, mutators(
            fn child(child: impl ToHtml) {
                let child = child.to_html();
                move |children| children.append(child)
            }
    ))]
    children: Html
}

The return value of the mutator function is omitted on purpose, but it's basically always going to be impl FnOnce(&mut <field-type>). It'd generate a child method on the builder, with the same arguments as the child function inside the mutators attribute field. With many Rust-ceremony omitted, it'd look something like:

impl ComponentBuilder {
    fn child(mut self, child: impl ToHtml) -> Self {
        // This is the exact same function from `mutators`
        fn child(child: impl ToHtml) {
            let child = child.to_html();
            move |children| children.append(child)
        }
        (child(child))(&mut self.fields.1);
        self
    }
}

yes, that is exactly what I need. I assume the default would be eagerly evaluated and filled in (::builder()) for such a field?

(In my case it would be

#[derive(TypedBuilder)]
struct Component {
    normal_field: String,
    #[builder(setter(skip), default, mutators(
            fn child(child: impl ToHtml) {
                move |children| child.write_to_html(children)
            }
    ))]
    children: Html
}

to avoid unnecessary allocations)

The reason to not do fn child(children: &mut Html, child: impl Html) is that you want to avoid ambiguity about which argument is the field?

Otherwise, one could also say, that the first argument is always the field, and allow:

fn child(children: &mut Html, child: impl ToHtml) {
  child.write_to_html(children);
}

Maybe something like this?

#[derive(TypedBuilder)]
struct Component {
    normal_field: String,
    #[builder(setter(skip), default, mutators(
            fn child(child: impl ToHtml) {
                let child = child.to_html();
                move |children| children.append(child)
            }
    ))]
    children: Html
}

if this is the api you want to go for, I'd be willing to implement this.

Another option:

#[derive(TypedBuilder)]
struct Component {
    normal_field: String,
    #[builder(setter(skip), default, mutators(
            fn child(self, child: impl ToHtml) {
                self.children.append(child.to_html());
            }
    ))]
    children: Html
}

Which will generate:

impl ComponentBuilder {
    fn child(mut self, child: impl ToHtml) -> Self {
        struct TypedBuilderFieldMutator<'a> {
            children: &'a mut Html,
        }

        impl TypedBuilderFieldMutator<'_> {
            fn child(&mut self, child: impl ToHtml) {
                self.children.append(child.to_html());
            }
        }

        TypedBuilderFieldMutator {
            children: &mut self.fields.1
        }.child(child);

        self
    }
}

This is simpler than my original proposition, because it does not have that weird closure returning. And it has the advantage over your proposition that the user won't have to repeat the type. Also, using self to access the field should feel natural.

I do wonder though if having self in the function is okay, or if it should be &mut self - which is more correct.

This is simpler than my original proposition, because it does not have that weird closure returning. And it has the advantage over your proposition that the user won't have to repeat the type. Also, using self to access the field should feel natural.

I do wonder though if having self in the function is okay, or if it should be &mut self - which is more correct.

Having access to self would make me assume, I could also modify other fields, with the current approach this wouldn't be allowed.

(Though one could, as a future extension, create global mutators, that get mutable access to all fields, marked as mutable.)

I'm not really worried about that, because they'd just get a compiler error. Maybe even a rust-analyzer linter error, if we do it right.

I do wonder, though, if mutators should be mutually exclusive to setter and default. Consider this (lets ditch the Html type for something simpler):

#[derive(TypedBuilder)]
struct Foo {
    #[builder(
        default = vec![1, 2],
        mutators(
            fn bar(self, value: u32) {
                self.bars.push(value);
            }
        ),
    )]
    bars: Vec<u32>,
}

What should this resolve to?

Foo::builder().bar(3).bar(4).build().bars

Should it be [3, 4] because we added some values so the default should be ignored? Or should it be [1, 2, 3, 4], because it starts form the default and each call to bar mutated that?

Also, how about this?

Foo::builder().bar(3).bars(vec![4, 5]).bar(6).build().bars

Should it be [3, 4, 5, 6], because that's all the numbers we set? Or should it be [4, 5, 6], because the call to bars had overriden whatever was there?

For both questions, the first answer is what would make sense to a native reader to the API, and the second is what would make sense from the implementation side. I don't like that they differ, so I'd rather just prevent the possibility that they'd be used together.

Actually, for the second question, it'd make more sense to just disallow it - because after the first bar, the field already has a value so bars cannot be set. bars should only be called as the first bars-setting method. And of course, if there is no default, bars must be called before any bar - otherwise we don't have a way to construct the initial value (unless we dictate that fields with mutators must always implement Default)

Of course, we want the ability to declare an initial value (in part, so that we can have non-Default fields with mutators). Not sure how to tie it to the mutators though. Maybe this?

#[derive(TypedBuilder)]
struct Foo {
    #[builder(
        mutators(
            init = vec![1, 2];

            fn bar(self, value: u32) {
                self.bars.push(value);
            }
        ),
    )]
    bars: Vec<u32>,
}

Though this can get confusing. So maybe just this?

#[derive(TypedBuilder)]
struct Foo {
    #[builder(
        init = vec![1, 2],
        mutators(
            fn bar(self, value: u32) {
                self.bars.push(value);
            }
        ),
    )]
    bars: Vec<u32>,
}

And make it an error to set init without mutators?

(Though one could, as a future extension, create global mutators, that get mutable access to all fields, marked as mutable.)

What if we do it like that from the get-go?

#[derive(TypedBuilder)]
struct Foo {
    #![builder(mutators(
            fn bar(self, value: u32) {
                self.bars.push(value);
            }
    ))]

    #[builder(
        delegated(init = vec![1, 2]), // maybe need better names?
    )]
    bars: Vec<u32>,
}

This syntax is starting to get ugly though. Maybe we can just move the mutator fields to be named fields under the builder strunct, and rely on the fact that using mutators does not change the generic type of the builder?

#[derive(TypedBuilder)]
struct Foo {
    #[builder(
        delegated(init = vec![1, 2]), // maybe need better names?
    )]
    bars: Vec<u32>,
}

impl<B> FooBuilder<B> {
    fn bar(mut self, value: u32) -> Self {
        self.bars.push(value);
        self
    }
}

I'm not sure that would work though. If it turns out we need to constratin B, it may make the API too complex to use...

I agree it makes sense to disallow a setter after a mutator, and I wouldn't mind making them exclusive.

If someone wants to have a setter still, it would be straight forward to define, independently of the syntax we choose, and they would need to figure out how it should behave themselves.

e.g. for a vec it could make sense to say

struct Struct {
  #![builder(mutators(
    fn bar(&mut self, value: u32) {
        self.bars.push(value);
    }
    fn bars(&mut self, value: &[u32]) {
        self.bars.extend_from_slice(value);
    }
  ))]
  // field is mutably exclusive with a predefined setter
  #[builder(field)] // this would mean default
  #[builder(field = vec![1, 2, 3])] // this would init with additional value
  #[builder(field(pub))] // would make field public
  #[builder(field(pub, init = vec![1, 2, 3]))] // would make field public
  bars: Vec<u32>,
}

Another approach would be to allow mutators, to be type-state dependend, i.e.:

struct Struct {
  #![builder(mutators(
    fn f1_f2_set(self: (f1, f2), a: u32, b: bool) {
        // this function would have mutable access to
        self.initialized_field;
        self.f1;
        self.f2;
        // but can only be called when f1 and f2 are set
    }
    
    fn only_init_fields_are_set(self, b: bool) {
        // This function can always be called, but only has
        // access to fields that start out initialized
    }
  )]
  f1: Vec<u32>,
  #[builder(mutator(
    fn extend_f2(self, value: u32) {
      // This function is only availible if `f2` is set prior.
      self.f2.push(value);
    }
  )]
  f2: Vec<u32>,
  #[builder(init)] // This field starts initialized with default
  #[builder(init = value)] // This field starts initialized with `value`
  initialized_field: Vec<u32>
}

This would seperate the ability to use a mutator from pre initialization of a field.

One thing to note is, that init should imply setter(skip).

We could (but that is independent of this IMO) create a setter(overwrite) that allows calling a setter twice or calling a setter on an init field.

One thing to note is, that init should imply setter(skip).

Which is why I'm worried about this name - which does not seem related to the mutators thing.

Ideally I'd like a name that could do this:

#[derive(TypedBuilder)]
struct Foo {
    #![builder(mutators(...))]
    // `bar` is a regular field with a generated setter:
    bar: Bar,

    // `baz` has no setter and uses mutators. It gets initialized to `Baz::default()`.
    #[builder(something)]
    baz: Baz,

    // `qux` has no setter and uses mutators. It gets initialized to `Qux::new()`.
    #[builder(something = Qux::new())]
    qux: Qux,
}

But I'm not sure what would be a good name for "something". "via_mutators" won't work because #[builder(via_mutators = Qux::new())] does not indicate that Qux::new() is the initial value.

Maybe something like this?

#[builder(via_mutators(init = Qux::new()))]
qux: Qux,

#[builder(via_mutators(init = Qux::new()))]
qux: Qux,

I can't think of anything better right now. The only other Idea I had was to make init require setter(skip) to ensure that this fact is understood (similarly to how setter(skip) requires default).

Actually, this got me thinking about the relationship with setter(skip).

setter(skip) requires that a field has a default and disables the setter… we could use setter(skip) to mark fields as init.

I'm not sure what implications this would have, though. One result would be, that the default expression of setter(skip) is called in builder() instead of build(). Which would be unnecessary if one doesn't plan on using mutators.

It might also be a bit unintuitive.

I still think it should not be called deafult in case of mutators, because that would imply that if you use the mutators something other than the default is used.

My TypedBuilderFieldMutator has a potential problem - because the fields are &mut references to the original fields, they have to be dereferenced for some operations.

This will probably not be an issue for most things, since you'd use methods on them anyway, but it may still be confusing.

Creating an impl block for the builder will overcome this limitation. Of course, that does require some big changes to the builder struct (namely - adding all mutator-able fields as struct fields, which means we can no longer use regular names for the fields the macro uses...). I'm not sure what would be better...

Creating an impl block for the builder will overcome this limitation. Of course, that does require some big changes to the builder struct (namely - adding all mutator-able fields as struct fields, which means we can no longer use regular names for the fields the macro uses...). I'm not sure what would be better...

This would also remove the option to integrate the mutators in the typestate pattern

My TypedBuilderFieldMutator has a potential problem - because the fields are &mut references to the original fields, they have to be dereferenced for some operations.

This will probably not be an issue for most things, since you'd use methods on them anyway, but it may still be confusing.

Actually, I think this can work:

struct Builder<A, B, C> {
    fields: (A, B, C),
}

impl<A, C> Builder<A, usize, C> {
    fn add_to_b(self, b: usize) -> Self {
        struct TypeBuilderFieldMutator_B {
            b: usize,
        }
        impl TypeBuilderFieldMutator_B {
            // original fn from #[builder(mutators...
            fn add_to_b(&mut self, b: usize) {
                self.b += b
            }
        }

        let Self { fields: (a, b, c) };

        let mut mutator = TypeBuilderFieldMutator_B { b };
        mutator.add_to_b(b);
        let TypeBuilderFieldMutator_B { b } = mutator;

        Self { fields: (a, b, c) }
    }
}

No mutable references needed

Another name could be set or pre-set, because it is as if the setter was executed from the get go.

Both set and pre-set only convey the correct meaning if you know what they do. If you just see them, they can be quite confusing.

Currently, TypedBuilder exposes the whole of fields as one generic Parameter.

struct StructBuilder<TypedBuilderFields = ((), ())> {
    fields: TypedBuilderFields,
    phantom: ::core::marker::PhantomData<()>,
}

I see two options to extend this for via_mutators fields:

Either, keep the generic, but always set it, to the type of the field:

struct StructBuilder<TypedBuilderFields = ((), String)> {
    fields: TypedBuilderFields,
    phantom: ::core::marker::PhantomData<()>,
}

Alternatively, we could switch from one generic to one per field, and avoid the via_muators fields:

struct StructBuilder<__field1: ()> {
    fields: (__field1, String),
    phantom: ::core::marker::PhantomData<()>,
}

Or move the via_mutator fields in a separate field

struct StructBuilder<TypedBuilderFields = ((),)> {
    fields: TypedBuilderFields,
    via_mutators_fields: (String,)
    phantom: ::core::marker::PhantomData<()>,
}

I'd lean towards the first option, as it seems like the least disrupting to existing implementation.

Should we have error functions for mutators with required fields, that are called without those fields set?

Akin to the build() functions for unset required fields?

I don't think the question of required fields should even be relevant. I see no reason to allow mutators access to fields that don't have via_mutators, and fields that have via_mutators will not have the concept of "required" - they will always be initialized.

I'm open to the idea of having required via_mutators fields, that must first be initialized and are only then open to mutation, but since this complicates both the interface and the implementation, I'll have to be convinced first that this is really needed.

I'm open to the idea of having required via_mutators fields, that must first be initialized and are only then open to mutation, but since this complicates both the interface and the implementation, I'll have to be convinced first that this is really needed.

Well, I was already implementing that before your response, but I don't think it actually complicates the code much:

There is some additional parsing for the syntax I choose (Open to change that):

rust-typed-builder/typed-builder-macro/src/util.rs

Lines 256 to 276 in 5d8714a

    
           if receiver.colon_token.is_some() { 
        
               let fields = match &*receiver.ty { 
        
                   Type::Paren(ty) => { 
        
                       vec![&*ty.elem] 
        
                   } 
        
                   Type::Tuple(ty) => ty.elems.iter().collect(), 
        
                   ty => { 
        
                       return Err(syn::Error::new_spanned( 
        
                           ty, 
        
                           "expected (field, field2) to specify required fields", 
        
                       )) 
        
                   } 
        
               }; 
        
               requires = fields 
        
                   .into_iter() 
        
                   .map(|field| match field { 
        
                       Type::Path(field) => field.path.require_ident().cloned(), 
        
                       other => Err(syn::Error::new_spanned(other, "expected field identifier")), 
        
                   }) 
        
                   .collect::<Result<_, _>>()?; 
        
               required_fields_span = receiver.ty.span();

In the actual macro expansion, there isn't much of a difference:

rust-typed-builder/typed-builder-macro/src/struct_info.rs

Lines 433 to 439 in 5d8714a

    
           for f @ FieldInfo { name, ty, .. } in self.included_fields() { 
        
               if f.builder_attr.via_mutators.is_some() || required_fields.remove(f.name) { 
        
                   ty_generics_tuple.elems.push(f.tuplized_type_ty_param()); 
        
                   mutator_ty_fields.push(quote!(#name: #ty)); 
        
                   mutator_destructure_fields.push(name); 
        
                   quote!((#name,),).to_tokens(&mut destructuring); 
        
               } else {

After seeing the error messages, I think the required_fields_span doesn't do anything, and I'll remove it, simplifying this a bit more.

	if receiver.colon_token.is_some() {
	let fields = match &*receiver.ty {
	Type::Paren(ty) => {
	vec![&*ty.elem]
	}
	Type::Tuple(ty) => ty.elems.iter().collect(),
	ty => {
	return Err(syn::Error::new_spanned(
	ty,
	"expected (field, field2) to specify required fields",
	))
	}
	};
	requires = fields
	.into_iter()
	.map(\|field\| match field {
	Type::Path(field) => field.path.require_ident().cloned(),
	other => Err(syn::Error::new_spanned(other, "expected field identifier")),
	})
	.collect::<Result<_, _>>()?;
	required_fields_span = receiver.ty.span();

	for f @ FieldInfo { name, ty, .. } in self.included_fields() {
	if f.builder_attr.via_mutators.is_some() \|\| required_fields.remove(f.name) {
	ty_generics_tuple.elems.push(f.tuplized_type_ty_param());
	mutator_ty_fields.push(quote!(#name: #ty));
	mutator_destructure_fields.push(name);
	quote!((#name,),).to_tokens(&mut destructuring);
	} else {