Keats / validator

Simple validation for Rust structs
MIT License
2.07k stars 151 forks source link

0.17.0: Code builds but custom validators are silently ignored #317

Closed ssokolow closed 2 months ago

ssokolow commented 7 months ago

I have a project in the process of being rewritten from an incomplete Python draft to an incomplete Rust draft and, after adjusting the code so the Dependabot PR for validator 0.17.0 would build, my integration tests for custom validators are failing with their "Validation should error out" failure message.

Experimenting with the validation functions seems to indicate they're just not getting called at all (Throwing in an unconditional panic! or std::process::exit doesn't change anything.) and, if this is an intended behaviour, then the subtlety of using the new API is such a footgun that it's still a bug.

I was hoping not to share this project until it was ready, but I don't know if I'll have time to make a minimal reproducer, so here's the repo (validator-0.17.0 branch). The Rust project is in the verify_files folder. (cargo test should be all you need to reproduce the problem. It's the integration tests which feed TOML into Serde and then invoke validator which are failing.)

Keats commented 7 months ago

I'll push 0.18 today or tomorrow and we can have a look at that point. You probably don't need to migrate anything (or few things) once 0.18 is released compared to 0.16. You can make the repo private in the meantime

ssokolow commented 7 months ago

Sorry for going silent. The last few days have been a mess (eg. the whole region was without power for much of the time I was awake on two of them). I'll try to fit this in tomorrow.

ssokolow commented 7 months ago

Obviously, that didn't happen, but I've tried it now. I've tested with 0.18 and I'm still getting the same behaviour.

Worked under 0.16. Tests fail with "Validation should error out: ()" in 0.18 after doing minimal type Tetris to get things compiling on 0.17 and above.

ssokolow commented 2 months ago

I finally made time to test with 0.18.1 and this new error message addresses the "too much footgun" issue I had with 0.17 and 0.18:

error: You need to set at least one validator on field `handlers`

         = note: If you want nested validation, use `#[validate(nested)]`