Keats / validator

Simple validation for Rust structs
MIT License
1.97k stars 141 forks source link

HasLen trait not working on custome impl #252

Closed omercotkd closed 1 year ago

omercotkd commented 1 year ago

Hey, I found a weird behavior of the HasLen trait, by impl the trait on any type one would expect that the length validation will be possible on the type:

use serde::Serialize;
use validator::{HasLen, Validate};

const MAX_FILE_SIZE: usize = 1024;

#[derive(Serialize, Validate)]
struct FileData {
    file_name: String,
    size: usize,
    content_type: String,
}

impl HasLen for &FileData {
    fn length(&self) -> u64 {
        self.size as u64
    }
}
#[derive(Validate)]
struct Foo {
    #[validate(length(max = "MAX_FILE_SIZE"))]
    file: FileData,
}

but this is not the case, I am receiving the following error Validator length can only be used on types String, &str etc. only the types that the crate impl I found that If I change the name of the struct to FileDatastr or FileDataString the above code sniped will compile.

After looking a bit in the source code, I found that the fn used to check if the length validation can be applied to a certain type is being made with this fn assert_has_len (validator_derive/src/lib.rs;509).

I think changing the assert_has_len to check if the field type impl HasLen and if so let the code compile.

Keats commented 1 year ago

It's an issue with the fact that the derive macros checks for specific types. These checks should be removed since in practice it's causing a lot of issues and not solving much.

omercotkd commented 1 year ago

I cloned the repo, and changed the above code to the following: (working on the "next" branch)

use validator::{Validate, ValidateLength};

--- 
impl ValidateLength for &FileData {
    fn length(&self) -> u64 {
        self.size as u64
    }
}
----

And I removed the assert_has_len(rust_ident.clone(), field_type, &field.ty); line I mentioned, and the code compiles as it should. considering the above is there a reason to keep the assert_has_len check? or I can open a pr to remove it?

edit: I will also add an example to the readme.

Keats commented 1 year ago

It will be removed for the next version, no need to do a PR.

omercotkd commented 1 year ago

Great thank you for letting me know, have a nice day!