serde-rs/json

Deserializing a map with integer keys doesn't work when wrapped in an enum

Closed this issue · 5 comments

Not sure what's causing this behavior, but I have a minimal reproduction:

use serde::{Deserialize, Serialize};
use serde_json;
use std::collections::BTreeMap;

#[derive(Serialize, Deserialize)]
#[serde(tag = "_")]
pub enum ItemTypes {
    #[serde(rename = "r")]
    Resource(Resource),
}

#[derive(Serialize, Deserialize)]
pub struct Resource {
    #[serde(rename = "s")]
    pub strings: BTreeMap<i32, String>,
}

fn main() {
    // This works:
    let resource_json_string = r#"{"s":{"2":"a","3":"b"}}"#;
    let _: Resource = serde_json::from_str(resource_json_string).unwrap();
    println!("Resource deserialized!");

    // But wrap the struct in an enum and it fails:
    let items_json_string = r#"{"_":"r","s":{"2":"a","3":"b"}}"#;
    let _: ItemTypes = serde_json::from_str(items_json_string).unwrap();
    
    // Error("invalid type: string \"2\", expected i32", line: 0, column: 0)
}

Try it out at: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=dbf570c164d60ccf734c375e05b79aa0

ping @dtolnay
Would you be so kind at least to point at the place which possible causes the problem?

We're hitting this in our codebase to and this makes us to omit using HashMap in our API, which is quite undesirable. So, we can try to fix it at least.

cc @evdokimovs

This is a duplicate of serde-rs/serde#1183. I'll close so that we keep the discussion in one place.

We will be better positioned to fix this after there's been progress on associated type defaults (rust-lang/rust#29661). Without that, this is an extremely complicated issue.

Thanks @dtolnay! In the meantime, does anyone have a workaround for this issue? I'm experimenting with providing a deserialize_with option for the map field, but I'm struggling with how to implement the function it'll point to. It'd be nice if I could get serde to handle deserializing to BTreeMap<String, String>, for example, and then it'd be simple enough for me to convert that to a BTreeMap<i32, String> myself.

Your deserialize_with function can directly call BTreeMap<String, String>'s Deserialize impl.

use serde::{de, Deserialize, Deserializer};
use std::collections::HashMap;
use std::fmt::Display;
use std::hash::Hash;
use std::str::FromStr;

fn de_int_key<'de, D, K, V>(deserializer: D) -> Result<HashMap<K, V>, D::Error>
where
    D: Deserializer<'de>,
    K: Eq + Hash + FromStr,
    K::Err: Display,
    V: Deserialize<'de>,
{
    let string_map = <HashMap<String, V>>::deserialize(deserializer)?;
    let mut map = HashMap::with_capacity(string_map.len());
    for (s, v) in string_map {
        let k = K::from_str(&s).map_err(de::Error::custom)?;
        map.insert(k, v);
    }
    Ok(map)
}

If you need to optimize for performance or memory usage, the following will likely do better by not allocating the string keys or the string to string map.

use serde::de::{self, Deserialize, DeserializeSeed, Deserializer, MapAccess, Visitor};
use std::collections::HashMap;
use std::fmt::{self, Display};
use std::hash::Hash;
use std::marker::PhantomData;
use std::str::FromStr;

fn de_int_key<'de, D, K, V>(deserializer: D) -> Result<HashMap<K, V>, D::Error>
where
    D: Deserializer<'de>,
    K: Eq + Hash + FromStr,
    K::Err: Display,
    V: Deserialize<'de>,
{
    struct KeySeed<K> {
        k: PhantomData<K>,
    }

    impl<'de, K> DeserializeSeed<'de> for KeySeed<K>
    where
        K: FromStr,
        K::Err: Display,
    {
        type Value = K;

        fn deserialize<D>(self, deserializer: D) -> Result<Self::Value, D::Error>
        where
            D: Deserializer<'de>,
        {
            deserializer.deserialize_str(self)
        }
    }

    impl<'de, K> Visitor<'de> for KeySeed<K>
    where
        K: FromStr,
        K::Err: Display,
    {
        type Value = K;

        fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
            formatter.write_str("a string")
        }

        fn visit_str<E>(self, string: &str) -> Result<Self::Value, E>
        where
            E: de::Error,
        {
            K::from_str(string).map_err(de::Error::custom)
        }
    }

    struct MapVisitor<K, V> {
        k: PhantomData<K>,
        v: PhantomData<V>,
    }

    impl<'de, K, V> Visitor<'de> for MapVisitor<K, V>
    where
        K: Eq + Hash + FromStr,
        K::Err: Display,
        V: Deserialize<'de>,
    {
        type Value = HashMap<K, V>;

        fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
            formatter.write_str("a map")
        }

        fn visit_map<A>(self, mut input: A) -> Result<Self::Value, A::Error>
        where
            A: MapAccess<'de>,
        {
            let mut map = HashMap::new();
            while let Some((k, v)) =
                input.next_entry_seed(KeySeed { k: PhantomData }, PhantomData)?
            {
                map.insert(k, v);
            }
            Ok(map)
        }
    }

    deserializer.deserialize_map(MapVisitor {
        k: PhantomData,
        v: PhantomData,
    })
}

Wow, that's super helpful @dtolnay, thanks so much!