From dbc5bebacd314f4cd1822aa5ba516de07cc0fbb5 Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Sun, 18 Oct 2020 00:19:25 +0200 Subject: [PATCH] Replace macro panics with syn::Error --- strum_macros/src/helpers/metadata.rs | 15 ++---- strum_macros/src/helpers/mod.rs | 16 ++++++ strum_macros/src/helpers/type_props.rs | 21 +++++--- strum_macros/src/helpers/variant_props.rs | 53 +++++++++++++------ strum_macros/src/lib.rs | 3 +- strum_macros/src/macros/enum_count.rs | 10 ++-- strum_macros/src/macros/enum_discriminants.rs | 4 +- strum_macros/src/macros/enum_iter.rs | 17 +++--- strum_macros/src/macros/enum_messages.rs | 6 +-- strum_macros/src/macros/enum_properties.rs | 6 +-- strum_macros/src/macros/enum_variant_names.rs | 4 +- strum_macros/src/macros/strings/as_ref_str.rs | 14 ++--- strum_macros/src/macros/strings/display.rs | 8 +-- .../src/macros/strings/from_string.rs | 46 ++++++++-------- strum_macros/src/macros/strings/to_string.rs | 8 +-- 15 files changed, 137 insertions(+), 94 deletions(-) diff --git a/strum_macros/src/helpers/metadata.rs b/strum_macros/src/helpers/metadata.rs index cbf06289..cd36f603 100644 --- a/strum_macros/src/helpers/metadata.rs +++ b/strum_macros/src/helpers/metadata.rs @@ -9,7 +9,7 @@ use syn::{ use super::case_style::CaseStyle; -mod kw { +pub mod kw { use syn::custom_keyword; // enum metadata @@ -31,29 +31,24 @@ mod kw { pub enum EnumMeta { SerializeAll { - serialize_all_kw: kw::serialize_all, + kw: kw::serialize_all, case_style: CaseStyle, }, } impl Parse for EnumMeta { fn parse(input: ParseStream) -> syn::Result { - let serialize_all_kw = input.parse::()?; + let kw = input.parse::()?; input.parse::()?; let case_style = input.parse()?; - Ok(EnumMeta::SerializeAll { - serialize_all_kw, - case_style, - }) + Ok(EnumMeta::SerializeAll { kw, case_style }) } } impl Spanned for EnumMeta { fn span(&self) -> Span { match self { - EnumMeta::SerializeAll { - serialize_all_kw, .. - } => serialize_all_kw.span(), + EnumMeta::SerializeAll { kw, .. } => kw.span(), } } } diff --git a/strum_macros/src/helpers/mod.rs b/strum_macros/src/helpers/mod.rs index 747ad745..77049eb4 100644 --- a/strum_macros/src/helpers/mod.rs +++ b/strum_macros/src/helpers/mod.rs @@ -6,3 +6,19 @@ pub mod case_style; mod metadata; pub mod type_props; pub mod variant_props; + +use proc_macro2::Span; +use quote::ToTokens; + +pub fn non_enum_error() -> syn::Error { + syn::Error::new(Span::call_site(), "This macro only supports enums.") +} + +pub fn occurrence_error(fst: T, snd: T, attr: &str) -> syn::Error { + let mut e = syn::Error::new_spanned( + snd, + format!("Found multiple occurrences of strum({})", attr), + ); + e.combine(syn::Error::new_spanned(fst, "first one here")); + e +} diff --git a/strum_macros/src/helpers/type_props.rs b/strum_macros/src/helpers/type_props.rs index d9bda387..c942c5d7 100644 --- a/strum_macros/src/helpers/type_props.rs +++ b/strum_macros/src/helpers/type_props.rs @@ -3,8 +3,9 @@ use quote::quote; use std::default::Default; use syn::{DeriveInput, Ident, Path}; -use crate::helpers::case_style::CaseStyle; -use crate::helpers::metadata::{DeriveInputExt, EnumDiscriminantsMeta, EnumMeta}; +use super::case_style::CaseStyle; +use super::metadata::{DeriveInputExt, EnumDiscriminantsMeta, EnumMeta}; +use super::occurrence_error; pub trait HasTypeProperties { fn get_type_properties(&self) -> syn::Result; @@ -25,28 +26,32 @@ impl HasTypeProperties for DeriveInput { let strum_meta = self.get_metadata()?; let discriminants_meta = self.get_discriminants_metadata()?; + let mut serialize_all_kw = None; for meta in strum_meta { match meta { - EnumMeta::SerializeAll { case_style, .. } => { - if output.case_style.is_some() { - panic!("found multiple values of serialize_all"); + EnumMeta::SerializeAll { case_style, kw } => { + if let Some(fst_kw) = serialize_all_kw { + return Err(occurrence_error(fst_kw, kw, "serialize_all")); } + serialize_all_kw = Some(kw); output.case_style = Some(case_style); } } } + let mut name_kw = None; for meta in discriminants_meta { match meta { EnumDiscriminantsMeta::Derive { paths, .. } => { output.discriminant_derives.extend(paths); } - EnumDiscriminantsMeta::Name { name, .. } => { - if output.discriminant_name.is_some() { - panic!("multiple occurrences of 'name'"); + EnumDiscriminantsMeta::Name { name, kw } => { + if let Some(fst_kw) = name_kw { + return Err(occurrence_error(fst_kw, kw, "name")); } + name_kw = Some(kw); output.discriminant_name = Some(name); } EnumDiscriminantsMeta::Other { path, nested } => { diff --git a/strum_macros/src/helpers/variant_props.rs b/strum_macros/src/helpers/variant_props.rs index ca6b3f0b..1077bcb2 100644 --- a/strum_macros/src/helpers/variant_props.rs +++ b/strum_macros/src/helpers/variant_props.rs @@ -1,8 +1,9 @@ use std::default::Default; use syn::{Ident, LitStr, Variant}; -use crate::helpers::case_style::{CaseStyle, CaseStyleHelpers}; -use crate::helpers::metadata::{VariantExt, VariantMeta}; +use super::case_style::{CaseStyle, CaseStyleHelpers}; +use super::metadata::{kw, VariantExt, VariantMeta}; +use super::occurrence_error; pub trait HasStrumVariantProperties { fn get_variant_properties(&self) -> syn::Result; @@ -10,8 +11,8 @@ pub trait HasStrumVariantProperties { #[derive(Clone, Eq, PartialEq, Debug, Default)] pub struct StrumVariantProperties { - pub is_disabled: bool, - pub default: bool, + pub disabled: Option, + pub default: Option, pub message: Option, pub detailed_message: Option, pub string_props: Vec<(LitStr, LitStr)>, @@ -59,37 +60,55 @@ impl HasStrumVariantProperties for Variant { let mut output = StrumVariantProperties::default(); output.ident = Some(self.ident.clone()); + let mut message_kw = None; + let mut detailed_message_kw = None; + let mut to_string_kw = None; + let mut disabled_kw = None; + let mut default_kw = None; for meta in self.get_metadata()? { match meta { - VariantMeta::Message { value, .. } => { - if output.message.is_some() { - panic!("message is set twice on the same variant"); + VariantMeta::Message { value, kw } => { + if let Some(fst_kw) = message_kw { + return Err(occurrence_error(fst_kw, kw, "message")); } + message_kw = Some(kw); output.message = Some(value); } - VariantMeta::DetailedMessage { value, .. } => { - if output.detailed_message.is_some() { - panic!("detailed message set twice on the same variant"); + VariantMeta::DetailedMessage { value, kw } => { + if let Some(fst_kw) = detailed_message_kw { + return Err(occurrence_error(fst_kw, kw, "detailed_message")); } + detailed_message_kw = Some(kw); output.detailed_message = Some(value); } VariantMeta::Serialize { value, .. } => { output.serialize.push(value); } - VariantMeta::ToString { value, .. } => { - if output.to_string.is_some() { - panic!("to_string is set twice on the same variant"); + VariantMeta::ToString { value, kw } => { + if let Some(fst_kw) = to_string_kw { + return Err(occurrence_error(fst_kw, kw, "to_string")); } + to_string_kw = Some(kw); output.to_string = Some(value); } - VariantMeta::Disabled(_) => { - output.is_disabled = true; + VariantMeta::Disabled(kw) => { + if let Some(fst_kw) = disabled_kw { + return Err(occurrence_error(fst_kw, kw, "disabled")); + } + + disabled_kw = Some(kw); + output.disabled = Some(kw); } - VariantMeta::Default(_) => { - output.default = true; + VariantMeta::Default(kw) => { + if let Some(fst_kw) = default_kw { + return Err(occurrence_error(fst_kw, kw, "default")); + } + + default_kw = Some(kw); + output.default = Some(kw); } VariantMeta::Props { props, .. } => { output.string_props.extend(props); diff --git a/strum_macros/src/lib.rs b/strum_macros/src/lib.rs index 5d2440a4..f36a0e20 100644 --- a/strum_macros/src/lib.rs +++ b/strum_macros/src/lib.rs @@ -658,7 +658,8 @@ pub fn enum_discriminants(input: proc_macro::TokenStream) -> proc_macro::TokenSt )] pub fn enum_count(input: proc_macro::TokenStream) -> proc_macro::TokenStream { let ast = syn::parse_macro_input!(input as DeriveInput); - let toks = macros::enum_count::enum_count_inner(&ast); + let toks = + macros::enum_count::enum_count_inner(&ast).unwrap_or_else(|err| err.to_compile_error()); debug_print_generated(&ast, &toks); toks.into() } diff --git a/strum_macros/src/macros/enum_count.rs b/strum_macros/src/macros/enum_count.rs index 4d492897..ada3a0fd 100644 --- a/strum_macros/src/macros/enum_count.rs +++ b/strum_macros/src/macros/enum_count.rs @@ -2,10 +2,12 @@ use proc_macro2::TokenStream; use quote::quote; use syn::{Data, DeriveInput}; -pub(crate) fn enum_count_inner(ast: &DeriveInput) -> TokenStream { +use crate::helpers::non_enum_error; + +pub(crate) fn enum_count_inner(ast: &DeriveInput) -> syn::Result { let n = match &ast.data { Data::Enum(v) => v.variants.len(), - _ => panic!("EnumCount can only be used with enums"), + _ => return Err(non_enum_error()), }; // Used in the quasi-quotation below as `#name` @@ -14,10 +16,10 @@ pub(crate) fn enum_count_inner(ast: &DeriveInput) -> TokenStream { // Helper is provided for handling complex generic types correctly and effortlessly let (impl_generics, ty_generics, where_clause) = ast.generics.split_for_impl(); - quote! { + Ok(quote! { // Implementation impl #impl_generics ::strum::EnumCount for #name #ty_generics #where_clause { const COUNT: usize = #n; } - } + }) } diff --git a/strum_macros/src/macros/enum_discriminants.rs b/strum_macros/src/macros/enum_discriminants.rs index f8430450..991b8f74 100644 --- a/strum_macros/src/macros/enum_discriminants.rs +++ b/strum_macros/src/macros/enum_discriminants.rs @@ -3,7 +3,7 @@ use quote::quote; use syn::parse_quote; use syn::{Data, DeriveInput}; -use crate::helpers::HasTypeProperties; +use crate::helpers::{non_enum_error, HasTypeProperties}; /// Attributes to copy from the main enum's variants to the discriminant enum's variants. /// @@ -17,7 +17,7 @@ pub fn enum_discriminants_inner(ast: &DeriveInput) -> syn::Result { let variants = match &ast.data { Data::Enum(v) => &v.variants, - _ => panic!("EnumDiscriminants only works on Enums"), + _ => return Err(non_enum_error()), }; // Derives for the generated enum diff --git a/strum_macros/src/macros/enum_iter.rs b/strum_macros/src/macros/enum_iter.rs index bbef9c51..b2150d64 100644 --- a/strum_macros/src/macros/enum_iter.rs +++ b/strum_macros/src/macros/enum_iter.rs @@ -1,8 +1,8 @@ -use proc_macro2::TokenStream; +use proc_macro2::{Span, TokenStream}; use quote::quote; use syn::{Data, DeriveInput, Ident}; -use crate::helpers::HasStrumVariantProperties; +use crate::helpers::{non_enum_error, HasStrumVariantProperties}; pub fn enum_iter_inner(ast: &DeriveInput) -> syn::Result { let name = &ast.ident; @@ -11,10 +11,11 @@ pub fn enum_iter_inner(ast: &DeriveInput) -> syn::Result { let vis = &ast.vis; if gen.lifetimes().count() > 0 { - panic!( - "Enum Iterator isn't supported on Enums with lifetimes. The resulting enums would \ - be unbounded." - ); + return Err(syn::Error::new( + Span::call_site(), + "This macro doesn't support enums with lifetimes. \ + The resulting enums would be unbounded.", + )); } let phantom_data = if gen.type_params().count() > 0 { @@ -26,7 +27,7 @@ pub fn enum_iter_inner(ast: &DeriveInput) -> syn::Result { let variants = match &ast.data { Data::Enum(v) => &v.variants, - _ => panic!("EnumIter only works on Enums"), + _ => return Err(non_enum_error()), }; let mut arms = Vec::new(); @@ -34,7 +35,7 @@ pub fn enum_iter_inner(ast: &DeriveInput) -> syn::Result { for variant in variants { use syn::Fields::*; - if variant.get_variant_properties()?.is_disabled { + if variant.get_variant_properties()?.disabled.is_some() { continue; } diff --git a/strum_macros/src/macros/enum_messages.rs b/strum_macros/src/macros/enum_messages.rs index 67ad6bc9..4557d794 100644 --- a/strum_macros/src/macros/enum_messages.rs +++ b/strum_macros/src/macros/enum_messages.rs @@ -2,14 +2,14 @@ use proc_macro2::TokenStream; use quote::quote; use syn::{Data, DeriveInput}; -use crate::helpers::{HasStrumVariantProperties, HasTypeProperties}; +use crate::helpers::{non_enum_error, HasStrumVariantProperties, HasTypeProperties}; pub fn enum_message_inner(ast: &DeriveInput) -> syn::Result { let name = &ast.ident; let (impl_generics, ty_generics, where_clause) = ast.generics.split_for_impl(); let variants = match &ast.data { Data::Enum(v) => &v.variants, - _ => panic!("EnumMessage only works on Enums"), + _ => return Err(non_enum_error()), }; let type_properties = ast.get_type_properties()?; @@ -46,7 +46,7 @@ pub fn enum_message_inner(ast: &DeriveInput) -> syn::Result { } // But you can disable the messages. - if variant_properties.is_disabled { + if variant_properties.disabled.is_some() { continue; } diff --git a/strum_macros/src/macros/enum_properties.rs b/strum_macros/src/macros/enum_properties.rs index ccb2b7fe..1a8147ab 100644 --- a/strum_macros/src/macros/enum_properties.rs +++ b/strum_macros/src/macros/enum_properties.rs @@ -2,14 +2,14 @@ use proc_macro2::TokenStream; use quote::quote; use syn::{Data, DeriveInput}; -use crate::helpers::HasStrumVariantProperties; +use crate::helpers::{non_enum_error, HasStrumVariantProperties}; pub fn enum_properties_inner(ast: &DeriveInput) -> syn::Result { let name = &ast.ident; let (impl_generics, ty_generics, where_clause) = ast.generics.split_for_impl(); let variants = match &ast.data { Data::Enum(v) => &v.variants, - _ => panic!("EnumProp only works on Enums"), + _ => return Err(non_enum_error()), }; let mut arms = Vec::new(); @@ -20,7 +20,7 @@ pub fn enum_properties_inner(ast: &DeriveInput) -> syn::Result { let mut bool_arms = Vec::new(); let mut num_arms = Vec::new(); // But you can disable the messages. - if variant_properties.is_disabled { + if variant_properties.disabled.is_some() { continue; } diff --git a/strum_macros/src/macros/enum_variant_names.rs b/strum_macros/src/macros/enum_variant_names.rs index 868f9892..b99f179d 100644 --- a/strum_macros/src/macros/enum_variant_names.rs +++ b/strum_macros/src/macros/enum_variant_names.rs @@ -2,7 +2,7 @@ use proc_macro2::TokenStream; use quote::quote; use syn::{Data, DeriveInput}; -use crate::helpers::{HasStrumVariantProperties, HasTypeProperties}; +use crate::helpers::{non_enum_error, HasStrumVariantProperties, HasTypeProperties}; pub fn enum_variant_names_inner(ast: &DeriveInput) -> syn::Result { let name = &ast.ident; @@ -11,7 +11,7 @@ pub fn enum_variant_names_inner(ast: &DeriveInput) -> syn::Result { let variants = match &ast.data { Data::Enum(v) => &v.variants, - _ => panic!("EnumVariantNames only works on Enums"), + _ => return Err(non_enum_error()), }; // Derives for the generated enum diff --git a/strum_macros/src/macros/strings/as_ref_str.rs b/strum_macros/src/macros/strings/as_ref_str.rs index 9e26413d..cc89954c 100644 --- a/strum_macros/src/macros/strings/as_ref_str.rs +++ b/strum_macros/src/macros/strings/as_ref_str.rs @@ -2,14 +2,14 @@ use proc_macro2::TokenStream; use quote::quote; use syn::{parse_quote, Data, DeriveInput}; -use crate::helpers::{HasStrumVariantProperties, HasTypeProperties}; +use crate::helpers::{non_enum_error, HasStrumVariantProperties, HasTypeProperties}; fn get_arms(ast: &DeriveInput) -> syn::Result> { let name = &ast.ident; let mut arms = Vec::new(); let variants = match &ast.data { Data::Enum(v) => &v.variants, - _ => panic!("This macro only works on Enums"), + _ => return Err(non_enum_error()), }; let type_properties = ast.get_type_properties()?; @@ -19,7 +19,7 @@ fn get_arms(ast: &DeriveInput) -> syn::Result> { let ident = &variant.ident; let variant_properties = variant.get_variant_properties()?; - if variant_properties.is_disabled { + if variant_properties.disabled.is_some() { continue; } @@ -38,9 +38,11 @@ fn get_arms(ast: &DeriveInput) -> syn::Result> { if arms.len() < variants.len() { arms.push(quote! { - _ => panic!("AsRef::::as_ref() or AsStaticRef::::as_static() \ - called on disabled variant.") - }) + _ => panic!( + "AsRef::::as_ref() or AsStaticRef::::as_static() \ + called on disabled variant.", + ) + }); } Ok(arms) diff --git a/strum_macros/src/macros/strings/display.rs b/strum_macros/src/macros/strings/display.rs index 1aefe789..740e437b 100644 --- a/strum_macros/src/macros/strings/display.rs +++ b/strum_macros/src/macros/strings/display.rs @@ -2,14 +2,14 @@ use proc_macro2::TokenStream; use quote::quote; use syn::{Data, DeriveInput, Fields}; -use crate::helpers::{HasStrumVariantProperties, HasTypeProperties}; +use crate::helpers::{non_enum_error, HasStrumVariantProperties, HasTypeProperties}; pub fn display_inner(ast: &DeriveInput) -> syn::Result { let name = &ast.ident; let (impl_generics, ty_generics, where_clause) = ast.generics.split_for_impl(); let variants = match &ast.data { Data::Enum(v) => &v.variants, - _ => panic!("Display only works on Enums"), + _ => return Err(non_enum_error()), }; let type_properties = ast.get_type_properties()?; @@ -19,7 +19,7 @@ pub fn display_inner(ast: &DeriveInput) -> syn::Result { let ident = &variant.ident; let variant_properties = variant.get_variant_properties()?; - if variant_properties.is_disabled { + if variant_properties.disabled.is_some() { continue; } @@ -36,7 +36,7 @@ pub fn display_inner(ast: &DeriveInput) -> syn::Result { } if arms.len() < variants.len() { - arms.push(quote! { _ => panic!("fmt() called on disabled variant.")}) + arms.push(quote! { _ => panic!("fmt() called on disabled variant.") }); } Ok(quote! { diff --git a/strum_macros/src/macros/strings/from_string.rs b/strum_macros/src/macros/strings/from_string.rs index 5b1d7c19..060bfdcb 100644 --- a/strum_macros/src/macros/strings/from_string.rs +++ b/strum_macros/src/macros/strings/from_string.rs @@ -1,50 +1,52 @@ use proc_macro2::TokenStream; use quote::quote; -use syn::{Data, DeriveInput}; +use syn::{Data, DeriveInput, Fields}; -use crate::helpers::{HasStrumVariantProperties, HasTypeProperties}; +use crate::helpers::{ + non_enum_error, occurrence_error, HasStrumVariantProperties, HasTypeProperties, +}; pub fn from_string_inner(ast: &DeriveInput) -> syn::Result { let name = &ast.ident; let (impl_generics, ty_generics, where_clause) = ast.generics.split_for_impl(); let variants = match &ast.data { Data::Enum(v) => &v.variants, - _ => panic!("FromString only works on Enums"), + _ => return Err(non_enum_error()), }; let type_properties = ast.get_type_properties()?; - let mut has_default = false; + let mut default_kw = None; let mut default = quote! { _ => ::std::result::Result::Err(::strum::ParseError::VariantNotFound) }; let mut arms = Vec::new(); for variant in variants { - use syn::Fields::*; let ident = &variant.ident; let variant_properties = variant.get_variant_properties()?; - if variant_properties.is_disabled { + if variant_properties.disabled.is_some() { continue; } - if variant_properties.default { - if has_default { - panic!("Can't have multiple default variants"); + if let Some(kw) = variant_properties.default { + if let Some(fst_kw) = default_kw { + return Err(occurrence_error(fst_kw, kw, "default")); } - if let Unnamed(fields) = &variant.fields { - if fields.unnamed.len() != 1 { - panic!("Default only works on unit structs with a single String parameter"); + match &variant.fields { + Fields::Unnamed(fields) if fields.unnamed.len() == 1 => {} + _ => { + return Err(syn::Error::new_spanned( + variant, + "Default only works on newtype structs with a single String field", + )) } - - default = quote! { - default => ::std::result::Result::Ok(#name::#ident (default.into())) - }; - } else { - panic!("Default only works on unit structs with a single String parameter"); } - has_default = true; + default_kw = Some(kw); + default = quote! { + default => ::std::result::Result::Ok(#name::#ident(default.into())) + }; continue; } @@ -52,13 +54,13 @@ pub fn from_string_inner(ast: &DeriveInput) -> syn::Result { let attrs = variant_properties.get_serializations(type_properties.case_style); let params = match &variant.fields { - Unit => quote! {}, - Unnamed(fields) => { + Fields::Unit => quote! {}, + Fields::Unnamed(fields) => { let defaults = ::std::iter::repeat(quote!(Default::default())).take(fields.unnamed.len()); quote! { (#(#defaults),*) } } - Named(fields) => { + Fields::Named(fields) => { let fields = fields .named .iter() diff --git a/strum_macros/src/macros/strings/to_string.rs b/strum_macros/src/macros/strings/to_string.rs index 7c05f506..267173a2 100644 --- a/strum_macros/src/macros/strings/to_string.rs +++ b/strum_macros/src/macros/strings/to_string.rs @@ -2,14 +2,14 @@ use proc_macro2::TokenStream; use quote::quote; use syn::{Data, DeriveInput}; -use crate::helpers::{HasStrumVariantProperties, HasTypeProperties}; +use crate::helpers::{non_enum_error, HasStrumVariantProperties, HasTypeProperties}; pub fn to_string_inner(ast: &DeriveInput) -> syn::Result { let name = &ast.ident; let (impl_generics, ty_generics, where_clause) = ast.generics.split_for_impl(); let variants = match &ast.data { Data::Enum(v) => &v.variants, - _ => panic!("ToString only works on Enums"), + _ => return Err(non_enum_error()), }; let type_properties = ast.get_type_properties()?; @@ -19,7 +19,7 @@ pub fn to_string_inner(ast: &DeriveInput) -> syn::Result { let ident = &variant.ident; let variant_properties = variant.get_variant_properties()?; - if variant_properties.is_disabled { + if variant_properties.disabled.is_some() { continue; } @@ -36,7 +36,7 @@ pub fn to_string_inner(ast: &DeriveInput) -> syn::Result { } if arms.len() < variants.len() { - arms.push(quote! { _ => panic!("to_string() called on disabled variant.")}) + arms.push(quote! { _ => panic!("to_string() called on disabled variant.") }); } Ok(quote! {