From 5b815b70016502d79a94c3e8beece948fee6aeaf Mon Sep 17 00:00:00 2001
From: Michael Smith <michael@spinda.net>
Date: Mon, 7 Aug 2017 17:22:26 -0700
Subject: [PATCH 1/3] Implement serialize_with for variants

As discussed in #1013, serialize_with functions attached to variants receive an
argument for each inner value contained within the variant. Internally such a
function is wired up to the serializer as if the variant were a newtype variant.
---
 serde_derive/src/bound.rs                     |  28 ++-
 serde_derive/src/de.rs                        |  11 +-
 serde_derive/src/ser.rs                       | 184 +++++++++++++-----
 serde_derive_internals/src/attr.rs            |  28 +++
 serde_derive_internals/src/check.rs           |  38 ++++
 .../with-variant/skip_ser_newtype_field.rs    |  25 +++
 .../with-variant/skip_ser_newtype_field_if.rs |  27 +++
 .../with-variant/skip_ser_struct_field.rs     |  29 +++
 .../with-variant/skip_ser_struct_field_if.rs  |  31 +++
 .../with-variant/skip_ser_tuple_field.rs      |  25 +++
 .../with-variant/skip_ser_tuple_field_if.rs   |  27 +++
 .../with-variant/skip_ser_whole_variant.rs    |  26 +++
 test_suite/tests/test_annotations.rs          |  68 +++++++
 test_suite/tests/test_gen.rs                  | 144 ++++++++++++++
 14 files changed, 631 insertions(+), 60 deletions(-)
 create mode 100644 test_suite/tests/compile-fail/with-variant/skip_ser_newtype_field.rs
 create mode 100644 test_suite/tests/compile-fail/with-variant/skip_ser_newtype_field_if.rs
 create mode 100644 test_suite/tests/compile-fail/with-variant/skip_ser_struct_field.rs
 create mode 100644 test_suite/tests/compile-fail/with-variant/skip_ser_struct_field_if.rs
 create mode 100644 test_suite/tests/compile-fail/with-variant/skip_ser_tuple_field.rs
 create mode 100644 test_suite/tests/compile-fail/with-variant/skip_ser_tuple_field_if.rs
 create mode 100644 test_suite/tests/compile-fail/with-variant/skip_ser_whole_variant.rs

diff --git a/serde_derive/src/bound.rs b/serde_derive/src/bound.rs
index 77e7824a..0160fa85 100644
--- a/serde_derive/src/bound.rs
+++ b/serde_derive/src/bound.rs
@@ -10,7 +10,7 @@ use std::collections::HashSet;
 
 use syn::{self, visit};
 
-use internals::ast::Container;
+use internals::ast::{Body, Container};
 use internals::attr;
 
 macro_rules! path {
@@ -88,7 +88,7 @@ pub fn with_bound<F>(
     bound: &syn::Path,
 ) -> syn::Generics
 where
-    F: Fn(&attr::Field) -> bool,
+    F: Fn(&attr::Field, Option<&attr::Variant>) -> bool,
 {
     struct FindTyParams {
         // Set of all generic type parameters on the current struct (A, B, C in
@@ -124,17 +124,27 @@ where
         .map(|ty_param| ty_param.ident.clone())
         .collect();
 
-    let relevant_tys = cont.body
-        .all_fields()
-        .filter(|&field| filter(&field.attrs))
-        .map(|field| &field.ty);
-
     let mut visitor = FindTyParams {
         all_ty_params: all_ty_params,
         relevant_ty_params: HashSet::new(),
     };
-    for ty in relevant_tys {
-        visit::walk_ty(&mut visitor, ty);
+    match cont.body {
+        Body::Enum(ref variants) => {
+            for variant in variants.iter() {
+                let relevant_fields = variant
+                    .fields
+                    .iter()
+                    .filter(|field| filter(&field.attrs, Some(&variant.attrs)));
+                for field in relevant_fields {
+                    visit::walk_ty(&mut visitor, field.ty);
+                }
+            }
+        }
+        Body::Struct(_, ref fields) => {
+            for field in fields.iter().filter(|field| filter(&field.attrs, None)) {
+                visit::walk_ty(&mut visitor, field.ty);
+            }
+        }
     }
 
     let new_predicates = generics
diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs
index 5ab6d0a9..b7fa3f1b 100644
--- a/serde_derive/src/de.rs
+++ b/serde_derive/src/de.rs
@@ -157,14 +157,17 @@ fn build_generics(cont: &Container) -> syn::Generics {
 // deserialized by us so we do not generate a bound. Fields with a `bound`
 // attribute specify their own bound so we do not generate one. All other fields
 // may need a `T: Deserialize` bound where T is the type of the field.
-fn needs_deserialize_bound(attrs: &attr::Field) -> bool {
-    !attrs.skip_deserializing() && attrs.deserialize_with().is_none() && attrs.de_bound().is_none()
+fn needs_deserialize_bound(field: &attr::Field, variant: Option<&attr::Variant>) -> bool {
+    !field.skip_deserializing() &&
+    field.deserialize_with().is_none() &&
+    field.de_bound().is_none() &&
+    variant.map_or(true, |variant| variant.deserialize_with().is_none())
 }
 
 // Fields with a `default` attribute (not `default=...`), and fields with a
 // `skip_deserializing` attribute that do not also have `default=...`.
-fn requires_default(attrs: &attr::Field) -> bool {
-    attrs.default() == &attr::Default::Default
+fn requires_default(field: &attr::Field, _variant: Option<&attr::Variant>) -> bool {
+    field.default() == &attr::Default::Default
 }
 
 // The union of lifetimes borrowed by each field of the container.
diff --git a/serde_derive/src/ser.rs b/serde_derive/src/ser.rs
index 9be9bddc..c904c45e 100644
--- a/serde_derive/src/ser.rs
+++ b/serde_derive/src/ser.rs
@@ -143,12 +143,16 @@ fn build_generics(cont: &Container) -> syn::Generics {
     }
 }
 
-// Fields with a `skip_serializing` or `serialize_with` attribute are not
-// serialized by us so we do not generate a bound. Fields with a `bound`
-// attribute specify their own bound so we do not generate one. All other fields
-// may need a `T: Serialize` bound where T is the type of the field.
-fn needs_serialize_bound(attrs: &attr::Field) -> bool {
-    !attrs.skip_serializing() && attrs.serialize_with().is_none() && attrs.ser_bound().is_none()
+// Fields with a `skip_serializing` or `serialize_with` attribute, or which
+// belong to a variant with a `serialize_with` attribute, are not serialized by
+// us so we do not generate a bound. Fields with a `bound` attribute specify
+// their own bound so we do not generate one. All other fields may need a `T:
+// Serialize` bound where T is the type of the field.
+fn needs_serialize_bound(field: &attr::Field, variant: Option<&attr::Variant>) -> bool {
+    !field.skip_serializing() &&
+    field.serialize_with().is_none() &&
+    field.ser_bound().is_none() &&
+    variant.map_or(true, |variant| variant.serialize_with().is_none())
 }
 
 fn serialize_body(cont: &Container, params: &Parameters) -> Fragment {
@@ -203,7 +207,7 @@ fn serialize_newtype_struct(
 
     let mut field_expr = get_field(params, field, 0);
     if let Some(path) = field.attrs.serialize_with() {
-        field_expr = wrap_serialize_with(params, field.ty, path, field_expr);
+        field_expr = wrap_serialize_field_with(params, field.ty, path, field_expr);
     }
 
     quote_expr! {
@@ -395,6 +399,19 @@ fn serialize_externally_tagged_variant(
     let type_name = cattrs.name().serialize_name();
     let variant_name = variant.attrs.name().serialize_name();
 
+    if let Some(path) = variant.attrs.serialize_with() {
+        let ser = wrap_serialize_variant_with(params, path, &variant);
+        return quote_expr! {
+            _serde::Serializer::serialize_newtype_variant(
+                __serializer,
+                #type_name,
+                #variant_index,
+                #variant_name,
+                #ser,
+            )
+        };
+    }
+
     match variant.style {
         Style::Unit => {
             quote_expr! {
@@ -410,7 +427,7 @@ fn serialize_externally_tagged_variant(
             let field = &variant.fields[0];
             let mut field_expr = quote!(__field0);
             if let Some(path) = field.attrs.serialize_with() {
-                field_expr = wrap_serialize_with(params, field.ty, path, field_expr);
+                field_expr = wrap_serialize_field_with(params, field.ty, path, field_expr);
             }
 
             quote_expr! {
@@ -460,6 +477,20 @@ fn serialize_internally_tagged_variant(
     let enum_ident_str = params.type_name();
     let variant_ident_str = variant.ident.as_ref();
 
+    if let Some(path) = variant.attrs.serialize_with() {
+        let ser = wrap_serialize_variant_with(params, path, &variant);
+        return quote_expr! {
+            _serde::private::ser::serialize_tagged_newtype(
+                __serializer,
+                #enum_ident_str,
+                #variant_ident_str,
+                #tag,
+                #variant_name,
+                #ser,
+            )
+        };
+    }
+
     match variant.style {
         Style::Unit => {
             quote_block! {
@@ -474,7 +505,7 @@ fn serialize_internally_tagged_variant(
             let field = &variant.fields[0];
             let mut field_expr = quote!(__field0);
             if let Some(path) = field.attrs.serialize_with() {
-                field_expr = wrap_serialize_with(params, field.ty, path, field_expr);
+                field_expr = wrap_serialize_field_with(params, field.ty, path, field_expr);
             }
 
             quote_expr! {
@@ -515,44 +546,57 @@ fn serialize_adjacently_tagged_variant(
     let variant_name = variant.attrs.name().serialize_name();
 
     let inner = Stmts(
-        match variant.style {
-            Style::Unit => {
-                return quote_block! {
-                    let mut __struct = try!(_serde::Serializer::serialize_struct(
-                        __serializer, #type_name, 1));
-                    try!(_serde::ser::SerializeStruct::serialize_field(
-                        &mut __struct, #tag, #variant_name));
-                    _serde::ser::SerializeStruct::end(__struct)
-                };
+        if let Some(path) = variant.attrs.serialize_with() {
+            let ser = wrap_serialize_variant_with(params, path, &variant);
+            quote_expr! {
+                _serde::Serialize::serialize(#ser, __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_with(params, field.ty, path, field_expr);
+        } else {
+            match variant.style {
+                Style::Unit => {
+                    return quote_block! {
+                        let mut __struct = try!(_serde::Serializer::serialize_struct(
+                            __serializer, #type_name, 1));
+                        try!(_serde::ser::SerializeStruct::serialize_field(
+                            &mut __struct, #tag, #variant_name));
+                        _serde::ser::SerializeStruct::end(__struct)
+                    };
                 }
+                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);
+                    }
 
-                quote_expr! {
-                    _serde::Serialize::serialize(#field_expr, __serializer)
+                    quote_expr! {
+                        _serde::Serialize::serialize(#field_expr, __serializer)
+                    }
+                }
+                Style::Tuple => {
+                    serialize_tuple_variant(TupleVariant::Untagged, params, &variant.fields)
+                }
+                Style::Struct => {
+                    serialize_struct_variant(
+                        StructVariant::Untagged,
+                        params,
+                        &variant.fields,
+                        &variant_name,
+                    )
                 }
-            }
-            Style::Tuple => {
-                serialize_tuple_variant(TupleVariant::Untagged, params, &variant.fields)
-            }
-            Style::Struct => {
-                serialize_struct_variant(
-                    StructVariant::Untagged,
-                    params,
-                    &variant.fields,
-                    &variant_name,
-                )
             }
         },
     );
 
     let fields_ty = variant.fields.iter().map(|f| &f.ty);
     let ref fields_ident: Vec<_> = match variant.style {
-        Style::Unit => unreachable!(),
+        Style::Unit => {
+            if variant.attrs.serialize_with().is_some() {
+                vec![]
+            } else {
+                unreachable!()
+            }
+        }
         Style::Newtype => vec![Ident::new("__field0")],
         Style::Tuple => {
             (0..variant.fields.len())
@@ -576,7 +620,11 @@ fn serialize_adjacently_tagged_variant(
 
     let (_, ty_generics, where_clause) = params.generics.split_for_impl();
 
-    let wrapper_generics = bound::with_lifetime_bound(&params.generics, "'__a");
+    let wrapper_generics = if let Style::Unit = variant.style {
+        params.generics.clone()
+    } else {
+        bound::with_lifetime_bound(&params.generics, "'__a")
+    };
     let (wrapper_impl_generics, wrapper_ty_generics, _) = wrapper_generics.split_for_impl();
 
     quote_block! {
@@ -612,6 +660,13 @@ fn serialize_untagged_variant(
     variant: &Variant,
     cattrs: &attr::Container,
 ) -> Fragment {
+    if let Some(path) = variant.attrs.serialize_with() {
+        let ser = wrap_serialize_variant_with(params, path, &variant);
+        return quote_expr! {
+            _serde::Serialize::serialize(#ser, __serializer)
+        };
+    }
+
     match variant.style {
         Style::Unit => {
             quote_expr! {
@@ -622,7 +677,7 @@ fn serialize_untagged_variant(
             let field = &variant.fields[0];
             let mut field_expr = quote!(__field0);
             if let Some(path) = field.attrs.serialize_with() {
-                field_expr = wrap_serialize_with(params, field.ty, path, field_expr);
+                field_expr = wrap_serialize_field_with(params, field.ty, path, field_expr);
             }
 
             quote_expr! {
@@ -808,7 +863,7 @@ fn serialize_tuple_struct_visitor(
                     .map(|path| quote!(#path(#field_expr)));
 
                 if let Some(path) = field.attrs.serialize_with() {
-                    field_expr = wrap_serialize_with(params, field.ty, path, field_expr);
+                    field_expr = wrap_serialize_field_with(params, field.ty, path, field_expr);
                 }
 
                 let ser = quote! {
@@ -850,7 +905,7 @@ fn serialize_struct_visitor(
                     .map(|path| quote!(#path(#field_expr)));
 
                 if let Some(path) = field.attrs.serialize_with() {
-                    field_expr = wrap_serialize_with(params, field.ty, path, field_expr)
+                    field_expr = wrap_serialize_field_with(params, field.ty, path, field_expr);
                 }
 
                 let ser = quote! {
@@ -866,21 +921,56 @@ fn serialize_struct_visitor(
         .collect()
 }
 
-fn wrap_serialize_with(
+fn wrap_serialize_field_with(
     params: &Parameters,
     field_ty: &syn::Ty,
     serialize_with: &syn::Path,
-    value: Tokens,
+    field_expr: Tokens,
+) -> Tokens {
+    wrap_serialize_with(params,
+                        serialize_with,
+                        &[field_ty],
+                        &[quote!(#field_expr)])
+}
+
+fn wrap_serialize_variant_with(
+    params: &Parameters,
+    serialize_with: &syn::Path,
+    variant: &Variant,
+) -> Tokens {
+    let field_tys: Vec<_> = variant.fields.iter().map(|field| field.ty).collect();
+    let field_exprs: Vec<_> = variant.fields.iter()
+        .enumerate()
+        .map(|(i, field)| {
+            let id = field.ident.as_ref().map_or_else(|| Ident::new(format!("__field{}", i)),
+                                                      |id| id.clone());
+            quote!(#id)
+        })
+        .collect();
+    wrap_serialize_with(params, serialize_with, field_tys.as_slice(), field_exprs.as_slice())
+}
+
+fn wrap_serialize_with(
+    params: &Parameters,
+    serialize_with: &syn::Path,
+    field_tys: &[&syn::Ty],
+    field_exprs: &[Tokens],
 ) -> Tokens {
     let this = &params.this;
     let (_, ty_generics, where_clause) = params.generics.split_for_impl();
 
-    let wrapper_generics = bound::with_lifetime_bound(&params.generics, "'__a");
+    let wrapper_generics = if field_exprs.len() == 0 {
+        params.generics.clone()
+    } else {
+        bound::with_lifetime_bound(&params.generics, "'__a")
+    };
     let (wrapper_impl_generics, wrapper_ty_generics, _) = wrapper_generics.split_for_impl();
 
+    let field_access = (0..field_exprs.len()).map(|n| Ident::new(format!("{}", n)));
+
     quote!({
         struct __SerializeWith #wrapper_impl_generics #where_clause {
-            value: &'__a #field_ty,
+            values: (#(&'__a #field_tys, )*),
             phantom: _serde::export::PhantomData<#this #ty_generics>,
         }
 
@@ -888,12 +978,12 @@ fn wrap_serialize_with(
             fn serialize<__S>(&self, __s: __S) -> _serde::export::Result<__S::Ok, __S::Error>
                 where __S: _serde::Serializer
             {
-                #serialize_with(self.value, __s)
+                #serialize_with(#(self.values.#field_access, )* __s)
             }
         }
 
         &__SerializeWith {
-            value: #value,
+            values: (#(#field_exprs, )*),
             phantom: _serde::export::PhantomData::<#this #ty_generics>,
         }
     })
diff --git a/serde_derive_internals/src/attr.rs b/serde_derive_internals/src/attr.rs
index caf306cf..2803da9d 100644
--- a/serde_derive_internals/src/attr.rs
+++ b/serde_derive_internals/src/attr.rs
@@ -510,6 +510,8 @@ pub struct Variant {
     skip_deserializing: bool,
     skip_serializing: bool,
     other: bool,
+    serialize_with: Option<syn::Path>,
+    deserialize_with: Option<syn::Path>,
 }
 
 impl Variant {
@@ -520,6 +522,8 @@ impl Variant {
         let mut skip_serializing = BoolAttr::none(cx, "skip_serializing");
         let mut rename_all = Attr::none(cx, "rename_all");
         let mut other = BoolAttr::none(cx, "other");
+        let mut serialize_with = Attr::none(cx, "serialize_with");
+        let mut deserialize_with = Attr::none(cx, "deserialize_with");
 
         for meta_items in variant.attrs.iter().filter_map(get_serde_meta_items) {
             for meta_item in meta_items {
@@ -569,6 +573,20 @@ impl Variant {
                         other.set_true();
                     }
 
+                    // Parse `#[serde(serialize_with = "...")]`
+                    MetaItem(NameValue(ref name, ref lit)) if name == "serialize_with" => {
+                        if let Ok(path) = parse_lit_into_path(cx, name.as_ref(), lit) {
+                            serialize_with.set(path);
+                        }
+                    }
+
+                    // Parse `#[serde(deserialize_with = "...")]`
+                    MetaItem(NameValue(ref name, ref lit)) if name == "deserialize_with" => {
+                        if let Ok(path) = parse_lit_into_path(cx, name.as_ref(), lit) {
+                            deserialize_with.set(path);
+                        }
+                    }
+
                     MetaItem(ref meta_item) => {
                         cx.error(format!("unknown serde variant attribute `{}`", meta_item.name()));
                     }
@@ -595,6 +613,8 @@ impl Variant {
             skip_deserializing: skip_deserializing.get(),
             skip_serializing: skip_serializing.get(),
             other: other.get(),
+            serialize_with: serialize_with.get(),
+            deserialize_with: deserialize_with.get(),
         }
     }
 
@@ -626,6 +646,14 @@ impl Variant {
     pub fn other(&self) -> bool {
         self.other
     }
+
+    pub fn serialize_with(&self) -> Option<&syn::Path> {
+        self.serialize_with.as_ref()
+    }
+
+    pub fn deserialize_with(&self) -> Option<&syn::Path> {
+        self.deserialize_with.as_ref()
+    }
 }
 
 /// Represents field attribute information
diff --git a/serde_derive_internals/src/check.rs b/serde_derive_internals/src/check.rs
index 84c958ea..6fe31615 100644
--- a/serde_derive_internals/src/check.rs
+++ b/serde_derive_internals/src/check.rs
@@ -15,6 +15,7 @@ use Ctxt;
 pub fn check(cx: &Ctxt, cont: &Container) {
     check_getter(cx, cont);
     check_identifier(cx, cont);
+    check_variant_skip_attrs(cx, cont);
 }
 
 /// Getters are only allowed inside structs (not enums) with the `remote`
@@ -94,3 +95,40 @@ fn check_identifier(cx: &Ctxt, cont: &Container) {
         }
     }
 }
+
+/// Skip-(de)serializing attributes are not allowed on variants marked
+/// (de)serialize_with.
+fn check_variant_skip_attrs(cx: &Ctxt, cont: &Container) {
+    let variants = match cont.body {
+        Body::Enum(ref variants) => variants,
+        Body::Struct(_, _) => {
+            return;
+        }
+    };
+
+    for variant in variants.iter() {
+        if variant.attrs.serialize_with().is_some() {
+            if variant.attrs.skip_serializing() {
+                cx.error(format!("variant `{}` cannot have both #[serde(serialize_with)] and \
+                                  #[serde(skip_serializing)]", variant.ident));
+            }
+
+            for (i, field) in variant.fields.iter().enumerate() {
+                let ident = field.ident.as_ref().map_or_else(|| format!("{}", i),
+                                                             |ident| format!("`{}`", ident));
+
+                if field.attrs.skip_serializing() {
+                    cx.error(format!("variant `{}` cannot have both #[serde(serialize_with)] and \
+                                      a field {} marked with #[serde(skip_serializing)]",
+                                     variant.ident, ident));
+                }
+
+                if field.attrs.skip_serializing_if().is_some() {
+                    cx.error(format!("variant `{}` cannot have both #[serde(serialize_with)] and \
+                                      a field {} marked with #[serde(skip_serializing_if)]",
+                                     variant.ident, ident));
+                }
+            }
+        }
+    }
+}
diff --git a/test_suite/tests/compile-fail/with-variant/skip_ser_newtype_field.rs b/test_suite/tests/compile-fail/with-variant/skip_ser_newtype_field.rs
new file mode 100644
index 00000000..162812d4
--- /dev/null
+++ b/test_suite/tests/compile-fail/with-variant/skip_ser_newtype_field.rs
@@ -0,0 +1,25 @@
+// Copyright 2017 Serde Developers
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#[macro_use]
+extern crate serde_derive;
+
+#[derive(Serialize)] //~ ERROR: proc-macro derive panicked
+//~^ HELP: variant `Newtype` cannot have both #[serde(serialize_with)] and a field 0 marked with #[serde(skip_serializing)]
+enum Enum {
+    #[serde(serialize_with = "serialize_some_newtype_variant")]
+    Newtype(#[serde(skip_serializing)] String),
+}
+
+fn serialize_some_newtype_variant<S>(_: &String) -> StdResult<S::Ok, S::Error>
+    where S: Serializer,
+{
+    unimplemented!()
+}
+
+fn main() { }
diff --git a/test_suite/tests/compile-fail/with-variant/skip_ser_newtype_field_if.rs b/test_suite/tests/compile-fail/with-variant/skip_ser_newtype_field_if.rs
new file mode 100644
index 00000000..065ca20d
--- /dev/null
+++ b/test_suite/tests/compile-fail/with-variant/skip_ser_newtype_field_if.rs
@@ -0,0 +1,27 @@
+// Copyright 2017 Serde Developers
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#[macro_use]
+extern crate serde_derive;
+
+#[derive(Serialize)] //~ ERROR: proc-macro derive panicked
+//~^ HELP: variant `Newtype` cannot have both #[serde(serialize_with)] and a field 0 marked with #[serde(skip_serializing_if)]
+enum Enum {
+    #[serde(serialize_with = "serialize_some_newtype_variant")]
+    Newtype(#[serde(skip_serializing_if = "always")] String),
+}
+
+fn serialize_some_newtype_variant<S>(_: &String) -> StdResult<S::Ok, S::Error>
+    where S: Serializer,
+{
+    unimplemented!()
+}
+
+fn always<T>(_: &T) -> bool { true }
+
+fn main() { }
diff --git a/test_suite/tests/compile-fail/with-variant/skip_ser_struct_field.rs b/test_suite/tests/compile-fail/with-variant/skip_ser_struct_field.rs
new file mode 100644
index 00000000..dc046b86
--- /dev/null
+++ b/test_suite/tests/compile-fail/with-variant/skip_ser_struct_field.rs
@@ -0,0 +1,29 @@
+// Copyright 2017 Serde Developers
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#[macro_use]
+extern crate serde_derive;
+
+#[derive(Serialize)] //~ ERROR: proc-macro derive panicked
+//~^ HELP: variant `Struct` cannot have both #[serde(serialize_with)] and a field `f1` marked with #[serde(skip_serializing)]
+enum Enum {
+    #[serde(serialize_with = "serialize_some_other_variant")]
+    Struct {
+        #[serde(skip_serializing)]
+        f1: String,
+        f2: u8,
+    },
+}
+
+fn serialize_some_other_variant<S>(_: &String, _: Option<&u8>, _: S) -> StdResult<S::Ok, S::Error>
+    where S: Serializer,
+{
+    unimplemented!()
+}
+
+fn main() { }
diff --git a/test_suite/tests/compile-fail/with-variant/skip_ser_struct_field_if.rs b/test_suite/tests/compile-fail/with-variant/skip_ser_struct_field_if.rs
new file mode 100644
index 00000000..fd42947d
--- /dev/null
+++ b/test_suite/tests/compile-fail/with-variant/skip_ser_struct_field_if.rs
@@ -0,0 +1,31 @@
+// Copyright 2017 Serde Developers
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#[macro_use]
+extern crate serde_derive;
+
+#[derive(Serialize)] //~ ERROR: proc-macro derive panicked
+//~^ HELP: variant `Struct` cannot have both #[serde(serialize_with)] and a field `f1` marked with #[serde(skip_serializing_if)]
+enum Enum {
+    #[serde(serialize_with = "serialize_some_newtype_variant")]
+    Struct {
+        #[serde(skip_serializing_if = "always")]
+        f1: String,
+        f2: u8,
+    },
+}
+
+fn serialize_some_other_variant<S>(_: &String, _: Option<&u8>, _: S) -> StdResult<S::Ok, S::Error>
+    where S: Serializer,
+{
+    unimplemented!()
+}
+
+fn always<T>(_: &T) -> bool { true }
+
+fn main() { }
diff --git a/test_suite/tests/compile-fail/with-variant/skip_ser_tuple_field.rs b/test_suite/tests/compile-fail/with-variant/skip_ser_tuple_field.rs
new file mode 100644
index 00000000..56cbfa45
--- /dev/null
+++ b/test_suite/tests/compile-fail/with-variant/skip_ser_tuple_field.rs
@@ -0,0 +1,25 @@
+// Copyright 2017 Serde Developers
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#[macro_use]
+extern crate serde_derive;
+
+#[derive(Serialize)] //~ ERROR: proc-macro derive panicked
+//~^ HELP: variant `Tuple` cannot have both #[serde(serialize_with)] and a field 0 marked with #[serde(skip_serializing)]
+enum Enum {
+    #[serde(serialize_with = "serialize_some_other_variant")]
+    Tuple(#[serde(skip_serializing)] String, u8),
+}
+
+fn serialize_some_other_variant<S>(_: &String, _: Option<&u8>, _: S) -> StdResult<S::Ok, S::Error>
+    where S: Serializer,
+{
+    unimplemented!()
+}
+
+fn main() { }
diff --git a/test_suite/tests/compile-fail/with-variant/skip_ser_tuple_field_if.rs b/test_suite/tests/compile-fail/with-variant/skip_ser_tuple_field_if.rs
new file mode 100644
index 00000000..c85db63d
--- /dev/null
+++ b/test_suite/tests/compile-fail/with-variant/skip_ser_tuple_field_if.rs
@@ -0,0 +1,27 @@
+// Copyright 2017 Serde Developers
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#[macro_use]
+extern crate serde_derive;
+
+#[derive(Serialize)] //~ ERROR: proc-macro derive panicked
+//~^ HELP: variant `Tuple` cannot have both #[serde(serialize_with)] and a field 0 marked with #[serde(skip_serializing_if)]
+enum Enum {
+    #[serde(serialize_with = "serialize_some_other_variant")]
+    Tuple(#[serde(skip_serializing_if = "always")] String, u8),
+}
+
+fn serialize_some_other_variant<S>(_: &String, _: Option<&u8>, _: S) -> StdResult<S::Ok, S::Error>
+    where S: Serializer,
+{
+    unimplemented!()
+}
+
+fn always<T>(_: &T) -> bool { true }
+
+fn main() { }
diff --git a/test_suite/tests/compile-fail/with-variant/skip_ser_whole_variant.rs b/test_suite/tests/compile-fail/with-variant/skip_ser_whole_variant.rs
new file mode 100644
index 00000000..8e3291e3
--- /dev/null
+++ b/test_suite/tests/compile-fail/with-variant/skip_ser_whole_variant.rs
@@ -0,0 +1,26 @@
+// Copyright 2017 Serde Developers
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#[macro_use]
+extern crate serde_derive;
+
+#[derive(Serialize)] //~ ERROR: proc-macro derive panicked
+//~^ HELP: variant `Unit` cannot have both #[serde(serialize_with)] and #[serde(skip_serializing)]
+enum Enum {
+    #[serde(serialize_with = "serialize_some_unit_variant")]
+    #[serde(skip_serializing)]
+    Unit,
+}
+
+fn serialize_some_unit_variant<S>(_: S) -> StdResult<S::Ok, S::Error>
+    where S: Serializer,
+{
+    unimplemented!()
+}
+
+fn main() { }
diff --git a/test_suite/tests/test_annotations.rs b/test_suite/tests/test_annotations.rs
index cb9642e8..3a31b775 100644
--- a/test_suite/tests/test_annotations.rs
+++ b/test_suite/tests/test_annotations.rs
@@ -811,6 +811,74 @@ fn test_serialize_with_enum() {
     );
 }
 
+#[derive(Debug, PartialEq, Serialize)]
+enum SerializeWithVariant {
+    #[serde(serialize_with="serialize_unit_variant_as_i8")]
+    Unit,
+
+    #[serde(serialize_with="SerializeWith::serialize_with")]
+    Newtype(i32),
+
+    #[serde(serialize_with="serialize_some_other_variant")]
+    Tuple(String, u8),
+
+    #[serde(serialize_with="serialize_some_other_variant")]
+    Struct {
+        f1: String,
+        f2: u8,
+    },
+}
+
+fn serialize_unit_variant_as_i8<S>(serializer: S) -> Result<S::Ok, S::Error>
+    where S: Serializer,
+{
+    serializer.serialize_i8(0)
+}
+
+fn serialize_some_other_variant<S>(f1: &String,
+                                   f2: &u8,
+                                   serializer: S)
+                                   -> Result<S::Ok, S::Error>
+    where S: Serializer,
+{
+    serializer.serialize_str(format!("{};{:?}", f1, f2).as_str())
+}
+
+#[test]
+fn test_serialize_with_variant() {
+    assert_ser_tokens(
+        &SerializeWithVariant::Unit,
+        &[
+            Token::NewtypeVariant { name: "SerializeWithVariant", variant: "Unit" },
+            Token::I8(0),
+        ],
+    );
+
+    assert_ser_tokens(
+        &SerializeWithVariant::Newtype(123),
+        &[
+            Token::NewtypeVariant { name: "SerializeWithVariant", variant: "Newtype" },
+            Token::Bool(true),
+        ],
+    );
+
+    assert_ser_tokens(
+        &SerializeWithVariant::Tuple("hello".into(), 0),
+        &[
+            Token::NewtypeVariant { name: "SerializeWithVariant", variant: "Tuple" },
+            Token::Str("hello;0"),
+        ],
+    );
+
+    assert_ser_tokens(
+        &SerializeWithVariant::Struct { f1: "world".into(), f2: 1 },
+        &[
+            Token::NewtypeVariant { name: "SerializeWithVariant", variant: "Struct" },
+            Token::Str("world;1"),
+        ],
+    );
+}
+
 #[derive(Debug, PartialEq, Deserialize)]
 struct DeserializeWithStruct<B>
 where
diff --git a/test_suite/tests/test_gen.rs b/test_suite/tests/test_gen.rs
index 5d708014..2c85a116 100644
--- a/test_suite/tests/test_gen.rs
+++ b/test_suite/tests/test_gen.rs
@@ -373,6 +373,124 @@ fn test_gen() {
         #[serde(with = "vis::SDef")]
         s: vis::S,
     }
+
+    #[derive(Serialize)]
+    enum ExternallyTaggedVariantWith {
+        #[allow(dead_code)]
+        Normal { f1: String },
+
+        #[serde(serialize_with = "ser_x")]
+        #[serde(deserialize_with = "de_x")]
+        #[allow(dead_code)]
+        Newtype(X),
+
+        #[serde(serialize_with = "serialize_some_other_variant")]
+        #[serde(deserialize_with = "deserialize_some_other_variant")]
+        #[allow(dead_code)]
+        Tuple(String, u8),
+
+        #[serde(serialize_with = "serialize_some_other_variant")]
+        #[serde(deserialize_with = "deserialize_some_other_variant")]
+        #[allow(dead_code)]
+        Struct {
+            f1: String,
+            f2: u8,
+        },
+
+        #[serde(serialize_with = "serialize_some_unit_variant")]
+        #[serde(deserialize_with = "deserialize_some_unit_variant")]
+        #[allow(dead_code)]
+        Unit,
+    }
+    assert_ser::<ExternallyTaggedVariantWith>();
+
+    #[derive(Serialize)]
+    #[serde(tag = "t")]
+    enum InternallyTaggedVariantWith {
+        #[allow(dead_code)]
+        Normal { f1: String },
+
+        #[serde(serialize_with = "ser_x")]
+        #[serde(deserialize_with = "de_x")]
+        #[allow(dead_code)]
+        Newtype(X),
+
+        #[serde(serialize_with = "serialize_some_other_variant")]
+        #[serde(deserialize_with = "deserialize_some_other_variant")]
+        #[allow(dead_code)]
+        Struct {
+            f1: String,
+            f2: u8,
+        },
+
+        #[serde(serialize_with = "serialize_some_unit_variant")]
+        #[serde(deserialize_with = "deserialize_some_unit_variant")]
+        #[allow(dead_code)]
+        Unit,
+    }
+    assert_ser::<InternallyTaggedVariantWith>();
+
+    #[derive(Serialize)]
+    #[serde(tag = "t", content = "c")]
+    enum AdjacentlyTaggedVariantWith {
+        #[allow(dead_code)]
+        Normal { f1: String },
+
+        #[serde(serialize_with = "ser_x")]
+        #[serde(deserialize_with = "de_x")]
+        #[allow(dead_code)]
+        Newtype(X),
+
+        #[serde(serialize_with = "serialize_some_other_variant")]
+        #[serde(deserialize_with = "deserialize_some_other_variant")]
+        #[allow(dead_code)]
+        Tuple(String, u8),
+
+        #[serde(serialize_with = "serialize_some_other_variant")]
+        #[serde(deserialize_with = "deserialize_some_other_variant")]
+        #[allow(dead_code)]
+        Struct {
+            f1: String,
+            f2: u8,
+        },
+
+        #[serde(serialize_with = "serialize_some_unit_variant")]
+        #[serde(deserialize_with = "deserialize_some_unit_variant")]
+        #[allow(dead_code)]
+        Unit,
+    }
+    assert_ser::<AdjacentlyTaggedVariantWith>();
+
+    #[derive(Serialize)]
+    #[serde(untagged)]
+    enum UntaggedVariantWith {
+        #[allow(dead_code)]
+        Normal { f1: String },
+
+        #[serde(serialize_with = "ser_x")]
+        #[serde(deserialize_with = "de_x")]
+        #[allow(dead_code)]
+        Newtype(X),
+
+        #[serde(serialize_with = "serialize_some_other_variant")]
+        #[serde(deserialize_with = "deserialize_some_other_variant")]
+        #[allow(dead_code)]
+        Tuple(String, u8),
+
+        #[serde(serialize_with = "serialize_some_other_variant")]
+        #[serde(deserialize_with = "deserialize_some_other_variant")]
+        #[allow(dead_code)]
+        Struct {
+            f1: String,
+            f2: u8,
+        },
+
+        #[serde(serialize_with = "serialize_some_unit_variant")]
+        #[serde(deserialize_with = "deserialize_some_unit_variant")]
+        #[allow(dead_code)]
+        Unit,
+    }
+    assert_ser::<UntaggedVariantWith>();
 }
 
 //////////////////////////////////////////////////////////////////////////
@@ -414,3 +532,29 @@ impl DeserializeWith for X {
         unimplemented!()
     }
 }
+
+pub fn serialize_some_unit_variant<S>(_: S) -> StdResult<S::Ok, S::Error>
+    where S: Serializer,
+{
+    unimplemented!()
+}
+
+pub fn deserialize_some_unit_variant<'de, D>(_: D) -> StdResult<(), D::Error>
+    where D: Deserializer<'de>,
+{
+    unimplemented!()
+}
+
+pub fn serialize_some_other_variant<S>(_: &String, _: &u8, _: S) -> StdResult<S::Ok, S::Error>
+    where S: Serializer,
+{
+    unimplemented!()
+}
+
+pub fn deserialize_some_other_variant<'de, D>(_: D) -> StdResult<(String, u8), D::Error>
+    where D: Deserializer<'de>,
+{
+    unimplemented!()
+}
+
+pub fn is_zero(n: &u8) -> bool { *n == 0 }

From 9fc180e62f4c8b424048ad2336b3a858b30d280b Mon Sep 17 00:00:00 2001
From: Michael Smith <michael@spinda.net>
Date: Mon, 14 Aug 2017 14:39:29 -0700
Subject: [PATCH 2/3] Implement deserialize_with for variants

Complements variant serialize_with and closes #1013.
---
 serde_derive/src/de.rs                        | 100 +++++++++++++++--
 serde_derive_internals/src/check.rs           |  18 +++
 .../with-variant/skip_de_newtype_field.rs     |  25 +++++
 .../with-variant/skip_de_struct_field.rs      |  29 +++++
 .../with-variant/skip_de_tuple_field.rs       |  25 +++++
 .../with-variant/skip_de_whole_variant.rs     |  26 +++++
 test_suite/tests/test_annotations.rs          | 104 +++++++++++++++---
 test_suite/tests/test_gen.rs                  |   8 +-
 8 files changed, 308 insertions(+), 27 deletions(-)
 create mode 100644 test_suite/tests/compile-fail/with-variant/skip_de_newtype_field.rs
 create mode 100644 test_suite/tests/compile-fail/with-variant/skip_de_struct_field.rs
 create mode 100644 test_suite/tests/compile-fail/with-variant/skip_de_tuple_field.rs
 create mode 100644 test_suite/tests/compile-fail/with-variant/skip_de_whole_variant.rs

diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs
index b7fa3f1b..b3589f55 100644
--- a/serde_derive/src/de.rs
+++ b/serde_derive/src/de.rs
@@ -375,7 +375,7 @@ fn deserialize_seq(
                         quote!(try!(_serde::de::SeqAccess::next_element::<#field_ty>(&mut __seq)))
                     }
                     Some(path) => {
-                        let (wrapper, wrapper_ty) = wrap_deserialize_with(
+                        let (wrapper, wrapper_ty) = wrap_deserialize_field_with(
                             params, field.ty, path);
                         quote!({
                             #wrapper
@@ -431,7 +431,7 @@ fn deserialize_newtype_struct(type_path: &Tokens, params: &Parameters, field: &F
             }
         }
         Some(path) => {
-            let (wrapper, wrapper_ty) = wrap_deserialize_with(params, field.ty, path);
+            let (wrapper, wrapper_ty) = wrap_deserialize_field_with(params, field.ty, path);
             quote!({
                 #wrapper
                 try!(<#wrapper_ty as _serde::Deserialize>::deserialize(__e)).value
@@ -1087,6 +1087,16 @@ fn deserialize_externally_tagged_variant(
     variant: &Variant,
     cattrs: &attr::Container,
 ) -> Fragment {
+    if let Some(path) = variant.attrs.deserialize_with() {
+        let (wrapper, wrapper_ty, unwrap_fn) =
+            wrap_deserialize_variant_with(params, &variant, path);
+        return quote_block! {
+            #wrapper
+            _serde::export::Result::map(
+                _serde::de::VariantAccess::newtype_variant::<#wrapper_ty>(__variant), #unwrap_fn)
+        };
+    }
+
     let variant_ident = &variant.ident;
 
     match variant.style {
@@ -1115,6 +1125,10 @@ fn deserialize_internally_tagged_variant(
     cattrs: &attr::Container,
     deserializer: Tokens,
 ) -> Fragment {
+    if variant.attrs.deserialize_with().is_some() {
+        return deserialize_untagged_variant(params, variant, cattrs, deserializer);
+    }
+
     let variant_ident = &variant.ident;
 
     match variant.style {
@@ -1140,6 +1154,16 @@ fn deserialize_untagged_variant(
     cattrs: &attr::Container,
     deserializer: Tokens,
 ) -> Fragment {
+    if let Some(path) = variant.attrs.deserialize_with() {
+        let (wrapper, wrapper_ty, unwrap_fn) =
+            wrap_deserialize_variant_with(params, &variant, path);
+        return quote_block! {
+            #wrapper
+            _serde::export::Result::map(
+                <#wrapper_ty as _serde::Deserialize>::deserialize(#deserializer), #unwrap_fn)
+        };
+    }
+
     let variant_ident = &variant.ident;
 
     match variant.style {
@@ -1201,7 +1225,7 @@ fn deserialize_externally_tagged_newtype_variant(
             }
         }
         Some(path) => {
-            let (wrapper, wrapper_ty) = wrap_deserialize_with(params, field.ty, path);
+            let (wrapper, wrapper_ty) = wrap_deserialize_field_with(params, field.ty, path);
             quote_block! {
                 #wrapper
                 _serde::export::Result::map(
@@ -1229,7 +1253,7 @@ fn deserialize_untagged_newtype_variant(
             }
         }
         Some(path) => {
-            let (wrapper, wrapper_ty) = wrap_deserialize_with(params, field.ty, path);
+            let (wrapper, wrapper_ty) = wrap_deserialize_field_with(params, field.ty, path);
             quote_block! {
                 #wrapper
                 _serde::export::Result::map(
@@ -1531,7 +1555,7 @@ fn deserialize_map(
                     }
                 }
                 Some(path) => {
-                    let (wrapper, wrapper_ty) = wrap_deserialize_with(
+                    let (wrapper, wrapper_ty) = wrap_deserialize_field_with(
                         params, field.ty, path);
                     quote!({
                         #wrapper
@@ -1664,7 +1688,7 @@ fn field_i(i: usize) -> Ident {
 /// in a trait to prevent it from accessing the internal `Deserialize` state.
 fn wrap_deserialize_with(
     params: &Parameters,
-    field_ty: &syn::Ty,
+    value_ty: Tokens,
     deserialize_with: &syn::Path,
 ) -> (Tokens, Tokens) {
     let this = &params.this;
@@ -1672,7 +1696,7 @@ fn wrap_deserialize_with(
 
     let wrapper = quote! {
         struct __DeserializeWith #de_impl_generics #where_clause {
-            value: #field_ty,
+            value: #value_ty,
             phantom: _serde::export::PhantomData<#this #ty_generics>,
             lifetime: _serde::export::PhantomData<&'de ()>,
         }
@@ -1695,6 +1719,68 @@ fn wrap_deserialize_with(
     (wrapper, wrapper_ty)
 }
 
+fn wrap_deserialize_field_with(
+    params: &Parameters,
+    field_ty: &syn::Ty,
+    deserialize_with: &syn::Path,
+) -> (Tokens, Tokens) {
+    wrap_deserialize_with(params, quote!(#field_ty), deserialize_with)
+}
+
+fn wrap_deserialize_variant_with(
+    params: &Parameters,
+    variant: &Variant,
+    deserialize_with: &syn::Path,
+) -> (Tokens, Tokens, Tokens) {
+    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 field_access = (0..variant.fields.len()).map(|n| Ident::new(format!("{}", n)));
+    let unwrap_fn = match variant.style {
+        Style::Struct => {
+            let field_idents = variant.fields.iter().map(|field| field.ident.as_ref().unwrap());
+            quote! {
+                {
+                    |__wrap| {
+                        #this::#variant_ident { #(#field_idents: __wrap.value.#field_access),* }
+                    }
+                }
+            }
+        }
+        Style::Tuple => {
+            quote! {
+                {
+                    |__wrap| {
+                        #this::#variant_ident(#(__wrap.value.#field_access),*)
+                    }
+                }
+            }
+        }
+        Style::Newtype => {
+            quote! {
+                {
+                    |__wrap| {
+                        #this::#variant_ident(__wrap.value)
+                    }
+                }
+            }
+        }
+        Style::Unit => {
+            quote! {
+                {
+                    |__wrap| { #this::#variant_ident }
+                }
+            }
+        }
+    };
+
+    (wrapper, wrapper_ty, unwrap_fn)
+}
+
 fn expr_is_missing(field: &Field, cattrs: &attr::Container) -> Fragment {
     match *field.attrs.default() {
         attr::Default::Default => {
diff --git a/serde_derive_internals/src/check.rs b/serde_derive_internals/src/check.rs
index 6fe31615..5d6a76ff 100644
--- a/serde_derive_internals/src/check.rs
+++ b/serde_derive_internals/src/check.rs
@@ -130,5 +130,23 @@ fn check_variant_skip_attrs(cx: &Ctxt, cont: &Container) {
                 }
             }
         }
+
+        if variant.attrs.deserialize_with().is_some() {
+            if variant.attrs.skip_deserializing() {
+                cx.error(format!("variant `{}` cannot have both #[serde(deserialize_with)] and \
+                                  #[serde(skip_deserializing)]", variant.ident));
+            }
+
+            for (i, field) in variant.fields.iter().enumerate() {
+                if field.attrs.skip_deserializing() {
+                    let ident = field.ident.as_ref().map_or_else(|| format!("{}", i),
+                                                                 |ident| format!("`{}`", ident));
+
+                    cx.error(format!("variant `{}` cannot have both #[serde(deserialize_with)] \
+                                      and a field {} marked with #[serde(skip_deserializing)]",
+                                     variant.ident, ident));
+                }
+            }
+        }
     }
 }
diff --git a/test_suite/tests/compile-fail/with-variant/skip_de_newtype_field.rs b/test_suite/tests/compile-fail/with-variant/skip_de_newtype_field.rs
new file mode 100644
index 00000000..391af1ba
--- /dev/null
+++ b/test_suite/tests/compile-fail/with-variant/skip_de_newtype_field.rs
@@ -0,0 +1,25 @@
+// Copyright 2017 Serde Developers
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#[macro_use]
+extern crate serde_derive;
+
+#[derive(Deserialize)] //~ ERROR: proc-macro derive panicked
+//~^ HELP: variant `Newtype` cannot have both #[serde(deserialize_with)] and a field 0 marked with #[serde(skip_deserializing)]
+enum Enum {
+    #[serde(deserialize_with = "deserialize_some_newtype_variant")]
+    Newtype(#[serde(skip_deserializing)] String),
+}
+
+fn deserialize_some_newtype_variant<'de, D>(_: D) -> StdResult<String, D::Error>
+    where D: Deserializer<'de>,
+{
+    unimplemented!()
+}
+
+fn main() { }
diff --git a/test_suite/tests/compile-fail/with-variant/skip_de_struct_field.rs b/test_suite/tests/compile-fail/with-variant/skip_de_struct_field.rs
new file mode 100644
index 00000000..7c563a8a
--- /dev/null
+++ b/test_suite/tests/compile-fail/with-variant/skip_de_struct_field.rs
@@ -0,0 +1,29 @@
+// Copyright 2017 Serde Developers
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#[macro_use]
+extern crate serde_derive;
+
+#[derive(Deserialize)] //~ ERROR: proc-macro derive panicked
+//~^ HELP: variant `Struct` cannot have both #[serde(deserialize_with)] and a field `f1` marked with #[serde(skip_deserializing)]
+enum Enum {
+    #[serde(deserialize_with = "deserialize_some_other_variant")]
+    Struct {
+        #[serde(skip_deserializing)]
+        f1: String,
+        f2: u8,
+    },
+}
+
+fn deserialize_some_other_variant<'de, D>(_: D) -> StdResult<(String, u8), D::Error>
+    where D: Deserializer<'de>,
+{
+    unimplemented!()
+}
+
+fn main() { }
diff --git a/test_suite/tests/compile-fail/with-variant/skip_de_tuple_field.rs b/test_suite/tests/compile-fail/with-variant/skip_de_tuple_field.rs
new file mode 100644
index 00000000..44fd0958
--- /dev/null
+++ b/test_suite/tests/compile-fail/with-variant/skip_de_tuple_field.rs
@@ -0,0 +1,25 @@
+// Copyright 2017 Serde Developers
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#[macro_use]
+extern crate serde_derive;
+
+#[derive(Deserialize)] //~ ERROR: proc-macro derive panicked
+//~^ HELP: variant `Tuple` cannot have both #[serde(deserialize_with)] and a field 0 marked with #[serde(skip_deserializing)]
+enum Enum {
+    #[serde(deserialize_with = "deserialize_some_other_variant")]
+    Tuple(#[serde(skip_deserializing)] String, u8),
+}
+
+fn deserialize_some_other_variant<'de, D>(_: D) -> StdResult<(String, u8), D::Error>
+    where D: Deserializer<'de>,
+{
+    unimplemented!()
+}
+
+fn main() { }
diff --git a/test_suite/tests/compile-fail/with-variant/skip_de_whole_variant.rs b/test_suite/tests/compile-fail/with-variant/skip_de_whole_variant.rs
new file mode 100644
index 00000000..68c015a2
--- /dev/null
+++ b/test_suite/tests/compile-fail/with-variant/skip_de_whole_variant.rs
@@ -0,0 +1,26 @@
+// Copyright 2017 Serde Developers
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#[macro_use]
+extern crate serde_derive;
+
+#[derive(Deserialize)] //~ ERROR: proc-macro derive panicked
+//~^ HELP: variant `Unit` cannot have both #[serde(deserialize_with)] and #[serde(skip_deserializing)]
+enum Enum {
+    #[serde(deserialize_with = "deserialize_some_unit_variant")]
+    #[serde(skip_deserializing)]
+    Unit,
+}
+
+fn deserialize_some_unit_variant<'de, D>(_: D) -> StdResult<(), D::Error>
+    where D: Deserializer<'de>,
+{
+    unimplemented!()
+}
+
+fn main() { }
diff --git a/test_suite/tests/test_annotations.rs b/test_suite/tests/test_annotations.rs
index 3a31b775..e0f6c8da 100644
--- a/test_suite/tests/test_annotations.rs
+++ b/test_suite/tests/test_annotations.rs
@@ -11,6 +11,7 @@ extern crate serde_derive;
 
 extern crate serde;
 use self::serde::{Serialize, Serializer, Deserialize, Deserializer};
+use self::serde::de::{self, Unexpected};
 
 extern crate serde_test;
 use self::serde_test::{Token, assert_tokens, assert_ser_tokens, assert_de_tokens,
@@ -811,18 +812,22 @@ fn test_serialize_with_enum() {
     );
 }
 
-#[derive(Debug, PartialEq, Serialize)]
-enum SerializeWithVariant {
+#[derive(Debug, PartialEq, Serialize, Deserialize)]
+enum WithVariant {
     #[serde(serialize_with="serialize_unit_variant_as_i8")]
+    #[serde(deserialize_with="deserialize_i8_as_unit_variant")]
     Unit,
 
     #[serde(serialize_with="SerializeWith::serialize_with")]
+    #[serde(deserialize_with="DeserializeWith::deserialize_with")]
     Newtype(i32),
 
-    #[serde(serialize_with="serialize_some_other_variant")]
+    #[serde(serialize_with="serialize_variant_as_string")]
+    #[serde(deserialize_with="deserialize_string_as_variant")]
     Tuple(String, u8),
 
-    #[serde(serialize_with="serialize_some_other_variant")]
+    #[serde(serialize_with="serialize_variant_as_string")]
+    #[serde(deserialize_with="deserialize_string_as_variant")]
     Struct {
         f1: String,
         f2: u8,
@@ -835,45 +840,112 @@ fn serialize_unit_variant_as_i8<S>(serializer: S) -> Result<S::Ok, S::Error>
     serializer.serialize_i8(0)
 }
 
-fn serialize_some_other_variant<S>(f1: &String,
-                                   f2: &u8,
-                                   serializer: S)
-                                   -> Result<S::Ok, S::Error>
+fn deserialize_i8_as_unit_variant<'de, D>(deserializer: D) -> Result<(), D::Error>
+    where D: Deserializer<'de>,
+{
+    let n = i8::deserialize(deserializer)?;
+    match n {
+        0 => Ok(()),
+        _ => Err(de::Error::invalid_value(Unexpected::Signed(n as i64), &"0")),
+    }
+}
+
+fn serialize_variant_as_string<S>(f1: &String,
+                                  f2: &u8,
+                                  serializer: S)
+                                  -> Result<S::Ok, S::Error>
     where S: Serializer,
 {
     serializer.serialize_str(format!("{};{:?}", f1, f2).as_str())
 }
 
+fn deserialize_string_as_variant<'de, D>(deserializer: D) -> Result<(String, u8), D::Error>
+    where D: Deserializer<'de>,
+{
+    let s = String::deserialize(deserializer)?;
+    let mut pieces = s.split(';');
+    let f1 = match pieces.next() {
+        Some(x) => x,
+        None => return Err(de::Error::invalid_length(0, &"2")),
+    };
+    let f2 = match pieces.next() {
+        Some(x) => x,
+        None => return Err(de::Error::invalid_length(1, &"2")),
+    };
+    let f2 = match f2.parse() {
+        Ok(n) => n,
+        Err(_) => {
+            return Err(de::Error::invalid_value(Unexpected::Str(f2), &"an 8-bit signed integer"));
+        }
+    };
+    Ok((f1.into(), f2))
+}
+
 #[test]
 fn test_serialize_with_variant() {
     assert_ser_tokens(
-        &SerializeWithVariant::Unit,
+        &WithVariant::Unit,
         &[
-            Token::NewtypeVariant { name: "SerializeWithVariant", variant: "Unit" },
+            Token::NewtypeVariant { name: "WithVariant", variant: "Unit" },
             Token::I8(0),
         ],
     );
 
     assert_ser_tokens(
-        &SerializeWithVariant::Newtype(123),
+        &WithVariant::Newtype(123),
         &[
-            Token::NewtypeVariant { name: "SerializeWithVariant", variant: "Newtype" },
+            Token::NewtypeVariant { name: "WithVariant", variant: "Newtype" },
             Token::Bool(true),
         ],
     );
 
     assert_ser_tokens(
-        &SerializeWithVariant::Tuple("hello".into(), 0),
+        &WithVariant::Tuple("hello".into(), 0),
         &[
-            Token::NewtypeVariant { name: "SerializeWithVariant", variant: "Tuple" },
+            Token::NewtypeVariant { name: "WithVariant", variant: "Tuple" },
             Token::Str("hello;0"),
         ],
     );
 
     assert_ser_tokens(
-        &SerializeWithVariant::Struct { f1: "world".into(), f2: 1 },
+        &WithVariant::Struct { f1: "world".into(), f2: 1 },
         &[
-            Token::NewtypeVariant { name: "SerializeWithVariant", variant: "Struct" },
+            Token::NewtypeVariant { name: "WithVariant", variant: "Struct" },
+            Token::Str("world;1"),
+        ],
+    );
+}
+
+#[test]
+fn test_deserialize_with_variant() {
+    assert_de_tokens(
+        &WithVariant::Unit,
+        &[
+            Token::NewtypeVariant { name: "WithVariant", variant: "Unit" },
+            Token::I8(0),
+        ],
+    );
+
+    assert_de_tokens(
+        &WithVariant::Newtype(123),
+        &[
+            Token::NewtypeVariant { name: "WithVariant", variant: "Newtype" },
+            Token::Bool(true),
+        ],
+    );
+
+    assert_de_tokens(
+        &WithVariant::Tuple("hello".into(), 0),
+        &[
+            Token::NewtypeVariant { name: "WithVariant", variant: "Tuple" },
+            Token::Str("hello;0"),
+        ],
+    );
+
+    assert_de_tokens(
+        &WithVariant::Struct { f1: "world".into(), f2: 1 },
+        &[
+            Token::NewtypeVariant { name: "WithVariant", variant: "Struct" },
             Token::Str("world;1"),
         ],
     );
diff --git a/test_suite/tests/test_gen.rs b/test_suite/tests/test_gen.rs
index 2c85a116..5ec186d0 100644
--- a/test_suite/tests/test_gen.rs
+++ b/test_suite/tests/test_gen.rs
@@ -374,7 +374,7 @@ fn test_gen() {
         s: vis::S,
     }
 
-    #[derive(Serialize)]
+    #[derive(Serialize, Deserialize)]
     enum ExternallyTaggedVariantWith {
         #[allow(dead_code)]
         Normal { f1: String },
@@ -404,7 +404,7 @@ fn test_gen() {
     }
     assert_ser::<ExternallyTaggedVariantWith>();
 
-    #[derive(Serialize)]
+    #[derive(Serialize, Deserialize)]
     #[serde(tag = "t")]
     enum InternallyTaggedVariantWith {
         #[allow(dead_code)]
@@ -430,7 +430,7 @@ fn test_gen() {
     }
     assert_ser::<InternallyTaggedVariantWith>();
 
-    #[derive(Serialize)]
+    #[derive(Serialize, Deserialize)]
     #[serde(tag = "t", content = "c")]
     enum AdjacentlyTaggedVariantWith {
         #[allow(dead_code)]
@@ -461,7 +461,7 @@ fn test_gen() {
     }
     assert_ser::<AdjacentlyTaggedVariantWith>();
 
-    #[derive(Serialize)]
+    #[derive(Serialize, Deserialize)]
     #[serde(untagged)]
     enum UntaggedVariantWith {
         #[allow(dead_code)]

From 552971196d4a933277645c6347d3b778c3c70e2e Mon Sep 17 00:00:00 2001
From: Michael Smith <michael@spinda.net>
Date: Wed, 16 Aug 2017 12:04:39 -0700
Subject: [PATCH 3/3] Fix Clippy errors in variant serialize_with tests

---
 test_suite/tests/test_annotations.rs | 2 +-
 test_suite/tests/test_gen.rs         | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/test_suite/tests/test_annotations.rs b/test_suite/tests/test_annotations.rs
index e0f6c8da..fb796111 100644
--- a/test_suite/tests/test_annotations.rs
+++ b/test_suite/tests/test_annotations.rs
@@ -850,7 +850,7 @@ fn deserialize_i8_as_unit_variant<'de, D>(deserializer: D) -> Result<(), D::Erro
     }
 }
 
-fn serialize_variant_as_string<S>(f1: &String,
+fn serialize_variant_as_string<S>(f1: &str,
                                   f2: &u8,
                                   serializer: S)
                                   -> Result<S::Ok, S::Error>
diff --git a/test_suite/tests/test_gen.rs b/test_suite/tests/test_gen.rs
index 5ec186d0..76c57db8 100644
--- a/test_suite/tests/test_gen.rs
+++ b/test_suite/tests/test_gen.rs
@@ -545,7 +545,7 @@ pub fn deserialize_some_unit_variant<'de, D>(_: D) -> StdResult<(), D::Error>
     unimplemented!()
 }
 
-pub fn serialize_some_other_variant<S>(_: &String, _: &u8, _: S) -> StdResult<S::Ok, S::Error>
+pub fn serialize_some_other_variant<S>(_: &str, _: &u8, _: S) -> StdResult<S::Ok, S::Error>
     where S: Serializer,
 {
     unimplemented!()