Keats/validator

Custom validation of optional fields broken in 0.17.0

Closed this issue · 4 comments

Hi!

I just tried to update to version 0.17.0 and I notice options are no longer automatically handled with a custom validation function. This worked fine in 0.16.1, and I couldn't find any indication in the documentation or git history as to how this case is intended to be handled so I assume it's a bug?

Error message:

error[E0308]: mismatched types
  --> ....
   |
44 | #[derive(Debug, serde::Deserialize, serde::Serialize, validator::Validate)]
   |                                                                      ^^^^^^^^^^^^^^^^^^^ expected `&...
49 |     #[validate(custom(function = "CountryCode::validate"))]
   |                                  ----------------------- arguments to this function are incorrect
   |
   = note: expected reference `&CountryCode`
              found reference `&std::option::Option<CountryCode>`

The code looks something something like this and works as intended when reverting back to version 0.16.1:

#[derive(Debug, serde::Serialize, serde::Deserialize]
struct CountryCode(pub String);

impl CountryCode {
    pub fn validate(&self) -> Result<(), validator::ValidationError> {
        if COUNTRY_CODES.contains(&self.0) {
            Ok(())
        } else {
            Err(validator::ValidationError::new(
                "not a valid ISO-3166-1 alpha-2 country code",
            ))
        }
    }
}

#[derive(Debug, serde::Deserialize, serde::Serialize, validator::Validate)]
#[serde(rename_all = "camelCase")]
pub struct Foo {
    #[validate(custom(function = "CountryCode::validate"))]
    pub country: Option<CountryCode>,
}

That's a bug

Ok so after looking at the code and the docs again, it was a bug that it worked before. Custom functions do take a reference to whatever the type is. In this case, the function signature should be:

fn validate_country_code(country_code: &Option<CountryCode>) -> Result<(), ValidationError> {

Ah ok, so I'll just have to create two separate functions if I want to handle both cases then, no biggie. But it would be awesome if it worked as before though 😃

Thanks anyway, awesome crate btw!

For reference, this is how I solved it:

impl CountryCode {
    pub fn validate(&self) -> Result<(), validator::ValidationError> {
        if COUNTRY_CODES.contains(&self.as_str()) {
            Ok(())
        } else {
            Err(validator::ValidationError::new(
                "not a valid ISO-3166-1 alpha-2 country code",
            ))
        }
    }

    pub fn validate_optional(value: &Option<Self>) -> Result<(), validator::ValidationError> {
        match value {
            Some(code) => Self::validate(code),
            None => Ok(()),
        }
    }
}

#[derive(Debug, serde::Deserialize, serde::Serialize, validator::Validate)]
#[serde(rename_all = "camelCase")]
pub struct Foo {
    #[validate(custom(function = "CountryCode::validate_optional"))]
    pub country: Option<CountryCode>,
}

Ok it's actually going to go back to what it was in 0.16 in the next version, my bad for the churn