Giter Club home page Giter Club logo

rust-derive-builder's People

Contributors

0g00 avatar adam-gleave avatar alexanderkjall avatar andy128k avatar azriel91 avatar badboy avatar blakejakopovic avatar chanced avatar colin-kiegel avatar colonelthirtytwo avatar davidb avatar dtolnay avatar eijebong avatar frisoft avatar ignatenkobrain avatar ijackson avatar killercup avatar leo848 avatar luro02 avatar mathstuf avatar mpalmer avatar nicholastmosher avatar red1bluelost avatar ryo33 avatar spearman avatar steadbytes avatar teddriggs avatar toku-sa-n avatar twilco avatar vishalsodani avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar

rust-derive-builder's Issues

Documentation comments appear in reversed order.

This …

struct Lorem {
    /// This is a doc comment for a field
    ///
    /// This is a second comment line
    field_with_doc_comment: String,
    ...
}

… turns into this …

struct Lorem {
    #[doc = r" This is a second comment line"]
    #[doc = r""]
    #[doc = r" This is a doc comment for a field"]
    field_with_doc_comment: String,
    ...
}

… when building with --pretty=expanded.

They then appear in the same reversed order within the HTML documents (via $ cargo doc).

failing doc test

I un-ignored this doc-test and somehow dev/githook.sh didn't catch this.

  • we don't have setter(name = "...") yet - my bad.
  • need to take a closer look at why dev/githook.sh didn't work

I'll fix this later https://travis-ci.org/colin-kiegel/rust-derive-builder/jobs/221262990#L2912. Have to go now. :-)

//! # #[cfg(feature = "nightlytests")]
//! #[derive(Builder, Debug, PartialEq)]
//! struct Ipsum {
//!     #[builder(try_setter, setter(into, name = "foo"))]
//!     pub dolor: u8,
//! }
error: proc-macro derive panicked
  --> <anon>:16:10
   |
16 | #[derive(Builder, Debug, PartialEq)]
   |          ^^^^^^^
   |
   = help: message: Unknown setter option `name` on field `dolor`.

PS: one way of fixing would be to implement name = "..." ^^

Allow author to opt out of public builder fields

It is considered a breaking change in Rust to add a field to a struct whose fields were all public in a previous release, as the added field can break pattern matches and struct literals. Many builder use cases do not require access to the partially-initialized fields, so exposing them increases the author's risk of a breaking change for no discernible use-case.

Proposal

Add a new option at the struct level: #[builder(fields(...))]. This option takes either:

  • public - for back-compat reasons, this probably needs to be the default. (I view this as unfortunate, since I think users should have to opt into broadening the API surface they expose)
  • private

This option will not be valid at the individual field level.

todo: test with multiple versions of custom_derive

We can not control which version of custom_derive is used by a client of this crate.

Just to be save, we should test with multiple versions of custom_derive. It would be sufficient if these tests only run on travis.

However I'am not sure, about the best way to achieve this.

  • it seems like dev-dependencies can't be opted out of
  • optional dependencies are fixed to a specific version
  • cargo features may only specify optional dependencies

http://doc.crates.io/manifest.html

One hacky approach might be usage of sed 's/placeholder/version/g' on a Cargo.toml template before cargo test.

don't fail if not necessary

General idea: Try not to fail on code which is ok for custom_derive and not of interest for derive_builder, like

  • unit-like structs struct Foo;
  • tuple-structs
  • enums

Does this make sense, or do we want to fail in these cases?

Note: The 'empty' struct struct Foo { } is equivalent to the unit-like struct, but we already don't fail there.

Support custom documentation for setters

Consider this builder:

custom_derive! {
    #[derive(Debug, Default, Builder)]
    struct Channel {
        //! This is a comment for token
        token: i32,
        special_info: i32
    }
}

It would be great if there was a way to add the comments as documentation for the fields:

//! This is a comment for token
fn token<VALUE: Into<i32>>(mut self, value: VALUE) -> Self {
    self.token = value.into();
    self
}

No clue if that is possible, though. 😄

Idea: Let the builder wrap a Result<T,Error>

Here is a loose idea. It is mostly meant as an inspiration..

It could be to have a builder that wraps a Result. This would allow the builder to validate fields during the build process, and keep track of errors. Rust already allows you to do this with the try!(), macro, and the ?-operator has been accepted as RFC 243, but I still think that it could be nice to have a builder that keeps track of errors.

Here is what the code could look like

// The following only works on unstable
![feature(try_from)]
use std::convert::{TryFrom, TryInto};

#[derive(Debug)]
enum  MyError {
    TooOld
}

// I am not fully sure if it makes sense to annotate the fields
// #[builder(PersonBuilder, MyError)]
#[derive(Default)]
struct Person {
    name: String,
    // #[try_from]
    age: Age,
    // #[try_from]
    mail: Mail
}

#[derive(Default)]
struct Age(u32);

impl TryFrom<u32> for Age {
    type Err = MyError;
    fn try_from(age: u32) -> Result<Age,MyError> {
        if age > 200 {
            Err(MyError::TooOld)
        } else {
            Ok(Age(age))
        }
    }
}

#[derive(Default)]
struct Mail(String);

// We should probably have TryFrom<&str> here
impl TryFrom<String> for Mail {
    type Err = MyError;
    fn try_from(mail: String) -> Result<Mail,MyError>{
        // Parsing goes here
        Ok(Mail(mail.into()))
    }
}

fn main() {

    let _person: Result<Person, MyError> = PersonBuilder::default()
        .name("bob".to_string())
        .age(25)
        .mail("[email protected]".to_string())
        .build();
}

Here is the code that could be derived behind the scenes

struct PersonBuilder {
    inner: Result<Person, MyError>
}

impl PersonBuilder {
    fn name(mut self, name: String) -> PersonBuilder {
        if let &mut  Ok(ref mut inner) = &mut self.inner {
            inner.name = name
        };
        self
    }

    // This code can probably be prettyfied
    fn age<T: TryInto<Age, Err=MyError>>(mut self, age: T)  -> PersonBuilder {
        match age.try_into() {
            Err(e) => if self.inner.is_ok() {
                self.inner = Err(e)
            },
            Ok(v) => if let &mut Ok(ref mut inner) = &mut self.inner {
                inner.age = v
            }
        };
        self
    }


    fn mail<T: TryInto<Mail, Err=MyError>>(mut self, mail: T) -> PersonBuilder {
        match mail.try_into() {
            Err(e) => if self.inner.is_ok() {
                self.inner = Err(e)
            },
            Ok(v) => if let &mut Ok(ref mut inner) = &mut self.inner {
                inner.mail = v
            }
        };
        self
    }

    fn build(self) -> Result<Person,MyError> {
        self.inner
    }
}

impl Default for PersonBuilder {
    fn default() -> PersonBuilder {
        PersonBuilder{
            inner: Ok(Person::default())
        }
    }
}

once-only setter: prevent overriding existing value

Current setters always override the previous value. But sometimes you want to emit a warning that the value was already set. E.g. derive_builder itself has the use case where it wants to warn you about something like #[builder(name="Foo", name="Bar")].

In such a case I could imagine a setting #[builder(setter(once))], which would error if the setter was called multiple times. The setter would need to change signature and the previous value could be contained in the error.

Atlernatives

  • let the user handle this situation manually via public builder fields or getters #7

Drawbacks

  • We already have fallible setters which are generic over TryFrom #72. These are activated via #[builder(try_setter)] and have a try_ prefix by default. It might be a bit weird to have to sets of fallible setters #[builder(try_setter)] and #[builder(setter(once))] which do different things. What if someone wants to have #[builder(try_setter(once))]? Would we even allow that? (We would need to wrap to different error types).
  • Again we face the problem that proc macro crates can't export items (sigh). But we would need to bring an error type into scope. Deriving unique (but otherwise identical) error types per builder seems to be an ugly hack.

idea: hidden fields

use case

User wants to expose custom setters instead, which are more complex, do additional checks, etc.

possible syntax

field annotation:

  • hide: #[builder(hide=true)]
  • unhide #[builder(hide=false)]

shorthand

  • hide: #[builder(hide)]
  • unhide: #[builder]

Example

#[derive(Builder)]
struct Foo {
    #[builder(hide)]
    foo: Vec<i32>
}

