BTreeMap json-serialization not supported

[AlisCode]

What happened?

It looks like insta does not support serializing the type BTreeMap<K, V> when K is a custom enum. serde_json does that fine, though.

Reproduction steps

Here is an example that reproduces the issue.

use serde::Serialize;

#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize)]
pub enum MyEnum {
    A,
    B,
    C,
}

#[test]
fn can_serialize() {
    let map = maplit::btreemap! {
        MyEnum::A => true,
        MyEnum::B => false,
        MyEnum::C => true,
    };
    // serde_json can serialize that BTreeMap
    assert!(serde_json::to_string_pretty(&map).is_ok());
}

#[test]
fn assert_get_map() {
    let map = maplit::btreemap! {
        MyEnum::A => true,
        MyEnum::B => false,
        MyEnum::C => true,
    };
    insta::assert_json_snapshot!(map);
}

The serialization through serde works fine, but insta fails to serialize with the following error :

.cargo/registry/src/index.crates.io-6f17d22bba15001f/insta-1.41.1/src/content/json.rs:188:25:
cannot serialize maps without string keys to JSON

Insta Version

1.41.1

rustc Version

1.80.1

What did you expect?

I expect my BTreeMap to be correctly serialized in JSON and a snapshot to be generated

[mitsuhiko]

The key that it tries to serialize is a Content::UnitVariant("MyEnum", 0, "A"). I'm not sure what the right thing is to do here. Maybe format it as MyEnum::A? I don't think we can reliably match what serde would do normally.

[AlisCode]

serde_json serializes it as "A" for sure, and as "a" if using e.g. #[serde(rename_all = "camelCase")], but I see that adding the rename to MyEnum changes it to Content::UnitVariant("MyEnum", 0, "a"), so it looks like that 3rd argument is what we're looking for ? I dont think MyEnum::A is the right way to go.

What do you think of adding a case for UnitVariant in the code that serializes a map, like this :

diff --git a/insta/src/content/json.rs b/insta/src/content/json.rs
index e4dd5e6..c03556c 100644
--- a/insta/src/content/json.rs
+++ b/insta/src/content/json.rs
@@ -180,6 +180,8 @@ impl Serializer {
                     let real_key = key.resolve_inner();
                     if let Content::String(ref s) = real_key {
                         self.write_escaped_str(s);
+                    } else if let Content::UnitVariant(_, _, s) = real_key {
+                        self.write_escaped_str(s);
                     } else if let Some(num) = real_key.as_i64() {
                         self.write_escaped_str(&num.to_string());
                     } else if let Some(num) = real_key.as_i128() {

I dont know if that would solve every other use cases, but it does work for the use case of this bug. If you approve of that idea, I'll happily create a PR :)

I don't think we can reliably match what serde would do normally.

I was surprised to learn that insta is not using serde_json under the hood, I wonder what the rationale is ?

[BillCarsonFr]

I faced a similar issue causing cannot serialize maps without string keys to JSON FWIW I used the following workaround by using serde_json::to_value

        with_settings!({sort_maps =>true}, {
            // We use directly the serde_json formatter here, because of a bug in insta
            // not serializing custom BTreeMap key enum https://github.com/mitsuhiko/insta/issues/689
            assert_json_snapshot! {
                serde_json::to_value(&room_event).unwrap(),
            }
        });

[mitsuhiko]

I was surprised to learn that insta is not using serde_json under the hood, I wonder what the rationale is ?

Mainly because of snapshot stability. Serde does not make any guarantees about the stability of serialization and has changed behavior multiple times. This has caused all kinds of snapshot instability over the years. Some reason why YAML is now vendored.