From 23ac36b559369ec6face09584c31bb96f0c58422 Mon Sep 17 00:00:00 2001 From: rsdy Date: Mon, 26 Sep 2022 11:55:16 +0100 Subject: [PATCH] Review follow-ups --- instant-xml-macros/src/case.rs | 44 ++++++++++++++++++++-------------- instant-xml-macros/src/de.rs | 7 ++++-- instant-xml-macros/src/lib.rs | 24 +++++++++++-------- instant-xml-macros/src/ser.rs | 8 +++++-- 4 files changed, 51 insertions(+), 32 deletions(-) diff --git a/instant-xml-macros/src/case.rs b/instant-xml-macros/src/case.rs index be59846..a6191f7 100644 --- a/instant-xml-macros/src/case.rs +++ b/instant-xml-macros/src/case.rs @@ -8,6 +8,10 @@ use std::ascii::AsciiExt; use std::fmt::{self, Debug, Display}; +use proc_macro2::Ident; +#[cfg(test)] +use proc_macro2::Span; + use self::RenameRule::*; /// The different possible ways to change case of fields in a struct, or variants in an enum. @@ -42,7 +46,7 @@ impl Default for RenameRule { } } -static RENAME_RULES: &[(&str, RenameRule)] = &[ +const RENAME_RULES: &[(&str, RenameRule)] = &[ ("\"lowercase\"", LowerCase), ("\"UPPERCASE\"", UpperCase), ("\"PascalCase\"", PascalCase), @@ -66,9 +70,10 @@ impl RenameRule { } /// Apply a renaming rule to an enum variant, returning the version expected in the source. - pub fn apply_to_variant(&self, variant: &str) -> String { + pub fn apply_to_variant(&self, ident: &Ident) -> String { + let variant = ident.to_string(); match *self { - None | PascalCase => variant.to_owned(), + None | PascalCase => variant, LowerCase => variant.to_ascii_lowercase(), UpperCase => variant.to_ascii_uppercase(), CamelCase => variant[..1].to_ascii_lowercase() + &variant[1..], @@ -82,18 +87,17 @@ impl RenameRule { } snake } - ScreamingSnakeCase => SnakeCase.apply_to_variant(variant).to_ascii_uppercase(), - KebabCase => SnakeCase.apply_to_variant(variant).replace('_', "-"), - ScreamingKebabCase => ScreamingSnakeCase - .apply_to_variant(variant) - .replace('_', "-"), + ScreamingSnakeCase => SnakeCase.apply_to_variant(ident).to_ascii_uppercase(), + KebabCase => SnakeCase.apply_to_variant(ident).replace('_', "-"), + ScreamingKebabCase => ScreamingSnakeCase.apply_to_variant(ident).replace('_', "-"), } } /// Apply a renaming rule to a struct field, returning the version expected in the source. - pub fn apply_to_field(&self, field: &str) -> String { + pub fn apply_to_field(&self, ident: &Ident) -> String { + let field = ident.to_string(); match *self { - None | LowerCase | SnakeCase => field.to_owned(), + None | LowerCase | SnakeCase => field, UpperCase => field.to_ascii_uppercase(), PascalCase => { let mut pascal = String::new(); @@ -111,12 +115,12 @@ impl RenameRule { pascal } CamelCase => { - let pascal = PascalCase.apply_to_field(field); + let pascal = PascalCase.apply_to_field(ident); pascal[..1].to_ascii_lowercase() + &pascal[1..] } ScreamingSnakeCase => field.to_ascii_uppercase(), KebabCase => field.replace('_', "-"), - ScreamingKebabCase => ScreamingSnakeCase.apply_to_field(field).replace('_', "-"), + ScreamingKebabCase => ScreamingSnakeCase.apply_to_field(ident).replace('_', "-"), } } } @@ -142,7 +146,7 @@ impl<'a> Display for ParseError<'a> { #[test] fn rename_variants() { - for &(original, lower, upper, camel, snake, screaming, kebab, screaming_kebab) in &[ + for &(name, lower, upper, camel, snake, screaming, kebab, screaming_kebab) in &[ ( "Outcome", "outcome", "OUTCOME", "outcome", "outcome", "OUTCOME", "outcome", "OUTCOME", ), @@ -159,10 +163,12 @@ fn rename_variants() { ("A", "a", "A", "a", "a", "A", "a", "A"), ("Z42", "z42", "Z42", "z42", "z42", "Z42", "z42", "Z42"), ] { - assert_eq!(None.apply_to_variant(original), original); + let original = &Ident::new(name, Span::call_site()); + + assert_eq!(None.apply_to_variant(original), name); assert_eq!(LowerCase.apply_to_variant(original), lower); assert_eq!(UpperCase.apply_to_variant(original), upper); - assert_eq!(PascalCase.apply_to_variant(original), original); + assert_eq!(PascalCase.apply_to_variant(original), name); assert_eq!(CamelCase.apply_to_variant(original), camel); assert_eq!(SnakeCase.apply_to_variant(original), snake); assert_eq!(ScreamingSnakeCase.apply_to_variant(original), screaming); @@ -176,7 +182,7 @@ fn rename_variants() { #[test] fn rename_fields() { - for &(original, upper, pascal, camel, screaming, kebab, screaming_kebab) in &[ + for &(name, upper, pascal, camel, screaming, kebab, screaming_kebab) in &[ ( "outcome", "OUTCOME", "Outcome", "outcome", "OUTCOME", "outcome", "OUTCOME", ), @@ -192,11 +198,13 @@ fn rename_fields() { ("a", "A", "A", "a", "A", "a", "A"), ("z42", "Z42", "Z42", "z42", "Z42", "z42", "Z42"), ] { - assert_eq!(None.apply_to_field(original), original); + let original = &Ident::new(name, Span::call_site()); + + assert_eq!(None.apply_to_field(original), name); assert_eq!(UpperCase.apply_to_field(original), upper); assert_eq!(PascalCase.apply_to_field(original), pascal); assert_eq!(CamelCase.apply_to_field(original), camel); - assert_eq!(SnakeCase.apply_to_field(original), original); + assert_eq!(SnakeCase.apply_to_field(original), name); assert_eq!(ScreamingSnakeCase.apply_to_field(original), screaming); assert_eq!(KebabCase.apply_to_field(original), kebab); assert_eq!(ScreamingKebabCase.apply_to_field(original), screaming_kebab); diff --git a/instant-xml-macros/src/de.rs b/instant-xml-macros/src/de.rs index 4ab418d..2d59c8c 100644 --- a/instant-xml-macros/src/de.rs +++ b/instant-xml-macros/src/de.rs @@ -6,7 +6,10 @@ use super::{discard_lifetimes, ContainerMeta, FieldMeta, Namespace, VariantMeta} pub(crate) fn from_xml(input: &syn::DeriveInput) -> TokenStream { let ident = &input.ident; - let meta = ContainerMeta::from_derive(input); + let meta = match ContainerMeta::from_derive(input) { + Ok(meta) => meta, + Err(e) => return e.to_compile_error(), + }; match &input.data { syn::Data::Struct(_) if meta.scalar => { @@ -224,7 +227,7 @@ fn process_field( Some(rename) => quote!(#rename), None => container_meta .rename_all - .apply_to_field(&field_name.to_string()) + .apply_to_field(field_name) .into_token_stream(), }; diff --git a/instant-xml-macros/src/lib.rs b/instant-xml-macros/src/lib.rs index a24d90d..b2c8ef1 100644 --- a/instant-xml-macros/src/lib.rs +++ b/instant-xml-macros/src/lib.rs @@ -37,23 +37,28 @@ struct ContainerMeta { } impl ContainerMeta { - fn from_derive(input: &syn::DeriveInput) -> ContainerMeta { + fn from_derive(input: &syn::DeriveInput) -> Result { let mut meta = ContainerMeta::default(); for item in meta_items(&input.attrs) { match item { - MetaItem::Attribute => panic!("attribute key invalid in container xml attribute"), + MetaItem::Attribute => { + return Err(syn::Error::new( + input.span(), + "attribute key invalid in container xml attribute", + )) + } MetaItem::Ns(ns) => meta.ns = ns, MetaItem::Rename(lit) => meta.rename = Some(lit), MetaItem::RenameAll(lit) => { meta.rename_all = match RenameRule::from_str(&lit.to_string()) { Ok(rule) => rule, - Err(err) => panic!("{err}"), + Err(err) => return Err(syn::Error::new(input.span(), err)), }; } MetaItem::Scalar => meta.scalar = true, } } - meta + Ok(meta) } } @@ -175,7 +180,7 @@ impl VariantMeta { Some(lit) => lit.into_token_stream(), None => container .rename_all - .apply_to_variant(&input.ident.to_string()) + .apply_to_variant(&input.ident) .to_token_stream(), }; @@ -491,13 +496,13 @@ fn meta_items(attrs: &[syn::Attribute]) -> Vec { (MetaState::Rename, TokenTree::Punct(punct)) if punct.as_char() == '=' => { MetaState::RenameValue } - (MetaState::RenameAll, TokenTree::Punct(punct)) if punct.as_char() == '=' => { - MetaState::RenameAllValue - } (MetaState::RenameValue, TokenTree::Literal(lit)) => { items.push(MetaItem::Rename(lit)); MetaState::Comma } + (MetaState::RenameAll, TokenTree::Punct(punct)) if punct.as_char() == '=' => { + MetaState::RenameAllValue + } (MetaState::RenameAllValue, TokenTree::Literal(lit)) => { items.push(MetaItem::RenameAll(lit)); MetaState::Comma @@ -810,7 +815,6 @@ mod tests { #[test] #[rustfmt::skip] - #[should_panic(expected = "unknown rename rule `rename_all = \"\\\"forgetaboutit\\\"\"`, expected one of \"\\\"lowercase\\\"\", \"\\\"UPPERCASE\\\"\", \"\\\"PascalCase\\\"\", \"\\\"camelCase\\\"\", \"\\\"snake_case\\\"\", \"\\\"SCREAMING_SNAKE_CASE\\\"\", \"\\\"kebab-case\\\"\", \"\\\"SCREAMING-KEBAB-CASE\\\"\"")] fn bogus_rename_all_not_permitted() { super::ser::to_xml(&parse_quote! { #[xml(rename_all = "forgetaboutit")] @@ -818,6 +822,6 @@ mod tests { field_1: String, field_2: u8, } - }).to_string().find("compile_error ! { \"attribute 'rename_all' invalid in field xml attribute\" }").unwrap(); + }).to_string().find("compile_error ! {").unwrap(); } } diff --git a/instant-xml-macros/src/ser.rs b/instant-xml-macros/src/ser.rs index 2b9d03c..d4b0ca1 100644 --- a/instant-xml-macros/src/ser.rs +++ b/instant-xml-macros/src/ser.rs @@ -7,7 +7,11 @@ use crate::Namespace; use super::{discard_lifetimes, ContainerMeta, FieldMeta, VariantMeta}; pub fn to_xml(input: &syn::DeriveInput) -> proc_macro2::TokenStream { - let meta = ContainerMeta::from_derive(input); + let meta = match ContainerMeta::from_derive(input) { + Ok(meta) => meta, + Err(e) => return e.to_compile_error(), + }; + match &input.data { syn::Data::Struct(_) if meta.scalar => { syn::Error::new(input.span(), "scalar structs are unsupported!").to_compile_error() @@ -151,7 +155,7 @@ fn process_named_field( Some(rename) => quote!(#rename), None => meta .rename_all - .apply_to_field(&field_name.to_string()) + .apply_to_field(field_name) .into_token_stream(), };