alternatives

  • hide
  • ignore
  • omit
  • skip
  • no_setter (i.e. #[builder(no_setter)] instead of #[builder(setter(skip))] ...)
  • ...

Support tuple structs

It appears that custom deriving a tuple struct does not work. Would be a very nice convenience to support those as well as regular structs.

DeprecationNotes generated from struct options aren't emitted

See #62 - the code attempts to send a deprecation warning when using #[builder(default)] at the struct level, but that gets swallowed and goes nowhere. This appears to be the result of two issues:

  1. The DeprecationNotes object was abandoned during the conversion from StructOptions to Builder.
  2. The DeprecationNotes::to_tokens() method was producing items that seem to have been silently ignored in the output.

Custom validation of built struct

It would be nice to allow custom code to validate an object before it is returned from the build method. This topic is touched upon in #53 and #56, but none of them are really about this, nor discuss it in detail.

I mean to validate the entire struct upon build, not individual fields on set. This is for situations where there are no invalid values for individual fields, but some combinations of field values are invalid.

Example

#[derive(Builder)]
#[builder(validatefn=my_validate)]
pub struct Foo {
    src: IpAddr,
    dst: IpAddr,
}

impl FooBuilder {
    fn my_validate(foo: &Foo) -> Result<(), String> {
        if foo.src.is_ipv4() == foo.dst.is_ipv4() {
            Ok(())
        } else {
            Err("Source and destination addresses not in same address family".to_owned());
        }
    }
}

// And the generated builder looks something like:
impl FooBuilder {
    fn build(&mut self) -> Result<Foo, String> {
        let foo: Result<Foo, String> = whatever_it_does_now_to_create_foo();
        foo.and_then(|foo| Self::my_validate(&foo).map(|_| foo))
    }
}

The value sent to validatefn should probably be limited to an identifier, a method name, and the code generated will be Self::$validatefn(&value)...

Structured errors

Structured errors, instead of String is discussed in issue #60. I really like the idea of returning structured errors and support that. This custom validation can be made to support that by changing the requirement on validatefn from (T is the type we are building):

Fn(&T) -> Result<(), String>

to

Fn(&T) -> Result<(), E> where E: std::error::Error

And introduce another structured error variant ValidationFailed(E), giving us a build method similar to:

impl FooBuilder {
    fn build(&mut self) -> Result<Foo, FooBuilderError> {
        let foo: Result<Foo, FooBuilderError> = whatever_it_does_now_to_create_foo();
        foo.and_then(|foo| {
            Self::my_validate(&foo)
                .map(|_| foo)
                .map_err(|e| FooBuilderError::ValidationFailed(e))
        })
    }
}

Allowing the user of derive_builder to return any error type they want in their validation function.

Wrong number of type arguments because Result returned instead of ::std::result::Result

I recently encountered an issue where I couldn't compile because the build() method tries to return a Result<T, E>, and because I've created my own Result type its signature is just Result<T>.

error[E0244]: wrong number of type arguments: expected 1, found 2
  --> src/connection.rs:64:10
   |
64 | #[derive(Builder)]
   |          ^^^^^^^ expected 1 type argument

Would you be able to adjust the generated code so it returns the fully qualified path ::std::result::Result?

i.e. change this:

fn build(&self) -> Result<Channel, String> {

to this:

fn build(&self) -> ::std::result::Result<Channel, String> {

In the meantime, if anyone else encounters this issue there's a fairly easy fix. You just need to rename your Result and Error type.

use errors::{Error as MyError, Result as MyResult};

dependency on custom_derive in "release" build?

Hm, I wonder about the dependency on custom_derive.

The thing is, src/derive_builder.rs does not really depend on it. Only our tests+examples and the consumer need the custom_derive crate.

Can we reflect this fact in Cargo.toml? Otherwise our consumers might end up building duplicates of the custom_derive crate. Alternatively we could perhaps re-export custom_derive, but this seems to contradict the general idea of using custom_derive with multiple extensions.

@killercup: Well, not really a big issue, but what do you think?

Infallible Build method

The build method currently fails, if a field value was not initialized and no default value was provided (possible since custom defaults #3 are implemented).

Proposal

Add a way to change the signature of the build method from Result<Foo, _> to Foo. This method would not panic. Instead derive_builder would deny that change of signature if the method is fallible.

Questions

How should this annotation be called? Here are two suggestions:

  • #[builder(infallible)]
  • #[builder(no_result)]
  • #[builder(deny_failure)]

Should this be scoped with other build-method-related options, e.g. inside - #[builder(buildfn(...))]?

Outlook

It is likely that derive_builder will have other failure modes in the future, such as field-specific custom validations (foo must be in 0..10, etc.). These features can not be activated if the infallible build method is also active.

Alternatives

  • Change the signature of the build method automatically when it cannot fail. This may be surprising if you have a lot of code depending on the current signature, then you add a default value to a field (which seems like a non-breaking change) and then nothing compiles anymore. Not good.
  • Always allow changing the signature to Foo even if the build method is fallible. Panic on failure. This does not seem to be idiomatic Rust - which we try to achieve as best as possible. If someone really wants that behaviour he/she can implement a custom wrapper (we will add means to rename the build method and make it private).

Lifetime support?

Just tried using rust-derive-builder for a struct with lifetimes:

#[macro_use] extern crate custom_derive;
#[macro_use] extern crate derive_builder;

custom_derive! {
    #[derive(Default, Builder)]
    struct Channel<'a> {
        token: &'a str,
        special_info: i32,
        // .. a whole bunch of other fields ..
    }
}

impl Channel {
    // All that's left to do for you is writing a method,
    // which actually *does* something. :-)
    pub fn build(&self) -> String {
        format!("The meaning of life is {}.", self.special_info)
    }
}

fn main() {
    // builder pattern, go, go, go!...
    let x = Channel::default().special_info(42u8).build();
    println!("{:?}", x);
}

I get the following error:

6:22 error: expected ident, found 'a
src/main.rs:6     struct Channel<'a> {
                                 ^~
error: Could not compile `derive-test`.

Can lifetimes be easily added? If not, are there any workarounds?

Generated Builder documentation snippet fails doc-test

For the following struct:

#[macro_use]
extern crate derive_builder;

#[derive(Builder)]
struct Foo { a: i32 }

and the current master (32a6437), cargo test fails the doc test because FooBuilder and Foo are not imported:

failures:

---- src/lib.rs - FooBuilder (line 8) stdout ----
	error[E0412]: cannot find type `FooBuilder` in this scope
 --> <anon>:2:16
  |
2 | let builder = <FooBuilder as Default>::default();
  |                ^^^^^^^^^^ not found in this scope

error[E0412]: cannot find type `Foo` in this scope
 --> <anon>:4:16
  |
4 | let _x: Result<Foo, String> = builder.build();
  |                ^^^ not found in this scope

error: aborting due to previous error(s)

I tried updating the builder doc template like so:

Builder for `{struct_name}`.

# Examples

```rust
# use {qualified_module_name}::{{{struct_name}, {builder_name}}};

let builder = <{builder_name} as Default>::default();
// .. call some setters on `builder`
let _x: Result<{struct_name}, String> = builder.build();
```

but I haven't figured out how to get the module name from the TokenTree or confirmed if it's possible / impossible.


Side note: I picked up master because I wanted the #![deny(missing_docs)] fix

Should builder methods use `mut self` or `&mut self`?

I haven't put much thought into it. @colin-kiegel wrote earlier (roughly translated):

IMHO, setter methods should take mut self instead of &mut self. This is more restricting, but in has two advantages—it allows us to write:

Channel::default()
    .token(raw_token)
    .into<X>();
  1. Channel::default() needn't be bound to anything. Without the move into the setter methods it would be out-of-scope too soon.
  2. It's possible to call the final into<X> (and similar methods) without .clone()

Optionally expose a static Target::builder() -> TargetBuilder method

Proposal: On the type where #[derive(Builder)] is present, generate an inherent impl block for that type which defines a new builder method. This has a couple usability perks:

  1. The caller doesn't have to import the type and the builder separately.
  2. The call is shorter: RequestOptions::builder() vs. RequestOptionsBuilder::default()

setter(skip) should use the custom default if that was defined

If a field is annotated with #[builder(setter(skip))] then the Default trait will be used to construct a value.

The same field (or even the whole struct) can be annotated with an explicit default #[builder(default="..."))]. If that is the case, this should be used to construct the value. This is currently not the case and a bug.

structured errors instead of `String`

The build method currently returns Result<Foo, String>. But representing errors as String is not very rustic.

It's also a show-stopper for using derive_builder with #![no_std] on the stable toolchain. The reason is that String on no_std requires the collections crate, which is marked as unstable.

These are already two reasons that representing errors as String is not optimal.

Proposal

Find a way to return structured errors instead of Strings. On std they should implement the usual fmt traits. On no_std they should be compliant with the stable toolchain.

Questions

The main question is, how do we get the structured errors into scope? This is unfortunately not straightforward, since derive_builder is a proc_macro crate.

proc-macro crate types cannot export any items other than functions tagged with #[proc_macro_derive] currently.

So we need to work around that limitation somehow. These ideas come to my mind right now

  1. We could add a derive_builder_common crate, which just exports traits / types / impls and must then be imported by any user of derive_builder.
  2. We could add a flag to derive_builder to emit the traits / types / impls directly into the target crate.
  3. Each derivation with derive_builder could generate unique error definitions for that struct.
  4. Wait until proc_macros can export items.

All these approaches have their own drawbacks.

  • 1+2 add boilerplate and complexity for users of derive_builder. The out-of-the-box experience is a bit impaired.
  • 3 introduces unnecessary redundancy (and complexity). This might get ugly if a user has multiple derived builders and does pattern matching on the errors.
  • 4 sounds like macros 2.0 which will most certainly take quite a while from now to land on stable.

cc @killercup

bors-ng integration

Moving discussion from #20 here:

Documentation is at http://homu.hellorust.com/
Authorized is whoever I put into the list.
This is all pretty manual right now (someday I will be as dynamic as homu.io)

Now to the sad part: It seems I would actually need owner rights to modify the right hooks, and I don't think you can give this to others on a user repository (only on organization repository).

So for now the integration has to wait until I figure out a way for you to execute this (or inserting the right hooks manually)

officially support `#![no_std]`

TODO: Add example + docs for #![no_std] scenario.

  • We'll need ::core::result::Result instead of ::std::result::Result etc.
  • ::core::... works iff there's an extern crate core; declaration.
  • String would require to explicitly add extern crate collections; (unless we find a way to introduce structured errors #60)

Add derives on derived FooBuilder struct

use case

User wants to have something like PartialEq for FooBuilder, but can't derive it since FooBuilder is already derived by itself.

possible API

We could add a struct annotation on Foo:

  • #[builder(derive(PartialEq, SomethingElse))]

example

#[derive(Builder)]
#[builder(derive(PartialEq, SomethingElse))]
struct Foo {
    foo: Vec<i32>
}

Would expand to:

#[derive(Default, Clone, PartialEq, SomethingElse)]
struct FooBuilder {
    foo: Option<Vec<i32>>
}

discussion

This crate should be conservative and derive as little as possible for FooBuilder by default. Because additional derives on FooBuilder would add corresponding requirements on the fields of Foo.

Status Quo:

  • FooBuilder impls Default and Clone (the latter is only needed for the immutable builder pattern)
  • All fields on Foo already require to implement ...
    • Clone, if they have a setter on FooBuilder
    • Default, if they don't have a setter on FooBuilder (.. well, at least with '#15: hidden fields' coming, and this might change with '#3: CustomDefault')

`no_std` is broken

We tried to introduce official support for no_std in #41 - but that was still broken in multiple ways and the test coverage obviously unsufficient. In it's current form it's probably not useful on no_std.

E.g. the features skip setter #44 and custom default #3 do not support no_std.

Implement TryFrom<Builder> for Target for all builders exposing a public build method

With #72 (fallible setters) merged and rust-lang/rust#33417 (TryFrom) approaching stabilization, having a TryFrom<Builder> for Target implementation for all builders with a generated build method will enable the following code:

let request = RequestBuilder::default()
    .from(-18000000)
    .sources(vec![Source::device_group(Oid::new(2))])
    .metric(Metric::builder()
        .stat_name("extrahop.device.ssl_server_detail")
        .field_name("cert_expiration"))?
    .total(true)
    .build()
    .expect("All inputs hardcoded");

Once TryFrom is stabilized I'm not planning to expose a struct-level attribute to control this. This seems too standard for a crate user not to want. #70 gives authors control over exposing the build method; this would look to that attribute and skip implementation if the build method was skipped.

I'm planning to take a stab at a PR for this.

optional setters

Right now all setters are mandatory. I.e. the build method of derive_builder will error if not all fields are initialized.

use case

User wants to make some setters optional. If no value is provided Default::default() should be used instead.

Outlook: custom defaults

possible api

field annotation:

  • optional: #[builder(optional="true")] or #[builder(mandatory="false")]
  • mandatory #[builder(optional="false")] or #[builder(mandatory="true")]

shorthand

  • optional: #[builder(optional)]
  • mandatory: #[builder(mandatory)]

example

#[derive(Builder)]
struct Foo {
    #[builder(optional)]
    foo: bool
}

#[test]
fn test() {
    let x = FooBuilder::default().build().unwrap();
                                       // ^~~ currently panics with `foo uninitialized`
    assert_eq!(x.foo, false);
}

open questions

  • API (see discussion)
  • What should be the default behaviour? (See this suggestion to switch back to "optional by default").

discussion

Conceptually the hardest part is to pick a good api (=annotation). :-)

This also has significant overlap with the idea to have custom defaults, e.g. I would expect that custom defaults implicitly make a setter optional. custom defaults wouldn't even make sense for mandatory setters. You could argue that an explicit notation for optional fields is not needed.

However most types implement Default, so a custom default may be unnecessary. Additionally the optional field annotation could be used for the whole struct, instead of individual fields - thereby inverting the opt-in to opt-out for that struct. In these cases an optional field annotation is more convenient than custom defaults - but it doesn't remove the overlap.

Now comes the hard part.

Should it be ...

  • optional="true" vs. optional="false"
  • .. the above with mandatory instead of optional
  • optional vs mandatory?
  • or the whole lot: optional, optional="true", mandatory="false" vs mandatory, mandatory="true", optional="false"
  • or something more extensible like style=optional vs. style=mandatory (where style is only a placeholder name)?

Right now I can't think of a reason to extend the binary set optional and mandatory.

My tendency is to allow the whole lot of optional, optional="true", mandatory="false" vs inverse. The main drawback I see is that it may clog the documentation a bit - at least if we want to cover all ways to do something.

alternatives

  • mandatory (inverse)
  • mandatory + optional
  • use_default
  • default (<- this may collides with the api for custom defaults)
  • style=... (more open)
  • voluntary instead of optional
  • ...

Forward the method to an internal struct?

I'd like to add builders to my struct, but the fields are "hidden" in an internal struct. For example, my structs look like this:

struct InternalStruct {
    a: bool,
    b: i64,
    // etc.
}
pub struct MyStruct {
    a: A,
    internal: InternalStruct,
}

It would be great to be able to add Builder to MyStruct and tell it to:

  1. "forward" the setters to the internal struc;
  2. Possibly ignore a (don't create a setter for it).

I use this to move InternalStruct in a serde_types.in.rs file to be Serialized/Deserialized. In my case above the type A cannot be serialized so I need to split the two structs. Also, I don't want a to have a setter since it's passed as MyStruct's constructor (hence 2.)

Also, I think this might be useful to encapsulate common members of multiple structs into a single one "a la" inheritance.

generate a (customizable) constructor

Currently all builder structs implement the Default trait. Using the Default trait is the proposed way to create a new builder instance right now.

As mentioned in this comment by @faern it would be nice to have a dedicated constructor. We could make the identifier customizable (defaulting to new) and we could offer opting out of generating a constructor.

Possible API

  • #[builder(constructor(name="brand_new", ...))]
  • #[builder(new(name="foo", private, ...))]
  • ...

Questions:

  • should we offer renaming the constructor?
  • should we offer to make the constructor private? (such that it can be wrapped - for easier re-ordering, or
    something else)?
  • how should we namespace these settings?

Outlook

This could allow better support for infallible build methods #56, if the build method would already take all required values (new(a, b) instead of new().a(a).b(b)).

Limitations:

  • the order of required fields in the constructor signature would most likely be identical with the order of fields. I would not like to offer an API for reordering fields, or something similar. In certain edge cases you might need implement the constructor manually - which will always be possible of course.

CustomDefault

The builder pattern allows us to set optional values on a struct—but how do we determine the default values for those fields? Rust's default answer (hehe) is of course std::default with its #[derive(Default)] and custom-written impl Default.

At first, we can

  • let the user add #[derive(Default)] so they can do MyStruct::default().setter(42),
  • or impl MyStruct { pub fn new(…) {…} } so they can do MyStruct::new(…).setter(42).

It would be really nice to also offer a way to derive/generate a Default impl with custom values, e.g.:

custom_derive! {
    #[derive(CustomDefault)]
    struct Channel {
        id: Uuid,
        token: Authentication,
        #[default(42i32)] // ← custom attribute
        special_info: i32,
    }
}

(How) should we do this? (I don't think it's necessary to have this early on, and it should probably be a separate crate.)

@colin-kiegel already suggestion this in DanielKeep/rust-custom-derive#18.

Getters

I wrote in #4 (comment):

I really want this to generate idiomatic Rust, so getters names would be prefixed with get_, e.g. get_foo() and get_foo_bar().

If I read rust-lang/rust#29599 correctly, it's not possible to generate ident on stable. If we were to do getters on nightly only, interpolate_idents might be helpful (usage example).

A more creative solution for stable might be creating a single get(&self) -> new_sub_module::Getter method, where Getter is a newtype that implements methods to get each of the struct fields, so you can do x.get().foo().

Field-level #[builder(default = __)] easier to use wrong than right

#[derive(Debug, PartialEq, Builder)]
struct Dolor {
    msg: String,
    
    #[builder(default = "self.msg")]
    z_failure: Option<String>
}

This produces the following error:

error[E0507]: cannot move out of borrowed content
    |
169 |     #[derive(Debug, PartialEq, Builder)]
    |                                ^^^^^^^ cannot move out of borrowed content

Alternatively, some really weird things can be accomplished:

#[derive(Debug, PartialEq, Builder)]
struct Dolor {
    
    #[builder(default)]
    msg: String,

    /// Prints 'hi' if not explicitly initialized.
    #[builder(default = "{println!(\"Hi!\"); Some(\"woot\".to_string())}")]
    z_failure: Option<String>
}

The only time the current implementation of this will work at the field level is if the user provides a literal or a value that implements Copy. Serde avoids this by requiring the value of default to be a function with the signature Fn() -> T, which it calls every time it needs a new instance. That feels much more hygienic than the current behavior (and makes using default in conjunction with helper or factory functions much cleaner).

@colin-kiegel, how would you feel about combining this with #61 and documenting the breaking change as "aligned default behavior with serde"?

more tests

  • expanded tokens
  • nice-to-have: compile fail tests with compiletest
  • nice-to-have: absence of clone in complex scenarios -- if we switch to &mut self signatures
    ...more?

#[builder(default)] on struct-level does not use Default impl of the struct

rustc version: 1.17.0-nightly (6eb9960d3 2017-03-19)
derive_builder version: 0.4.0

When a struct has a non-derived impl Default, those values do not get used in the output of the builder.

Repro

#[macro_use]
extern crate derive_builder;

#![cfg(test)]

#[derive(Debug, Clone, PartialEq, Eq, Builder)]
#[builder(default, setter(into))]
pub struct Example {
    pub a_flag: bool,
    pub limit: Option<String>,
    pub third: String,
}

impl Default for Example {
    fn default() -> Self {
        Example {
            a_flag: false,
            limit: Some("20".into()),
            third: "Hey".to_string(),
        }
    }
}

fn main() {
    let struct_update = Example {
        a_flag: true,
        ..Example::default()
    };
    
    let builder = ExampleBuilder::default().a_flag(true).build().unwrap();
    
    assert_eq!(builder, struct_update);
}

This panics with the following error:

panicked at 'assertion failed: `(left == right)` (left: `Example { a_flag: true, limit: None, third: "" }`, right: `Example { a_flag: true, limit: Some("20"), third: "Hey" }`)'

Expected: The resulting structs are equal.

Typesafe builders

With macros 1.1, it should be easy to implement builders like this one. This would allow the type-checker to confirm that all required (non-defaulting) fields are set exactly once. Would you consider an addition to the library implementing this feature? I'd be happy to put together a PR if there is interest.

Automatic documentation does not work

From the generic test as output by cargo expand --test generic_structs:

#[allow(dead_code)]
impl<T> GenLorem<T> {
    /// Set `$field_name`
    ///
    /// auto-generated by [derive_builder](https://crates.io/crates/derive_builder)
    fn ipsum<VALUE: Into<String>>(mut self, value: VALUE) -> Self {
        self.ipsum = value.into();
        self
    }
    /// Set `$field_name`
    ///
    /// auto-generated by [derive_builder](https://crates.io/crates/derive_builder)
    fn dolor<VALUE: Into<T>>(mut self, value: VALUE) -> Self {
        self.dolor = value.into();
        self
    }
}

Custom struct definition

derive_builder could skip the struct definition and leave that to the user. It would then only generate the setters.

Possible Use Cases

  • de/serialization of the builder struct or other complex derives - as an alternative to extending custom derives #43 further (cc @TedDriggs).
  • additional fields (and logic) which can not be derived directly from the target struct

Limitations / Drawbacks

  • the user would need to exactly match the signature and error messages will most likely be not very helpful due to lacking span information, like "some type was not as expected .. ok, which one?". But on the other hand it's really just wrapping everything in an Option.
  • it would be very unusual, or even surprising, to have a #[derive(...)] struct A generate impls on a struct B which is defined manually

Example

/// Derive builder, but skip the struct definition.
#[derive(Builder)]
#[builder(custom="ConfigBuilder")]
pub struct Config {
    pub appliance: Appliance,
    pub manual_limit: Option<usize>,
}

/// Manual implementation of the builder struct with `serde` derive
#[derive(Serde)]
pub struct ConfigBuilder {
    appliance: Option<Appliance>,

    #[serde(default, rename = "limit")]
    manual_limit: Option<Option<usize>>,
}

skip/rename/private build method

Right now we always generate the build method with a fixed name, and we give it the same privacy as the builder struct.

It would be nice to customize all of that, such that users can wrap a private build method, or just skip it entirely and do their own thing.

This should be rather straightforward to implement. If anyone is interested to start contributing to this crate, this would be a good entry point. :-)

The major question is, how to namespace these options

  • #[builder(build_fn(skip, private, name="finalize"))]
  • #[builder(buildfn(...))]
  • #[builder(build(...))]
  • #[builder(fn(...))]

All of the possible names I can currently think of look a bit weird.

Setter prefix / Option<> setters?

Hi,

I'm not sure whether such a thing is possible, but I like to prefix my setters with with_, so I have:

Foo::new()
    .with_bar(42)
    .with_baz("foofoo")
    .with_something(Else::new())

And if there is a field which is an Option<> I also like to have the without_* functions, so I can effectively:

Foo::new()
     .with_bar(42) // sets `self.bar = Some(42)`
     .without_bar() // sets `self.bar = None`

is such a thing possible?

macro equivalent for `if ($x == $y) { branch!() }`

I think sooner rather than later we want to do some comparisons and branching like if ($x == $y) { branch!() } based on two captured variables

custom_derive! {
        #[derive(Debug, CustomDefault, Builder(require(id, token)))]
        struct Channel {
            id: Uuid,
            token: Authentication,
            special_info: i32,
    }
}

In this example we will parse id: Uuid and have to decide whether we have seen this field name before in require(id, token).

Here is a proof-of-concept that such comparisons are working (however the default case will need rust 1.12, current nightly):
https://gist.github.com/colin-kiegel/c64128d1aa32ba21decffc8cbad28962

missing_docs

The following fails

#![deny(missing_docs)]
#[derive(Builder)]
struct Foo {
    bar: bool
}

with error: missing documentation for a method.

While doing this we should probably add some other denys to our tests, to be sure.

Use full path for Result

I think you should use std::result::Result instead Result in this place.
Cause: if declare or use custom type Result it will throw error: error[E0244]: wrong number of type arguments: expected 1, found 2

Can not derive on structs which have reference to generic structs

Hello there;
I'm trying to derive builder for the following struct but it gives compiler error :

/// Common properties of steering behaviors 
#[derive(Builder)]
pub struct SteeringBehavior<'a, T: 'a + Real> {
    /// ownew of this behavior upon which the calculations will occur
    pub owner: &'a Steerable<T>,
    /// is this behavior enabled
    pub enabled: bool,
    /// limitations on speed and velocity calculations
    pub limiter: &'a Limiter<T>,
}

Compiler error :

error[E0309]: the parameter type `T` may not live long enough
--> src/steering_behavior.rs:6:10
   |
6 | #[derive(Builder)]
   | ^^^^^^^
   |
= help: consider adding an explicit lifetime bound `T: 'a`...
note: ...so that the reference type `&'a steerable::Steerable<T> + 'a` does not outlive the data it points at

Change default build_fn name from `build` to `try_into` in preparation for `TryFrom`/`TryInto` traits?

TryFrom<T> and TryInto<T> are about to be stabilized. These would be the idiomatic builder traits IMO. And they are likely to end up in the std prelude, so you wouldn't need to import them.

The current build method, could be replaced by TryFrom (which implies TryInto)

impl TryFrom<FooBuilder>: Foo {
    type Err = String;

    fn try_from(b: FooBuilder) -> Result<Foo, String> {
         // ...
    }
}

Removing FooBuilder::build() in favour of TryFrom::try_from() would be a breaking change.

However we could keep the current default fn build() until TryFrom is stabilised. Then let build wrap around TryFrom and keep it this way virtually forever (unless opted out).

Alternatively, we could try to get rid of the wrapping build() fn as early as possible. This could happen in two steps (a) rename build() to try_into() and (b) switch to TryFrom, while builder.try_into() should still magically work in almost all cases. There could still be an option to opt into creating such a wrapper function with a custom name.

derive_builder is still young, so we may deprecate things more easily now rather than later. The question is, where do we want to end up long term? Do we want to keep a build() fn wrapper as default behaviour, or do we want to advocate using TryFrom and not have a wrapper by default?

Is it possible to add generic setters?

Ideally I would want some setters to recieve, for example somthing like Option<Into<Type>>, even just Into<Type>. For example ToSocketAddrs. Or functions.

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.