Support special setters for collections

[Veetaha]

This issue is based on prior thoughts in the alternatives section of the docs.

Design

I don't have a good design for the attribute yet. There are many unresolved questions.

• What should be the name of the attribute? The name should make it clear that it generates setter(s?) that extend a collection (push (front or back? for VecDeque), insert).
• Where should the attribute reside? Should it be at the level of #[builder(attr_name)] or #[builder(setters(attr_name))]?
• Should the setter accept Self by &mut or by value? The typestate won't change after a value is pushed into a colleciton, so &mut self is possible. However by value seems like a more natural fit to the current design where the setters always require self by value.
  • Maybe we should generate a couple of setters for &mut and by-value self?
  • What name pattern would this couple of setters use?
  • How much configrability should we provide for this?

Do We Need This At All?

How often would people want such setters? If that would be needed at a rare occasion, then people could use custom fields (#189) with custom methods to implement this.

I'll keep this issue on hold

Prior Art

• #[builder(setter(each))] in derive_builder
• Implicit special setters for collections in buildstructor
• Java Lombok @Singular

A note for the community from the maintainers

Please vote on this issue by adding a 👍 reaction to help the maintainers with prioritizing it. You may add a comment describing your real use case related to this issue for us to better understand the problem domain.

[andrzejressel]

In Java there is lombok - they do it by separate annotation @Singular on field, that can take a string that would be new method name.

[Veetaha]

I've published a 3.1 release of bon that adds #[builder(field)] annotation allowing you to define custom fields. This can be used in combination with custom methods to define special setters for collections. I believe this may be good enough for many use cases since it provides great flexibility to how you'd like these special setters to look like.

See the example of this feature in the new Custom Fields guide.

@andrzejressel do you think #[builder(field)] would cover your use case, or would you like to have some attribute to generate setters for collections via bon? IIRC you are using bon in generated code, so defining custom fields/methods could be fine in this context, since you may define their template in one place.

[jvatic]

@Veetaha I'm attempting to use #[builder(field)] for adding a custom setter to push an item into a Vec. However, I don't see any way of making such a field required without resorting to what feels like an ugly work-around, e.g.:

pub struct MyStruct {
    entries: Vec<usize>,
}

// using a method builder so `entries_marker` doesn't have to be part of the struct
#[bon]
impl MyStruct {
    #[builder]
    pub fn new(
        #[builder(field = Vec::new())] entries: Vec<usize>,
        #[builder(setters(vis = ""), overwritable)]
        #[allow(unused)]
        entries_marker: bool, // work-around to make `entries` required
    ) -> Self {
        Self {
            entries,
        }
    }
}

impl<S: State> MyStructBuilder<S> {
    pub fn entry(
        mut self,
        entry: usize,
    ) -> MyStructBuilder<SetEntriesMarker<S>> {
        self.entries.push(entry);
        self.entries_marker(true)
    }
    
    pub fn entries(
        mut self,
        entries: Vec<usize>,
    ) -> MyStructBuilder<SetEntriesMarker<S>>
    where
        S::EntriesMarker: IsUnset,
    {
        self.entries = entries;
        self.entries_marker(true)
    }
}

Is there in fact a clean way of doing this currently?

While the above example works, it requires using the experimental-overwritable feature, and error messages are not the best (the member Unset<entries_marker> was not set, but this method requires it to be set—which could be traded for less intuitive variable names).

I'd feel a lot better about it if there was a State I could generate to determine when the entries field is set (based on my custom methods), which would also require some way of constructing the builder type for that state to return.

I'd love to be able to use bon for this edge case ;)

[Veetaha]

Hi @jvatic, unfortunately, I don't have a cleaner solution to this except maybe suggest that you'd use entries name instead of entries_marker for the marker field, and call the internal Vec field something else to improve error messages, and use a unit type () instead of bool for the marker.

Another way to approach this (which is also not very clean), is to accept the first or last item of the collection as a positional parameter of the start_fn or finish_fn.

To have a clean solution, this requires a special type state, and I think it's not that easy to do without the pitfall of SetEntries<SetEntries<SetEntrires<...>>> state nesting that accumulates on every setter call. This is what's currently blocking the stabilisation of the overwritable feature (described in detail in the comment #149 (comment), btw this comment also contains a bit different more complex solution to this problem). I haven't found a solution to the problem of "state nesting on every setter call" with the current state of Rust, and that's why it's still marked as experimental.

But in general, I'm not a fan of "pusher" methods on builders. To my mind, the problem of building collections should usually be solved separately (i.e. build a collection separately from the builder syntax). If you'd really like to ensure the collection is non-empty at compile time, you could use a primitive like Vec1, which would encode this property in the type itself.

[jvatic]

Thanks for the suggestions and context @Veetaha! After thinking this over a bit more, I've decided I don't really need to support calling the singleton setter more than once for my use case—it's more of a convenience for if there's a single item in the collection, so I can just call the generated collection setter from a custom singleton one.

[Veetaha]

Some ideas for the potential syntax here taken from Discord discussions.

#[builder(collection(
    extend_one(vis, doc, name, into, with),
    extend(vis, doc, name, into, with),
))]

with the short syntax of #[builder(collection(extend_one = name, extend = name))]. No check for "at least one" will be possible. No type state will be added for collection members. No setter that accepts the collection itself is added. The extend method accepts an IntoIterator