Skip to content
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

Consider using an existing rust validation library? #2

Closed
jmetz opened this issue Feb 12, 2024 · 4 comments
Closed

Consider using an existing rust validation library? #2

jmetz opened this issue Feb 12, 2024 · 4 comments

Comments

@jmetz
Copy link

jmetz commented Feb 12, 2024

When looking into the feasibility of this recently, I came across eg

Perhaps it would make more sense to use one of these for the "easy" fields and only worry about the more complicated validation logic ourselves - or is there a reason not to?

@Tomaz-Vieira
Copy link
Collaborator

Tomaz-Vieira commented Feb 12, 2024

So I looked into the libraries and if I understand correctly, the flow is something like this:

pub struct User{
  #[validate(minimum = 0)]
  #[validate(maximum = 120)] // users can be at most 120 years old
  age: u8,
}

let user = User{age: 255}; // instantiate an invalid user
if let Err(err) = user.validate(){
  # handle invalid value
}

It's nice that we don't have to write the validation code ourselves, but it does come at a cost: Because this validation is not done at construction-time, it is possible to have invalid Users floating around the code, meaning that any code that touches a User can't be sure that it's dealing with a valid User, and that limits the compiler's ability to catch errors; the compiler can never enforce that we are actually calling .validate() and then handling the errors accordingly.

Still, what we have today might not be too different from that... Under bioimg_spec::rdf we have the stuff that is serialized directly to YAML, and under bioimg_spec::runtime we have structs that are parsed, "valid" objects, constructed from those in bioimg_spec::rdf. So, the rdf::* stuff is like "syntactic" checking and the runtime::* stuff is like "semantic" validation. So it's conceivable we could use those validation libraries inside runtime::*, though I don't know if it will actually make our lives easier than to simply write the validation code 🤔

I'll come back to this one runtime::* is more populated so I can have a feel of how annoying the validation code is actually getting.

@jmetz
Copy link
Author

jmetz commented Feb 12, 2024

Hmm I guess this is almost always going to be used for runtime validation - that's kinda the point of the library.

The idea of enforcing compile-time validation, for the rare scenario where someone is hard-coding an RDF in rust... I'm not sure it made sense to spend time on that tbh 🤷

That said, you've done that now I guess. I'm still not 100% clear as to the utility... am testing now.

@jmetz
Copy link
Author

jmetz commented Feb 12, 2024

I just tested your argument, and it does not really work as you had intentioned - see #3

@jmetz
Copy link
Author

jmetz commented Feb 13, 2024

See #3

@jmetz jmetz closed this as not planned Won't fix, can't repro, duplicate, stale Feb 13, 2024
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

No branches or pull requests

2 participants