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
Consider serializing map integer keys as strings #45
Comments
I stumbled into the "key must be a string" error while using the newtype pattern (https://aturon.github.io/features/types/newtype.html).
Also I often use the small string optimization (around https://github.com/fitzgen/inlinable_string) to avoid the heap allocations. I gather Serde won't let me do |
@ArtemGr it shouldn't be a problem, you just need to make sure that your Edit: oh wrt newtypes, I'm not sure if it's a good idea to make the default serialization work in this case. You created a newtype out of a reason (type safety), so I'm not sure if serde should break that as the default. But I'm open to be convinced otherwise ;) |
Oh, thanks for the hint @oli-obk. I thought the problem is the string check but it looks like it's the lack of newtype support. I sure would like for Serde to support the newtype pattern, maybe with some kind of new tag. |
serde does support newtypes. It's just a question of whether it would be a good default to serialize newtypes exactly like their inner type. Right now that's not the default. |
Yeah, what I mean is the transparent newtype support. When we write But with newtype pattern it suddenly works differently, Serde wants to actually write down in JSON that
I guess defaulting transparent newtype support would be a breaking change. It would be much easier to just stick a new tag on it. |
What do you mean by that? You can already add |
This: "to serialize newtypes exactly like their inner type".
That's what I'm trying to avoid. |
I don't understand where you want to add a new tag to prevent a breaking change (since you obviously dislike |
or
Where InlString should be treated as a string because it implements
What doesn't work currently: a) b) Using |
I'd be fine with newtype structs being serialized without serializing the wrapper struct, that's why I added support for it. Frankly I thought I already implemented this for JSON. This would need a major version bump though since it'd change the serialization format. |
That behavior would be very surprising to me and I was not able to reproduce it. Can you share your code? Here is mine. #![feature(custom_derive, plugin)]
#![plugin(serde_macros)]
use std::collections::btree_map::BTreeMap;
extern crate serde_json;
pub type Foo = String;
#[derive(Serialize, Deserialize)]
pub struct Bar(pub BTreeMap<Foo, i32>);
fn main() {
let mut b = Bar(BTreeMap::new());
b.0.insert("foo".to_string(), 123);
// prints {"foo":123}
println!("{}", serde_json::to_string(&b).unwrap());
} |
Ditto Erick, I would have expected this to work already. This is a bug that we need to fix. |
Turns out it's kind of a corner case. I'm not serializing to a String, instead I have an untyped JSON structure (a part of technical debt) that I work with as simply Here's some code:
With Serde 0.7.0 it prints
I'm glad to see that the first line printed is indeed a transparent newtype. My use case is the second one though and here Serde seems to wrap the inner type in array. |
Seems like a bug in Value. I filed #77 to look into it. |
Another data point: Go allows integer keys: package main
import (
"fmt"
"encoding/json"
)
func main() {
m := map[int]string{1: "s"}
j, err := json.Marshal(m)
if err != nil {
panic(err)
}
fmt.Println(string(j)) // {"1":"s"}
} |
I think we can close this now 😃 |
Right now serde_json rejects serializing maps with integer keys because semantically speaking, JSON only supports maps with string keys. There are workarounds in serde 0.7 with the new
#[serde(serialize_with="...", deserialize_with="...)]
(see this gist), but it still can be annoying if this keeps causing problems.Is there any real value about erroring out on non-key values?
The text was updated successfully, but these errors were encountered: