Skip to content

Conversation

@LeMarwin
Copy link

Problem:

  • nest_all_fields adds #[validate(nested)] for all fields, making the macro unusable in a situation where some fields have specific validation instruction.
  • adding #[validate(skip)] along with other #[validate] attrs skips field validation entirely

Solution: only add #[validate(nested)] to fields that don't have other validation attributes.

Essentially this makes nest_all_fields semantically closer to validate_all_fields but inserts #[validate(nested)] if no validation instruction is present. But renaming it is a breaking change

Concrete example of what I wanted to enable:

#[derive(Validate)]
#[validate(nest_all_fields)]
struct Test {
    #[validate(skip)]
    _a: Nested,
    b: NestedValidated,
    #[validate(range(min = 10))]
    c: u8,
}

It's obvious that this should work: we gave explicit validation instruction, so there's no need to insert #[validate(nested)]

Moreover, this PR enables something like this:

#[derive(Validate)]
#[validate(nest_all_fields)]
struct Test {
    #[validate(skip)]
    _a: Nested,
    b: NestedValidated,
    #[validate(custom(function = "validate_c"))]
    c: NestedValidated,
}

which follows the same logic as the previous example, and is quite handy in general

Problem:
* `nest_all_fields` adds #[validate(nested)] for all fields, making the macro unusable in a situation where some fields have specific validation instruction.
* adding #[validate(skip)] along with other #[validate] attrs skips field validation entirely

Solution: only add #[validate(nested)] to fields that don't have other validation attributes
@Keats
Copy link
Owner

Keats commented Apr 21, 2025

How about things like that https://github.com/Keats/validator/blob/master/validator_derive_tests/tests/nested.rs#L273-L336 eg containers where you have length validation but also want to validate the values

@LeMarwin
Copy link
Author

LeMarwin commented Apr 22, 2025

How about things like that https://github.com/Keats/validator/blob/master/validator_derive_tests/tests/nested.rs#L273-L336 eg containers where you have length validation but also want to validate the values

Okay, I missed that usecase since tests didn't break.

I guess that makes this PR a breaking change and thus undesirable, since if someone relied on #[validate(nest_all_fields)] to insert #[validate(nested)] for containers that would no longer be the case and this change would be silent.

How about adding validate_all_fields as a container-level attribute? So that the compiler actually checks that all fields have some kind of validation instruction and adds #[validate(nested)] if none is present?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants