Review follow-ups

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(),
+            ScreamingSnakeCase => SnakeCase.apply_to_variant(ident).to_ascii_uppercase(),
-            KebabCase => SnakeCase.apply_to_variant(variant).replace('_', "-"),
+            KebabCase => SnakeCase.apply_to_variant(ident).replace('_', "-"),
-            ScreamingKebabCase => ScreamingSnakeCase
+            ScreamingKebabCase => ScreamingSnakeCase.apply_to_variant(ident).replace('_', "-"),
-                .apply_to_variant(variant)
-                .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);

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(),
     };
 

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<ContainerMeta, syn::Error> {
         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<MetaItem> {
             (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();
     }
 }

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(),
     };