Open SteelAlloy opened 1 month ago
Maybe something like
#[derive(Builder)]
struct Example {
#[builder(flag)]
color: bool,
}
It would generate three methods:
color() -> true
no_color() -> false
set_color(value: bool)
I think a method that accepts a value is still useful in many cases to make conditional building painless. For example, if the value of color should be computed from an env var, the user will still be able to use set_color(std::env::var("COLOR").is_ok())
instead of doing an if-else
dance of calling color/no_color methods.
In fact the if-else
dance is the thing I tried to avoid and enforce the users of bon
not to do by not providing such a "flag" API initially. I think it can be harmful for the API users who don't want to just hardcode the value to true or false but rather get the value for the boolean from some dynamic source (like env vars, CLI args, etc.).
Anyway this feature by itself can just provide 3 setters to simplify the syntax for the cases where the API is used with hardcoded values a lot.
Also, this is how I imagine the syntax to override the names of each of the 3 setters:
struct Example {
#[builder(
// the values specified here are the ones that will be used by default (#[builder(flag)])
flag(
true = color, // sets the bool to `true`
false = no_color, // sets the bool to `false`
set = set_color // accepts a `bool` argument
),
)]
color: bool,
}
I'm not sure about flag(set = ...)
naming yet though. I'd be happy to hear any other suggestions. Maybe instead it could be arg
and then the default name would be arg_color
(but this seems too unconventional).
For example, AWS SDK for Rust uses a similar convention of set_
prefix for methods that accept Option<T>
, while methods that accept T
have no prefix.
Also, what about Option<bool>
or #[builder(default)]
fields? I suppose it would then generate 4 setters.
#[derive(Builder)]
struct Example {
#[builder(flag)]
color: Option<bool>,
}
There will be methods:
color() -> Some(true)
no_color() -> Some(false)
set_color(value: bool)
maybe_color(value: Option<bool>)
But.. no_color
doesn't look very intuitive for optional members as the callers may think as if it sets the value to None
or the default value.
This has to be supported because booleans are often annotated with #[builder(default)]
and sometimes the value of that default is default = true
. So there has to be a method that accepts an Option<bool>
where None
uses the default to make evolving such API possible without breaking changes (i.e. changing the required boolean flag field to optional).
I like the no_color
naming convention because it aligns with naming patterns in CLI applications, but what I currently don't like about this design is the potential confusion with None
value for optional boolean fields. But maybe not to the extent of blocking this feature...
struct Example { #[builder( // the values specified here are the ones that will be used by default (#[builder(flag)]) flag( true = color, // sets the bool to `true` false = no_color, // sets the bool to `false` set = set_color // accepts a `bool` argument ), )] color: bool, }
The naming sounds good to me π
Need to think about the potential confusion with options, that's an aspect I hadn't considered.
Alternatively, the false
setter may just be omitted by default. Since having that many setters also complicates the API. I wonder if that'd really be a frequent use case for the false
setter.
Also, I'm currently working on a wider feature that allows adding arbitrary setters and methods to the builder that will cover this use case, although it'll require you to write your own impl block with methods which is a bit more code, but will at least make it possible to create such API by yourself. After that is done we may have the #[builder(flag)]
syntax sugar
Such a feature would be great, I'm currently having trouble with custom methods in my builders I don't mind writing my own impl block
I've started work on the feature for custom method extensions of builder type and stable type state signature in https://github.com/elastio/bon/pull/145.
See the usage shown in the PR description there for an example of how you may implement this feature using this new upcoming flexible low-level API.
I still need to add some syntax for hiding setters (overriding their visibility and renaming them) to make this use case possible, which I'll do as part of that PR. Unfortunatelly this update will require a 3.0 version bump of bon
because it generates new items in the user's scope (namely a module named after the builder, but in snake case). It should be a minor breaking change such that 99% of users can upgrade without any migration, but still potentially breaking.
I'm revisiting the design for this yet again as part of the bigger 3.0
design effort.
I propose to make this syntax sugar a bit more limited, but probably more intuitive.
// Generates two setters for booleans:
// - `my_lovely_flag() -> true`
// - `with_my_lovely_flag(bool)`
//
// It also automatically implies that the setters are optional to call.
// The default value is `false` automatically. The `#[builder(default)]`
// attribute is not allowed with the `flag` attribute.
#[builder(flag)]
my_lovely_flag: bool,
I decided to change the by-value setter prefix to with_
, which is more convetional for setters that accept self
by value. I'd like to reserve the set_
prefix convention for by-&mut self
setters (future #[builder(mutable)]
attribute).
In the future there will be a member-level #[builder(required)]
attribute (supported with #[builder(on(_, required))]
at the top-level) that can be combined with flag fields to make sure the user doesn't omit calling their setters (exhaustiveness).
Hi there, thanks for your great work on this crate!
I can relate that this flag
feature would be a great enhancement to make the generated API feel more natural.
I think we could recycle the logic applied to Option
s (with the maybe_
prefix) to bool
s and even any other type with a default value.
The flag
attribute would then allow any type with a default value to opt-in the current behavior of optional values, just preventing the exponential growth of the API. Example:
pub struct MyType<T> {
my_option: Option<T>,
my_flag: bool,
}
#[bon]
impl<T> MyType<T> {
#[builder]
fn new(my_option: Option<T>, my_flag: #[builder(flag)] bool) -> Self {
Self { my_option, my_flag }
}
}
MyType::builder().maybe_my_option(Some(t)).build();
MyType::builder().maybe_my_option(None).build();
MyType::builder().my_option(t).build();
MyType::builder().maybe_my_flag(true).build();
MyType::builder().maybe_my_flag(false).build();
MyType::builder().my_flag().build();
Also, this flag
feature could be extended to support enum
s.
Here's an example use case with redis's SET
request.
With the following AST:
pub enum Condition {
// if not exists
Nx,
// if already exists
Xx,
}
pub struct Duration<P>(i64, PhantomData<P>);
pub struct UnixTimestamp<P>(Duration<P>);
pub struct Seconds;
pub struct Milliseconds;
pub enum Expiration {
Ex(Duration<Seconds>),
ExAt(UnixTimestamp<Seconds>),
Px(Duration<Milliseconds>),
PxAt(UnixTimestamp<Milliseconds>),
KeepTtl,
}
pub struct SetRequest<'a> {
key: &'a [u8],
value: &'a [u8],
condition: Option<Condition>,
expiration: Option<Expiration>,
get: bool
}
Ideally, I'd like to be able to define the builder as such:
use bon::bon;
#[bon]
impl<'a> SetRequest<'a> {
#[builder]
fn new<K, V>(
key: &'a (impl ?Sized + AsRef<[u8]>),
value: &'a (impl ?Sized + AsRef<[u8]>),
#[builder(flag)] condition: Option<Condition>,
#[builder(flag)] expiration: Option<Expiration>,
#[builder(flag)] get: bool,
) -> Self {
Self {
key: key.as_ref(),
value: value.as_ref(),
condition,
expiration,
get
}
}
}
And it would provide the following usages:
SetRequest::builder()
.key("hello")
.value("world")
.maybe_condition(Some(Condition::Nx))
.maybe_expiration(Some(Expiration::Ex(Duration::<Seconds>::try_from(42)?)))
.maybe_get(true)
.build()?;
SetRequest::builder()
.key("hello")
.value("world")
.condition(Condition::Nx)
.expiration(Expiration::Ex(Duration::<Seconds>::try_from(42)?))
.get()
.build()?;
SetRequest::builder()
.key("hello")
.value("world")
.nx()
.ex(Duration::<Seconds>::try_from(42)?)
.get()
.build()?;
If you feel like this is interesting, it'd be happy to contribute :)
PS: Sorry for the long post, I figured it would be easier to explain with actual code and real world example ^^"
Hi, thank you for sharing your ideas!
#[builder(flag)] condition: Option<Condition>,
I don't understand what the attribute #[builder(flag)]
changes for members of type Option<T>
. Members of type Option<T>
already generate two setters {member}(T)
and maybe_{member}(Option<T>)
by default, so there is no need for additional attribute to enable that behavior.
I think we could recycle the logic applied to
Option
s (with themaybe_
prefix) tobool
s and even any other type with a default value.
The maybe_
setter is generated for members with #[builder(default)]
by default as well.
The prefix maybe_
was chosen to denote a setter that takes an Option<T>
value. The naming of such setter implies "the setter accepts a value that can be None
, and if it's None
- then use the default value". In case of boolean flags, the default value is always false
, and that additional setter accepts a bool
(not an Option<bool>
). Therefore, it would be confusing to have maybe_flag(bool)
, because the word "maybe" implies there could be a "lack of value", when in fact there is no "lack of value" to represent for a bool
(it always has a value: true
or false
).
Therefore, I think maybe_{member}
terminology doesn't really fit to boolean flags. I propose to use the with_{member}(bool)
naming instead, which doesn't imply a potential "lack of value".
Wow, thanks for the fast and thorough reply.
I didn't know that the builder(default)
attribute was already generating the maybe_{member}
setters, that's great!
I totally understand now why we should have a different prefix for bool
s.
My idea with the builder(flag)
attribute on an optional value was that it could somehow "traverse" the Option
allowing to use the variants of an enum
as flags, like this:
SetRequest::builder()
.key("hello")
.value("world")
.nx() // sugar for `.condition(Condition::Nx)` or `.maybe_condition(Some(Condition::Nx))`
.ex(42.try_into()?) // sugar for `.expiration(Expiration::Ex(42.try_into()?))` or `.maybe_expiration(Some(Expiration::Ex(42.try_into()?)))`
.get() // sugar for `get(true)` which will become `.with_get(true)` as far as I understand
.build()?;
My idea with the builder(flag) attribute on an optional value was that it could somehow "traverse" the Option allowing to use the variants of an enum as flags
Aha, now I see the idea. There are several problems with this design, such that I think, it shouldn't be part of #[builder(flag)]
or some other specialized attribute. Here is why.
The fundamental limitation of macros (both declarative and procedural) is that they only see "the code" they were applied to. For example:
#[bon]
impl<'a> SetRequest<'a> {
#[builder]
fn new<K, V>(
key: &'a (impl ?Sized + AsRef<[u8]>),
value: &'a (impl ?Sized + AsRef<[u8]>),
#[builder(flag)] condition: Option<Condition>,
#[builder(flag)] expiration: Option<Expiration>,
#[builder(flag)] get: bool,
) -> Self {
Self {
key: key.as_ref(),
value: value.as_ref(),
condition,
expiration,
get
}
}
}
Here, the code generation is performed by the active #[bon]
attribute. This is "the code" that it sees. Now, imagine you are the #[bon]
attribute. Do you know if Condition
and Expiration
are enum
s or struct
s from only this context? Even if you assume that these two types are enum
s, how do you know what variants they have? The problem is that you don't know. The macro only sees the words Condition
and Expiration
, but nothing more than that... So it can't generate setters for every enum variant having this context provided to it by the compiler.
Although, there are some hacks to provide more context about the Condition
and Expiration
types structure that would involve annotating these types with additional macros. But.. They involve generating declarative macro callbacks, and this approach is quite fragile, and doesn't work that well when the types come from external modules.
This feature of generating a setter for every enum variant looks rather exotic to my taste. My gut feeling is that there isn't a considerable demand for this feature out there. So.. it doesn't really make sense to provide syntax sugar for this case. The attribute #[builder(flag)]
is fine because I do see how many people would like to use it frequently. But.. for the enums case I don't, and thus even if we had smth like #[builder(enum)]
for that, it would only be used so rarely, that the cognitive overhead of this feature isn't worth the syntax saving, as for me.
However, I did say the words "syntax sugar". The term "syntax sugar" means just a shorter notation for what's already possible to express. So it doesn't mean that this API isn't physically possible to express with bon
, it will be possible to define such API, albeit it will take a bit more characters to type to do that.
I'm currently working on a big PR stabilizing the builder's type state API (https://github.com/elastio/bon/pull/145). It would allow you to add custom setters to the builder struct directly, by defining your own impl block for the builder. So this will be possible to achieve using the following lower-level code. It already compiles from my PR's branch (https://github.com/elastio/bon/pull/145):
use bon::bon;
use std::marker::PhantomData;
#[bon]
impl<'a> SetRequest<'a> {
#[builder]
fn new(
key: &'a (impl ?Sized + AsRef<[u8]>),
value: &'a (impl ?Sized + AsRef<[u8]>),
condition: Option<Condition>,
expiration: Option<Expiration>,
#[builder(setters(name = set_get))] get: bool,
) -> Self {
Self {
key: key.as_ref(),
value: value.as_ref(),
condition,
expiration,
get,
}
}
}
// Import type-state API tooling from the generated module
use set_request_builder::{IsUnset, SetCondition, SetExpiration, SetGet, State};
// Define a custom impl block for the builder with custom setters
impl<'a, S: State, K, V> SetRequestBuilder<'a, K, V, S>
where
K: ?Sized + AsRef<[u8]>,
V: ?Sized + AsRef<[u8]>,
{
// Custom setter for the enum
fn nx(self) -> SetRequestBuilder<'a, K, V, SetCondition<S>>
// Setters need to have a `where` clause requiring that they can only be called if the
// member wasn't set yet (i.e. the state for the member implements `IsUnset` trait)
where
S::Condition: IsUnset,
{
self.condition(Condition::Nx)
}
// Custom setter for the enum that accepts a value
fn ex(self, duration: Duration<Seconds>) -> SetRequestBuilder<'a, K, V, SetExpiration<S>>
where
S::Expiration: IsUnset,
{
self.expiration(Expiration::Ex(duration))
}
// Custom boolean `flag` setter. Note that `#[builder(flag)]` isn't implemented yet, so in the
// mean time it will be possible to just use this API to implement flag-like setters, but the
// `#[builder(flag)]` feature should be available in the `3.1` release shortly after `3.0` is released
fn get(self) -> SetRequestBuilder<'a, K, V, SetGet<S>>
where
S::Get: IsUnset,
{
self.set_get(true)
}
}
pub enum Condition {
// if not exists
Nx,
// if already exists
Xx,
}
pub struct Duration<P>(i64, PhantomData<P>);
impl<P> Duration<P> {
fn new(value: i64) -> Self {
Self(value, PhantomData)
}
}
pub struct UnixTimestamp<P>(Duration<P>);
pub struct Seconds;
pub struct Milliseconds;
pub enum Expiration {
Ex(Duration<Seconds>),
ExAt(UnixTimestamp<Seconds>),
Px(Duration<Milliseconds>),
PxAt(UnixTimestamp<Milliseconds>),
KeepTtl,
}
pub struct SetRequest<'a> {
key: &'a [u8],
value: &'a [u8],
condition: Option<Condition>,
expiration: Option<Expiration>,
get: bool,
}
fn main() {
SetRequest::builder()
.key("hello")
.value("world")
.maybe_condition(Some(Condition::Nx))
.maybe_expiration(Some(Expiration::Ex(Duration::new(42))))
.set_get(true)
.build();
SetRequest::builder()
.key("hello")
.value("world")
.condition(Condition::Nx)
.expiration(Expiration::Ex(Duration::new(42)))
.get()
.build();
SetRequest::builder()
.key("hello")
.value("world")
.nx()
.ex(Duration::new(42))
.get()
.build();
}
Btw, I just noticed, that the impl block with the #[bon]
attribute can be avoided. In the next version of bon
it will be possible to express such a builder using the derive(Builder)
on the struct directly thanks to the new #[builder(with)]
attribute that overrides the signature of the generated setters:
#[derive(bon::Builder)]
pub struct SetRequest<'a> {
#[builder(with = |value: &'a (impl ?Sized + AsRef<[u8]>)| value.as_ref())]
key: &'a [u8],
#[builder(with = |value: &'a (impl ?Sized + AsRef<[u8]>)| value.as_ref())]
value: &'a [u8],
condition: Option<Condition>,
expiration: Option<Expiration>,
// In `3.1` (after 3.0) this will just be `#[builder(flag)]`
#[builder(setters(name = set_get))]
get: bool,
}
But you'll still need to have a custom impl block for the specialized enum setters
I see you've given deep thoughts in the overall design already and it all makes sense. I actually prefer your alternative :)
3.0.0-rc
version was published. I'll prepare a blog post for the release and switch it to 3.0.0
on November 13-th
Hey there,
Bon is great but it still feels weird to write
.property(true)
for booleans and I feel that an option to shorten this usage would be a nice addition.The syntax is just an example of how I would see it implemented :
And then use it without specifying
true
:For falsy values, one could think of adding the same method with a prefix (
no_
by default) :And then :
Feedback is welcome, it would greatly improve my builders!
A note for the community from the maintainers
Please vote on this issue by adding a π reaction to help the maintainers with prioritizing it. You may add a comment describing your real use case related to this issue for us to better understand the problem domain.