Merge pull request 1992 from Mingun/unnecessary-deserialize-with

serde-rs · Aug 21, 2021 · c1ce03b · c1ce03b
2 parents a587eb8 + e0fc467
commit c1ce03b
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 16 deletions.
diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs
@@ -1728,6 +1728,8 @@ fn deserialize_externally_tagged_variant(
     }
 }
 
+// Generates significant part of the visit_seq and visit_map bodies of visitors
+// for the variants of internally tagged enum.
 fn deserialize_internally_tagged_variant(
     params: &Parameters,
     variant: &Variant,
@@ -1779,11 +1781,9 @@ fn deserialize_untagged_variant(
     deserializer: TokenStream,
 ) -> Fragment {
     if let Some(path) = variant.attrs.deserialize_with() {
-        let (wrapper, wrapper_ty, unwrap_fn) = wrap_deserialize_variant_with(params, variant, path);
+        let unwrap_fn = unwrap_to_variant_closure(params, variant, false);
         return quote_block! {
-            #wrapper
-            _serde::__private::Result::map(
-                <#wrapper_ty as _serde::Deserialize>::deserialize(#deserializer), #unwrap_fn)
+            _serde::__private::Result::map(#path(#deserializer), #unwrap_fn)
         };
     }
 
@@ -2883,44 +2883,67 @@ fn wrap_deserialize_variant_with(
     variant: &Variant,
     deserialize_with: &syn::ExprPath,
 ) -> (TokenStream, TokenStream, TokenStream) {
-    let this = &params.this;
-    let variant_ident = &variant.ident;
-
     let field_tys = variant.fields.iter().map(|field| field.ty);
     let (wrapper, wrapper_ty) =
         wrap_deserialize_with(params, &quote!((#(#field_tys),*)), deserialize_with);
 
+    let unwrap_fn = unwrap_to_variant_closure(params, variant, true);
+
+    (wrapper, wrapper_ty, unwrap_fn)
+}
+
+// Generates closure that converts single input parameter to the final value.
+fn unwrap_to_variant_closure(
+    params: &Parameters,
+    variant: &Variant,
+    with_wrapper: bool,
+) -> TokenStream {
+    let this = &params.this;
+    let variant_ident = &variant.ident;
+
+    let (arg, wrapper) = if with_wrapper {
+        (
+            quote!{ __wrap },
+            quote!{ __wrap.value },
+        )
+    } else {
+        let field_tys = variant.fields.iter().map(|field| field.ty);
+        (
+            quote!{ __wrap: (#(#field_tys),*) },
+            quote!{ __wrap },
+        )
+    };
+
     let field_access = (0..variant.fields.len()).map(|n| {
         Member::Unnamed(Index {
             index: n as u32,
             span: Span::call_site(),
         })
     });
-    let unwrap_fn = match variant.style {
+
+    match variant.style {
         Style::Struct if variant.fields.len() == 1 => {
             let member = &variant.fields[0].member;
             quote! {
-                |__wrap| #this::#variant_ident { #member: __wrap.value }
+                |#arg| #this::#variant_ident { #member: #wrapper }
             }
         }
         Style::Struct => {
             let members = variant.fields.iter().map(|field| &field.member);
             quote! {
-                |__wrap| #this::#variant_ident { #(#members: __wrap.value.#field_access),* }
+                |#arg| #this::#variant_ident { #(#members: #wrapper.#field_access),* }
             }
         }
         Style::Tuple => quote! {
-            |__wrap| #this::#variant_ident(#(__wrap.value.#field_access),*)
+            |#arg| #this::#variant_ident(#(#wrapper.#field_access),*)
         },
         Style::Newtype => quote! {
-            |__wrap| #this::#variant_ident(__wrap.value)
+            |#arg| #this::#variant_ident(#wrapper)
         },
         Style::Unit => quote! {
-            |__wrap| #this::#variant_ident
+            |#arg| #this::#variant_ident
         },
-    };
-
-    (wrapper, wrapper_ty, unwrap_fn)
+    }
 }
 
 fn expr_is_missing(field: &Field, cattrs: &attr::Container) -> Fragment {

diff --git a/test_suite/tests/test_gen.rs b/test_suite/tests/test_gen.rs
@@ -823,3 +823,26 @@ where
 {
     vec.first().serialize(serializer)
 }
+
+//////////////////////////////////////////////////////////////////////////
+
+#[derive(Debug, PartialEq, Deserialize)]
+#[serde(tag = "tag")]
+enum InternallyTagged {
+    #[serde(deserialize_with = "deserialize_generic")]
+    Unit,
+
+    #[serde(deserialize_with = "deserialize_generic")]
+    Newtype(i32),
+
+    #[serde(deserialize_with = "deserialize_generic")]
+    Struct { f1: String, f2: u8 },
+}
+
+fn deserialize_generic<'de, T, D>(deserializer: D) -> StdResult<T, D::Error>
+where
+    T: Deserialize<'de>,
+    D: Deserializer<'de>,
+{
+    T::deserialize(deserializer)
+}