Trait validation

[pintariching]

I've implemented trait validation as mentioned in #205.

For now I've just implemented it for the length validation just to see if I'm on the right track. I've also copied the tests just to be sure it works.

Now if you want length validation for a custom type you just have to write an impl block for it like

impl ValidateLength for CustomType {
    fn validate_length(self, min: Option<u64>, max: Option<u64>, equal: Option<u64>) -> bool {
        // write length validation in here 
    }
}

[tqwewe]

It seems like if the user wants to implement ValidateLength on their own type, they'd need to also implement HaLen?

Might it be simpler if HasLen is not involved?

[Keats]

Typo on the name and it shouldn't use HasLen at all. Then we can just create a macro to generate all the impls we want for each type. With this trait, HasLen doesn't exist anymore

[Keats]

Typo on the name and it shouldn't use HasLen at all. Then we can just create a macro to generate all the impls we want for each type. With this trait, HasLen doesn't exist anymore

[pintariching]

Okay I changed the trait and updated the implementations. The length extraction has to be done inside the impl for every type out there. I just added the temporary function validate_length_for_trait so the crate still builds.

[pintariching]

I tried making a macro with macro_rules! for quickly implementing for all the types but my macro-fu is not on that level. I'm not sure how to specify lifetimes so the macro makes something like this

impl<'a> HasLen for &'a String {
    fn length(&self) -> u64 {
        self.chars().count() as u64
    }
}

from something like this

impl_validate_length!(&'a String)

[pintariching]

I have implemented all the impls that had the HasLen trait by hand. I'm unsure how to make a macro. I also added a derive test can_validate_custom_impl_for_length that shows the functionality that I managed to get to work. Right now you have to implement the length validation by yourself unless validator::length::validate_length_for_trait is made public

[tqwewe]

What made you decide to go with self.chars().count() instead of self.len()?
And why use u64 everywhere instead of usize?

[pintariching]

I used self.chars().count() because the HasLen trait used them too and I didn't really think about it. The same is for u64 instead of usize. The validate_length function used Option<u64>. I can refactor the trait to use self.len() and usize if it makes any difference.

[Keats]

Strings should use self.chars().count()

[tqwewe]

@Keats interesting, what is the advantage of that over .len()?

[Keats]

fn main() {
    let txt = "日本語";
    println!("len {:?} vs chars {:?}", txt.len(), txt.chars().count());
}

will print

len 9 vs chars 3

In this case no one reasonable will think of len as number of bytes used (.len()) vs number of characters, which is what people think of when they think of a len for a string.

[tqwewe]

Wow very interesting! Makes sense to use chars count then

[pintariching]

Now I'm unsure what path to go. Do we let users implement the validation logic by themselves like so

#[derive(Debug, Serialize)]
struct CustomString(String);

impl validator::ValidateLength for &CustomString {
    fn validate_length(&self, min: Option<u64>, max: Option<u64>, equal: Option<u64>) -> bool {
        let length = self.0.chars().count() as u64;

        if let Some(eq) = equal {
            return length == eq;
        } else {
            if let Some(m) = min {
	    if length < m {
	        return false;
	    }
        }
        if let Some(m) = max {
	    if length > m {
                return false;
	        }
            }
        }
        true
    }
}

This fees like cumebrsome and I would prefer if the user just has to implement a length function like

#[derive(Debug, Serialize)]
struct CustomString(String);

impl validator::ValidateLength for &CustomString {
    fn length(&self) -> u64 {
        self.0.chars().count() as u64
    }
}

And maybe leave the first example for the custom validator?

[Keats]

I don't see what this length method gives us though? It's not validating anything

[pintariching]

It's just there so the user can specify how to get the length value from a custom type. This is then used in the validate_length function with value.length().

Now that I think about it, it's the same as it was before with the HasLen trait so I just renamed it 😅.

[pintariching]

How about this, the trait ValidateLength implements a default validate_length() function that you can override if you want. Or you can just implement length() for your custom type that the default implementation uses if you don't want to change the logic.

[Keats]

I think that looks good

[Keats]

That implies allocation, it should return a &str instead

[pintariching]

Returning a &str is impossible if I want to return a formatted string for example

fn to_email_string(&self) -> &str {
    format!("{}@{}", self.user_part, self.domain_part).as_str()
}

How about returning Cow?

[Keats]

Cow is fine then

[Keats]

Thoughts on letting crates handle validator support themselves like they do for serde?

[Keats]

I'm tempted to remove that one from validator, it's currently too generic to be easily usable and can easily be added manually if needed.

[pintariching]

Maybe leaving an example on how to add it with a custom validator after removing it would be a good idea?

[svenstaro]

What's the status on this? Is there anything that can be picked up here?

[Keats]

Just need some time to take that PR in and re-do the derive impl. I should have some more free time from next week on and could use a little break from Zola.

[svenstaro]

Just a friendly ping @Keats :) We could also try to assist as we use this lib quite a bit.

[Keats]

Yes that would be great. I started to have a look at the derive crate and it seems nicer to use than 7 years ago. It also made me realise we were not handling the rename_all of serde afaik so that rename feature kinda sucked but no one complained? Maybe it's not super useful in the end.

The library itself should be that PR but I don't know whether we should do it in 2 phases (validation as trait -> derive rewrite later) or try to do it in one go.
I would probably go with the 2 phases since it would be too much work to do it in a single phase.

I've created a next branch so we can work on it without causing issues on master