Keats / validator

Simple validation for Rust structs
MIT License
1.97k stars 141 forks source link

Add ValidateAll derive macro which automatically adds nested validation to all fields #267

Open davidlang42 opened 1 year ago

davidlang42 commented 1 year ago

My suggestion here is to add a second derive macro ValidateAll which operates similarly to the original (and unmodified) Validate macro, except if any fields don't have any validation, they are assumed to have Nested validation (as if they had the attribute #[validate].

The use case here is when you have a Struct with many fields, most of which are more nested Structs. Down a few layers you probably have a Struct with more typical validation, such as:

#[derive(Validate)]
struct Address {
    name: String,
    #[validate(range(max = 99))]
    house_number: u8
    street: String
    #[validate(phone)]
    phone_number: String
    suburb: String
}

but in the upper layers you have many fields, all of which need Nested validation, writing #[validate] on every 2nd line would be quite verbose. So instead of having this:

#[derive(Validate)]
struct Order {
    #[validate]
    customer_number: u16,
    #[validate]
    billing_address: Address,
    #[validate]
    postal_address: Address,
    #[validate(email)]
    email: String,
    backup_address: Address
    #[validate]
    item_type: Item,
    #[validate(range(min = 1))]
    item_quantity: u8,
    #[validate]
    payment_method: Payment,
    #[validate]
    referrer: MarketingSource
}

you can simply put:

#[derive(ValidateAll)]
struct Order {
    customer_number: CustomerId,
    billing_address: Address,
    postal_address: Address,
    #[validate(email)]
    email: String,
    backup_address: Address
    item_type: Item,
    #[validate(range(min = 1))]
    item_quantity: u8,
    payment_method: Payment,
    referrer: MarketingSource
}

However there is another, and much more important catch here. The first option is not just verbose but its also prone to missing a field, meaning you think you are validating the nested data but you actually aren't. If you look closely, you'll see I forgot the #[validate] attribute on backup_address in the first example, but its not obvious at all. Once I use ValidateAll instead, it not only does the nesting for me, but if any of the types don't implement Validate (for example MarketingSource) then I'll get a compiler error telling me I forgot something.

I can't help but think this will benefit many people in many scenarios, but also I'm not tied to the exact implementation. If you don't like the extra derive macro ValidateAll another idea I had was adding a struct level attribute like #[validate(mandatory_on_all_fields)] which makes the Validate derive macro do the same thing as above.

Also in this PR is a new field level attribute #[validate(always_valid)] for the odd time when you want to use ValidateAll but some fields genuinely don't need validation.

Keats commented 1 year ago

That should be an option on the current validate macro rather than a complete new impl. There is a full rewrite ongoing (https://github.com/Keats/validator/pull/262) so that's probably not going to be merged.

cc @pintariching

davidlang42 commented 1 year ago

I've rewritten it has a struct attribute (#[validate(nest_all_fields)]) rather than new macro, as you suggested.

I understand the rewrite is ongoing, but as it doesn't seem very close to being merged I'm hoping you might consider merging this version (not being pushy though, just putting it out there).

It was actually a fairly small change to make this work (once I understood how the existing derive macro worked) so thanks for structuring well in the first place!

Also, the unit tests were quite helpful in making sure I didn't break this, they all pass now and I've added 2 more to account for the new functionality I've added.

pintariching commented 1 year ago

With this change, all the fields have to be nested structs, otherwise the macro calls .validate() on any other type. You would need a #[validate(skip)] on fields you don't want validated and I don't think it's implemented in the main branch?

I can get this working in the rewrite.

Keats commented 1 year ago

With this change, all the fields have to be nested structs, otherwise the macro calls .validate() on any other type. You would need a #[validate(skip)] on fields you don't want validated and I don't think it's implemented in the main branch?

Yep you would definitely need a skip attribute as well

davidlang42 commented 1 year ago

With this change, all the fields have to be nested structs, otherwise the macro calls .validate() on any other type. You would need a #[validate(skip)] on fields you don't want validated and I don't think it's implemented in the main branch?

I can get this working in the rewrite.

I included this! I just didn't call it skip. I called it alwaysvalid but I can rename to skip if you'd rather :)

jayvdb commented 6 months ago

IMO skip is the usual term used for this.