Feature request: noclobber field attribute

[coriolinus]

It is sometimes useful to ensure that a builder value, once set, is not allowed to be reset before calling build. For example:

// allowed in current builder code

#[derive(Builder)]
struct Lorem {
    ipsum: u32,
    dolor: u32,
}

let lorem = LoremBuilder::default().ipsum(123).ipsum(456).dolor(789).build();
assert_eq!(lorem.ipsum, 456);

Instead, I'd like something like:

// allowed in current builder code

#[derive(Builder)]
struct Lorem {
    #[builder(noclobber)
    ipsum: u32,
    dolor: u32,
}

// this doesn't work
// let lorem = LoremBuilder::default().ipsum(123).ipsum(456).dolor(789).build();

There are at least two different patterns which could potentially be used to prevent duplicate sets: typestates and fallible results.

The typestate method is in my opinion somewhat more elegant, though it makes autogenerated documentation for the builder somewhat worse. In this method, the builder gains a generic attribute for every field, and the build method is only implemented for the appropriate typestate:

// this is all generated code
// note I'm freehanding all this, syntax errors are likely
// also this pattern only works with consuming builders, which might rule it out

struct LoremBuilder<Ipsum, Dolor> {
    // otherwise exactly identical to the lorem builder declaration
}

// this depends on appropriate `Set` and `Unset` definitions
impl<Ipsum, Dolor> LoremBuilder<Ipsum, Dolor> where Ipsum: Unset {
    pub fn ipsum(self, ipsum: u32) -> LoremBuilder<Set, Dolor> {
        self.ipsum = Some(ipsum);
    }
}

Fallible setters seem more likely to compose well with other existing derive-builder features. In that case, what we end up with is

let err = LoremBuilder::default().ipsum(123)?.ipsum(456).unwrap_err();
assert_eq!(err.to_string(), "attempted to call `ipsum(...)` twice in `LoremBuilder`");

I have no strong opinions about which pattern gets used, but one way or the other, support for noclobber would be useful to me.

[TedDriggs]

Can you provide more information about why you're trying to prevent "clobbering" of a builder value? Builders are nothing but bags of values that know how to turn themselves into the "target" type, and I'm struggling to imagine a scenario where swapping values in such a bag could do any actual harm.

All RAII-type "work" or cross-field validation "work" should either be happening before passing a value to the builder, in which case noclobber doesn't prevent that additional work from being done, or when the builder's build method is creating an instance of the deriving struct, at which point there's no concept of "clobbering" because we are operating on the builder struct at a single moment in time.

[coriolinus]

The underlying idea is to use the builder as a kind of typestate machine, so that a field value, once set, is known to be definitely expressed in the final struct. It's possible to do this work ahead of time, but it's also possible to do this work within the type system, and the latter can be preferable sometimes.

In other words, yes, I can (and have, for the immediate case) written ahead-of-time validation which ensures that we don't attempt to redundantly set particular values. However, it would have been even easier for me if I didn't have to write that, and could instead write #[builder(noclobber)] and get the same effect.

[TedDriggs]

Typesafe builders were initially discussed in #33, and the conclusion since then has been that they're interesting, but out of scope for derive_builder:

1. The code generated for typesafe builders would be very different from the code for the current builders, so there's not much reason to include both approaches in one crate
2. A number of derive_builder's options would not make sense with typesafe builders at all, or their behaviors would need to be fully rethought - for example, what would the signature of a try-setter on a typesafe builder be?

Since typesafe builders are out of scope, that leaves only the alternative of a Result-returning setter. I'm not convinced that this is particularly Rust-like, since it pushes programmer errors to runtime, and it encourages code whose control flow is complex enough that single-assignment of a value is not trivially proven. Even setting that concern aside, this composes with the rest of the crate poorly:

• Setters for noclobber fields would all need question marks or error-handling code added
• Any behavior of noclobber with each setters is confusing.
• A try-setter's return type would end up being Result<Result<&mut TheBuilder, derive_builder::FieldAlreadySetError>, E>

Existing Support/Workarounds

derive_builder already supports required, set-once fields by using #[builder(custom_constructor)] attribute and #[builder(setter(custom))] on the field(s) in question. This will prevent the builder from exposing impl Default, allowing the builder to have a manual impl block with a new function that takes the required set-once field(s). Hiding the setter for that field will prevent the value from changing after the builder is initialized.

Optional set-once fields are possible two ways: Either by adding custom methods to the builder in an impl block, or by adding alternate constructor functions to the builder, e.g. with_optional_set_once_thing. Having these require custom code is good in my opinion, because it forces the author of the crate deriving a builder to think about the specific behaviors they want in these scenarios, avoiding the possibility of confusion over how different derive_builder attributes might interact (e.g. each and try_setter)

Finally, if the desire to avoid clobbering comes from the builder's caller instead of the builder itself, using #[builder(field(public))] or adding custom impl methods would allow the caller to check if a value was present before providing one of their own.

Conclusion

Given the problems described above and the existence of workarounds, I would prefer not to address this within derive_builder.

Thank you for submitting this idea, and taking the time to provide a great writeup of the concept, the proposed experience, and the reasoning behind it. While this particular idea isn't a fit for derive_builder, I hope your project is successful and that you contribute more ideas to derive_builder in the future.