Support MapKeySerializer::serialize_some

[LPGhatguy]

This PR updates MapKeySerializer::serialize_some to match Serializer::serialize_some.

Currently, all calls to serialize_some are rejected with the key_must_be_a_string error that many other variants use. Because there is no distinction between Some(T) and T in JSON, I think that serializing the underlying type directly is a better behavior for this function.

I think this change is useful for serializing types that have internal nullability. In a project I am working on, there is a bitpattern in a type I am serializing that I would like serialize as null. To behave correctly with as many serializers as I can, I use serialize_some and serialize_none. When the type is used as a key, this causes serde_json to panic, even if the value is normally fine as a key.

One argument against this change is that it creates a new footgun. Users may serialize a type like HashMap<Option<String>, T> with keys that are all Some(...), and then be surprised when the type fails to serialize with a key that is None. Currently, I think that any types that are valid map keys are always valid map keys, which is desirable.

I'm hoping to get feedback by opening this PR to figure out if this is the right direction... or not!

[dtolnay]

Thanks — this seems fine to me.

Deserializing Map<Option<String>, T> is already permitted.

serde_json::from_str::<BTreeMap<Option<String>, ()>>(r#"{"...":null}"#).unwrap();

json/src/de.rs

Lines 2170 to 2176 in ba29a89

	fn deserialize_option<V>(self, visitor: V) -> Result<V::Value>
	where
	V: de::Visitor<'de>,
	{
	// Map keys cannot be null.
	visitor.visit_some(self)
	}

When the type is used as a key, this causes serde_json to panic, even if the value is normally fine as a key.

Serde_json does not panic in this situation. Most likely serde_json returned an error and that got turned into a panic by something else in your project.

Users may serialize a type like HashMap<Option<String>, T> with keys that are all Some(...), and then be surprised when the type fails to serialize with a key that is None.

This is not concerning to me. As a general principle, serde operates in terms of values, not types. Being able to serialize or deserialize one particular value of a type does not convey a guarantee that you can serialize or deserialize other values of the same Rust type. For example, on all existing serde_json versions, you can serialize Map<Option<String>, T> already (as long as the map is empty). It is not meaningful to consider whether the "type" itself is supported by serde.

[LPGhatguy]

Thank you for the quick response and turnaround!

Serde_json does not panic in this situation. Most likely serde_json returned an error and that got turned into a panic by something else in your project.

Whoops, my bad! I'm not sure why I took the shortcut from "error" to "panic" here. 😅

This is not concerning to me. As a general principle, serde operates in terms of values, not types. Being able to serialize or deserialize one particular value of a type does not convey a guarantee that you can serialize or deserialize other values of the same Rust type. For example, on all existing serde_json versions, you can serialize Map<Option, T> already (as long as the map is empty). It is not meaningful to consider whether the "type" itself is supported by serde.

This makes a lot of sense. Already being able to deserialize the value, and the empty map already working anyways are great examples.