diff --git a/serde/Cargo.toml b/serde/Cargo.toml index 2209d58b..97d8287d 100644 --- a/serde/Cargo.toml +++ b/serde/Cargo.toml @@ -9,13 +9,13 @@ documentation = "https://serde-rs.github.io/serde/serde/serde/index.html" readme = "../README.md" keywords = ["serde", "serialization"] -[dependencies.num] -version = "^0.1.27" -default-features = false - [features] -nightly = [] -num-impls = ["num-bigint", "num-complex", "num-rational"] +nightly = ["clippy"] num-bigint = ["num/bigint"] num-complex = ["num/complex"] +num-impls = ["num-bigint", "num-complex", "num-rational"] num-rational = ["num/rational"] + +[dependencies] +clippy = { version = "^0.0.35", optional = true } +num = { version = "^0.1.27", default-features = false } diff --git a/serde/src/de/impls.rs b/serde/src/de/impls.rs index 59925707..f0e184ed 100644 --- a/serde/src/de/impls.rs +++ b/serde/src/de/impls.rs @@ -234,7 +234,7 @@ impl Visitor for StringVisitor { fn visit_str(&mut self, v: &str) -> Result where E: Error, { - Ok(v.to_string()) + Ok(v.to_owned()) } fn visit_string(&mut self, v: String) -> Result @@ -247,12 +247,12 @@ impl Visitor for StringVisitor { where E: Error, { match str::from_utf8(v) { - Ok(s) => Ok(s.to_string()), + Ok(s) => Ok(s.to_owned()), Err(_) => Err(Error::type_mismatch(Type::String)), } } - fn visit_byte_buf<'a, E>(&mut self, v: Vec) -> Result + fn visit_byte_buf(&mut self, v: Vec) -> Result where E: Error, { match String::from_utf8(v) { diff --git a/serde/src/lib.rs b/serde/src/lib.rs index e4072d1c..b3e3148e 100644 --- a/serde/src/lib.rs +++ b/serde/src/lib.rs @@ -10,7 +10,10 @@ //! [github repository](https://github.com/serde-rs/serde) #![doc(html_root_url="https://serde-rs.github.io/serde/serde")] -#![cfg_attr(feature = "nightly", feature(collections, enumset, nonzero, step_trait, zero_one))] +#![cfg_attr(feature = "nightly", feature(collections, enumset, nonzero, plugin, step_trait, + zero_one))] +#![cfg_attr(feature = "nightly", plugin(clippy))] +#![cfg_attr(feature = "nightly", allow(linkedlist))] #![deny(missing_docs)] diff --git a/serde/src/ser/impls.rs b/serde/src/ser/impls.rs index 173f6c17..7d01a384 100644 --- a/serde/src/ser/impls.rs +++ b/serde/src/ser/impls.rs @@ -566,8 +566,8 @@ impl MapVisitor for MapIteratorVisitor { match self.iter.next() { Some((key, value)) => { - let value = try!(serializer.serialize_map_elt(key, value)); - Ok(Some(value)) + try!(serializer.serialize_map_elt(key, value)); + Ok(Some(())) } None => Ok(None) } diff --git a/serde/src/ser/mod.rs b/serde/src/ser/mod.rs index 06fa7bb8..19dd8212 100644 --- a/serde/src/ser/mod.rs +++ b/serde/src/ser/mod.rs @@ -322,6 +322,7 @@ pub trait Serializer { } /// A trait that is used by a `Serialize` to iterate through a sequence. +#[cfg_attr(feature = "nightly", allow(len_without_is_empty))] pub trait SeqVisitor { /// Serializes a sequence item in the serializer. /// @@ -338,6 +339,7 @@ pub trait SeqVisitor { } /// A trait that is used by a `Serialize` to iterate through a map. +#[cfg_attr(feature = "nightly", allow(len_without_is_empty))] pub trait MapVisitor { /// Serializes a map item in the serializer. /// diff --git a/serde_codegen/Cargo.toml b/serde_codegen/Cargo.toml index cc7a5911..072e440a 100644 --- a/serde_codegen/Cargo.toml +++ b/serde_codegen/Cargo.toml @@ -11,7 +11,7 @@ keywords = ["serde", "serialization"] [features] default = ["with-syntex"] -nightly = ["quasi_macros"] +nightly = ["clippy", "quasi_macros"] with-syntex = ["quasi/with-syntex", "quasi_codegen", "quasi_codegen/with-syntex", "syntex", "syntex_syntax"] [build-dependencies] @@ -20,6 +20,7 @@ syntex = { version = "^0.26.0", optional = true } [dependencies] aster = { version = "^0.10.0", default-features = false } +clippy = { version = "^0.0.35", optional = true } quasi = { version = "^0.4.0", default-features = false } quasi_macros = { version = "^0.4.0", optional = true } syntex = { version = "^0.26.0", optional = true } diff --git a/serde_codegen/src/attr.rs b/serde_codegen/src/attr.rs index d71ef09f..6a028367 100644 --- a/serde_codegen/src/attr.rs +++ b/serde_codegen/src/attr.rs @@ -4,6 +4,7 @@ use std::collections::HashSet; use syntax::ast; use syntax::attr; use syntax::ext::base::ExtCtxt; +use syntax::print::pprust::meta_item_to_string; use syntax::ptr::P; use aster; @@ -32,7 +33,7 @@ impl FieldAttrs { /// Return a set of formats that the field has attributes for. pub fn formats(&self) -> HashSet> { match self.names { - FieldNames::Format{ref formats, default: _} => { + FieldNames::Format { ref formats, .. } => { let mut set = HashSet::new(); for (fmt, _) in formats.iter() { set.insert(fmt.clone()); @@ -70,7 +71,7 @@ impl FieldAttrs { pub fn default_key_expr(&self) -> &P { match self.names { FieldNames::Global(ref expr) => expr, - FieldNames::Format{formats: _, ref default} => default, + FieldNames::Format { ref default, .. } => default, } } @@ -104,6 +105,7 @@ impl FieldAttrs { } pub struct FieldAttrsBuilder<'a> { + cx: &'a ExtCtxt<'a>, builder: &'a aster::AstBuilder, skip_serializing_field: bool, skip_serializing_field_if_empty: bool, @@ -114,8 +116,10 @@ pub struct FieldAttrsBuilder<'a> { } impl<'a> FieldAttrsBuilder<'a> { - pub fn new(builder: &'a aster::AstBuilder) -> FieldAttrsBuilder<'a> { + pub fn new(cx: &'a ExtCtxt<'a>, + builder: &'a aster::AstBuilder) -> FieldAttrsBuilder<'a> { FieldAttrsBuilder { + cx: cx, builder: builder, skip_serializing_field: false, skip_serializing_field_if_empty: false, @@ -126,7 +130,7 @@ impl<'a> FieldAttrsBuilder<'a> { } } - pub fn field(mut self, field: &ast::StructField) -> FieldAttrsBuilder<'a> { + pub fn field(mut self, field: &ast::StructField) -> Result, ()> { match field.node.kind { ast::NamedField(name, _) => { self.name = Some(self.builder.expr().str(name)); @@ -137,28 +141,36 @@ impl<'a> FieldAttrsBuilder<'a> { self.attrs(&field.node.attrs) } - pub fn attrs(self, attrs: &[ast::Attribute]) -> FieldAttrsBuilder<'a> { - attrs.iter().fold(self, FieldAttrsBuilder::attr) + pub fn attrs(mut self, attrs: &[ast::Attribute]) -> Result, ()> { + for attr in attrs { + self = try!(self.attr(attr)); + } + + Ok(self) } - pub fn attr(self, attr: &ast::Attribute) -> FieldAttrsBuilder<'a> { + pub fn attr(mut self, attr: &ast::Attribute) -> Result, ()> { match attr.node.value.node { ast::MetaList(ref name, ref items) if name == &"serde" => { attr::mark_used(&attr); - items.iter().fold(self, FieldAttrsBuilder::meta_item) + for item in items { + self = try!(self.meta_item(item)); + } + + Ok(self) } _ => { - self + Ok(self) } } } - pub fn meta_item(mut self, meta_item: &P) -> FieldAttrsBuilder<'a> { + pub fn meta_item(mut self, meta_item: &P) -> Result, ()> { match meta_item.node { ast::MetaNameValue(ref name, ref lit) if name == &"rename" => { let expr = self.builder.expr().build_lit(P(lit.clone())); - self.name(expr) + Ok(self.name(expr)) } ast::MetaList(ref name, ref items) if name == &"rename" => { for item in items { @@ -172,23 +184,27 @@ impl<'a> FieldAttrsBuilder<'a> { _ => { } } } - self + + Ok(self) } ast::MetaWord(ref name) if name == &"default" => { - self.default() + Ok(self.default()) } ast::MetaWord(ref name) if name == &"skip_serializing" => { - self.skip_serializing_field() + Ok(self.skip_serializing_field()) } ast::MetaWord(ref name) if name == &"skip_serializing_if_empty" => { - self.skip_serializing_field_if_empty() + Ok(self.skip_serializing_field_if_empty()) } ast::MetaWord(ref name) if name == &"skip_serializing_if_none" => { - self.skip_serializing_field_if_none() + Ok(self.skip_serializing_field_if_none()) } _ => { - // Ignore unknown meta variables for now. - self + self.cx.span_err( + meta_item.span, + &format!("unknown serde field attribute `{}`", + meta_item_to_string(meta_item))); + Err(()) } } } @@ -256,46 +272,59 @@ impl ContainerAttrs { } } -pub struct ContainerAttrsBuilder { +pub struct ContainerAttrsBuilder<'a> { + cx: &'a ExtCtxt<'a>, deny_unknown_fields: bool, } -impl ContainerAttrsBuilder { - pub fn new() -> ContainerAttrsBuilder { +impl<'a> ContainerAttrsBuilder<'a> { + pub fn new(cx: &'a ExtCtxt) -> Self { ContainerAttrsBuilder { + cx: cx, deny_unknown_fields: false, } } - pub fn attrs(self, attrs: &[ast::Attribute]) -> ContainerAttrsBuilder { - attrs.iter().fold(self, ContainerAttrsBuilder::attr) + pub fn attrs(mut self, attrs: &[ast::Attribute]) -> Result { + for attr in attrs { + self = try!(self.attr(attr)); + } + + Ok(self) } - pub fn attr(self, attr: &ast::Attribute) -> ContainerAttrsBuilder { + pub fn attr(mut self, attr: &ast::Attribute) -> Result { match attr.node.value.node { ast::MetaList(ref name, ref items) if name == &"serde" => { attr::mark_used(&attr); - items.iter().fold(self, ContainerAttrsBuilder::meta_item) + for item in items { + self = try!(self.meta_item(item)); + } + + Ok(self) } _ => { - self + Ok(self) } } } - pub fn meta_item(self, meta_item: &P) -> ContainerAttrsBuilder { + pub fn meta_item(self, meta_item: &P) -> Result { match meta_item.node { ast::MetaWord(ref name) if name == &"deny_unknown_fields" => { - self.deny_unknown_fields() + Ok(self.deny_unknown_fields()) } _ => { - // Ignore unknown meta variables for now. - self + self.cx.span_err( + meta_item.span, + &format!("unknown serde container attribute `{}`", + meta_item_to_string(meta_item))); + Err(()) } } } - pub fn deny_unknown_fields(mut self) -> ContainerAttrsBuilder { + pub fn deny_unknown_fields(mut self) -> Self { self.deny_unknown_fields = true; self } diff --git a/serde_codegen/src/de.rs b/serde_codegen/src/de.rs index 7d04e9dd..f7deccba 100644 --- a/serde_codegen/src/de.rs +++ b/serde_codegen/src/de.rs @@ -57,13 +57,13 @@ pub fn expand_derive_deserialize( .segment(item.ident).with_generics(impl_generics.clone()).build() .build(); - let body = deserialize_body( - cx, - &builder, - &item, - &impl_generics, - ty.clone(), - ); + let body = match deserialize_body(cx, &builder, &item, &impl_generics, ty.clone()) { + Ok(body) => body, + Err(()) => { + // An error occured, but it should have been reported already. + return; + } + }; let where_clause = &impl_generics.where_clause; @@ -86,8 +86,8 @@ fn deserialize_body( item: &Item, impl_generics: &ast::Generics, ty: P, -) -> P { - let container_attrs = field::container_attrs(cx, item); +) -> Result, ()> { + let container_attrs = try!(field::container_attrs(cx, item)); match item.node { ast::ItemStruct(ref variant_data, _) => { @@ -129,7 +129,7 @@ fn deserialize_item_struct( span: Span, variant_data: &ast::VariantData, container_attrs: &ContainerAttrs, -) -> P { +) -> Result, ()> { match *variant_data { ast::VariantData::Unit(_) => { deserialize_unit_struct( @@ -186,14 +186,14 @@ fn deserialize_visitor( trait_generics: &ast::Generics, forward_ty_params: Vec, forward_tys: Vec> -) -> (P, P, P, ast::Generics) { +) -> Result<(P, P, P, ast::Generics), ()> { if trait_generics.ty_params.is_empty() && forward_tys.is_empty() { - ( + Ok(( builder.item().tuple_struct("__Visitor").build(), builder.ty().id("__Visitor"), builder.expr().id("__Visitor"), trait_generics.clone(), - ) + )) } else { let placeholders : Vec<_> = trait_generics.ty_params.iter() .map(|t| builder.ty().id(t.ident)) @@ -203,7 +203,7 @@ fn deserialize_visitor( ty_params.extend(trait_generics.ty_params.into_vec()); trait_generics.ty_params = P::from_vec(ty_params); - ( + Ok(( builder.item().tuple_struct("__Visitor") .generics().with(trait_generics.clone()).build() .with_tys({ @@ -241,7 +241,7 @@ fn deserialize_visitor( }) .build(), trait_generics, - ) + )) } } @@ -265,10 +265,10 @@ fn deserialize_unit_struct( cx: &ExtCtxt, builder: &aster::AstBuilder, type_ident: Ident, -) -> P { +) -> Result, ()> { let type_name = builder.expr().str(type_ident); - quote_expr!(cx, { + Ok(quote_expr!(cx, { struct __Visitor; impl ::serde::de::Visitor for __Visitor { @@ -291,7 +291,7 @@ fn deserialize_unit_struct( } deserializer.deserialize_unit_struct($type_name, __Visitor) - }) + })) } fn deserialize_newtype_struct( @@ -300,16 +300,15 @@ fn deserialize_newtype_struct( type_ident: Ident, impl_generics: &ast::Generics, ty: P, -) -> P { +) -> Result, ()> { let where_clause = &impl_generics.where_clause; - let (visitor_item, visitor_ty, visitor_expr, visitor_generics) = - deserialize_visitor( - builder, - impl_generics, - vec![deserializer_ty_param(builder)], - vec![deserializer_ty_arg(builder)], - ); + let (visitor_item, visitor_ty, visitor_expr, visitor_generics) = try!(deserialize_visitor( + builder, + impl_generics, + vec![deserializer_ty_param(builder)], + vec![deserializer_ty_arg(builder)], + )); let visit_seq_expr = deserialize_seq( cx, @@ -320,7 +319,7 @@ fn deserialize_newtype_struct( let type_name = builder.expr().str(type_ident); - quote_expr!(cx, { + Ok(quote_expr!(cx, { $visitor_item impl $visitor_generics ::serde::de::Visitor for $visitor_ty $where_clause { @@ -343,7 +342,7 @@ fn deserialize_newtype_struct( } deserializer.deserialize_newtype_struct($type_name, $visitor_expr) - }) + })) } fn deserialize_tuple_struct( @@ -353,16 +352,15 @@ fn deserialize_tuple_struct( impl_generics: &ast::Generics, ty: P, fields: usize, -) -> P { +) -> Result, ()> { let where_clause = &impl_generics.where_clause; - let (visitor_item, visitor_ty, visitor_expr, visitor_generics) = - deserialize_visitor( - builder, - impl_generics, - vec![deserializer_ty_param(builder)], - vec![deserializer_ty_arg(builder)], - ); + let (visitor_item, visitor_ty, visitor_expr, visitor_generics) = try!(deserialize_visitor( + builder, + impl_generics, + vec![deserializer_ty_param(builder)], + vec![deserializer_ty_arg(builder)], + )); let visit_seq_expr = deserialize_seq( cx, @@ -373,7 +371,7 @@ fn deserialize_tuple_struct( let type_name = builder.expr().str(type_ident); - quote_expr!(cx, { + Ok(quote_expr!(cx, { $visitor_item impl $visitor_generics ::serde::de::Visitor for $visitor_ty $where_clause { @@ -388,7 +386,7 @@ fn deserialize_tuple_struct( } deserializer.deserialize_tuple_struct($type_name, $fields, $visitor_expr) - }) + })) } fn deserialize_seq( @@ -430,7 +428,7 @@ fn deserialize_struct_as_seq( builder: &aster::AstBuilder, struct_path: ast::Path, fields: &[ast::StructField], -) -> P { +) -> Result, ()> { let let_values: Vec> = (0 .. fields.len()) .map(|i| { let name = builder.id(format!("__field{}", i)); @@ -463,13 +461,13 @@ fn deserialize_struct_as_seq( ) .build(); - quote_expr!(cx, { + Ok(quote_expr!(cx, { $let_values try!(visitor.end()); Ok($result) - }) + })) } fn deserialize_struct( @@ -480,36 +478,36 @@ fn deserialize_struct( ty: P, fields: &[ast::StructField], container_attrs: &ContainerAttrs, -) -> P { +) -> Result, ()> { let where_clause = &impl_generics.where_clause; - let (visitor_item, visitor_ty, visitor_expr, visitor_generics) = deserialize_visitor( + let (visitor_item, visitor_ty, visitor_expr, visitor_generics) = try!(deserialize_visitor( builder, &impl_generics, vec![deserializer_ty_param(builder)], vec![deserializer_ty_arg(builder)], - ); + )); let type_path = builder.path().id(type_ident).build(); - let visit_seq_expr = deserialize_struct_as_seq( + let visit_seq_expr = try!(deserialize_struct_as_seq( cx, builder, type_path.clone(), fields, - ); + )); - let (field_visitor, fields_stmt, visit_map_expr) = deserialize_struct_visitor( + let (field_visitor, fields_stmt, visit_map_expr) = try!(deserialize_struct_visitor( cx, builder, type_path.clone(), fields, container_attrs - ); + )); let type_name = builder.expr().str(type_ident); - quote_expr!(cx, { + Ok(quote_expr!(cx, { $field_visitor $visitor_item @@ -535,7 +533,7 @@ fn deserialize_struct( $fields_stmt deserializer.deserialize_struct($type_name, FIELDS, $visitor_expr) - }) + })) } fn deserialize_item_enum( @@ -546,7 +544,7 @@ fn deserialize_item_enum( ty: P, enum_def: &EnumDef, container_attrs: &ContainerAttrs -) -> P { +) -> Result, ()> { let where_clause = &impl_generics.where_clause; let type_name = builder.expr().str(type_ident); @@ -557,7 +555,7 @@ fn deserialize_item_enum( enum_def.variants.iter() .map(|variant| { let expr = builder.expr().str(variant.node.name); - attr::FieldAttrsBuilder::new(builder) + attr::FieldAttrsBuilder::new(cx, builder) .name(expr) .build() }) @@ -585,37 +583,35 @@ fn deserialize_item_enum( }; // Match arms to extract a variant from a string - let variant_arms: Vec<_> = enum_def.variants.iter() - .enumerate() - .map(|(i, variant)| { - let variant_name = builder.pat().enum_() - .id("__Field").id(format!("__field{}", i)).build() - .build(); + let mut variant_arms = vec![]; + for (i, variant) in enum_def.variants.iter().enumerate() { + let variant_name = builder.pat().enum_() + .id("__Field").id(format!("__field{}", i)).build() + .build(); - let expr = deserialize_variant( - cx, - builder, - type_ident, - impl_generics, - ty.clone(), - variant, - container_attrs, - ); - - quote_arm!(cx, $variant_name => { $expr }) - }) - .chain(ignored_arm.into_iter()) - .collect(); - - let (visitor_item, visitor_ty, visitor_expr, visitor_generics) = - deserialize_visitor( + let expr = try!(deserialize_variant( + cx, builder, + type_ident, impl_generics, - vec![deserializer_ty_param(builder)], - vec![deserializer_ty_arg(builder)], - ); + ty.clone(), + variant, + container_attrs, + )); - quote_expr!(cx, { + let arm = quote_arm!(cx, $variant_name => { $expr }); + variant_arms.push(arm); + } + variant_arms.extend(ignored_arm.into_iter()); + + let (visitor_item, visitor_ty, visitor_expr, visitor_generics) = try!(deserialize_visitor( + builder, + impl_generics, + vec![deserializer_ty_param(builder)], + vec![deserializer_ty_arg(builder)], + )); + + Ok(quote_expr!(cx, { $variant_visitor $visitor_item @@ -635,7 +631,7 @@ fn deserialize_item_enum( $variants_stmt deserializer.deserialize_enum($type_name, VARIANTS, $visitor_expr) - }) + })) } fn deserialize_variant( @@ -646,21 +642,21 @@ fn deserialize_variant( ty: P, variant: &ast::Variant, container_attrs: &ContainerAttrs, -) -> P { +) -> Result, ()> { let variant_ident = variant.node.name; match variant.node.data { ast::VariantData::Unit(_) => { - quote_expr!(cx, { + Ok(quote_expr!(cx, { try!(visitor.visit_unit()); Ok($type_ident::$variant_ident) - }) + })) } ast::VariantData::Tuple(ref args, _) if args.len() == 1 => { - quote_expr!(cx, { + Ok(quote_expr!(cx, { let val = try!(visitor.visit_newtype()); Ok($type_ident::$variant_ident(val)) - }) + })) } ast::VariantData::Tuple(ref fields, _) => { deserialize_tuple_variant( @@ -696,16 +692,15 @@ fn deserialize_tuple_variant( generics: &ast::Generics, ty: P, fields: usize, -) -> P { +) -> Result, ()> { let where_clause = &generics.where_clause; - let (visitor_item, visitor_ty, visitor_expr, visitor_generics) = - deserialize_visitor( - builder, - generics, - vec![deserializer_ty_param(builder)], - vec![deserializer_ty_arg(builder)], - ); + let (visitor_item, visitor_ty, visitor_expr, visitor_generics) = try!(deserialize_visitor( + builder, + generics, + vec![deserializer_ty_param(builder)], + vec![deserializer_ty_arg(builder)], + )); let visit_seq_expr = deserialize_seq( cx, @@ -714,7 +709,7 @@ fn deserialize_tuple_variant( fields, ); - quote_expr!(cx, { + Ok(quote_expr!(cx, { $visitor_item impl $visitor_generics ::serde::de::Visitor for $visitor_ty $where_clause { @@ -728,7 +723,7 @@ fn deserialize_tuple_variant( } visitor.visit_tuple($fields, $visitor_expr) - }) + })) } fn deserialize_struct_variant( @@ -740,7 +735,7 @@ fn deserialize_struct_variant( ty: P, fields: &[ast::StructField], container_attrs: &ContainerAttrs, -) -> P { +) -> Result, ()> { let where_clause = &generics.where_clause; let type_path = builder.path() @@ -748,30 +743,29 @@ fn deserialize_struct_variant( .id(variant_ident) .build(); - let visit_seq_expr = deserialize_struct_as_seq( + let visit_seq_expr = try!(deserialize_struct_as_seq( cx, builder, type_path.clone(), fields, - ); + )); - let (field_visitor, fields_stmt, field_expr) = deserialize_struct_visitor( + let (field_visitor, fields_stmt, field_expr) = try!(deserialize_struct_visitor( cx, builder, type_path, fields, container_attrs, - ); + )); - let (visitor_item, visitor_ty, visitor_expr, visitor_generics) = - deserialize_visitor( - builder, - generics, - vec![deserializer_ty_param(builder)], - vec![deserializer_ty_arg(builder)], - ); + let (visitor_item, visitor_ty, visitor_expr, visitor_generics) = try!(deserialize_visitor( + builder, + generics, + vec![deserializer_ty_param(builder)], + vec![deserializer_ty_arg(builder)], + )); - quote_expr!(cx, { + Ok(quote_expr!(cx, { $field_visitor $visitor_item @@ -797,7 +791,7 @@ fn deserialize_struct_variant( $fields_stmt visitor.visit_struct(FIELDS, $visitor_expr) - }) + })) } fn deserialize_field_visitor( @@ -969,21 +963,21 @@ fn deserialize_struct_visitor( struct_path: ast::Path, fields: &[ast::StructField], container_attrs: &ContainerAttrs, -) -> (Vec>, P, P) { +) -> Result<(Vec>, P, P), ()> { let field_visitor = deserialize_field_visitor( cx, builder, - field::struct_field_attrs(cx, builder, fields), + try!(field::struct_field_attrs(cx, builder, fields)), container_attrs ); - let visit_map_expr = deserialize_map( + let visit_map_expr = try!(deserialize_map( cx, builder, struct_path, fields, container_attrs, - ); + )); let fields_expr = builder.expr().addr_of().slice() .with_exprs( @@ -1003,7 +997,7 @@ fn deserialize_struct_visitor( const FIELDS: &'static [&'static str] = $fields_expr; ).unwrap(); - (field_visitor, fields_stmt, visit_map_expr) + Ok((field_visitor, fields_stmt, visit_map_expr)) } fn deserialize_map( @@ -1012,7 +1006,7 @@ fn deserialize_map( struct_path: ast::Path, fields: &[ast::StructField], container_attrs: &ContainerAttrs, -) -> P { +) -> Result, ()> { // Create the field names for the fields. let field_names: Vec = (0 .. fields.len()) .map(|i| builder.id(format!("__field{}", i))) @@ -1045,8 +1039,10 @@ fn deserialize_map( .chain(ignored_arm.into_iter()) .collect(); + let field_attrs = try!(field::struct_field_attrs(cx, builder, fields)); + let extract_values: Vec> = field_names.iter() - .zip(field::struct_field_attrs(cx, builder, fields).iter()) + .zip(field_attrs.iter()) .map(|(field_name, field_attr)| { let missing_expr = if field_attr.use_default() { quote_expr!(cx, ::std::default::Default::default()) @@ -1099,7 +1095,7 @@ fn deserialize_map( ) .build(); - quote_expr!(cx, { + Ok(quote_expr!(cx, { $let_values while let Some(key) = try!(visitor.visit_key()) { @@ -1113,5 +1109,5 @@ fn deserialize_map( try!(visitor.end()); Ok($result) - }) + })) } diff --git a/serde_codegen/src/field.rs b/serde_codegen/src/field.rs index a0a992f5..43fe4f90 100644 --- a/serde_codegen/src/field.rs +++ b/serde_codegen/src/field.rs @@ -2,23 +2,29 @@ use syntax::ast; use syntax::ext::base::ExtCtxt; use aster; -use attr::{ContainerAttrs, ContainerAttrsBuilder, FieldAttrs, FieldAttrsBuilder}; +use attr; pub fn struct_field_attrs( - _cx: &ExtCtxt, + cx: &ExtCtxt, builder: &aster::AstBuilder, fields: &[ast::StructField], -) -> Vec { - fields.iter() - .map(|field| { - FieldAttrsBuilder::new(builder).field(field).build() - }) - .collect() +) -> Result, ()> { + let mut attrs = vec![]; + for field in fields { + let builder = attr::FieldAttrsBuilder::new(cx, builder); + let builder = try!(builder.field(field)); + let attr = builder.build(); + attrs.push(attr); + } + + Ok(attrs) } pub fn container_attrs( - _cx: &ExtCtxt, + cx: &ExtCtxt, container: &ast::Item, -) -> ContainerAttrs { - ContainerAttrsBuilder::new().attrs(container.attrs()).build() -} \ No newline at end of file +) -> Result { + let builder = attr::ContainerAttrsBuilder::new(cx); + let builder = try!(builder.attrs(container.attrs())); + Ok(builder.build()) +} diff --git a/serde_codegen/src/lib.rs b/serde_codegen/src/lib.rs index af94ac5d..83e768a6 100644 --- a/serde_codegen/src/lib.rs +++ b/serde_codegen/src/lib.rs @@ -1,3 +1,5 @@ +#![cfg_attr(feature = "nightly", plugin(clippy))] +#![cfg_attr(feature = "nightly", allow(used_underscore_binding))] #![cfg_attr(not(feature = "with-syntex"), feature(rustc_private, plugin))] #![cfg_attr(not(feature = "with-syntex"), plugin(quasi_macros))] @@ -16,6 +18,9 @@ extern crate syntax; #[cfg(not(feature = "with-syntex"))] extern crate rustc_plugin; +#[cfg(not(feature = "with-syntex"))] +use syntax::feature_gate::AttributeType; + #[cfg(feature = "with-syntex")] include!(concat!(env!("OUT_DIR"), "/lib.rs")); @@ -70,4 +75,6 @@ pub fn register(reg: &mut rustc_plugin::Registry) { syntax::parse::token::intern("derive_Deserialize"), syntax::ext::base::MultiDecorator( Box::new(de::expand_derive_deserialize))); + + reg.register_attribute("serde".to_owned(), AttributeType::Normal); } diff --git a/serde_codegen/src/ser.rs b/serde_codegen/src/ser.rs index f42f7a0d..f8c63649 100644 --- a/serde_codegen/src/ser.rs +++ b/serde_codegen/src/ser.rs @@ -11,7 +11,7 @@ use syntax::ext::base::{Annotatable, ExtCtxt}; use syntax::ext::build::AstBuilder; use syntax::ptr::P; -use field::struct_field_attrs; +use field; pub fn expand_derive_serialize( cx: &mut ExtCtxt, @@ -53,13 +53,13 @@ pub fn expand_derive_serialize( .segment(item.ident).with_generics(impl_generics.clone()).build() .build(); - let body = serialize_body( - cx, - &builder, - &item, - &impl_generics, - ty.clone(), - ); + let body = match serialize_body(cx, &builder, &item, &impl_generics, ty.clone()) { + Ok(body) => body, + Err(()) => { + // An error occured, but it should have been reported already. + return; + } + }; let where_clause = &impl_generics.where_clause; @@ -82,7 +82,11 @@ fn serialize_body( item: &Item, impl_generics: &ast::Generics, ty: P, -) -> P { +) -> Result, ()> { + // Note: While we don't have any container attributes, we still want to try to + // parse them so we can report a proper error if we get passed an unknown attribute. + let _ = try!(field::container_attrs(cx, item)); + match item.node { ast::ItemStruct(ref variant_data, _) => { serialize_item_struct( @@ -107,7 +111,7 @@ fn serialize_body( } _ => { cx.span_bug(item.span, - "expected ItemStruct or ItemEnum in #[derive(Serialize)]") + "expected ItemStruct or ItemEnum in #[derive(Serialize)]"); } } } @@ -120,7 +124,7 @@ fn serialize_item_struct( ty: P, span: Span, variant_data: &ast::VariantData, -) -> P { +) -> Result, ()> { match *variant_data { ast::VariantData::Unit(_) => { serialize_unit_struct( @@ -171,20 +175,24 @@ fn serialize_unit_struct( cx: &ExtCtxt, builder: &aster::AstBuilder, type_ident: Ident -) -> P { +) -> Result, ()> { let type_name = builder.expr().str(type_ident); - quote_expr!(cx, serializer.serialize_unit_struct($type_name)) + Ok(quote_expr!(cx, + serializer.serialize_unit_struct($type_name) + )) } fn serialize_newtype_struct( cx: &ExtCtxt, builder: &aster::AstBuilder, type_ident: Ident -) -> P { +) -> Result, ()> { let type_name = builder.expr().str(type_ident); - quote_expr!(cx, serializer.serialize_newtype_struct($type_name, &self.0)) + Ok(quote_expr!(cx, + serializer.serialize_newtype_struct($type_name, &self.0) + )) } fn serialize_tuple_struct( @@ -194,7 +202,7 @@ fn serialize_tuple_struct( impl_generics: &ast::Generics, ty: P, fields: usize, -) -> P { +) -> Result, ()> { let (visitor_struct, visitor_impl) = serialize_tuple_struct_visitor( cx, builder, @@ -209,7 +217,7 @@ fn serialize_tuple_struct( let type_name = builder.expr().str(type_ident); - quote_expr!(cx, { + Ok(quote_expr!(cx, { $visitor_struct $visitor_impl serializer.serialize_tuple_struct($type_name, Visitor { @@ -217,7 +225,7 @@ fn serialize_tuple_struct( state: 0, _structure_ty: ::std::marker::PhantomData::<&$ty>, }) - }) + })) } fn serialize_struct( @@ -227,8 +235,13 @@ fn serialize_struct( impl_generics: &ast::Generics, ty: P, fields: &[ast::StructField], -) -> P { - let (visitor_struct, visitor_impl) = serialize_struct_visitor( +) -> Result, ()> { + let value_exprs = fields.iter().map(|field| { + let name = field.node.ident().expect("struct has unnamed field"); + quote_expr!(cx, &self.value.$name) + }); + + let (visitor_struct, visitor_impl) = try!(serialize_struct_visitor( cx, builder, ty.clone(), @@ -238,15 +251,12 @@ fn serialize_struct( .build_ty(ty.clone()), fields, impl_generics, - fields.iter().map(|field| { - let name = field.node.ident().expect("struct has unnamed field"); - quote_expr!(cx, &self.value.$name) - }) - ); + value_exprs, + )); let type_name = builder.expr().str(type_ident); - quote_expr!(cx, { + Ok(quote_expr!(cx, { $visitor_struct $visitor_impl serializer.serialize_struct($type_name, Visitor { @@ -254,7 +264,7 @@ fn serialize_struct( state: 0, _structure_ty: ::std::marker::PhantomData::<&$ty>, }) - }) + })) } fn serialize_item_enum( @@ -264,27 +274,28 @@ fn serialize_item_enum( impl_generics: &ast::Generics, ty: P, enum_def: &ast::EnumDef, -) -> P { - let arms: Vec = enum_def.variants.iter() - .enumerate() - .map(|(variant_index, variant)| { - serialize_variant( - cx, - builder, - type_ident, - impl_generics, - ty.clone(), - variant, - variant_index, - ) - }) - .collect(); +) -> Result, ()> { + let mut arms = vec![]; - quote_expr!(cx, + for (variant_index, variant) in enum_def.variants.iter().enumerate() { + let arm = try!(serialize_variant( + cx, + builder, + type_ident, + impl_generics, + ty.clone(), + variant, + variant_index, + )); + + arms.push(arm); + } + + Ok(quote_expr!(cx, match *self { $arms } - ) + )) } fn serialize_variant( @@ -295,7 +306,7 @@ fn serialize_variant( ty: P, variant: &ast::Variant, variant_index: usize, -) -> ast::Arm { +) -> Result { let type_name = builder.expr().str(type_ident); let variant_ident = variant.node.name; let variant_name = builder.expr().str(variant_ident); @@ -306,7 +317,7 @@ fn serialize_variant( .id(type_ident).id(variant_ident).build() .build(); - quote_arm!(cx, + Ok(quote_arm!(cx, $pat => { ::serde::ser::Serializer::serialize_unit_variant( serializer, @@ -315,7 +326,7 @@ fn serialize_variant( $variant_name, ) } - ) + )) }, ast::VariantData::Tuple(ref fields, _) if fields.len() == 1 => { let field = builder.id("__simple_value"); @@ -324,7 +335,8 @@ fn serialize_variant( .id(type_ident).id(variant_ident).build() .with_pats(Some(field).into_iter()) .build(); - quote_arm!(cx, + + Ok(quote_arm!(cx, $pat => { ::serde::ser::Serializer::serialize_newtype_variant( serializer, @@ -334,7 +346,7 @@ fn serialize_variant( __simple_value, ) } - ) + )) }, ast::VariantData::Tuple(ref fields, _) => { let field_names: Vec = (0 .. fields.len()) @@ -361,7 +373,9 @@ fn serialize_variant( field_names, ); - quote_arm!(cx, $pat => { $expr }) + Ok(quote_arm!(cx, + $pat => { $expr } + )) } ast::VariantData::Struct(ref fields, _) => { let field_names: Vec<_> = (0 .. fields.len()) @@ -386,7 +400,7 @@ fn serialize_variant( ) .build(); - let expr = serialize_struct_variant( + let expr = try!(serialize_struct_variant( cx, builder, type_name, @@ -396,9 +410,11 @@ fn serialize_variant( ty, fields, field_names, - ); + )); - quote_arm!(cx, $pat => { $expr }) + Ok(quote_arm!(cx, + $pat => { $expr } + )) } } } @@ -463,7 +479,7 @@ fn serialize_struct_variant( structure_ty: P, fields: &[ast::StructField], field_names: Vec, -) -> P { +) -> Result, ()> { let value_ty = builder.ty().tuple() .with_tys( fields.iter().map(|field| { @@ -483,7 +499,7 @@ fn serialize_struct_variant( ) .build(); - let (visitor_struct, visitor_impl) = serialize_struct_visitor( + let (visitor_struct, visitor_impl) = try!(serialize_struct_visitor( cx, builder, structure_ty.clone(), @@ -495,9 +511,9 @@ fn serialize_struct_variant( .tup_field(i) .field("value").self_() }) - ); + )); - quote_expr!(cx, { + Ok(quote_expr!(cx, { $visitor_struct $visitor_impl serializer.serialize_struct_variant($type_name, $variant_index, $variant_name, Visitor { @@ -505,7 +521,7 @@ fn serialize_struct_variant( state: 0, _structure_ty: ::std::marker::PhantomData::<&$structure_ty>, }) - }) + })) } fn serialize_tuple_struct_visitor( @@ -583,12 +599,12 @@ fn serialize_struct_visitor( fields: &[ast::StructField], generics: &ast::Generics, value_exprs: I, -) -> (P, P) +) -> Result<(P, P), ()> where I: Iterator>, { let value_exprs = value_exprs.collect::>(); - let field_attrs = struct_field_attrs(cx, builder, fields); + let field_attrs = try!(field::struct_field_attrs(cx, builder, fields)); let arms: Vec = field_attrs.iter() .zip(value_exprs.iter()) @@ -651,7 +667,7 @@ fn serialize_struct_visitor( }) .fold(quote_expr!(cx, 0), |sum, expr| quote_expr!(cx, $sum + $expr)); - ( + Ok(( quote_item!(cx, struct Visitor $visitor_impl_generics $where_clause { state: usize, @@ -683,5 +699,5 @@ fn serialize_struct_visitor( } } ).unwrap(), - ) + )) } diff --git a/serde_macros/Cargo.toml b/serde_macros/Cargo.toml index 93b01fc0..8845c072 100644 --- a/serde_macros/Cargo.toml +++ b/serde_macros/Cargo.toml @@ -13,9 +13,11 @@ name = "serde_macros" plugin = true [dependencies] +clippy = "^0.0.35" serde_codegen = { version = "*", path = "../serde_codegen", default-features = false, features = ["nightly"] } [dev-dependencies] +compiletest_rs = "^0.0.11" num = "^0.1.27" rustc-serialize = "^0.3.16" serde = { version = "*", path = "../serde", features = ["nightly", "num-impls"] } diff --git a/serde_macros/benches/bench.rs b/serde_macros/benches/bench.rs index 519c189f..7b545011 100644 --- a/serde_macros/benches/bench.rs +++ b/serde_macros/benches/bench.rs @@ -1,4 +1,5 @@ #![feature(custom_attribute, custom_derive, plugin, test)] +#![plugin(clippy)] #![plugin(serde_macros)] extern crate num; diff --git a/serde_macros/src/lib.rs b/serde_macros/src/lib.rs index 17e39bb4..edd2addc 100644 --- a/serde_macros/src/lib.rs +++ b/serde_macros/src/lib.rs @@ -1,4 +1,5 @@ -#![feature(plugin_registrar, rustc_private)] +#![feature(plugin, plugin_registrar, rustc_private)] +#![plugin(clippy)] extern crate serde_codegen; extern crate rustc_plugin; diff --git a/serde_macros/tests/compile-fail/reject-unknown-attributes.rs b/serde_macros/tests/compile-fail/reject-unknown-attributes.rs new file mode 100644 index 00000000..5a4d2776 --- /dev/null +++ b/serde_macros/tests/compile-fail/reject-unknown-attributes.rs @@ -0,0 +1,30 @@ +#![feature(custom_attribute, custom_derive, plugin)] +#![plugin(serde_macros)] + +extern crate serde; + +#[derive(Serialize)] +#[serde(abc="xyz")] //~ unknown serde container attribute `abc = "xyz"` +struct Foo { + x: u32, +} + +#[derive(Deserialize)] +#[serde(abc="xyz")] //~ unknown serde container attribute `abc = "xyz"` +struct Foo { + x: u32, +} + +#[derive(Serialize)] +struct Foo { + #[serde(abc="xyz")] //~ unknown serde field attribute `abc = "xyz"` + x: u32, +} + +#[derive(Deserialize)] +struct Foo { + #[serde(abc="xyz")] //~ unknown serde field attribute `abc = "xyz"` + x: u32, +} + +fn main() { } diff --git a/serde_macros/tests/compile_tests.rs b/serde_macros/tests/compile_tests.rs new file mode 100644 index 00000000..27c212b4 --- /dev/null +++ b/serde_macros/tests/compile_tests.rs @@ -0,0 +1,25 @@ +extern crate compiletest_rs as compiletest; + +use std::path::PathBuf; +use std::env::var; + +fn run_mode(mode: &'static str) { + let mut config = compiletest::default_config(); + + let cfg_mode = mode.parse().ok().expect("Invalid mode"); + + config.target_rustcflags = Some("-L target/debug/ -L target/debug/deps/".to_owned()); + if let Ok(name) = var::<&str>("TESTNAME") { + let s : String = name.to_owned(); + config.filter = Some(s) + } + config.mode = cfg_mode; + config.src_base = PathBuf::from(format!("tests/{}", mode)); + + compiletest::run_tests(&config); +} + +#[test] +fn compile_test() { + run_mode("compile-fail"); +} diff --git a/serde_macros/tests/test.rs b/serde_macros/tests/test.rs index 26ab0042..2f2af797 100644 --- a/serde_macros/tests/test.rs +++ b/serde_macros/tests/test.rs @@ -6,3 +6,5 @@ extern crate serde; extern crate test; include!("../../serde_tests/tests/test.rs.in"); + +mod compile_tests; diff --git a/serde_tests/Cargo.toml b/serde_tests/Cargo.toml index 4e0dba9a..16f8d75b 100644 --- a/serde_tests/Cargo.toml +++ b/serde_tests/Cargo.toml @@ -10,6 +10,9 @@ readme = "README.md" keywords = ["serialization"] build = "build.rs" +[features] +nightly = ["clippy", "serde/nightly"] + [build-dependencies] syntex = { version = "^0.26.0" } syntex_syntax = { version = "^0.26.0" } @@ -21,6 +24,9 @@ rustc-serialize = "^0.3.16" serde = { version = "*", path = "../serde", features = ["num-impls"] } syntex = "^0.26.0" +[dependencies] +clippy = { version = "^0.0.35", optional = true } + [[test]] name = "test" path = "tests/test.rs" diff --git a/serde_tests/benches/bench.rs b/serde_tests/benches/bench.rs index 181c4c61..db86c6a6 100644 --- a/serde_tests/benches/bench.rs +++ b/serde_tests/benches/bench.rs @@ -1,4 +1,6 @@ #![feature(test)] +#![cfg_attr(feature = "nightly", feature(plugin))] +#![cfg_attr(feature = "nightly", plugin(clippy))] extern crate num; extern crate rustc_serialize; diff --git a/serde_tests/benches/bench_enum.rs b/serde_tests/benches/bench_enum.rs index 1f6d5124..ecd3114b 100644 --- a/serde_tests/benches/bench_enum.rs +++ b/serde_tests/benches/bench_enum.rs @@ -433,7 +433,7 @@ fn bench_decoder_dog(b: &mut Bencher) { #[bench] fn bench_decoder_frog(b: &mut Bencher) { b.iter(|| { - let animal = Animal::Frog("Henry".to_string(), 349); + let animal = Animal::Frog("Henry".to_owned(), 349); let mut d = decoder::AnimalDecoder::new(animal.clone()); let value: Animal = Decodable::decode(&mut d).unwrap(); @@ -457,7 +457,7 @@ fn bench_deserializer_dog(b: &mut Bencher) { #[bench] fn bench_deserializer_frog(b: &mut Bencher) { b.iter(|| { - let animal = Animal::Frog("Henry".to_string(), 349); + let animal = Animal::Frog("Henry".to_owned(), 349); let mut d = deserializer::AnimalDeserializer::new(animal.clone()); let value: Animal = Deserialize::deserialize(&mut d).unwrap(); diff --git a/serde_tests/benches/bench_struct.rs b/serde_tests/benches/bench_struct.rs index 5fef379c..03ae4310 100644 --- a/serde_tests/benches/bench_struct.rs +++ b/serde_tests/benches/bench_struct.rs @@ -632,7 +632,7 @@ mod deserializer { fn bench_decoder_0_0(b: &mut Bencher) { b.iter(|| { let mut map = HashMap::new(); - map.insert("abc".to_string(), Some('c')); + map.insert("abc".to_owned(), Some('c')); let outer = Outer { inner: vec!(), @@ -671,11 +671,11 @@ fn bench_decoder_1_0(b: &mut Bencher) { fn bench_decoder_1_5(b: &mut Bencher) { b.iter(|| { let mut map = HashMap::new(); - map.insert("1".to_string(), Some('a')); - map.insert("2".to_string(), None); - map.insert("3".to_string(), Some('b')); - map.insert("4".to_string(), None); - map.insert("5".to_string(), Some('c')); + map.insert("1".to_owned(), Some('a')); + map.insert("2".to_owned(), None); + map.insert("3".to_owned(), Some('b')); + map.insert("4".to_owned(), None); + map.insert("5".to_owned(), Some('c')); let outer = Outer { inner: vec!( @@ -734,11 +734,11 @@ fn bench_deserializer_1_0(b: &mut Bencher) { fn bench_deserializer_1_5(b: &mut Bencher) { b.iter(|| { let mut map = HashMap::new(); - map.insert("1".to_string(), Some('a')); - map.insert("2".to_string(), None); - map.insert("3".to_string(), Some('b')); - map.insert("4".to_string(), None); - map.insert("5".to_string(), Some('c')); + map.insert("1".to_owned(), Some('a')); + map.insert("2".to_owned(), None); + map.insert("3".to_owned(), Some('b')); + map.insert("4".to_owned(), None); + map.insert("5".to_owned(), Some('c')); let outer = Outer { inner: vec!( diff --git a/serde_tests/tests/test.rs b/serde_tests/tests/test.rs index 416704ce..7fefeebc 100644 --- a/serde_tests/tests/test.rs +++ b/serde_tests/tests/test.rs @@ -1,3 +1,6 @@ +#![cfg_attr(feature = "nightly", feature(plugin))] +#![cfg_attr(feature = "nightly", plugin(clippy))] + extern crate num; extern crate serde; diff --git a/serde_tests/tests/test_de.rs b/serde_tests/tests/test_de.rs index 9e0b2f83..0655a493 100644 --- a/serde_tests/tests/test_de.rs +++ b/serde_tests/tests/test_de.rs @@ -99,12 +99,12 @@ declare_tests! { test_char { 'a' => vec![Token::Char('a')], 'a' => vec![Token::Str("a")], - 'a' => vec![Token::String("a".to_string())], + 'a' => vec![Token::String("a".to_owned())], } test_string { - "abc".to_string() => vec![Token::Str("abc")], - "abc".to_string() => vec![Token::String("abc".to_string())], - "a".to_string() => vec![Token::Char('a')], + "abc".to_owned() => vec![Token::Str("abc")], + "abc".to_owned() => vec![Token::String("abc".to_owned())], + "a".to_owned() => vec![Token::Char('a')], } test_option { None:: => vec![Token::Unit], diff --git a/serde_tests/tests/test_ser.rs b/serde_tests/tests/test_ser.rs index 6a454814..a5947508 100644 --- a/serde_tests/tests/test_ser.rs +++ b/serde_tests/tests/test_ser.rs @@ -63,7 +63,7 @@ declare_ser_tests! { } test_str { "abc" => &[Token::Str("abc")], - "abc".to_string() => &[Token::Str("abc")], + "abc".to_owned() => &[Token::Str("abc")], } test_option { None:: => &[Token::Option(false)], diff --git a/serde_tests/tests/token.rs b/serde_tests/tests/token.rs index c9e6ef78..43502567 100644 --- a/serde_tests/tests/token.rs +++ b/serde_tests/tests/token.rs @@ -326,7 +326,7 @@ impl de::Error for Error { fn end_of_stream() -> Error { Error::EndOfStreamError } fn unknown_field(field: &str) -> Error { - Error::UnknownFieldError(field.to_string()) + Error::UnknownFieldError(field.to_owned()) } fn missing_field(field: &'static str) -> Error { @@ -396,7 +396,6 @@ impl de::Deserializer for Deserializer fn deserialize(&mut self, mut visitor: V) -> Result where V: de::Visitor, { - println!("visit {:?}", self.tokens.peek()); match self.tokens.next() { Some(Token::Bool(v)) => visitor.visit_bool(v), Some(Token::Isize(v)) => visitor.visit_isize(v),