Fix incorrect representation of tuple variants with skipped fields

serde_derive/src/internals/ast.rs

@@ -30,11 +30,54 @@ pub enum Data<'a> {
 /// A variant of an enum.
 pub struct Variant<'a> {
     pub ident: syn::Ident,
+    /// #[serde(...)] attributes of this variant
     pub attrs: attr::Variant,
+    /// Style used by Rust code to represent enum variant. Can differ from the
+    /// style of the serialized form due to fields skipping
     pub style: Style,
     pub fields: Vec<Field<'a>>,
     pub original: &'a syn::Variant,
 }
+impl<'a> Variant<'a> {
+    /// Effective style of serialized representation used for deserialization,
+    /// calculated by taking into account fields skipped for deserialization.
+    pub fn de_style(&self) -> Style {
+        effective_style(
+            self.style,
+            self.fields
+                .iter()
+                .filter(|f| !f.attrs.skip_deserializing())
+                .count(),
+        )
+    }
+
+    /// Effective style of serialized representation used for serialization,
+    /// calculated by taking into account fields skipped for serialization.
+    pub fn ser_style(&self) -> Style {
+        effective_style(
+            self.style,
+            self.fields
+                .iter()
+                .filter(|f| !f.attrs.skip_serializing())
+                .count(),
+        )
+    }
+}
+
+fn effective_style(style: Style, fields: usize) -> Style {
+    match (style, fields) {
+        (Style::Unit, _) => Style::Unit,
+
+        (Style::Newtype, 0) => Style::Unit,
+        (Style::Newtype, _) => Style::Newtype,
+
+        (Style::Tuple, 0) => Style::Unit,
+        (Style::Tuple, 1) => Style::Newtype,
+        (Style::Tuple, _) => Style::Tuple,
+
+        (Style::Struct, _) => Style::Struct,
+    }
+}
 
 /// A field of a struct.
 pub struct Field<'a> {
@@ -44,7 +87,7 @@ pub struct Field<'a> {
     pub original: &'a syn::Field,
 }
 
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, PartialEq, Eq)]
 pub enum Style {
     /// Named fields.
     Struct,

serde_derive/src/internals/attr.rs

@@ -586,7 +586,7 @@ impl Container {
             },
             ser_bound: ser_bound.get(),
             de_bound: de_bound.get(),
-            tag: decide_tag(cx, item, untagged, internal_tag, content),
+            tag: decide_tag(cx, untagged, internal_tag, content),
             type_from: type_from.get(),
             type_try_from: type_try_from.get(),
             type_into: type_into.get(),
@@ -685,7 +685,6 @@ impl Container {
 
 fn decide_tag(
     cx: &Ctxt,
-    item: &syn::DeriveInput,
     untagged: BoolAttr,
     internal_tag: Attr<String>,
     content: Attr<String>,
@@ -697,25 +696,7 @@ fn decide_tag(
     ) {
         (None, None, None) => TagType::External,
         (Some(_), None, None) => TagType::None,
-        (None, Some((_, tag)), None) => {
-            // Check that there are no tuple variants.
-            if let syn::Data::Enum(data) = &item.data {
-                for variant in &data.variants {
-                    match &variant.fields {
-                        syn::Fields::Named(_) | syn::Fields::Unit => {}
-                        syn::Fields::Unnamed(fields) => {
-                            if fields.unnamed.len() != 1 {
-                                let msg =
-                                    "#[serde(tag = \"...\")] cannot be used with tuple variants";
-                                cx.error_spanned_by(variant, msg);
-                                break;
-                            }
-                        }
-                    }
-                }
-            }
-            TagType::Internal { tag }
-        }
+        (None, Some((_, tag)), None) => TagType::Internal { tag },
         (Some((untagged_tokens, _)), Some((tag_tokens, _)), None) => {
             let msg = "enum cannot be both untagged and internally tagged";
             cx.error_spanned_by(untagged_tokens, msg);

serde_derive/src/internals/check.rs

@@ -11,6 +11,7 @@ pub fn check(cx: &Ctxt, cont: &mut Container, derive: Derive) {
     check_flatten(cx, cont);
     check_identifier(cx, cont);
     check_variant_skip_attrs(cx, cont);
+    check_internal_no_tuples(cx, cont, derive);
     check_internal_tag_field_name_conflict(cx, cont);
     check_adjacent_tag_conflict(cx, cont);
     check_transparent(cx, cont, derive);
@@ -260,6 +261,29 @@ fn check_variant_skip_attrs(cx: &Ctxt, cont: &Container) {
     }
 }
 
+// The tag of an internally-tagged struct variant must not be the same as either
+// one of its fields, as this would result in duplicate keys in the serialized
+// output and/or ambiguity in the to-be-deserialized input.
+fn check_internal_no_tuples(cx: &Ctxt, cont: &Container, derive: Derive) {
+    let variants = match &cont.data {
+        Data::Enum(variants) => variants,
+        Data::Struct(_, _) => return,
+    };
+
+    if let TagType::Internal { .. } = cont.attrs.tag() {
+        for variant in variants {
+            if derive == Derive::Serialize && variant.ser_style() == Style::Tuple
+                || derive == Derive::Deserialize && variant.de_style() == Style::Tuple
+            {
+                cx.error_spanned_by(
+                    variant.original,
+                    "#[serde(tag = \"...\")] cannot be used with tuple variants",
+                );
+            }
+        }
+    }
+}
+
 // The tag of an internally-tagged struct variant must not be the same as either
 // one of its fields, as this would result in duplicate keys in the serialized
 // output and/or ambiguity in the to-be-deserialized input.

serde_derive/src/internals/mod.rs

@@ -13,7 +13,7 @@ use syn::Type;
 pub use self::ctxt::Ctxt;
 pub use self::receiver::replace_receiver;
 
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, PartialEq, Eq)]
 pub enum Derive {
     Serialize,
     Deserialize,

serde_derive/src/ser.rs

@@ -3,7 +3,7 @@ use crate::internals::ast::{Container, Data, Field, Style, Variant};
 use crate::internals::{attr, replace_receiver, Ctxt, Derive};
 use crate::{bound, dummy, pretend, this};
 use proc_macro2::{Span, TokenStream};
-use quote::{quote, quote_spanned};
+use quote::{quote, quote_spanned, ToTokens};
 use syn::spanned::Spanned;
 use syn::{parse_quote, Ident, Index, Member};
 
@@ -176,7 +176,7 @@ fn serialize_body(cont: &Container, params: &Parameters) -> Fragment {
                 serialize_tuple_struct(params, fields, &cont.attrs)
             }
             Data::Struct(Style::Newtype, fields) => {
-                serialize_newtype_struct(params, &fields[0], &cont.attrs)
+                serialize_newtype_struct(params, 0, &fields[0], &cont.attrs)
             }
             Data::Struct(Style::Unit, _) => serialize_unit_struct(&cont.attrs),
         }
@@ -225,19 +225,18 @@ fn serialize_unit_struct(cattrs: &attr::Container) -> Fragment {
 
 fn serialize_newtype_struct(
     params: &Parameters,
+    index: usize,
     field: &Field,
     cattrs: &attr::Container,
 ) -> Fragment {
+    // For `struct Tuple1as0(#[serde(skip)] u8);` cases
+    if field.attrs.skip_serializing() {
+        return serialize_tuple_struct(params, &[], cattrs);
+    }
+
     let type_name = cattrs.name().serialize_name();
 
-    let mut field_expr = get_member(
-        params,
-        field,
-        &Member::Unnamed(Index {
-            index: 0,
-            span: Span::call_site(),
-        }),
-    );
+    let mut field_expr = get_member(params, field, &Member::Unnamed(Index::from(index)));
     if let Some(path) = field.attrs.serialize_with() {
         field_expr = wrap_serialize_field_with(params, field.ty, path, &field_expr);
     }
@@ -265,6 +264,14 @@ fn serialize_tuple_struct(
         .filter(|(_, field)| !field.attrs.skip_serializing())
         .peekable();
 
+    match serialized_fields.clone().count() {
+        1 => {
+            let (index, field) = serialized_fields.next().unwrap();
+            return serialize_newtype_struct(params, index, field, cattrs);
+        }
+        _ => {}
+    }
+
     let let_mut = mut_if(serialized_fields.peek().is_some());
 
     let len = serialized_fields
@@ -513,7 +520,7 @@ fn serialize_externally_tagged_variant(
         };
     }
 
-    match effective_style(variant) {
+    match variant.ser_style() {
         Style::Unit => {
             quote_expr! {
                 _serde::Serializer::serialize_unit_variant(
@@ -525,13 +532,7 @@ fn serialize_externally_tagged_variant(
             }
         }
         Style::Newtype => {
-            let field = &variant.fields[0];
-            let mut field_expr = quote!(__field0);
-            if let Some(path) = field.attrs.serialize_with() {
-                field_expr = wrap_serialize_field_with(params, field.ty, path, &field_expr);
-            }
-
-            let span = field.original.span();
+            let (field_expr, span) = newtype_field(params, &variant);
             let func = quote_spanned!(span=> _serde::Serializer::serialize_newtype_variant);
             quote_expr! {
                 #func(
@@ -590,7 +591,7 @@ fn serialize_internally_tagged_variant(
         };
     }
 
-    match effective_style(variant) {
+    match variant.ser_style() {
         Style::Unit => {
             quote_block! {
                 let mut __struct = _serde::Serializer::serialize_struct(
@@ -601,13 +602,7 @@ fn serialize_internally_tagged_variant(
             }
         }
         Style::Newtype => {
-            let field = &variant.fields[0];
-            let mut field_expr = quote!(__field0);
-            if let Some(path) = field.attrs.serialize_with() {
-                field_expr = wrap_serialize_field_with(params, field.ty, path, &field_expr);
-            }
-
-            let span = field.original.span();
+            let (field_expr, span) = newtype_field(params, &variant);
             let func = quote_spanned!(span=> _serde::__private::ser::serialize_tagged_newtype);
             quote_expr! {
                 #func(
@@ -647,7 +642,7 @@ fn serialize_adjacently_tagged_variant(
             _serde::Serialize::serialize(#ser, __serializer)
         }
     } else {
-        match effective_style(variant) {
+        match variant.ser_style() {
             Style::Unit => {
                 return quote_block! {
                     let mut __struct = _serde::Serializer::serialize_struct(
@@ -658,13 +653,7 @@ fn serialize_adjacently_tagged_variant(
                 };
             }
             Style::Newtype => {
-                let field = &variant.fields[0];
-                let mut field_expr = quote!(__field0);
-                if let Some(path) = field.attrs.serialize_with() {
-                    field_expr = wrap_serialize_field_with(params, field.ty, path, &field_expr);
-                }
-
-                let span = field.original.span();
+                let (field_expr, span) = newtype_field(params, &variant);
                 let func = quote_spanned!(span=> _serde::ser::SerializeStruct::serialize_field);
                 return quote_block! {
                     let mut __struct = _serde::Serializer::serialize_struct(
@@ -757,20 +746,14 @@ fn serialize_untagged_variant(
         };
     }
 
-    match effective_style(variant) {
+    match variant.ser_style() {
         Style::Unit => {
             quote_expr! {
                 _serde::Serializer::serialize_unit(__serializer)
             }
         }
         Style::Newtype => {
-            let field = &variant.fields[0];
-            let mut field_expr = quote!(__field0);
-            if let Some(path) = field.attrs.serialize_with() {
-                field_expr = wrap_serialize_field_with(params, field.ty, path, &field_expr);
-            }
-
-            let span = field.original.span();
+            let (field_expr, span) = newtype_field(params, &variant);
             let func = quote_spanned!(span=> _serde::Serialize::serialize);
             quote_expr! {
                 #func(#field_expr, __serializer)
@@ -784,6 +767,23 @@ fn serialize_untagged_variant(
     }
 }
 
+fn newtype_field(params: &Parameters, variant: &Variant) -> (TokenStream, Span) {
+    let (i, field) = variant
+        .fields
+        .iter()
+        .enumerate()
+        .find(|(_, field)| !field.attrs.skip_serializing())
+        .expect("checked in Variant::ser_style");
+
+    let mut field_expr =
+        Ident::new(&format!("__field{}", i), Span::call_site()).into_token_stream();
+    if let Some(path) = field.attrs.serialize_with() {
+        field_expr = wrap_serialize_field_with(params, field.ty, path, &field_expr);
+    }
+
+    (field_expr, field.original.span())
+}
+
 enum TupleVariant {
     ExternallyTagged {
         type_name: String,
@@ -1274,13 +1274,6 @@ fn get_member(params: &Parameters, field: &Field, member: &Member) -> TokenStrea
     }
 }
 
-fn effective_style(variant: &Variant) -> Style {
-    match variant.style {
-        Style::Newtype if variant.fields[0].attrs.skip_serializing() => Style::Unit,
-        other => other,
-    }
-}
-
 enum StructTrait {
     SerializeMap,
     SerializeStruct,