rust-typed-builder
rust-typed-builder copied to clipboard
Support field dependencies
As a concrete example, when creating a customer in the Stripe API, there’s an argument tax_percent
that is only permitted if plan
has also been set. It is an error to set tax_percent
if plan
isn’t set, so it’s best to statically enforce that where possible.
I picture code looking like this:
#[derive(TypedBuilder)]
struct Customer {
…
plan: T,
#[builder(requires = "plan", default)]
tax_percent: Option<T>,
…
}
As schemed here, setting requires
simply means “you are not permitted to override the default unless this field has been set”. Thus all requires
things would need default
or default_code
.
An alternative scheme would allow you to specify different defaults depending on whether the required field was defined or not, e.g. replace default{,_code}
with dependency_not_met_default{,_code}
and dependency_met_default{,_code}
(strawman names selected for lack of ambiguity).
This would work very well in conjunction with #6, extending “requires” to support arbitrary groups (named, or with any()
&c. as in my comment there just now).
There are two ways to do this:
-
Forbid setting the dependent fields until its dependency is met. This means that:
Customer::builder().tax_percent(...).plan(...).build()
would be illegal, because even though you used both fields when you set the
tax_percent
there wasn't aplan
yet. Instead, you'd have to use:Customer::builder().plan(...).tax_percent(...).build()
-
Forbid building with unfulfilled dependencies. So
Customer::builder().tax_percent(...).plan(...).build()
would be OK, but
Customer::builder().tax_percent(...).build()
wouldn't.
While the second option seem more intuitive, I prefer the first option due to technical limitations:
- I can't freely code when to allow
build()
and when to forbid it - I have to build a complex set ofimpl
blocks to generate the required behavior. Doing this for theimpl
blocks of the individual dependant fields is much easier than doing it for the finalimpl
block of thebuild()
. - I can't get a proper error message, because I rely on the nonexistence of the
build()
method. By making thetax_percent()
fail to compile it will be easier for the user to determine what went wrong.
I had been assuming the first option. The second option would definitely be much, much messier technically, and would make life hard in IDEs and native error messages, and really scramble the docs on the builder type (which I value).
BTW, I’m quite happy to implement this myself.
I have need for this exact case too.
@idanarye Can I try this?
Sorry for spam with these questions, but it is frustrating to realize that the maintainer is not welcoming some PR after you have submitted it. Therefore I always ask first :)
I thought about it and any
is complicated and has exponential scaling because you would have to implement the setter of the dependent field for the following configurations for the example any(a, b, c)
:
- a set, b unset, c unset
- a unset, b set, c unset
- a unset, b unset, c set
- a set, b set, c unset
- a set, b unset, c set
- a unset, b set, c set
- a set, b set, c set
This means that we have 2^3 - 1 = 7
combinations. For n
fields in any
, we get 2^n - 1
⚠️
Let's say we remove any
. Then we have to think about all
and not
.
all
produces only one setter implementation. Therefore, I would implement it :D
not(a)
on b
is not helpful at all! It can be reflected in the setter of b
, but we could just set a
after setting b
.
Note that any
is not really exponential. Your any(a, b, c)
example only needs 3 cases:
- a set
- a unset, b set
- a unset, b unset, c set
WOW, I did not think about that!
I did not come up with it either - it's just very similar to what @seb-bl's did in #30.
I did think about it for many hours and I do finally have results!
Let's look at any(a, b, c)
to understand the notation that I did use:
a b c
S _ _
U S _
U U S
This means that _
stands for generic, U
stands for unset, S
stands for set.
The rows (except the first one) are the needed implementations.
all(a, b, c)
is simple:
a b c
S S S
Now let's take a look at any(all(a, b), c)
:
a b c
S S _
? ? S
Problem: We can not replace both ?
with U
because then we can't set the field if c
and (a
xor b
) are set.
This case has two solutions. But first, let's think about how these "implementation boards" are built.
- A normal field sets only itself in an implementation row.
-
all
sets all groups in an implementation row. -
any
generates an implementation row for every group.
After all S
are placed in the board, fill the rest with _
:
a b c
S S _
_ _ S
Now, cast a diagonal "U
shadow" of the two S
in the first row in the following way with S
under the "shadow":
a b c
S S _
U _ S
S U S
This is one solution! The other solution which reduces the number of implementations is to sort the rows first based on their number of S
entries:
a b c
_ _ S
S S _
Now we cast one "shadow" and we are done:
a b c
_ _ S
S S U
We know what problems we can get with combined any
and all
. How does the number of implementations scale?
Well, I have written an algorithm to test that: https://codeberg.org/mo8it/sus-impls
@idanarye I think that I did close all bugs in my algorithm groups
. Do you want to try it out?
- Clone the repo
- Run
cargo run 'GROUPS'
. For example:cargo r 'all(any(a, b, c), any(d, e, f))'
. You can use letters froma
toz
.
I have tests even with implementations on real Rust traits to make sure that there are no conflicts. It is possible that some weird case is not covered. Would it be possible to implement it and say in the release notes that it is kind of in beta?
The algorithm can be used for https://github.com/idanarye/rust-typed-builder/issues/6 too!
A few words on how it works? Are you normalizing the condition?
I did start explaining the algorithm in the README. I will ping you when it is done.
Why would a normalization be needed? My plan was to check for conflicts in the input and error after finding one. An example for an input conflict is any(a, all(a, b))
which can be detected in the implementation matrix:
a b
S _
S S
I did implement that check today.
It may be an overkill, but a normalization can potentially just remove these conflicts instead of rejecting them.
I admit though that I hadn't study the algorithm yet, so I don't know how complicated it is. And I don't see an already existing crate for it.
I could remove the conflicts without normalization. But I don't think that the additional performance cost is worth it. I don't see why we should allow writing conflicting rules in the first place.
I would even go that far to think about adding a crate feature to optionally disable that input conflicts check because it is O(n^2)
where n
is the number of initial implementation rows.
Maybe we could use a feature like "less_debugging_sugar" that also removes the implementations for repeated and missing fields. But I am not sure how much it would improve the compilation time.
The documentation is finally done and I did fix some more bugs! I have been working on this algorithm for more than a week :D I did underestimate this issue xD
You can read the README and try out the algorithm by running the binary as described: https://codeberg.org/mo8it/sus-impls
@idanarye I am waiting for your feedback before starting a PR to use the algorithm in this crate.
How do we want to integrate the algorithm? Do we keep it as a separate crate or do we just add it to this one as a subcrate? The AGPL3 license is conflicting, right? I would downgrade it for this crate :/
Yea, I'd rather keep this crate copyleft-free.
I wrote and documented the algorithm. The license is compatible. I also started a draft PR. I just have to sit down and finish that PR 😅 Sorry, I got distracted by a lot of stuff.
I wrote and documented the algorithm. The license is compatible. I also started a draft PR. I just have to sit down and finish that PR 😅
Sorry, I got distracted by a lot of stuff.
My bad for the pressure. Simply expressing it didn't have to be copyleft :-) didn't mean to rush anyone haha
I tried to get back to it again but it would require some time to get comfortable with the macros again. Therefore, I closed the PR https://github.com/idanarye/rust-typed-builder/pull/96 to not let you wait even longer and would ask @idanarye if he wants to implement it in typed-builder. I am ready to help on the sus_impls
side if something is required. Just let me know.