Release 1.0.164

Previously if someone wrote an enum containing:

- `A` (untagged)
- `B` (tagged)
- `C` (tagged)
- `D` (untagged)
- `E` (tagged)
- `F` (untagged)

serde_derive would produce errors referring to B and E only, saying
you're supposed to put untagged variants at the end. The choice of B and
E for this error doesn't make a lot of sense because in order to resolve
the issue, the user must either:

- move A and D down

or:

- move B, C, and E up.

This commit changes the error to appear on A and D instead.

.github/workflows/ci.yml

@@ -149,6 +149,7 @@ jobs:
     steps:
       - uses: actions/checkout@v3
       - uses: dtolnay/rust-toolchain@miri
+      - run: cargo miri setup
       - run: cd serde && cargo miri test --features derive,rc,unstable
         env:
           MIRIFLAGS: -Zmiri-strict-provenance

serde/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "serde"
-version = "1.0.163" # remember to update html_root_url and serde_derive dependency
+version = "1.0.164" # remember to update html_root_url and serde_derive dependency
 authors = ["Erick Tryzelaar <erick.tryzelaar@gmail.com>", "David Tolnay <dtolnay@gmail.com>"]
 build = "build.rs"
 categories = ["encoding", "no-std"]
@@ -15,7 +15,7 @@ repository = "https://github.com/serde-rs/serde"
 rust-version = "1.19"
 
 [dependencies]
-serde_derive = { version = "=1.0.163", optional = true, path = "../serde_derive" }
+serde_derive = { version = "=1.0.164", optional = true, path = "../serde_derive" }
 
 [dev-dependencies]
 serde_derive = { version = "1.0", path = "../serde_derive" }

serde/src/lib.rs

@@ -93,7 +93,7 @@
 ////////////////////////////////////////////////////////////////////////////////
 
 // Serde types in rustdoc of other crates get linked to here.
-#![doc(html_root_url = "https://docs.rs/serde/1.0.163")]
+#![doc(html_root_url = "https://docs.rs/serde/1.0.164")]
 // Support using Serde without the standard library!
 #![cfg_attr(not(feature = "std"), no_std)]
 // Unstable functionality only if the user asks for it. For tracking and

serde/src/private/de.rs

@@ -2193,6 +2193,14 @@ mod content {
         }
     }
 
+    impl<'a, 'de: 'a, E> Copy for ContentRefDeserializer<'a, 'de, E> {}
+
+    impl<'a, 'de: 'a, E> Clone for ContentRefDeserializer<'a, 'de, E> {
+        fn clone(&self) -> Self {
+            *self
+        }
+    }
+
     struct EnumRefDeserializer<'a, 'de: 'a, E>
     where
         E: de::Error,

serde_derive/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "serde_derive"
-version = "1.0.163" # remember to update html_root_url
+version = "1.0.164" # remember to update html_root_url
 authors = ["Erick Tryzelaar <erick.tryzelaar@gmail.com>", "David Tolnay <dtolnay@gmail.com>"]
 categories = ["no-std"]
 description = "Macros 1.1 implementation of #[derive(Serialize, Deserialize)]"

serde_derive/src/de.rs

@@ -15,9 +15,7 @@ use this;
 use std::collections::BTreeSet;
 use std::ptr;
 
