colin-kiegel / rust-derive-builder

derive builder implementation for rust structs
https://colin-kiegel.github.io/rust-derive-builder/
Apache License 2.0
1.28k stars 82 forks source link

Non-Clone / `self`-consuming `build` #212

Closed dan-fritchman closed 3 years ago

dan-fritchman commented 3 years ago

Love the crate, thanks for all of the work on it.

I see the generated build methods Clone::clone each attribute. For example in the readme code: https://github.com/colin-kiegel/rust-derive-builder#how-it-works

fn build(
        &self, // <= looking here 
    ) -> Result<Channel, ChannelBuilderError> {
        Ok(Channel {
            id: match self.id {
                Some(ref value) => Clone::clone(value), // <= and here 
                None => // error stuff
            },

For non-Clone members, it would seem this (or a version of it) could consume self instead, and use its attributes directly, along the lines of:

fn build (
        self, // <= consuming self
    ) -> Result<Channel, ChannelBuilderError> {
        let ChannelBuilder { id, ... } = self;
        Ok(Channel {
            id: match id {
                Some(value) => value, // <= 
                None => // error stuff
            },

I suppose the build-method may not want to do this all the time (I guess in the case it's building more than one copy).

Builder has quite a few (very helpful) options, I've been looking through them for something like this and not finding it. Is such a thing supported? If not, is there an alternate recommendation for non-cloning attributes (maybe an annotation to leave them off the builder)?

Thanks!

TedDriggs commented 3 years ago

You should be able to do this with the #[builder(pattern = "owned")] attribute (relevant source code)

dan-fritchman commented 3 years ago

Thanks! My impression was the BuilderPattern options were designed to change the return type of the setter methods. I see it changes the build method as well. For example, this:

#[derive(Default, Debug)]
struct NonClone;

#[derive(Default, Builder, Debug)]
#[builder(pattern="owned", setter(into), private)]
struct BuildOwned {
    non_clone: NonClone
}

Expands to:

#[allow(dead_code)]
impl BuildOwnedBuilder {
    #[allow(unused_mut)]
    fn non_clone<VALUE: ::std::convert::Into<NonClone>>(self, value: VALUE) -> Self {
        let mut new = self;
        new.non_clone = ::std::option::Option::Some(value.into());
        new
    }
    #[doc = "Builds a new `BuildOwned`.\n\n# Errors\n\nIf a required field has not been initialized.\n"]
    fn build(self) -> ::std::result::Result<BuildOwned, ::std::string::String> {
        Ok(BuildOwned {
            non_clone: match self.non_clone {
                Some(value) => value,
                None => // error stuff 
            },
        })
    }

This is great.
One caveat, I don't really want the "consume and return self" setter behavior. Is there some other options that decouples these two?

TedDriggs commented 3 years ago

There is not.

dan-fritchman commented 3 years ago

Thanks, closing.

Mart-Bogdan commented 2 years ago

You should be able to do this with the #[builder(pattern = "owned")] attribute (relevant source code)

Perhaps more info should be added to readme.

Basically all that is written there:

Builder patterns: You can opt into other builder patterns by preceding your struct (or field) with #[builder(pattern = "owned")] or #[builder(pattern = "immutable")].

So I went to search if issue already exists, or I should create one, Turns out this is already implemented, but not documented in obvious way.

P.S. it is in documentation https://docs.rs/derive_builder/latest/derive_builder/#owned-aka-consuming but would be nice to have it in Readme, and perhaps more visible in documentation as well.