Getters

[killercup]

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().

[nielsle]

It would be great to have a solution that allowed the user to use the builder as something like a rust-typemap. (Yes I know it is out of scope, but it would be great to have.). I tried to propose something similar over here

DanielKeep/rust-custom-derive#19

[colin-kiegel]

@nielsle thx for your suggestion. But I don't see that in this crate, because it's a different use case with different requirements.

But feel free to create a fork of derive_builder and replace https://github.com/colin-kiegel/rust-derive-builder/blob/master/src/derive_builder.rs#L47 and https://github.com/colin-kiegel/rust-derive-builder/blob/master/src/derive_builder.rs#L80

[colin-kiegel]

as long as we can't make this idiomatic, I would rate a workaround with low priority. It is already possible to make the fields public, which seems good enough as a workaround.

[colin-kiegel]

If a field is unset but has a default value, should the getter return None or the default value?

Note: Default values can be arbitrary functions with side-effects.

[TedDriggs]

@colin-kiegel I was thinking about that as part of the work on #85. As shown in the sample code in #79, I'd like to expose at least private getters with the signature get_foo(&self) -> Result<T, String>, which returns the explicit value of foo, the default value of foo(which may require invoking the default at the struct level), or an uninitialized field error. These private getters would make writing a validator function and/or custom build function much cleaner.

The downside seems to be that they require doing one of the following:

1. Take &mut self so that they can cache the default value after computing it once. Using them in conjunction with the validator hook would mean the validator had to take &mut self, and that would require changing the signature of the build method.
2. Cache the default value, but use a Cell to avoid calling the default methods more than once. This field would have to be private, which is a breaking change if anyone is depending on struct update syntax without a trailing .. clause.
3. Eagerly compute default values in FooBuilder::default() and prepopulate the fields with the values. This would interfere with any attempt at a set-once model.

[colin-kiegel]

What about this use case, where someone wants to query the builder whether a field is already set? If we don't include computing the default value, then we could allow this via get_foo(&self) -> Option<T>. It seems like we can't have both.

If we decide to include computing custom defaults, we could also

0. Accept that calling get_foo(&self) can be expensive, when custom defaults are defined.

[TedDriggs]

The "is set" case can be checked by querying the field directly, especially if the author is writing a custom build function or a validator.

[colin-kiegel]

Ok, I was thinking about the three-crate-scenario (i) derive_builder, (ii) author of builder, (iii) user of builder. I'm wondering if there could be a use case where the user of the builder wants to inspect the input already given to the builder. You're saying this would only be possible if the author of the builder already made the fields public (which is probably an un-idiomatic choice as discussed in #86).

Maybe such a use case is unlikely. I can only imagine a cases where the user of the builder uses dynamic input and either (a) wants to conditionally apply a default value which is different from the stock default, but the setters have once-only-semantics or (b) wants to display the given inputs somehow, like "please review your PizzaOrder then confirm". Both cases don't seem like day-to-day use-cases.

[colin-kiegel]

Another possible problem I can imagine is this.

I would very much like #[builder(default)] to be a non-breaking addition. That is, again in the three crate-scenario, if the author of the builder adds #[builder(default)] to a previously undefaulted field, it should not break any previously working code for the user of the builder. This would imply that we can't change signatures when #[builder(default)] is added to a field.

Getters would have to return Result even if their only failure mode would be None. Could this be awkward or annoying for users of the builder?

And is it really safe to say that users of the builder should not depend on getters to fail in a specific situation (like no value was supplied)? Because otherwise adding #[builder(default)] to a field would be a breaking change.

[TedDriggs]

This issue hasn't had any movement in ~5 years, so I'm concluding that this doesn't come up much in practice and am closing the issue.

[ijackson]

The use-case makes sense to me. However, this is the first time I've heard of it being needed, so I'm not sure how widespread the need is.

Thanks.

Well, Arti is the first project I've done significant work on that uses builder patterns for its configuration, but I understand the builder pattern is quite common. I don't think anything we're doing in Arti is particularly unique. Rust is still quite young in some of these areas, so I don't find it surprising that I'm breaking new ground.

Within Arti: I looked at my outstanding work and my tree has 24 uses of sub_builder. I am expecting to want to introduce more. I'm finding it really pleasant in practice.

In approach 2, would the getters described in #7 be sufficient?

I don't think they would be right, because the accessors need to take &mut self and return &mut SubStructBuilder.

You'd still have to repeat the field name in the build block, but that's literally adjacent to the field declaration so you'd remove the non-local name repetition of the getter, and since an F2-rename of the field would immediately break the build I think that would be maintainable.

That's true. But, I have seem soo many bugs where even adjacent repetition has been botched. This is still very true in Rust (where everyone runs rustfmt, which sadly can't maintain tabuler layout). If we end up writing code like this, the bug is too easy to write and too hard to spot:

    #[builder(field(
        type = "LoremOrIpsumBuilder",
        build = r#"self.lorem.build().map_err(|e| e.within("lorem")?"#
    ), setter(custom))]
    lorem: LoremOrIpsum,

    #[builder(field(
        type = "LoremOrIpsumBuilder",
        build = r#"self.lorem.build().map_err(|e| e.within("ipsum")?"#
    ), setter(custom))]
    ipsum: LoremOrIpsum,

    #[builder(field(
        type = "DolorBuilder",
        build = r#"self.dolor.build().map_err(|e| e.within("dolor")?"#
    ), setter(custom))]
    dolor: Dolor,

Could you do this the other way round, and have an attribute macro that did the necessary rewriting of SuperStruct?

I believe that would be possible, but I don't think it is a very good design.

It seems like an antipattern to set out to make a proc macro whose purpose is to massage the input for a diffeerent proc macro. The layering of abstractions becomes quite confusing, and the "wrapper" macro now has to unparse the inner proc-macro's input. I think this is a thing one would only do if there were compelling reasons why the "inner" macro couldn't be modified to add the desired features.

(It's a different matter if the two proc macros are semantically orthogonal: I often combine proc macros and am sometimes disappointed when the interact when I wish they wouldn't...)