Skip to content

Support input validation in builders #34

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mirosval opened this issue Aug 5, 2024 · 14 comments
Closed

Support input validation in builders #34

mirosval opened this issue Aug 5, 2024 · 14 comments
Labels
design needed The feature requires more design effort feature request A new feature is requested

Comments

@mirosval
Copy link

mirosval commented Aug 5, 2024

Hello and thank you for writing this lib!

I'm wondering if extending this lib with validation would be in scope. What I'm imagining is an interface where I can specify input validators (e.g. string -> uuid). The builder would then run them all and collect the errors. The type of the build method would be fn build() -> Result<T, Vec<E>> or something like this.

It is a common scenario in a lot of software to write types that have some guaranteed invariants, but are built from some basic wire types. Validating all of the fields manually is a lot of boilerplate. I've attempted to do a PoC of this in my crate called valibuk, but I don't have the capacity to properly maintain an OSS project.

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.

@Veetaha Veetaha added design needed The feature requires more design effort feature request A new feature is requested labels Aug 5, 2024
@Veetaha
Copy link
Collaborator

Veetaha commented Aug 5, 2024

UPD: If you are here to learn about fallible builders, then visit this page in docs which goes over the fallible builders in detail. It was created after this issue was posted.

Original response:


Hi, thank you for opening the issue! I can see this within the scope of bon. I understand you'd like some short syntax for validating the inputs and collecting a Vec of errors. This requires some design for the attributes interface for such a feature. I've seen some other crates that can do validation and your PoC will also be a good reference for this feature. It'll require some work, but we can try to get there, although it won't be an immediate priority for bon.

Right now, with the current state of bon it's possible to do validation, but it requires writing a bit more code by using #[builder] on the type's new() method. It allows you to define any custom logic that you'd like inside of the new() method. The builder will use your implementation in the build() method:

use anyhow::{Context, Error};
use bon::bon;
use uuid::Uuid;

struct User {
    name: String,
    id: Uuid,
    group_id: Uuid,
}

#[bon]
impl User {
    #[builder]
    fn new(name: String, id: &str, group_id: &str) -> Result<Self, Vec<Error>> {
        let mut errors = vec![];

        if name.contains("@#!$") {
            errors.push(anyhow::anyhow!("Name contains invalid characters: {name}"));
        }

        let id = Uuid::parse_str(id)
            .with_context(|| format!("Invalid UUID: {id}"))
            .map_err(|err| errors.push(err));

        let group_id = Uuid::parse_str(group_id)
            .with_context(|| format!("Invalid UUID: {group_id}"))
            .map_err(|err| errors.push(err));

        if !errors.is_empty() {
            return Err(errors);
        }

        Ok(Self {
            name,
            // Unwraps are meh.. but not critical. `errors.is_empty()` check ensures no panics here
            id: id.unwrap(),
            group_id: group_id.unwrap(),
        })
    }
}

let result = User::builder()
    .name("username")
    .id("a1da0850-03dc-4f53-8e54-e609b28d17e8")
    .group_id("fbb1efc2-bd9b-425f-a97d-b7ffc6430a1b")
    .build();
    
if let Err(errors) = result {
    // handle errors
}

UPD: there is another way to do validation by performing it directly in setters. For more info, there is this page in documentation that goes over the fallible builders in detail.

@zanedp
Copy link

zanedp commented Aug 23, 2024

This would be a great example to have in the documentation. I was looking for exactly the kind of thing you're showing here where the builder can succeed or fail to build, and couldn't find it, and then luckily saw it here in this issue.

@Veetaha
Copy link
Collaborator

Veetaha commented Aug 23, 2024

Makes sense, I'll add it to the docs together with some other common patterns with the coming release.

@Veetaha
Copy link
Collaborator

Veetaha commented Aug 26, 2024

I added an example to the docs at the "Fallible builders" page. There are some other patterns described in the "Patterns" section. I recommend you to check them out.

See the 2.0 release blog post for details.

@dzmitry-lahoda
Copy link
Contributor

What are possible options of validating inputs regarding ability to reference already set fields?

@Veetaha
Copy link
Collaborator

Veetaha commented Sep 8, 2024

What are possible options of validating inputs regarding ability to reference already set fields?

I'm not sure I understand the full context of this question. Currently bon provides a way to generate a builder from a fallible method where all the fields are already set and available.

@dzmitry-lahoda
Copy link
Contributor

I refer to this text

If you have a use case for some convenience attributes to do automatic validations using the #[builder] macro with the struct syntax, then add a 👍 reaction to this Github issue.

If to introduce convenience attributes to do automatic validations, there is problem if need to cross validate fields, so one field validation depends on how other field validated.

@Veetaha
Copy link
Collaborator

Veetaha commented Oct 25, 2024

Hey guys, I've created an attribute #[builder(with = closure)] that allows customizing the setters.
Among other, it also accepts the following syntax: #[builder(with = |param: ty, ...| -> Result<_, ErrorType> { custom_logic })]. It can be used to make the setter fallible. Example:

#[derive(bon::Builder)]
struct Example {
    #[builder(with = |string: &str| -> Result<_, std::num::ParseIntError> {
        string.parse()
    })]
    level: u32,

    #[builder(with = |name: String| -> anyhow::Result<_> {
        if name.len() > 10 {
            return Err(anyhow::anyhow!("name is too long: {name}"));
        }
        Ok(name)
    })]
    name: String,
}

Example::builder()
    .level("10")?
    .name("hello".to_string())?
    .build();

Example::builder()
    .level("10")?
    // Setter that accepts `Option<String>` is also fallible
    .maybe_name(Some("hello".to_string()))?
    .build();

Of course you don't have to write all the validation logic inside of an attribute. You can write it in a separate function and just invoke that one inside of the closure. Unfortunatelly, this syntax requires a closure. This is needed because the builder macro needs to see the signature of the function (input types and the type of the result/error).

As a last resort it's possible to define custom methods (including custom setters) on the builder struct itself. An example of that is in #145 PR description, but I'll add extensive documentation about that soon.

It's not released yet, but this feature is already in master (merged via #145). I'll release it as part of bon 3.0. Let me know if you have any feedback.

cc all the people who put a like under the issue:

@MeGaGiGaGon, @yeswalrus, @Songtronix, @dzmitry-lahoda, @elefant-dev, @wilzbach, @dsully, @ferreira-tb and other who commented (they get a notification automatically).

@Veetaha Veetaha changed the title Input-validating builders Support input validation in builders Nov 10, 2024
@evbo
Copy link

evbo commented Feb 16, 2025

would there be a way to express: for any field in this struct of type String, validate its length > 0?

That's almost what I thought this doc was describing

example:

#[derive(bon::Builder)]
#[builder(with = |value: String| -> anyhow::Result<_> {
        if name.len() > 10 {
            return Err(anyhow::anyhow!("name is too long: {name}"));
        }
        Ok(name)
    })]
struct MyStruct {
    stuff: String, // applies to this
    other: String // applies to this
    nah: u8 // doesn't apply here
}

@Veetaha
Copy link
Collaborator

Veetaha commented Feb 16, 2025

Hi @evbo. Currently, it's not possible to do it that way, but you can use a shared function and do it like this:

#[derive(bon::Builder)]
struct MyStruct {
    #[builder(with = |value: String| -> anyhow::Result<_> { validate_str(value) })]
    stuff: String,

    #[builder(with = |value: String| -> anyhow::Result<_> { validate_str(value) })]
    other: String,

    nah: u8,
}

fn validate_str(value: String) -> anyhow::Result<String> {
    if value.len() > 10 {
        return Err(anyhow::anyhow!("value is too long: {value}"));
    }
    Ok(value)
}

Doing it at the struct level yields a really complex feature and behavior. I tried to lay out my thoughts about it in #152.

Also, consider making the finishing function of the builder itself fallible instead of making every setter fallible. See the difference between fallible builder and setter in this section

@JakubWasilczyk
Copy link

Hi @Veetaha, I've had an idea on a small improvement on the current fallible setter pattern, and another for the fallible builder:

Fallible setter

A small cool improvement would be to accept not only closures but also functions, like so:

#[derive(bon::Builder)]
struct MyStruct {
    // previously #[builder(with = |value: String| -> anyhow::Result<_> { validate_str(value) })]
    #[builder(with = validate_str)]
    stuff: String,

    // previously #[builder(with = |value: String| -> anyhow::Result<_> { validate_str(value) })]
    #[builder(with = validate_str)]
    other: String,

    nah: u8,
}

fn validate_str(value: String) -> anyhow::Result<String> {
    if value.len() > 10 {
        return Err(anyhow::anyhow!("value is too long: {value}"));
    }
    Ok(value)
}

Since generally speaking, most people would make a bunch of functions that validate the same way and reuse them, with the same error type. I don't know how much work would that take though, I imagine a bunch. If this new idea is something you think can be made I could try to make a PR.

Fallible builder