-pub fn expand_derive_deserialize(
+pub fn expand_derive_deserialize(input: &mut syn::DeriveInput) -> syn::Result<TokenStream> {
-    input: &mut syn::DeriveInput,
-) -> Result<TokenStream, Vec<syn::Error>> {
     replace_receiver(input);
 
     let ctxt = Ctxt::new();
@@ -1168,6 +1166,22 @@ fn deserialize_enum(
     params: &Parameters,
     variants: &[Variant],
     cattrs: &attr::Container,
+) -> Fragment {
+    // The variants have already been checked (in ast.rs) that all untagged variants appear at the end
+    match variants.iter().position(|var| var.attrs.untagged()) {
+        Some(variant_idx) => {
+            let (tagged, untagged) = variants.split_at(variant_idx);
+            let tagged_frag = Expr(deserialize_homogeneous_enum(params, tagged, cattrs));
+            deserialize_untagged_enum_after(params, untagged, cattrs, Some(tagged_frag))
+        }
+        None => deserialize_homogeneous_enum(params, variants, cattrs),
+    }
+}
+
+fn deserialize_homogeneous_enum(
+    params: &Parameters,
+    variants: &[Variant],
+    cattrs: &attr::Container,
 ) -> Fragment {
     match cattrs.tag() {
         attr::TagType::External => deserialize_externally_tagged_enum(params, variants, cattrs),
@@ -1668,6 +1682,16 @@ fn deserialize_untagged_enum(
     params: &Parameters,
     variants: &[Variant],
     cattrs: &attr::Container,
+) -> Fragment {
+    let first_attempt = None;
+    deserialize_untagged_enum_after(params, variants, cattrs, first_attempt)
+}
+
+fn deserialize_untagged_enum_after(
+    params: &Parameters,
+    variants: &[Variant],
+    cattrs: &attr::Container,
+    first_attempt: Option<Expr>,
 ) -> Fragment {
     let attempts = variants
         .iter()
@@ -1677,12 +1701,10 @@ fn deserialize_untagged_enum(
                 params,
                 variant,
                 cattrs,
-                quote!(
+                quote!(__deserializer),
-                    _serde::__private::de::ContentRefDeserializer::<__D::Error>::new(&__content)
-                ),
             ))
         });
-
+    let attempts = first_attempt.into_iter().chain(attempts);
     // TODO this message could be better by saving the errors from the failed
     // attempts. The heuristic used by TOML was to count the number of fields
     // processed before an error, and use the error that happened after the
@@ -1697,6 +1719,7 @@ fn deserialize_untagged_enum(
 
     quote_block! {
         let __content = try!(<_serde::__private::de::Content as _serde::Deserialize>::deserialize(__deserializer));
+        let __deserializer = _serde::__private::de::ContentRefDeserializer::<__D::Error>::new(&__content);
 
         #(
             if let _serde::__private::Ok(__ok) = #attempts {

serde_derive/src/internals/ast.rs

@@ -140,7 +140,7 @@ fn enum_from_ast<'a>(
     variants: &'a Punctuated<syn::Variant, Token![,]>,
     container_default: &attr::Default,
 ) -> Vec<Variant<'a>> {
-    variants
+    let variants: Vec<Variant> = variants
         .iter()
         .map(|variant| {
             let attrs = attr::Variant::from_ast(cx, variant);
@@ -154,7 +154,20 @@ fn enum_from_ast<'a>(
                 original: variant,
             }
         })
-        .collect()
+        .collect();
+
+    let index_of_last_tagged_variant = variants
+        .iter()
+        .rposition(|variant| !variant.attrs.untagged());
+    if let Some(index_of_last_tagged_variant) = index_of_last_tagged_variant {
+        for variant in &variants[..index_of_last_tagged_variant] {
+            if variant.attrs.untagged() {
+                cx.error_spanned_by(&variant.ident, "all variants with the #[serde(untagged)] attribute must be placed at the end of the enum");
+            }
+        }
+    }
+
+    variants
 }
 
 fn struct_from_ast<'a>(

serde_derive/src/internals/attr.rs

@@ -740,6 +740,7 @@ pub struct Variant {
     serialize_with: Option<syn::ExprPath>,
     deserialize_with: Option<syn::ExprPath>,
     borrow: Option<BorrowAttribute>,
+    untagged: bool,
 }
 
 struct BorrowAttribute {
@@ -762,6 +763,7 @@ impl Variant {
         let mut serialize_with = Attr::none(cx, SERIALIZE_WITH);
         let mut deserialize_with = Attr::none(cx, DESERIALIZE_WITH);
         let mut borrow = Attr::none(cx, BORROW);
+        let mut untagged = BoolAttr::none(cx, UNTAGGED);
 
         for attr in &variant.attrs {
             if attr.path() != SERDE {
@@ -879,6 +881,8 @@ impl Variant {
                             cx.error_spanned_by(variant, msg);
                         }
                     }
+                } else if meta.path == UNTAGGED {
+                    untagged.set_true(&meta.path);
                 } else {
                     let path = meta.path.to_token_stream().to_string().replace(' ', "");
                     return Err(
@@ -905,6 +909,7 @@ impl Variant {
             serialize_with: serialize_with.get(),
             deserialize_with: deserialize_with.get(),
             borrow: borrow.get(),
+            untagged: untagged.get(),
         }
     }
 
@@ -956,6 +961,10 @@ impl Variant {
     pub fn deserialize_with(&self) -> Option<&syn::ExprPath> {
         self.deserialize_with.as_ref()
     }
+
+    pub fn untagged(&self) -> bool {
+        self.untagged
+    }
 }
 
 /// Represents field attribute information

serde_derive/src/internals/ctxt.rs

@@ -44,12 +44,19 @@ impl Ctxt {
     }
 
     /// Consume this object, producing a formatted error string if there are errors.
-    pub fn check(self) -> Result<(), Vec<syn::Error>> {
+    pub fn check(self) -> syn::Result<()> {
-        let errors = self.errors.borrow_mut().take().unwrap();
+        let mut errors = self.errors.borrow_mut().take().unwrap().into_iter();
-        match errors.len() {
+
-            0 => Ok(()),
+        let mut combined = match errors.next() {
-            _ => Err(errors),
+            Some(first) => first,
+            None => return Ok(()),
+        };
+
+        for rest in errors {
+            combined.combine(rest);
         }
+
+        Err(combined)
     }
 }
 

serde_derive/src/lib.rs

@@ -13,7 +13,7 @@
 //!
 //! [https://serde.rs/derive.html]: https://serde.rs/derive.html
 
-#![doc(html_root_url = "https://docs.rs/serde_derive/1.0.163")]
+#![doc(html_root_url = "https://docs.rs/serde_derive/1.0.164")]
 #![allow(unknown_lints, bare_trait_objects)]
 // Ignored clippy lints
 #![allow(
@@ -92,7 +92,7 @@ mod try;
 pub fn derive_serialize(input: TokenStream) -> TokenStream {
     let mut input = parse_macro_input!(input as DeriveInput);
     ser::expand_derive_serialize(&mut input)
-        .unwrap_or_else(to_compile_errors)
+        .unwrap_or_else(syn::Error::into_compile_error)
         .into()
 }
 
@@ -100,11 +100,6 @@ pub fn derive_serialize(input: TokenStream) -> TokenStream {
 pub fn derive_deserialize(input: TokenStream) -> TokenStream {
     let mut input = parse_macro_input!(input as DeriveInput);
     de::expand_derive_deserialize(&mut input)
-        .unwrap_or_else(to_compile_errors)
+        .unwrap_or_else(syn::Error::into_compile_error)
         .into()
 }
-
-fn to_compile_errors(errors: Vec<syn::Error>) -> proc_macro2::TokenStream {
-    let compile_errors = errors.iter().map(syn::Error::to_compile_error);
-    quote!(#(#compile_errors)*)
-}

serde_derive/src/ser.rs

@@ -10,9 +10,7 @@ use internals::{attr, replace_receiver, Ctxt, Derive};
 use pretend;
 use this;
 
-pub fn expand_derive_serialize(
+pub fn expand_derive_serialize(input: &mut syn::DeriveInput) -> syn::Result<TokenStream> {
-    input: &mut syn::DeriveInput,
-) -> Result<TokenStream, Vec<syn::Error>> {
     replace_receiver(input);
 
     let ctxt = Ctxt::new();
@@ -475,17 +473,19 @@ fn serialize_variant(
             }
         };
 
-        let body = Match(match cattrs.tag() {
+        let body = Match(match (cattrs.tag(), variant.attrs.untagged()) {
-            attr::TagType::External => {
+            (attr::TagType::External, false) => {
                 serialize_externally_tagged_variant(params, variant, variant_index, cattrs)
             }
-            attr::TagType::Internal { tag } => {
+            (attr::TagType::Internal { tag }, false) => {
                 serialize_internally_tagged_variant(params, variant, cattrs, tag)
             }
-            attr::TagType::Adjacent { tag, content } => {
+            (attr::TagType::Adjacent { tag, content }, false) => {
                 serialize_adjacently_tagged_variant(params, variant, cattrs, tag, content)
             }
-            attr::TagType::None => serialize_untagged_variant(params, variant, cattrs),
+            (attr::TagType::None, _) | (_, true) => {
+                serialize_untagged_variant(params, variant, cattrs)
+            }
         });
 
         quote! {

serde_derive_internals/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "serde_derive_internals"
-version = "0.27.0" # remember to update html_root_url
+version = "0.28.0" # remember to update html_root_url
 authors = ["Erick Tryzelaar <erick.tryzelaar@gmail.com>", "David Tolnay <dtolnay@gmail.com>"]
 description = "AST representation used by Serde derive macros. Unstable."
 documentation = "https://docs.rs/serde_derive_internals"

serde_derive_internals/lib.rs

@@ -1,4 +1,4 @@
-#![doc(html_root_url = "https://docs.rs/serde_derive_internals/0.27.0")]
+#![doc(html_root_url = "https://docs.rs/serde_derive_internals/0.28.0")]
 #![allow(unknown_lints, bare_trait_objects)]
 // Ignored clippy lints
 #![allow(

serde_test/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "serde_test"
-version = "1.0.163" # remember to update html_root_url
+version = "1.0.164" # remember to update html_root_url
 authors = ["Erick Tryzelaar <erick.tryzelaar@gmail.com>", "David Tolnay <dtolnay@gmail.com>"]
 build = "build.rs"
 categories = ["development-tools::testing"]

serde_test/src/lib.rs

@@ -140,7 +140,7 @@
 //! # }
 //! ```
 
-#![doc(html_root_url = "https://docs.rs/serde_test/1.0.163")]
+#![doc(html_root_url = "https://docs.rs/serde_test/1.0.164")]
 #![cfg_attr(feature = "cargo-clippy", allow(renamed_and_removed_lints))]
 // Ignored clippy lints
 #![cfg_attr(feature = "cargo-clippy", allow(float_cmp, needless_doctest_main))]

test_suite/tests/test_annotations.rs

@@ -2442,6 +2442,162 @@ fn test_untagged_enum_containing_flatten() {
     );
 }
 
+#[test]
+fn test_partially_untagged_enum() {
+    #[derive(Serialize, Deserialize, PartialEq, Debug)]
+    enum Exp {
+        Lambda(u32, Box<Exp>),
+        #[serde(untagged)]
+        App(Box<Exp>, Box<Exp>),
+        #[serde(untagged)]
+        Var(u32),
+    }
+    use Exp::*;
+
+    let data = Lambda(0, Box::new(App(Box::new(Var(0)), Box::new(Var(0)))));
+    assert_tokens(
+        &data,
+        &[
+            Token::TupleVariant {
+                name: "Exp",
+                variant: "Lambda",
+                len: 2,
+            },
+            Token::U32(0),
+            Token::Tuple { len: 2 },
+            Token::U32(0),
+            Token::U32(0),
+            Token::TupleEnd,
+            Token::TupleVariantEnd,
+        ],
+    );
+}
+
+#[test]
+fn test_partially_untagged_enum_generic() {
+    trait Trait<T> {
+        type Assoc;
+        type Assoc2;
+    }
+
+    #[derive(Serialize, Deserialize, PartialEq, Debug)]
+    enum E<A, B, C>
+    where
+        A: Trait<C, Assoc2 = B>,
+    {
+        A(A::Assoc),
+        #[serde(untagged)]
+        B(A::Assoc2),
+    }
+
+    impl<T> Trait<T> for () {
+        type Assoc = T;
+        type Assoc2 = bool;
+    }
+
+    type MyE = E<(), bool, u32>;
+    use E::*;
+
+    assert_tokens::<MyE>(&B(true), &[Token::Bool(true)]);
+
+    assert_tokens::<MyE>(
+        &A(5),
+        &[
+            Token::NewtypeVariant {
+                name: "E",
+                variant: "A",
+            },
+            Token::U32(5),
+        ],
+    );
+}
+
+#[test]
+fn test_partially_untagged_enum_desugared() {
+    #[derive(Serialize, Deserialize, PartialEq, Debug)]
+    enum Test {
+        A(u32, u32),
+        B(u32),
+        #[serde(untagged)]
+        C(u32),
+        #[serde(untagged)]
+        D(u32, u32),
+    }
+    use Test::*;
+
+    mod desugared {
+        use super::*;
+        #[derive(Serialize, Deserialize, PartialEq, Debug)]
+        pub(super) enum Test {
+            A(u32, u32),
+            B(u32),
+        }
+    }
+    use desugared::Test as TestTagged;
+
+    #[derive(Serialize, Deserialize, PartialEq, Debug)]
+    #[serde(untagged)]
+    enum TestUntagged {
+        Tagged(TestTagged),
+        C(u32),
+        D(u32, u32),
+    }
+
+    impl From<Test> for TestUntagged {
+        fn from(test: Test) -> Self {
+            match test {
+                A(x, y) => TestUntagged::Tagged(TestTagged::A(x, y)),
+                B(x) => TestUntagged::Tagged(TestTagged::B(x)),
+                C(x) => TestUntagged::C(x),
+                D(x, y) => TestUntagged::D(x, y),
+            }
+        }
+    }
+
+    fn assert_tokens_desugared(value: Test, tokens: &[Token]) {
+        assert_tokens(&value, tokens);
+        let desugared: TestUntagged = value.into();
+        assert_tokens(&desugared, tokens);
+    }
+
+    assert_tokens_desugared(
+        A(0, 1),
+        &[
+            Token::TupleVariant {
+                name: "Test",
+                variant: "A",
+                len: 2,
+            },
+            Token::U32(0),
+            Token::U32(1),
+            Token::TupleVariantEnd,
+        ],
+    );
+
+    assert_tokens_desugared(
+        B(1),
+        &[
+            Token::NewtypeVariant {
+                name: "Test",
+                variant: "B",
+            },
+            Token::U32(1),
+        ],
+    );
+
+    assert_tokens_desugared(C(2), &[Token::U32(2)]);
+
+    assert_tokens_desugared(
+        D(3, 5),
+        &[
+            Token::Tuple { len: 2 },
+            Token::U32(3),
+            Token::U32(5),
+            Token::TupleEnd,
+        ],
+    );
+}
+
 #[test]
 fn test_flatten_untagged_enum() {
     #[derive(Serialize, Deserialize, PartialEq, Debug)]

test_suite/tests/ui/enum-representation/partially_tagged_wrong_order.rs

@@ -0,0 +1,10 @@
+use serde_derive::Serialize;
+
+#[derive(Serialize)]
+enum E {
+    #[serde(untagged)]
+    A(u8),
+    B(String),
+}
+
+fn main() {}

test_suite/tests/ui/enum-representation/partially_tagged_wrong_order.stderr

@@ -0,0 +1,5 @@
+error: all variants with the #[serde(untagged)] attribute must be placed at the end of the enum
+ --> tests/ui/enum-representation/partially_tagged_wrong_order.rs:6:5
+  |
+6 |     A(u8),
+  |     ^