When a serde(untagged) enum can't deser, show all the reasons why

I was writing some code recently that uses `serde(untagged)` for
an enum when deserializing certain JSON schema, but I was unhappy
with the quality of the error messages I was getting, because it
doesn't really tell you why each possibility doesn't work.

I noticed that there is a TODO item in the `serde_derive` proc
macro code that generates this deserialization.

I decided that trying to improve the error messages upstream in
serde-derive is easier than trying to change how I'm using serde,
so I took a stab at implementing this TODO, and updating the tests
that test error messages, and writing some more tests.

I have tried to follow the patterns and conventions that I have
seen elsewhere in the serde-derive source code, and I think that
the code gen is good in that it uses `format_args!` like other
parts of serde error handling and avoids making a new dynamic memory
allocation. When untagged deserialization fails, the errors are
collected on the stack rather than in a new dynamically-sized
container.

Let me know if you think this is a good direction, I'm happy to
iterate if this patch is interesting to you. Thanks for building
serde, it's great.

Author: cbeck88

serde_derive/src/de.rs

@@ -1686,16 +1686,38 @@ fn deserialize_untagged_enum(
     );
     let fallthrough_msg = cattrs.expecting().unwrap_or(&fallthrough_msg);
 
-    quote_block! {
+    // If all variants cannot be deserialized, we will try to write an error
+    // message explaining why it failed for each one
+    // The format string we are building will have the following structure:
+    // "data did not match any variant of untagged enum{}\nvar1: {}\nvar2: {}\nvar3: {}"
+    let mut err_format_string = fallthrough_msg.to_owned();
+    let mut num_variants = 0usize;
+    for var in variants.iter().filter(|variant| !variant.attrs.skip_deserializing()) {
+        err_format_string.push_str("\n");
+        err_format_string.push_str(&var.ident.to_string());
+        err_format_string.push_str(": {}");
+        num_variants += 1;
+     }
+
+     // We need two copies of this iterator, and it's not cloneable
+     let err_identifiers1 = (0..num_variants).map(|idx| format_ident!("_err{}", idx));
+     let err_identifiers2 = (0..num_variants).map(|idx| format_ident!("_err{}", idx));
+
+     quote_block! {
         let __content = try!(<_serde::__private::de::Content as _serde::Deserialize>::deserialize(__deserializer));
 
         #(
-            if let _serde::__private::Ok(__ok) = #attempts {
-                return _serde::__private::Ok(__ok);
-            }
+            let #err_identifiers1 = match #attempts {
+                _serde::__private::Ok(__ok) => {
+                    return _serde::__private::Ok(__ok);
+                },
+                _serde::__private::Err(__err) => {
+                    __err
+                },
+            };
         )*
 
-        _serde::__private::Err(_serde::de::Error::custom(#fallthrough_msg))
+        _serde::__private::Err(_serde::de::Error::custom(format_args!(#err_format_string, #(#err_identifiers2),*)))
     }
 }
 

test_suite/tests/test_annotations.rs

@@ -2776,7 +2776,8 @@ fn test_expecting_message_untagged_tagged_enum() {
         Untagged,
     }
 
-    assert_de_tokens_error::<Enum>(&[Token::Str("Untagged")], r#"something strange..."#);
+    assert_de_tokens_error::<Enum>(&[Token::Str("Untagged")], r#"something strange...
+Untagged: invalid type: string "Untagged", expected unit variant Enum::Untagged"#);
 }
 
 #[test]

test_suite/tests/test_macros.rs

@@ -662,7 +662,13 @@ fn test_untagged_enum() {
 
     assert_de_tokens_error::<Untagged>(
         &[Token::Tuple { len: 1 }, Token::U8(1), Token::TupleEnd],
-        "data did not match any variant of untagged enum Untagged",
+        r#"data did not match any variant of untagged enum Untagged
+A: invalid type: sequence, expected struct variant Untagged::A
+B: invalid type: sequence, expected struct variant Untagged::B
+C: invalid type: sequence, expected unit variant Untagged::C
+D: invalid type: sequence, expected u8
+E: invalid type: sequence, expected a string
+F: invalid length 1, expected tuple variant Untagged::F with 2 elements"#,
     );
 
     assert_de_tokens_error::<Untagged>(
@@ -673,10 +679,67 @@ fn test_untagged_enum() {
             Token::U8(3),
             Token::TupleEnd,
         ],
-        "data did not match any variant of untagged enum Untagged",
+        r#"data did not match any variant of untagged enum Untagged
+A: invalid type: sequence, expected struct variant Untagged::A
+B: invalid type: sequence, expected struct variant Untagged::B
+C: invalid type: sequence, expected unit variant Untagged::C
+D: invalid type: sequence, expected u8
+E: invalid type: sequence, expected a string
+F: invalid length 3, expected 2 elements in sequence"#,
     );
 }
 
+#[test]
+fn test_untagged_enum_with_disallow_unknown_fields() {
+    #[derive(Debug, PartialEq, Serialize, Deserialize)]
+    #[serde(untagged)]
+    #[serde(deny_unknown_fields)]
+    enum Untagged {
+        A { a: String },
+        B { b: String, #[serde(default)] c: Vec<String> },
+    }
+
+    assert_de_tokens_error::<Untagged>(
+        &[
+            Token::Map { len: Some(1) },
+            Token::Str("d"),
+            Token::Str("foo"),
+            Token::MapEnd
+        ],
+        r#"data did not match any variant of untagged enum Untagged
+A: unknown field `d`, expected `a`
+B: unknown field `d`, expected `b` or `c`"#);
+
+    assert_de_tokens_error::<Untagged>(
+        &[
+            Token::Map { len: Some(2) },
+            Token::Str("a"),
+            Token::Str("foo"),
+            Token::Str("c"),
+            Token::Seq { len: Some(1) },
+            Token::Str("bar"),
+            Token::SeqEnd,
+            Token::MapEnd
+        ],
+        r#"data did not match any variant of untagged enum Untagged
+A: unknown field `c`, expected `a`
+B: unknown field `a`, expected `b` or `c`"#);
+
+    assert_de_tokens_error::<Untagged>(
+        &[
+            Token::Map { len: Some(1) },
+            Token::Str("c"),
+            Token::Seq { len: Some(1) },
+            Token::Str("bar"),
+            Token::SeqEnd,
+            Token::MapEnd
+        ],
+        r#"data did not match any variant of untagged enum Untagged
+A: unknown field `c`, expected `a`
+B: missing field `b`"#);
+
+}
+
 #[test]
 fn test_internally_tagged_enum() {
     #[derive(Debug, PartialEq, Serialize, Deserialize)]