Here I have more ideas than actual solutions. But the current drawback is that to make a builder fallible I have to create a "new" function that is fallible. But if I create that "new" function I have to maintain that now also other than the struct itself. The beauty of the Builder derive macro is that it updates itself when new fields are added, with the current fallible builder solution I can't have that beauty anymore and have to maintain both the struct and now also the new function.

What could be a solution?

So generally speaking when someone wants to do validate a field manually it's already working with the fallible setter, so that is done, but in most cases people use already existing solutions for validations (the classic don't reinvent the wheel). For example I use validify, and general usage would be to add a derive called Validify and over the fields the validate attributes. Then when needed just validate the struct by calling .validify() which returns either a void or an error. I've checked other solutions and they are quite similar to each other: derive, add attributes, call a function.

So to simplify the boilerplate a little, I thought, it would be cool to add a function call before the build completes, something like this:

#[derive(bon::Builder, validify::Validify)]
#[builder(pre_finish_fn = validify_values)]
struct MyStruct {
    #[modify(lowercase, trim)]
    #[validate(length(equal = 8))]
    stuff: String,

    #[modify(capitalize)]
    #[validate(length(equal = 14))]
    other: String,

    nah: u8,
}

/// Values here are mut because validify has also modifiers, that change the original data.
pub fn validate_values<T>(mut values: T) -> Result<T, ValidationErrors> {
    values.validify()?;
    values
}

Also the pre_finish_fn could change the finish_fn return value to whatever the pre_finish_fn returns.

So the .build() could look like this:

pub fn build() -> Result<T, ValidationErrors> {
    let result = Self {...};
    pre_finish_fn(result)
}

I might be proposing something completely stupid and insane, but would it be possible to do something like this? I'm completely open to contribute and help implementing/designing a solution similar to this.

@Veetaha
Copy link
Collaborator

Veetaha commented Apr 9, 2025

A small cool improvement would be to accept not only closures but also functions

The problem with this approach is that the builder macro needs to see the full signature of the function passed to with because it has to generate a wrapper setter method with almost the same signature. Macros don't have access to that kind of semantic/type information; that's why this limitation exists where you have to spell the types of parameters and return type manually.

