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

Add a deserialize_option_with helper #576

Closed
emk opened this issue Oct 8, 2016 · 7 comments
Closed

Add a deserialize_option_with helper #576

emk opened this issue Oct 8, 2016 · 7 comments

Comments

@emk
Copy link

emk commented Oct 8, 2016

I've been making heavy use of serde, and it's been a real help!

Use case

Imagine I have a custom deserialization function for type semver::VersionReq, which we can parse from a string:

/// Deserialize a `semver::VersionReq`.
fn deserialize_version_req<D>
    (deserializer: &mut D)
     -> result::Result<Option<semver::VersionReq>, D::Error>
    where D: serde::Deserializer
{
    let s = try!(String::deserialize(deserializer));
    semver::VersionReq::parse(&s).map_err(|err| {
        // Just convert the underlying error type into a string and
        // pass it to serde as a custom error.
        serde::Error::custom(format!("{}", err))
    })
}

But when I want to use it, it's sometimes wrapped in Option:

struct ProjectConfig {
    /// A semantic version requirement specifying compatible versions of
    /// this tool.
    #[serde(deserialize_with = "deserialize_opt_version_req")]
    cage_version: Option<semver::VersionReq>,
}

Possible implementation

I would like to be able to write another helper function:

/// Deserialize am optional `semver::VersionReq`.
fn deserialize_opt_version_req<D>
    (deserializer: &mut D)
     -> result::Result<Option<semver::VersionReq>, D::Error>
    where D: serde::Deserializer
{
    deserialize_option_with(deserializer, deserialize_version_req)
}

…which calls a hypothetical deserialize_option_with that has the following signature:

pub fn deserialize_option_with<T, D, F>(d: &mut D, f: F) -> Result<Option<T>, D::Error>
    where T: serde::Deserialize,
          <T as str::FromStr>::Err: Display,
          D: serde::Deserializer,
          F: Fn(&mut D) -> result::Result<T, D::Error>

Unfortunately, I find it extremely hard to store type F inside a Visitor implementation, so I can't provide an implementation.

@emk
Copy link
Author

emk commented Oct 8, 2016

This is as close as I can get, minus the leftover cruft:

pub fn deserialize_option_with<T, D, F>(d: &mut D, f: F)
                                        -> result::Result<Option<T>, D::Error>
    where T: serde::Deserialize,
          D: serde::Deserializer,
          F: for<D1: Deserializer> Fn(&mut D1) -> result::Result<T, D1::Error>
{
    /// Declare an internal visitor type to handle our input.
    struct OptVisitor<F> {
        wrapped_fn: F,
    }

    impl<T, F> serde::de::Visitor for OptVisitor<F>
        where T: serde::Deserialize,
              F: for<D1: Deserializer> Fn(&mut D1) -> result::Result<T, D1::Error>
    {
        type Value = Option<T>;

        fn visit_none<E>(&mut self) -> result::Result<Self::Value, E>
            where E: serde::Error
        {
            Ok(None)
        }

        fn visit_some<D>(&mut self,
                         deserializer: &mut D)
                         -> result::Result<Self::Value, D::Error>
            where D: serde::de::Deserializer
        {
            self.wrapped_fn(deserializer).map(Some)
        }
    }

    d.deserialize_option(OptVisitor { wrapped_fn: f, phantom: PhantomData })
}

Unfortunately, the syntax for<D1: Deserializer> Fn(&mut D1) -> result::Result<T, D1::Error> isn't legal Rust at the moment, because it would require something like rust-lang/rfcs#1481. I'll poke at this a bit longer and see if I can do something with trait objects.

@emk
Copy link
Author

emk commented Oct 8, 2016

I'm finding it hard to work around this problem with other approaches. For example, if we restrict our deserialize_option_with function to only work on FromStr types, we can write something like:

/// Deserialize a type that we can parse using `std::str::FromStr`.
fn deserialize_parsable<D, T>(deserializer: &mut D) -> result::Result<T, D::Error>
    where D: serde::Deserializer,
          T: str::FromStr,
          <T as str::FromStr>::Err: fmt::Display
{
    try!(String::deserialize(deserializer))
        .parse()
        .map_err(|e| serde::Error::custom(format!("{}", e)))
}

/// Deserialize an `Option` wrapping a type that we can parse using
/// `std::str::FromStr`.
fn deserialize_parsable_opt<D, T>(deserializer: &mut D)
                                  -> result::Result<Option<T>, D::Error>
    where D: serde::Deserializer,
          T: str::FromStr,
          <T as str::FromStr>::Err: fmt::Display
{
    /// Declare an internal visitor type to handle our input.
    struct OptVisitor<T>(PhantomData<T>);

    impl<T> serde::de::Visitor for OptVisitor<T>
        where T: str::FromStr,
              <T as str::FromStr>::Err: fmt::Display
    {
        type Value = Option<T>;

        fn visit_none<E>(&mut self) -> result::Result<Self::Value, E>
            where E: serde::Error
        {
            Ok(None)
        }

        fn visit_some<D>(&mut self,
                         deserializer: &mut D)
                         -> result::Result<Self::Value, D::Error>
            where D: serde::de::Deserializer
        {
            deserialize_parsable(deserializer).map(Some)
        }
    }

    deserializer.deserialize_option(OptVisitor(PhantomData))
}

But this fails with:

error[E0277]: the trait bound `T: serde::Deserialize` is not satisfied
  --> src/example.rs:50:13
   |
50 |     impl<T> serde::de::Visitor for OptVisitor<T>
   |             ^^^^^^^^^^^^^^^^^^ trait `T: serde::Deserialize` not satisfied
   |
   = help: consider adding a `where T: serde::Deserialize` bound
   = note: required because of the requirements on the impl of `serde::Deserialize` for `std::option::Option<T>`
   = note: required by `serde::de::Visitor`

…because we can apparently only implement visitor for Option<T> if T already implements Deserialize (which defeats the whole purpose).

I think I need to define a custom wrapper type.

@emk
Copy link
Author

emk commented Oct 8, 2016

OK, this is the closest working approximation that I can find to a generic deserialize_option_with function, but it's limited to types that implement FromStr:

/// Deserialize a type that we can parse using `std::str::FromStr`.
fn deserialize_parsable<D, T>(deserializer: &mut D) -> result::Result<T, D::Error>
    where D: serde::Deserializer,
          T: str::FromStr,
          <T as str::FromStr>::Err: fmt::Display
{
    try!(String::deserialize(deserializer))
        .parse()
        .map_err(|e| serde::Error::custom(format!("{}", e)))
}

/// Deserialize an `Option` wrapping a type that we can parse using
/// `std::str::FromStr`.
fn deserialize_parsable_opt<D, T>(deserializer: &mut D)
                                  -> result::Result<Option<T>, D::Error>
    where D: serde::Deserializer,
          T: str::FromStr,
          <T as str::FromStr>::Err: fmt::Display
{
    /// A wrapper type that allows us to declare `deserialize` for
    /// something carrying an `Option<T>` value.
    struct Wrap<T>(Option<T>);

    #[allow(unused_qualifications)]
    impl<T> serde::Deserialize for Wrap<T>
        where T: str::FromStr,
              <T as str::FromStr>::Err: fmt::Display
    {
        fn deserialize<D>(deserializer: &mut D) -> result::Result<Self, D::Error>
            where D: serde::Deserializer
        {
            /// Declare an internal visitor type to handle our input.
            struct OptVisitor<T>(PhantomData<T>);

            impl<T> serde::de::Visitor for OptVisitor<T>
                where T: str::FromStr,
                <T as str::FromStr>::Err: fmt::Display
            {
                type Value = Wrap<T>;

                fn visit_none<E>(&mut self) -> result::Result<Self::Value, E>
                    where E: serde::Error
                {
                    Ok(Wrap(None))
                }

                fn visit_some<D>(&mut self,
                                 deserializer: &mut D)
                                 -> result::Result<Self::Value, D::Error>
                    where D: serde::de::Deserializer
                {
                    deserialize_parsable(deserializer).map(|v| Wrap(Some(v)))
                }
            }

            deserializer.deserialize_option(OptVisitor(PhantomData))
        }
    }

    Wrap::deserialize(deserializer).map(|wrap| wrap.0)
}

The extra Wrap type is necessary to implement Deserialize so that we can implement serde::de::Visitor. The #[allow(unused_qualifications)] attribute supresses some truly weird compiler messages:

warning: unnecessary qualification
  --> src/project_config.in.rs:50:13
   |
50 |     impl<T> serde::Deserialize for Wrap<T>
   |             ^^^^^^^^^^^^^^^^^^
   |

Since it's relatively difficult to write even a partial workaround for something like deserialize_option_with, it would be nice to have something like this in the base serde library.

@TedDriggs
Copy link
Contributor

I ran into a similar problem when I had a Vec containing a type external to my crate. I considered using the newtype-with-deref pattern, but that meant that the docs for the outer type would be pointing to my newtype and not to the actual type. Because it was a Vec, I couldn't easily use deserialize_with, as I would've had to write a custom deserializer.

I'm not sure deserialize_option_with stands above deserialize_vec_with in this case... maybe a deserialize_item_with could cover both cases?

@dtolnay
Copy link
Member

dtolnay commented Feb 20, 2017

Yes we definitely want this for vecs and maps too - tracked in #723. @TedDriggs would you be interested in working on a PR for this?

@TedDriggs
Copy link
Contributor

Interested? Yes. Unfortunately, looking at the code I think this might be beyond my Rust skills at the moment, and the needs of my work project ended up being better solved by defining a local type so my time to take this on would be more limited.

@dtolnay dtolnay added this to the v1.0 milestone Apr 6, 2017
@dtolnay dtolnay removed this from the v1.0 milestone Apr 8, 2017
@dtolnay
Copy link
Member

dtolnay commented Apr 8, 2017

I am folding this into #723 because we would like an approach that applies to sequence and map types as well as Option.

@dtolnay dtolnay closed this as completed Apr 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants