iddm / serde-aux

An auxiliary serde library providing helpful functions for serialisation and deserialisation for containers, struct fields and others.
MIT License
152 stars 26 forks source link

Followup: Implement helper to define custom parser #16

Closed vbrandl closed 3 years ago

vbrandl commented 3 years ago

Following my last comment in #15, I added another helper to use a custom parser (other than T::from_str). While this works as expected, the function names are getting comically long: deserialize_vec_from_string_or_vec_on_separator_with_parser. That's almost javaesque. I'm getting SimpleBeanFactoryMonitorSingletonFactory flashbacks here :D

vbrandl commented 3 years ago

Here's a suggestion for a better API for this case. IMO, this is the more idiomatic solution but it clearly breaks with the rest of this library, so I can totally see this as out of scope. Clearly, the existing deserialize_vec_from_string_or_vec can still be exposed for easy use while giving the caller a good API to create custom deserializers.

If you're willing to use this API, I'll push the changes.

/// Builder to create a parser, that parses a separated string or a vec into a vec.
///
/// # Example:
///
/// ```rust
/// use serde_aux::prelude::*;
/// use std::str::FromStr;
///
/// fn parser<'de, D, T>(deserializer: D) -> Result<Vec<T>, D::Error>
/// where
///     D: serde::Deserializer<'de>,
///     T: FromStr + serde::Deserialize<'de> + 'static,
///     <T as FromStr>::Err: std::fmt::Display,
/// {
///     StringOrVecToVec::default().to_deserializer()(deserializer)
/// }
///
/// #[derive(serde::Serialize, serde::Deserialize, Debug)]
/// struct MyStruct {
///     #[serde(deserialize_with = "parser")]
///     list: Vec<i32>,
/// }
///
/// fn main() {
///     let s = r#" { "list": "1,2,3,4" } "#;
///     let a: MyStruct = serde_json::from_str(s).unwrap();
///     assert_eq!(&a.list, &[1, 2, 3, 4]);
///
///     let s = r#" { "list": [1,2,3,4] } "#;
///     let a: MyStruct = serde_json::from_str(s).unwrap();
///     assert_eq!(&a.list, &[1, 2, 3, 4]);
/// }
/// ```
pub struct StringOrVecToVec<T, E> {
    separator: Box<dyn Fn(char) -> bool>,
    parser: Box<dyn Fn(&str) -> Result<T, E>>,
}

impl<'de, T> Default for StringOrVecToVec<T, T::Err>
where
    T: FromStr + serde::Deserialize<'de> + 'static,
    <T as FromStr>::Err: std::fmt::Display,
{
    fn default() -> Self {
        Self::new(|c| c == ',', T::from_str)
    }
}

impl<'de, T> StringOrVecToVec<T, T::Err>
where
    T: FromStr + serde::Deserialize<'de> + 'static,
    <T as FromStr>::Err: std::fmt::Display,
{
    /// Create a `StringOrVecToVec` builder with a custom separator. `T::from_str` is used to parse
    /// the elements of the list.
    ///
    /// # Example:
    ///
    /// ```rust
    /// use serde_aux::prelude::*;
    /// use std::str::FromStr;
    ///
    /// fn parser<'de, D, T>(deserializer: D) -> Result<Vec<T>, D::Error>
    /// where
    ///     D: serde::Deserializer<'de>,
    ///     T: FromStr + serde::Deserialize<'de> + 'static,
    ///     <T as FromStr>::Err: std::fmt::Display,
    /// {
    ///     StringOrVecToVec::with_separator(|c| c == '-' || c == '+').to_deserializer()(deserializer)
    /// }
    ///
    /// #[derive(serde::Serialize, serde::Deserialize, Debug)]
    /// struct MyStruct {
    ///     #[serde(deserialize_with = "parser")]
    ///     list: Vec<i32>,
    /// }
    ///
    /// fn main() {
    ///     let s = r#" { "list": "1-2+3-4" } "#;
    ///     let a: MyStruct = serde_json::from_str(s).unwrap();
    ///     assert_eq!(&a.list, &[1, 2, 3, 4]);
    ///
    ///     let s = r#" { "list": [1,2,3,4] } "#;
    ///     let a: MyStruct = serde_json::from_str(s).unwrap();
    ///     assert_eq!(&a.list, &[1, 2, 3, 4]);
    /// }
    /// ```
    pub fn with_separator(separator: impl Fn(char) -> bool + 'static) -> Self {
        Self::new(separator, T::from_str)
    }
}

