feat(codegen): Switch attributes to using using paths not expressions

README.md

@@ -689,18 +689,17 @@ Variant Annotations:
 
 Field Annotations:
 
-| Annotation                             | Function                                                           |
+| Annotation                             | Function                                                                                                   |
-| ----------                             | --------                                                           |
+| ----------                             | --------                                                                                                   |
-| `#[serde(rename="name")`               | Serialize and deserialize this field with the given name           |
+| `#[serde(rename="name")`               | Serialize and deserialize this field with the given name                                                   |
-| `#[serde(rename(serialize="name1"))`   | Serialize this field with the given name                           |
+| `#[serde(rename(serialize="name1"))`   | Serialize this field with the given name                                                                   |
-| `#[serde(rename(deserialize="name1"))` | Deserialize this field with the given name                         |
+| `#[serde(rename(deserialize="name1"))` | Deserialize this field with the given name                                                                 |
-| `#[serde(default)`                     | If the value is not specified, use the `Default::default()`        |
+| `#[serde(default)`                     | If the value is not specified, use the `Default::default()`                                                |
-| `#[serde(default="$expr")`             | If the value is not specified, use the `$expr` expression          |
+| `#[serde(default="$path")`             | Call the path to a function `fn() -> T` to build the value                                                 |
-| `#[serde(skip_serializing)`            | Do not serialize this value                                        |
+| `#[serde(skip_serializing)`            | Do not serialize this value                                                                                |
-| `#[serde(skip_serializing_if="$expr")` | Do not serialize this value if the `$expr` expression returns true |
+| `#[serde(skip_serializing_if="$path")` | Do not serialize this value if this function `fn(&T) -> bool` returns `false`                              |
-| `#[serde(serialize_with="$expr")`      | Use the `$expr` expression to serialize this field                 |
+| `#[serde(serialize_with="$path")`      | Call a function `fn<T, S>(&T, &mut S) -> Result<(), S::Error> where S: Serializer` to serialize this value |
-| `#[serde(deserialize_with="$expr")`    | Use the `$expr` expression to deserialize this field               |
+| `#[serde(deserialize_with="$path")`    | Call a function `fn<T, D>(&mut D) -> Result<T, D::Error> where D: Deserializer` to deserialize this value  |
-
 
 Serialization Formats Using Serde
 =================================

serde_codegen/src/attr.rs

@@ -4,6 +4,7 @@ use syntax::attr;
 use syntax::codemap::Span;
 use syntax::ext::base::ExtCtxt;
 use syntax::fold::Folder;
+use syntax::parse::parser::PathParsingMode;
 use syntax::parse::token;
 use syntax::parse;
 use syntax::print::pprust::{lit_to_string, meta_item_to_string};
@@ -181,7 +182,8 @@ impl FieldAttrs {
     pub fn from_field(cx: &ExtCtxt,
                       container_ty: &P<ast::Ty>,
                       generics: &ast::Generics,
-                      field: &ast::StructField) -> Result<Self, Error> {
+                      field: &ast::StructField,
+                      is_enum: bool) -> Result<Self, Error> {
         let builder = AstBuilder::new();
 
         let field_ident = match field.node.ident() {
@@ -225,10 +227,7 @@ impl FieldAttrs {
                     // Parse `#[serde(default="...")]`
                     ast::MetaItemKind::NameValue(ref name, ref lit) if name == &"default" => {
                         let wrapped_expr = wrap_default(
-                            cx,
+                            try!(parse_lit_into_path(cx, name, lit)),
-                            &field.node.ty,
-                            generics,
-                            try!(parse_lit_into_expr(cx, name, lit)),
                         );
 
                         field_attrs.default_expr_if_missing = Some(wrapped_expr);
@@ -242,10 +241,9 @@ impl FieldAttrs {
                     // Parse `#[serde(skip_serializing_if="...")]`
                     ast::MetaItemKind::NameValue(ref name, ref lit) if name == &"skip_serializing_if" => {
                         let expr = wrap_skip_serializing(
-                            cx,
+                            field_ident,
-                            container_ty,
+                            try!(parse_lit_into_path(cx, name, lit)),
-                            generics,
+                            is_enum,
-                            try!(parse_lit_into_expr(cx, name, lit)),
                         );
 
                         field_attrs.skip_serializing_field_if = Some(expr);
@@ -257,7 +255,9 @@ impl FieldAttrs {
                             cx,
                             container_ty,
                             generics,
-                            try!(parse_lit_into_expr(cx, name, lit)),
+                            field_ident,
+                            try!(parse_lit_into_path(cx, name, lit)),
+                            is_enum,
                         );
 
                         field_attrs.serialize_with = Some(expr);
@@ -269,7 +269,7 @@ impl FieldAttrs {
                             cx,
                             &field.node.ty,
                             generics,
-                            try!(parse_lit_into_expr(cx, name, lit)),
+                            try!(parse_lit_into_path(cx, name, lit)),
                         );
 
                         field_attrs.deserialize_with = Some(expr);
@@ -349,9 +349,10 @@ impl FieldAttrs {
 pub fn get_struct_field_attrs(cx: &ExtCtxt,
                               container_ty: &P<ast::Ty>,
                               generics: &ast::Generics,
-                              fields: &[ast::StructField]) -> Result<Vec<FieldAttrs>, Error> {
+                              fields: &[ast::StructField],
+                              is_enum: bool) -> Result<Vec<FieldAttrs>, Error> {
     fields.iter()
-        .map(|field| FieldAttrs::from_field(cx, container_ty, generics, field))
+        .map(|field| FieldAttrs::from_field(cx, container_ty, generics, field, is_enum))
         .collect()
 }
 
@@ -441,7 +442,7 @@ impl<'a, 'b> Folder for Respanner<'a, 'b> {
     }
 }
 
-fn parse_lit_into_expr(cx: &ExtCtxt, name: &str, lit: &ast::Lit) -> Result<P<ast::Expr>, Error> {
+fn parse_lit_into_path(cx: &ExtCtxt, name: &str, lit: &ast::Lit) -> Result<ast::Path, Error> {
     let source: &str = match lit.node {
         ast::LitKind::Str(ref source, _) => &source,
         _ => {
@@ -471,10 +472,8 @@ fn parse_lit_into_expr(cx: &ExtCtxt, name: &str, lit: &ast::Lit) -> Result<P<ast
 
     let mut parser = parse::new_parser_from_tts(cx.parse_sess(), cx.cfg(), tts);
 
-    let expr = parser.parse_expr();
+    let path = match parser.parse_path(PathParsingMode::LifetimeAndTypesWithoutColons) {
-
+        Ok(path) => path,
-    let expr = match expr {
-        Ok(expr) => expr,
         Err(mut e) => {
             e.emit();
             return Err(Error);
@@ -490,55 +489,39 @@ fn parse_lit_into_expr(cx: &ExtCtxt, name: &str, lit: &ast::Lit) -> Result<P<ast
         }
     }
 
-    Ok(expr)
+    Ok(path)
 }
 
 /// This function wraps the expression in `#[serde(default="...")]` in a function to prevent it
 /// from accessing the internal `Deserialize` state.
-fn wrap_default(cx: &ExtCtxt,
+fn wrap_default(path: ast::Path) -> P<ast::Expr> {
-                field_ty: &P<ast::Ty>,
+    AstBuilder::new().expr().call()
-                generics: &ast::Generics,
+        .build_path(path)
-                expr: P<ast::Expr>) -> P<ast::Expr> {
+        .build()
-    let builder = AstBuilder::new();
-
-    // Quasi-quoting doesn't do a great job of expanding generics into paths, so manually build it.
-    let fn_path = builder.path()
-        .segment("__serde_default")
-            .with_generics(generics.clone())
-            .build()
-        .build();
-
-    let where_clause = &generics.where_clause;
-
-    quote_expr!(cx, {
-        fn __serde_default $generics() -> $field_ty $where_clause {
-            $expr
-        }
-        $fn_path()
-    })
 }
 
 /// This function wraps the expression in `#[serde(skip_serializing_if="...")]` in a trait to
 /// prevent it from accessing the internal `Serialize` state.
-fn wrap_skip_serializing(cx: &ExtCtxt,
+fn wrap_skip_serializing(field_ident: ast::Ident,
-                         container_ty: &P<ast::Ty>,
+                         path: ast::Path,
-                         generics: &ast::Generics,
+                         is_enum: bool) -> P<ast::Expr> {
-                         expr: P<ast::Expr>) -> P<ast::Expr> {
+    let builder = AstBuilder::new();
-    let where_clause = &generics.where_clause;
 
-    quote_expr!(cx, {
+    let expr = builder.expr()
-        trait __SerdeShouldSkipSerializing {
+        .field(field_ident)
-            fn __serde_should_skip_serializing(&self) -> bool;
+        .field("value")
-        }
+        .self_();
 
-        impl $generics __SerdeShouldSkipSerializing for $container_ty $where_clause {
+    let expr = if is_enum {
-            fn __serde_should_skip_serializing(&self) -> bool {
+        expr
-                $expr
+    } else {
-            }
+        builder.expr().ref_().build(expr)
-        }
+    };
 
-        self.value.__serde_should_skip_serializing()
+    builder.expr().call()
-    })
+        .build_path(path)
+        .arg().build(expr)
+        .build()
 }
 
 /// This function wraps the expression in `#[serde(serialize_with="...")]` in a trait to
@@ -546,7 +529,28 @@ fn wrap_skip_serializing(cx: &ExtCtxt,
 fn wrap_serialize_with(cx: &ExtCtxt,
                        container_ty: &P<ast::Ty>,
                        generics: &ast::Generics,
-                       expr: P<ast::Expr>) -> P<ast::Expr> {
+                       field_ident: ast::Ident,
+                       path: ast::Path,
+                       is_enum: bool) -> P<ast::Expr> {
+    let builder = AstBuilder::new();
+
+    let expr = builder.expr()
+        .field(field_ident)
+        .self_();
+
+    let expr = if is_enum {
+        expr
+    } else {
+        builder.expr().ref_().build(expr)
+    };
+
+    let expr = builder.expr().call()
+        .build_path(path)
+        .arg().build(expr)
+        .arg()
+            .id("serializer")
+        .build();
+
     let where_clause = &generics.where_clause;
 
     quote_expr!(cx, {
@@ -598,22 +602,7 @@ fn wrap_serialize_with(cx: &ExtCtxt,
 fn wrap_deserialize_with(cx: &ExtCtxt,
                          field_ty: &P<ast::Ty>,
                          generics: &ast::Generics,
-                         expr: P<ast::Expr>) -> P<ast::Expr> {
+                         path: ast::Path) -> P<ast::Expr> {
-    let builder = AstBuilder::new();
-
-    let fn_generics = builder.from_generics(generics.clone())
-        .ty_param("__D")
-            .bound()
-                .trait_(
-                    builder.path()
-                        .global()
-                        .ids(&["serde", "de", "Deserializer"])
-                        .build()
-                )
-                .build()
-            .build()
-        .build();
-
     // Quasi-quoting doesn't do a great job of expanding generics into paths, so manually build it.
     let ty_path = AstBuilder::new().path()
         .segment("__SerdeDeserializeWithStruct")
@@ -621,15 +610,9 @@ fn wrap_deserialize_with(cx: &ExtCtxt,
             .build()
         .build();
 
-    let fn_where_clause = &fn_generics.where_clause;
     let where_clause = &generics.where_clause;
 
     quote_expr!(cx, {
-        fn __serde_deserialize_with $fn_generics(deserializer: &mut __D)
-            -> Result<$field_ty, __D::Error> $fn_where_clause {
-            $expr
-        }
-
         struct __SerdeDeserializeWithStruct $generics $where_clause {
             value: $field_ty,
         }
@@ -638,7 +621,7 @@ fn wrap_deserialize_with(cx: &ExtCtxt,
             fn deserialize<D>(deserializer: &mut D) -> Result<Self, D::Error>
                 where D: ::serde::de::Deserializer
             {
-                let value = try!(__serde_deserialize_with(deserializer));
+                let value = try!($path(deserializer));
                 Ok(__SerdeDeserializeWithStruct { value: value })
             }
         }

serde_codegen/src/de.rs

@@ -506,7 +506,8 @@ fn deserialize_struct(
         &ty,
         impl_generics,
         fields,
-        container_attrs
+        container_attrs,
+        false,
     ));
 
     let type_name = container_attrs.deserialize_name_expr();
@@ -762,6 +763,7 @@ fn deserialize_struct_variant(
         generics,
         fields,
         container_attrs,
+        true,
     ));
 
     let (visitor_item, visitor_ty, visitor_expr, visitor_generics) = try!(deserialize_visitor(
@@ -926,6 +928,7 @@ fn deserialize_struct_visitor(
     generics: &ast::Generics,
     fields: &[ast::StructField],
     container_attrs: &attr::ContainerAttrs,
+    is_enum: bool,
 ) -> Result<(Vec<P<ast::Item>>, ast::Stmt, P<ast::Expr>), Error> {
     let field_exprs = fields.iter()
         .map(|field| {
@@ -933,7 +936,8 @@ fn deserialize_struct_visitor(
                 attr::FieldAttrs::from_field(cx,
                                              container_ty,
                                              generics,
-                                             field)
+                                             field,
+                                             is_enum)
             );
             Ok(field_attrs.deserialize_name_expr())
         })
@@ -954,6 +958,7 @@ fn deserialize_struct_visitor(
         generics,
         fields,
         container_attrs,
+        is_enum,
     ));
 
     let fields_expr = builder.expr().ref_().slice()
@@ -985,6 +990,7 @@ fn deserialize_map(
     generics: &ast::Generics,
     fields: &[ast::StructField],
     container_attrs: &attr::ContainerAttrs,
+    is_enum: bool,
 ) -> Result<P<ast::Expr>, Error> {
     // Create the field names for the fields.
     let field_names: Vec<ast::Ident> = (0 .. fields.len())
@@ -993,7 +999,7 @@ fn deserialize_map(
 
     let field_attrs: Vec<_> = try!(
         fields.iter()
-            .map(|field| attr::FieldAttrs::from_field(cx, container_ty, generics, field))
+            .map(|field| attr::FieldAttrs::from_field(cx, container_ty, generics, field, is_enum))
             .collect()
     );
 

serde_codegen/src/ser.rs

@@ -256,6 +256,7 @@ fn serialize_struct(
         builder.id("serialize_struct_elt"),
         fields,
         impl_generics,
+        false,
     ));
 
     let type_name = container_attrs.serialize_name_expr();
@@ -547,6 +548,7 @@ fn serialize_struct_variant(
         builder.id("serialize_struct_variant_elt"),
         fields,
         &variant_generics,
+        true,
     ));
 
     let container_name = container_attrs.serialize_name_expr();
@@ -644,9 +646,10 @@ fn serialize_struct_visitor(
     serializer_method: ast::Ident,
     fields: &[ast::StructField],
     generics: &ast::Generics,
+    is_enum: bool,
 ) -> Result<(P<ast::Item>, P<ast::Item>), Error> {
     let field_attrs = try!(
-        attr::get_struct_field_attrs(cx, &structure_ty, generics, fields)
+        attr::get_struct_field_attrs(cx, &structure_ty, generics, fields, is_enum)
     );
 
     let arms: Vec<ast::Arm> = fields.iter().zip(field_attrs.iter())

serde_tests/tests/test_annotations.rs

@@ -53,7 +53,7 @@ struct DefaultStruct<A, B: Default, C> where C: Trait {
     a1: A,
     #[serde(default)]
     a2: B,
-    #[serde(default="Trait::my_default()")]
+    #[serde(default="Trait::my_default")]
     a3: C,
 }
 
@@ -100,7 +100,7 @@ enum DefaultEnum<A, B: Default, C> where C: Trait {
         a1: A,
         #[serde(default)]
         a2: B,
-        #[serde(default="Trait::my_default()")]
+        #[serde(default="Trait::my_default")]
         a3: C,
     }
 }
@@ -393,7 +393,7 @@ struct SkipSerializingStruct<'a, B, C> where C: Trait {
     a: &'a i8,
     #[serde(skip_serializing)]
     b: B,
-    #[serde(skip_serializing_if="self.c.should_skip()")]
+    #[serde(skip_serializing_if="Trait::should_skip")]
     c: C,
 }
 
@@ -445,7 +445,7 @@ enum SkipSerializingEnum<'a, B, C> where C: Trait {
         a: &'a i8,
         #[serde(skip_serializing)]
         _b: B,
-        #[serde(skip_serializing_if="self.c.should_skip()")]
+        #[serde(skip_serializing_if="Trait::should_skip")]
         c: C,
     }
 }
@@ -495,7 +495,7 @@ fn test_skip_serializing_enum() {
 #[derive(Debug, PartialEq, Serialize)]
 struct SerializeWithStruct<'a, B> where B: Trait {
     a: &'a i8,
-    #[serde(serialize_with="self.b.serialize_with(serializer)")]
+    #[serde(serialize_with="Trait::serialize_with")]
     b: B,
 }
 
@@ -547,7 +547,7 @@ fn test_serialize_with_struct() {
 enum SerializeWithEnum<'a, B> where B: Trait {
     Struct {
         a: &'a i8,
-        #[serde(serialize_with="self.b.serialize_with(serializer)")]
+        #[serde(serialize_with="Trait::serialize_with")]
         b: B,
     }
 }
@@ -599,7 +599,7 @@ fn test_serialize_with_enum() {
 #[derive(Debug, PartialEq, Deserialize)]
 struct DeserializeWithStruct<B> where B: Trait {
     a: i8,
-    #[serde(deserialize_with="Trait::deserialize_with(deserializer)")]
+    #[serde(deserialize_with="Trait::deserialize_with")]
     b: B,
 }
 
@@ -650,7 +650,7 @@ fn test_deserialize_with_struct() {
 enum DeserializeWithEnum<B> where B: Trait {
     Struct {
         a: i8,
-        #[serde(deserialize_with="Trait::deserialize_with(deserializer)")]
+        #[serde(deserialize_with="Trait::deserialize_with")]
         b: B,
     }
 }