The return type is important in this case because there is special treatment in the case if the return type annotation is a Result<_, ...> (the setter method's body and signature works slightly differently for this return type, because it for example needs to wrap the returned builder in Result<Builder>). So, technically, the builder macro can't physically work with arbitrary functions. To make #[builder(with = func_name)] work the builder macro would have to assume that the function has some specific signature like |param: T| -> Result<T> (where T is the type of field). But then what Error type should it assume? Should it assume there exists a Result<T, E = PredefinedDefaultErrorType> in scope?

But I am not sure that having this specific signature be the default make senses at this point. Why this specific signature and not an infallible one |param: T| -> T? Unfortunately, I don't see the most obvious and intuitive function signature that with should expect from a function passed to it. That's why I haven't made any "default signature" assumption in its design so far.

I understand that this fallible signature would make sense if bon's primary use case was validations like the Validify crate you mentioned, where making an assumption about the default signature for with make sense.

People use with with so many different shapes of closures that making a specific one of them special is hard. Unfortunately, I don't have statistics of what signatures are used with with the most often, so it's just my speculation. All I can do is search for #[builder(with = across all github to look for usage patterns in open-source code (luckily this syntax is pretty unique to bon). I am always happy to be proven wrong in any of my speculations, though!

Also, I'm not sure how popular the fallible setter approach even is. To me, it is nicer to have a single fallible finishing function while setters are infallible so that you have to handle the error only in a single place instead of a many setters returning results. This brings us to your next suggestion about fallible builders.


The idea of a pre-validate function in the finishing function makes sense to me. And it's technically already possible like this.

#[derive(bon::Builder)]
// Make the generated finishing function private and give it a different name.
// We'll make our own wrapper over it.
#[builder(finish_fn(vis = "", name = build_internal))]
struct Example {
    a: Option<u32>,
    b: bool,
    c: String,
}

// Now define a custom finishing function on the builder
impl<S: example_builder::IsComplete> ExampleBuilder<S> {
    // This signature is fully customizable - you can use all your Rust power here
    // E.g. make it `async` or `unsafe`, require some input parameters, make it generic,
    // return a `Result`, or even `impl Trait`.
    fn build(self) -> String {
        // Delegate to the default generated finishing function
        let example = self.build_internal();
        // Here we could do anything we want. Including validating
        // the returned `example` object, or converting it to a different
        // value like a `String` in this case
        format!(
            "Example {{ a: {:?}, b: {}, c: {} }}",
            example.a, example.b, example.c
        )
    }
}

fn main() {
    // For the users nothing changes - it's still the same builder pattern
    // except that `build()` now returns a `String`, but it could return a `Result` too.
    let example = Example::builder()
        .a(42)
        .b(true)
        .c("Hello".to_string())
        .build();

    println!("{}", example);
}

I think this way of doing it without any additional macro syntax is already good enough.

There is also a feature request about it in typed-builder that's still open (idanarye/rust-typed-builder#67). The main concern (that I also predicted independently) is that people want this "pre-build" function to change the return type of the value, because they want to make invalid states unrepresentable in the type system. The Validify crate that you shared doesn't do that I suppose, because it takes a &mut T and doesn't change the type of T.

The approach of writing your own new() annotated with #[builder] solves the problem of having an object in invalid state at any point in time, but yeah - it's a bit more boilerpate (even though it also gives you a lot of powers).

Also the pre_finish_fn could change the finish_fn return value to whatever the pre_finish_fn returns.

If you pass an arbitrary function to pre_finish_fn you'll face with the same problem that I described about the setters in the first part of this comment. The builder macro has to know the exact return type of that function because the macro would generate a wrapper method for that function that needs to spell the return type exactly. If we were to have that pre_finish_fn it would also have to accept a closure syntax with all types annotated just like with does.

However, let's step back a bit and first discuss the approach I showed above that lets you have that custom finishing function today in the released version of bon. Does it look okay to you? Because, it doesn't look too bad to me, and we could just recommend using it in the docs instead of adding a new pre_build_fn attribute.

@JakubWasilczyk
Copy link

JakubWasilczyk commented Apr 9, 2025

The problem with this approach is that the builder macro needs to see the full signature of the function passed to with because it has to generate a wrapper setter method with almost the same signature. Macros don't have access to that kind of semantic/type information; that's why this limitation exists where you have to spell the types of parameters and return type manually.

This makes perfect sense. An hour after I wrote that comment I realised that you can't do it due to the macros limitations. My smöl brain here didn't think about it earlier.

The idea of a pre-validate function in the finishing function makes sense to me. And it's technically already possible like this.

#[derive(bon::Builder)]
// Make the generated finishing function private and give it a different name.
// We'll make our own wrapper over it.
#[builder(finish_fn(vis = "", name = build_internal))]
struct Example {
    a: Option<u32>,
    b: bool,
    c: String,
}

// Now define a custom finishing function on the builder
impl<S: example_builder::IsComplete> ExampleBuilder<S> {
    // This signature is fully customizable - you can use all your Rust power here
    // E.g. make it `async` or `unsafe`, require some input parameters, make it generic,
    // return a `Result`, or even `impl Trait`.
    fn build(self) -> String {
        // Delegate to the default generated finishing function
        let example = self.build_internal();
        // Here we could do anything we want. Including validating
        // the returned `example` object, or converting it to a different
        // value like a `String` in this case
        format!(
            "Example {{ a: {:?}, b: {}, c: {} }}",
            example.a, example.b, example.c
        )
    }
}

fn main() {
    // For the users nothing changes - it's still the same builder pattern
    // except that `build()` now returns a `String`, but it could return a `Result` too.
    let example = Example::builder()
        .a(42)
        .b(true)
        .c("Hello".to_string())
        .build();

    println!("{}", example);
}

I think this way of doing it without any additional macro syntax is already good enough.

I think that this solution is actually great. I would suggest putting that in the docs of the fallible builder cause I would say that it's way better than the new approach. I didn't think about it at all.

@Veetaha
Copy link
Collaborator

Veetaha commented Apr 11, 2025

@JakubWasilczyk I added the custom finishing function example to the fallible builder docs in #277.

I'd like to close this issue for now, since I believe the current possibilities with the "Fallible Builders" are flexible enough for people to introduce their own validation macros in combination with bon's builder.

I think at this point, I'd rather delegate the validation attributes syntax sugar to other crates that specialize on validations. Be it validify or some other crate of your love and choice. Validation logic is a very broad topic and a huge additional feature that requires maintenance. And, I don't have the enthusiasm to support that big of a zoo of validation attributes today.

The most I think bon should do on the input validation ground - is to make it as easy as possible to BYOVL (bring your own validation library). So if you have any concrete ideas of that spirit, I'm open to seeing issues about that.

Note that things may change in the future. Never say never!

@Veetaha Veetaha closed this as completed Apr 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design needed The feature requires more design effort feature request A new feature is requested
Projects
None yet
Development

No branches or pull requests

6 participants