Allow f32 and f64 map keys

[overdrivenpotato]

Now we allow objects with f32 or f64 string-serialized keys. For example:

{ "1.123": "foo", "2.0": "bar" }

The test_deny_float_key test has been inverted, and the deserialize_integer_key macros have been renamed to deserialize_numeric_key in order to support f32 and f64.

[dtolnay]

Would you be able to elaborate on the use case for this?

In general Rust does not make it easy to work with float map keys, because Eq is typically required of map keys, and floats do not implement it.

[overdrivenpotato]

Would you be able to elaborate on the use case for this?

This is useful for serializing data types like BTreeMap<NotNan<f32>, T>, using a type like NotNan or OrderedFloat from the ordered-float crate. A type like that could be useful in a few situations, e.g. a sorted map for timestamp -> event data, or to maintain an ordered list that allows for insertion between integer indices.

[dtolnay]

.parse() is not appropriate to use here for floats. That accepts a bunch of syntax that is not considered a valid float in json. For example .0 or 00. Please make sure this accepts only representations that would be valid f32/f64 outside of a map key.

[overdrivenpotato]

I've updated this to delegate to the parent Deserializer via self.de.method(...). This unfortunately makes the error messages a bit worse, as we now get something like:

expected value at line 1 column 3

Instead of:

"invalid type: string \"x\", expected f32 at line 1 column 4"

I noticed a similar issue here is present with integer keys, this example works on serde_json master:

extern crate serde_json;
extern crate serde;

fn main() {
    dbg!(serde_json::from_str::<std::collections::HashMap<i32, i32>>(r#"{"00":0}"#));
}

Is it worth updating this as well?

[dtolnay]

Yes, good call. I filed #1034 to follow up.

[overdrivenpotato]

In an effort to patch #1034, I've also updated integer key parsing to use Deserializer::*. This still exhibits the above error degredation.

Would it make sense here to parse the key into an intermediate string buffer, and then deserialize the integer value using crate::from_str? This would result in the original error messages for unexpected string keys, but may have a performance impact.

[dtolnay]

This needs a check for leading whitespace, otherwise it would end up accepting potentially invalid JSON syntax.

For example this is not a legal JSON document:

{"
1":null}

// [dependencies]
// ordered-float = { version = "3", features = ["serde"] }
// serde_json = "1"

use ordered_float::NotNan;
use std::collections::BTreeMap;

fn main() {
    let invalid_json = r#"
        {"
        1":null}
    "#;

    let map: BTreeMap<NotNan<f64>, ()> = serde_json::from_str(invalid_json).unwrap();
    println!("{:?}", map);
}

[dtolnay]

That'll fix the error message problem too. JSON numbers must start with 0-9 or -. If the upcoming character is not one of those, you can avoid calling the function that would produced the bad message in this situation, and directly create a useful "invalid value" error instead.

[overdrivenpotato]

I've patched this with a check for whitespace characters. I don't quite follow what you mean about fixing the error, the misleading text is still present, no? E.g. this example:

fn main() {
    dbg!(serde_json::from_str::<std::collections::HashMap<i32, i32>>(r#"{"abc":0}"#));
}

Previously would print invalid type: string "abc", expected i32 at line 1 column 6, but after this changeset it prints expected value at line 1 column 3.

[dtolnay]

This is what I was thinking of doing to give a more meaningful error: #1035.

[dtolnay]

The serialization looks good!

I left one more comment up in #1027 (review) about the deserialization.

[dtolnay]

Thank you!