impl<T, E> StringOrVecToVec<T, E> {
    /// Create a deserializer with a custom separator and parsing function.
    ///
    /// # Example:
    ///
    /// ```rust
    /// use serde_aux::prelude::*;
    /// use std::str::FromStr;
    ///
    /// fn parser<'de, D, T>(deserializer: D) -> Result<Vec<T>, D::Error>
    /// where
    ///     D: serde::Deserializer<'de>,
    ///     T: FromStr + serde::Deserialize<'de> + 'static,
    ///     <T as FromStr>::Err: std::fmt::Display,
    /// {
    ///     StringOrVecToVec::new(|c| c == '-' || c == '+', |s| s.trim().parse()).to_deserializer()(deserializer)
    /// }
    ///
    /// #[derive(serde::Serialize, serde::Deserialize, Debug)]
    /// struct MyStruct {
    ///     #[serde(deserialize_with = "parser")]
    ///     list: Vec<i32>,
    /// }
    ///
    /// fn main() {
    ///     let s = r#" { "list": "1 - 2    +  3-    4    " } "#;
    ///     let a: MyStruct = serde_json::from_str(s).unwrap();
    ///     assert_eq!(&a.list, &[1, 2, 3, 4]);
    ///
    ///     let s = r#" { "list": [1,2,3,4] } "#;
    ///     let a: MyStruct = serde_json::from_str(s).unwrap();
    ///     assert_eq!(&a.list, &[1, 2, 3, 4]);
    /// }
    /// ```
    pub fn new(
        separator: impl Fn(char) -> bool + 'static,
        parser: impl Fn(&str) -> Result<T, E> + 'static,
    ) -> Self {
        Self {
            separator: Box::new(separator),
            parser: Box::new(parser),
        }
    }

    /// Create a deserializer with a custom parsing function. The input string will be separated on
    /// `,`.
    ///
    /// # Example:
    ///
    /// ```rust
    /// use serde_aux::prelude::*;
    /// use std::str::FromStr;
    ///
    /// fn parser<'de, D, T>(deserializer: D) -> Result<Vec<T>, D::Error>
    /// where
    ///     D: serde::Deserializer<'de>,
    ///     T: FromStr + serde::Deserialize<'de> + 'static,
    ///     <T as FromStr>::Err: std::fmt::Display,
    /// {
    ///     StringOrVecToVec::with_parser(|s| s.trim().parse()).to_deserializer()(deserializer)
    /// }
    ///
    /// #[derive(serde::Serialize, serde::Deserialize, Debug)]
    /// struct MyStruct {
    ///     #[serde(deserialize_with = "parser")]
    ///     list: Vec<i32>,
    /// }
    ///
    /// fn main() {
    ///     let s = r#" { "list": "1 , 2    ,  3,    4    " } "#;
    ///     let a: MyStruct = serde_json::from_str(s).unwrap();
    ///     assert_eq!(&a.list, &[1, 2, 3, 4]);
    ///
    ///     let s = r#" { "list": [1,2,3,4] } "#;
    ///     let a: MyStruct = serde_json::from_str(s).unwrap();
    ///     assert_eq!(&a.list, &[1, 2, 3, 4]);
    /// }
    /// ```
    pub fn with_parser(parser: impl Fn(&str) -> Result<T, E> + 'static) -> Self {
        Self::new(|c| c == ',', parser)
    }

    /// Creates the actual deserializer from this builder.
    pub fn to_deserializer<'de, D>(
        self,
    ) -> impl Fn(D) -> Result<Vec<T>, <D as serde::Deserializer<'de>>::Error>
    where
        D: serde::Deserializer<'de>,
        T: serde::Deserialize<'de>,
        E: std::fmt::Display,
    {
        #[derive(Deserialize)]
        #[serde(untagged)]
        enum StringOrVec<T> {
            String(String),
            Vec(Vec<T>),
        }

        move |deserializer| match StringOrVec::<T>::deserialize(deserializer)? {
            StringOrVec::String(s) => s
                .split(&self.separator)
                .map(&self.parser)
                .collect::<Result<Vec<_>, _>>()
                .map_err(serde::de::Error::custom),
            StringOrVec::Vec(v) => Ok(v),
        }
    }
}
iddm commented 3 years ago

Thanks for another PR! Yes, I also don't like such long names, I just wanted to let people immediately use a convenient deserialiser without requiring writing functions using them first. For such use cases as you are doing here it is, of course, not suitable and is tiresome to maintain and support, so I appreciate your idea of doing it this way instead. I'll review it when you mark it as ready. :call_me_hand:

P.S. I have already tried to redesign the crate and move away from "field" and "container" attributes for various reasons, but have never finished it fully.

iddm commented 3 years ago

Are you planning on doing anything else or ready to have it merged?

vbrandl commented 3 years ago

I think this is fine. Otherwise this PR will get to big