Improve error messages related to transparent

serde_derive/src/de.rs

@@ -15,7 +15,7 @@ use syn::{self, Ident, Index, Member};
 use bound;
 use fragment::{Expr, Fragment, Match, Stmts};
 use internals::ast::{Container, Data, Field, Style, Variant};
-use internals::{attr, Ctxt};
+use internals::{attr, Ctxt, Derive};
 use pretend;
 use try;
 
@@ -23,7 +23,7 @@ use std::collections::BTreeSet;
 
 pub fn expand_derive_deserialize(input: &syn::DeriveInput) -> Result<TokenStream, String> {
     let ctxt = Ctxt::new();
-    let cont = Container::from_ast(&ctxt, input);
+    let cont = Container::from_ast(&ctxt, input, Derive::Deserialize);
     precondition(&ctxt, &cont);
     try!(ctxt.check());
 
@@ -355,7 +355,7 @@ fn deserialize_transparent(cont: &Container, params: &Parameters) -> Fragment {
     };
 
     let this = &params.this;
-    let transparent_field = find_transparent_field(fields);
+    let transparent_field = fields.iter().find(|f| f.attrs.transparent()).unwrap();
 
     let path = match transparent_field.attrs.deserialize_with() {
         Some(path) => quote!(#path),
@@ -383,34 +383,6 @@ fn deserialize_transparent(cont: &Container, params: &Parameters) -> Fragment {
     }
 }
 
-fn find_transparent_field<'a>(fields: &'a [Field<'a>]) -> &'a Field<'a> {
-    let mut transparent_field = None;
-
-    for field in fields {
-        if field.attrs.skip_deserializing() {
-            continue;
-        }
-        if field.attrs.default().is_none() {
-            if let syn::Type::Path(ref ty) = field.ty {
-                if let Some(seg) = ty.path.segments.last() {
-                    if seg.into_value().ident == "PhantomData" {
-                        continue;
-                    }
-                }
-            }
-            if transparent_field.is_some() {
-                panic!("Ambiguous transparent field");
-            }
-            transparent_field = Some(field);
-        }
-    }
-
-    match transparent_field {
-        Some(transparent_field) => transparent_field,
-        None => panic!("No field can be transparent"),
-    }
-}
-
 fn deserialize_from(type_from: &syn::Type) -> Fragment {
     quote_block! {
         _serde::export::Result::map(

serde_derive/src/internals/ast.rs

@@ -8,7 +8,7 @@
 
 use internals::attr;
 use internals::check;
-use internals::Ctxt;
+use internals::{Ctxt, Derive};
 use syn;
 use syn::punctuated::Punctuated;
 
@@ -47,7 +47,7 @@ pub enum Style {
 }
 
 impl<'a> Container<'a> {
-    pub fn from_ast(cx: &Ctxt, item: &'a syn::DeriveInput) -> Container<'a> {
+    pub fn from_ast(cx: &Ctxt, item: &'a syn::DeriveInput, derive: Derive) -> Container<'a> {
         let mut attrs = attr::Container::from_ast(cx, item);
 
         let mut data = match item.data {
@@ -86,13 +86,13 @@ impl<'a> Container<'a> {
             attrs.mark_has_flatten();
         }
 
-        let item = Container {
+        let mut item = Container {
             ident: item.ident.clone(),
             attrs: attrs,
             data: data,
             generics: &item.generics,
         };
-        check::check(cx, &item);
+        check::check(cx, &mut item, derive);
         item
     }
 }

serde_derive/src/internals/attr.rs

@@ -776,6 +776,7 @@ pub struct Field {
     borrowed_lifetimes: BTreeSet<syn::Lifetime>,
     getter: Option<syn::ExprPath>,
     flatten: bool,
+    transparent: bool,
 }
 
 /// Represents the default to use for a field when deserializing.
@@ -1077,6 +1078,7 @@ impl Field {
             borrowed_lifetimes: borrowed_lifetimes,
             getter: getter.get(),
             flatten: flatten.get(),
+            transparent: false,
         }
     }
 
@@ -1136,6 +1138,14 @@ impl Field {
     pub fn flatten(&self) -> bool {
         self.flatten
     }
+
+    pub fn transparent(&self) -> bool {
+        self.transparent
+    }
+
+    pub fn mark_transparent(&mut self) {
+        self.transparent = true;
+    }
 }
 
 type SerAndDe<T> = (Option<T>, Option<T>);

serde_derive/src/internals/check.rs

@@ -8,19 +8,19 @@
 
 use internals::ast::{Container, Data, Field, Style};
 use internals::attr::{EnumTag, Identifier};
-use internals::Ctxt;
-use syn::Member;
+use internals::{Ctxt, Derive};
+use syn::{Member, Type};
 
 /// Cross-cutting checks that require looking at more than a single attrs
 /// object. Simpler checks should happen when parsing and building the attrs.
-pub fn check(cx: &Ctxt, cont: &Container) {
+pub fn check(cx: &Ctxt, cont: &mut Container, derive: Derive) {
     check_getter(cx, cont);
     check_flatten(cx, cont);
     check_identifier(cx, cont);
     check_variant_skip_attrs(cx, cont);
     check_internal_tag_field_name_conflict(cx, cont);
     check_adjacent_tag_conflict(cx, cont);
-    check_transparent(cx, cont);
+    check_transparent(cx, cont, derive);
 }
 
 /// Getters are only allowed inside structs (not enums) with the `remote`
@@ -280,7 +280,7 @@ fn check_adjacent_tag_conflict(cx: &Ctxt, cont: &Container) {
 }
 
 /// Enums and unit structs cannot be transparent.
-fn check_transparent(cx: &Ctxt, cont: &Container) {
+fn check_transparent(cx: &Ctxt, cont: &mut Container, derive: Derive) {
     if !cont.attrs.transparent() {
         return;
     }
@@ -293,14 +293,40 @@ fn check_transparent(cx: &Ctxt, cont: &Container) {
         cx.error("#[serde(transparent)] is not allowed with #[serde(into = \"...\")]");
     }
 
-    match cont.data {
+    let fields = match cont.data {
         Data::Enum(_) => {
             cx.error("#[serde(transparent)] is not allowed on an enum");
+            return;
         }
         Data::Struct(Style::Unit, _) => {
             cx.error("#[serde(transparent)] is not allowed on a unit struct");
+            return;
+        }
+        Data::Struct(_, ref mut fields) => fields,
+    };
+
+    let mut transparent_field = None;
+
+    for field in fields {
+        if allow_transparent(field, derive) {
+            if transparent_field.is_some() {
+                cx.error("#[serde(transparent)] requires struct to have at most one transparent field");
+                return;
+            }
+            transparent_field = Some(field);
+        }
+    }
+
+    match transparent_field {
+        Some(transparent_field) => transparent_field.attrs.mark_transparent(),
+        None => match derive {
+            Derive::Serialize => {
+                cx.error("#[serde(transparent)] requires at least one field that is not skipped");
+            }
+            Derive::Deserialize => {
+                cx.error("#[serde(transparent)] requires at least one field that is neither skipped nor has a default");
+            }
         }
-        _ => {}
     }
 }
 
@@ -310,3 +336,18 @@ fn member_message(member: &Member) -> String {
         Member::Unnamed(ref i) => i.index.to_string(),
     }
 }
+
+fn allow_transparent(field: &Field, derive: Derive) -> bool {
+    if let Type::Path(ref ty) = *field.ty {
+        if let Some(seg) = ty.path.segments.last() {
+            if seg.into_value().ident == "PhantomData" {
+                return false;
+            }
+        }
+    }
+
+    match derive {
+        Derive::Serialize => !field.attrs.skip_serializing(),
+        Derive::Deserialize => !field.attrs.skip_deserializing() && field.attrs.default().is_none(),
+    }
+}

serde_derive/src/internals/mod.rs

@@ -14,3 +14,9 @@ pub use self::ctxt::Ctxt;
 
 mod case;
 mod check;
+
+#[derive(Copy, Clone)]
+pub enum Derive {
+    Serialize,
+    Deserialize,
+}

serde_derive/src/ser.rs

@@ -13,13 +13,13 @@ use syn::{self, Ident, Index, Member};
 use bound;
 use fragment::{Fragment, Match, Stmts};
 use internals::ast::{Container, Data, Field, Style, Variant};
-use internals::{attr, Ctxt};
+use internals::{attr, Ctxt, Derive};
 use pretend;
 use try;
 
 pub fn expand_derive_serialize(input: &syn::DeriveInput) -> Result<TokenStream, String> {
     let ctxt = Ctxt::new();
-    let cont = Container::from_ast(&ctxt, input);
+    let cont = Container::from_ast(&ctxt, input, Derive::Serialize);
     precondition(&ctxt, &cont);
     try!(ctxt.check());
 
@@ -194,7 +194,7 @@ fn serialize_transparent(cont: &Container, params: &Parameters) -> Fragment {
     };
 
     let self_var = params.self_var;
-    let transparent_field = find_transparent_field(fields);
+    let transparent_field = fields.iter().find(|f| f.attrs.transparent()).unwrap();
     let member = &transparent_field.member;
 
     let path = match transparent_field.attrs.serialize_with() {
@@ -207,32 +207,6 @@ fn serialize_transparent(cont: &Container, params: &Parameters) -> Fragment {
     }
 }
 
-fn find_transparent_field<'a>(fields: &'a [Field<'a>]) -> &'a Field<'a> {
-    let mut transparent_field = None;
-
-    for field in fields {
-        if field.attrs.skip_serializing() {
-            continue;
-        }
-        if let syn::Type::Path(ref ty) = field.ty {
-            if let Some(seg) = ty.path.segments.last() {
-                if seg.into_value().ident == "PhantomData" {
-                    continue;
-                }
-            }
-        }
-        if transparent_field.is_some() {
-            panic!("Ambiguous transparent field");
-        }
-        transparent_field = Some(field);
-    }
-
-    match transparent_field {
-        Some(transparent_field) => transparent_field,
-        None => panic!("No field can be transparent"),
-    }
-}
-
 fn serialize_into(params: &Parameters, type_into: &syn::Type) -> Fragment {
     let self_var = &params.self_var;
     quote_block! {