Replace length(min,max) with range operator
#31
Comments
Can you expand on this? What information does
As for the syntax, I politely disagree. I don't think ranges are a good match here! Ranges are primarily used for slicing, which is a kind of indexing operation. |
Because it's a type, it contains more information than just the values. This could be passed to the
I didn't know
Ranges are much more powerful than that. They can be used to specify any kind of range, not only "indexes"
Anything that has either a start or end (or even none of them!) can be represented using ranges. And of course, as expected by the Rust compiler team, Ranges are heavily optimised for performance
Validator wasn't designed with a composable design, and it shows on things like this. For instance, see Keats/validator#205
Rust has a rich type system we could leverage to our advantage to perform validation. We just need to use it |
My understanding is that the That being said, maybe there's some type system magic niche somewhere that I'm not aware of which could improve performance for length checks in
I don't think changing the syntax makes any difference in terms of composability here. Do you have any concrete examples?
Yep, I agree, which is why the validation rules in |
I had something else on my mind while writing that comment. To my knowledge, there are no performance benefits intrinsic to the
As you noted, the macro could emit ranges and delegate the bound checks to the specific impl. Syntax could be left as it is. |
I see, I think I also misunderstood what you were suggesting. Correct me if I'm wrong, but you're saying that this code: garde/garde/src/rules/range.rs Line 19 in 62292a1 Should accept some |
Rather delegate the bounds checking to the particular pub trait HasLength<RangeType> {
fn check(&self, range: RangeType) -> Result<(),Error>;
}
// Most collections will do this
impl HasLength<T, RangeType: RangeBounds> for Vec<T>{
fn check(&self, range: RangeType) -> Result<(),Error> {
if range.contains(self.len()) { Ok(()) } else { Err(...) }
}
}
// Other collections, where `len()` is O(N) would choose any other impl |
|
I see, but in case of I still think the #[derive(Validate)]
struct Foo {
#[garde(range(min='a', max='z'))]
value: char,
}but that's a separate issue. (#32) |
Yea thats a good one To be honest, i think everything could be generalised even further using traits. Something like this: pub trait Validator<T> {
type Context;
fn validate(&self, ctx: &Context, value: T) -> garde::Result {
todo!()
}
}
// Anything that impls AsRef<str> would support `Email`
impl <T: AsRef<str>> Validator<T> for Email {
type Context = ();
fn validate(&self, ctx: &Context, value: T) -> garde::Result {
todo!()
}
}
// ...use garde::validators::Email;
use garde::validators::Regex;
use garde_extensions::SuperComplexValidation;
#[derive(Validate)]
pub struct Person {
#[validate(Email)]
name: String,
// Range impls Validate<Vec<T>>, Validate<LinkedList<T>>,...
#[validate(18..=20)]
age: NonZeroUsize,
// impl AsRef<str>, so it could support `Regex`
#[validate(Regex(r"[\w/]{1,20}")]
foo: camino::Path,
// Combine multiple validators by using comma
// Allows custom validators, provided by the community
// vvvvvvvvvvvvvvvvvvvvvv
#[validate(10.., SuperComplexValidation { entropy: 100 })]
another_one: String,
}This would generate something like this: impl Validate for Person {
type Context = ();
fn validate(&self, context: Context) -> garde::Result {
Email.validate(&context, &self.name)?;
(18..=20).validate(&context, &self.age)?;
// ...
Ok(())
}
}I kinda love this API, as it makes dead simple to reason about the control flow of the program, simplifies the macro A LOT and provides third party crates with a contract (trait) to build upon, instead of relying on |
|
But now You could solve it by having an
In this case, there's no difference between a trait and a Fn, because you couldn't store any state in the validator anyway, it would have to be a unit struct (and a function is already a unit struct that implements the function traits 🙂). Maybe the context API could be improved to support passing any kind of state to custom validators, so libraries could provide reusable validators including the context they require. Not sure how, though! |
|
But anyway, seeing as the use case:
Is addressed by implementing |
Given that emails are expected to be valid strings, I don't see how this is an issue. Specially when using
Also, the docs list
See the
https://docs.rs/axum/latest/axum/#sharing-state-with-handlers Check In a nutshell: What the API I described earlier offers is to ditch the macro based design in favour of a trait based one |
This example is a bit contrived, but say you want to validate an email stored as a The current
The rules being deeply integrated into the macro is a feature, one which I don't want to ditch, because a good macro can significantly improve the user experience. To give a more concrete example, the
I know about this approach, but it's based on storing the state as |
The
You wouldn't be implementing an pub struct RopeyValidator;
impl Validator<ropey::Rope> for RopeyValidator {
type Context = RopeyRegex; // Or something like that
fn validate(&self, ctx: &Context, value: ropey::Rope) -> garde::Result {
if ctx.matches(value) { Ok(()) } else { Err(...) }
}
}This API ditches the idea of hardcoded rules in favour of generic Validators (implementors of the trait
It can also increase significantly complexity and compile times. Thats one of the main reasons most people ran away from heavy macro frameworks like Rocket. They generate too much code and makes rustc compile too slow!!
Thats worrisome. I don't see that documented on the docs. And it's definitely a behaviour I would like to disable. I don't need to benchmark it to know it increases compilation times on medium sized projects. Moreover, there are already existing solutions to this problem (https://docs.rs/lazy-regex/latest/lazy_regex/). Why would your implementation be better? I would consider regex compilation out of scope for this crate
Right! That example is incomplete. Because // Either this or the `lazy_regex` crate I linked earlier.
// 1. User is able to opt in into regex validation
// 2. Regex can be called elsewhere.
// 3. No more macro magic behind the curtains
lazy_static::lazy_static! {
static ref RE: Regex = Regex::new("...").unwrap();
}
#[derive(Validate)]
pub struct Person {
// Use RE directly, as it impls `Validate<T> where T: AsRef<str>`
#[validate(RE]
foo: String,
}
Nope, that was |
|
I created a POC of the API. I included an example https://github.com/Altair-Bueno/garde-poc-validator Note: For the sake of simplicity, i'm using ranges, but it can be anything else |
As far as I can tell, that would require you to create N*M validators, where N is the number of types, and M is the number of rules. I think a less contrived example would be any string type which does not deref to
A lot of people actually love Rocket's approach, but they "ran away" from it because it was unmaintained. I know at least I did.
You aren't disabling compile time validation of your regular expressions, you're just moving that work to a different macro. I plan to replace full
Only for To reiterate, the macro being deeply integrated with rules is a feature, and I have no plans to ditch it. Feel free to build out your POC into a more complete library and then use that instead! :) |
Compile times. The more work the macro does, the slower it will compile. Its the same deal with sqlx and other macros that do too much
Yes you can, or at least I haven't found any problems whatsoever with #[derive(FromContext)
pub ValidatorContext {
context1: (),
context2: FooContext,
// ...
}
Roger. Thanks for your time :) |
The range operator
..should be the syntax used by garde to performHasLengthvalidation. This would also provide more information toHasLengthimplementations, making them more performant.Example
The text was updated successfully, but these errors were encountered: