From 89a071c0f79e7625fbe345a1edb9ff029ec16318 Mon Sep 17 00:00:00 2001 From: Piotr Dulikowski Date: Thu, 28 Sep 2023 14:42:55 +0200 Subject: [PATCH] scylla-macros: report errors as compile errors, not panics Our current procedural macros handle errors by panicking. This leads to compilation errors which inform the user about that fact first, and then explain the real issue in the "help" part: ``` error: proc-macro derive panicked --> examples/user-defined-type.rs:6:10 | 6 | #[derive(FromUserType)] | ^^^^^^^^^^^^ | = help: message: derive(FromUserType) works only on structs! ``` This commit gets rid of `.expect()` and `panic()` calls. Instead, errors are now propagated via `Result` and properly converted to a compilation error at the end. This approach results in nicer error messages and allows to attach the message to a particular part of the original code that could cause the problem. After the changes, error messages look like this: ``` error: derive(FromUserType) works only for structs with named fields --> examples/user-defined-type.rs:7:1 | 7 | enum Foo { | ^^^^ ``` --- scylla-macros/src/from_row.rs | 10 +++++----- scylla-macros/src/from_user_type.rs | 10 +++++----- scylla-macros/src/into_user_type.rs | 10 +++++----- scylla-macros/src/lib.rs | 12 ++++++++---- scylla-macros/src/parser.rs | 28 ++++++++++++++++++++++------ scylla-macros/src/value_list.rs | 10 +++++----- 6 files changed, 50 insertions(+), 30 deletions(-) diff --git a/scylla-macros/src/from_row.rs b/scylla-macros/src/from_row.rs index c4caa405b6..027a8b3a0e 100644 --- a/scylla-macros/src/from_row.rs +++ b/scylla-macros/src/from_row.rs @@ -3,10 +3,10 @@ use quote::{quote, quote_spanned}; use syn::{spanned::Spanned, DeriveInput}; /// #[derive(FromRow)] derives FromRow for struct -pub fn from_row_derive(tokens_input: TokenStream) -> TokenStream { - let item = syn::parse::(tokens_input).expect("No DeriveInput"); - let path = crate::parser::get_path(&item).expect("No path"); - let struct_fields = crate::parser::parse_named_fields(&item, "FromRow"); +pub fn from_row_derive(tokens_input: TokenStream) -> Result { + let item = syn::parse::(tokens_input)?; + let path = crate::parser::get_path(&item)?; + let struct_fields = crate::parser::parse_named_fields(&item, "FromRow")?; let struct_name = &item.ident; let (impl_generics, ty_generics, where_clause) = item.generics.split_for_impl(); @@ -56,5 +56,5 @@ pub fn from_row_derive(tokens_input: TokenStream) -> TokenStream { } }; - TokenStream::from(generated) + Ok(TokenStream::from(generated)) } diff --git a/scylla-macros/src/from_user_type.rs b/scylla-macros/src/from_user_type.rs index de3a2adb7d..0b1efa54ac 100644 --- a/scylla-macros/src/from_user_type.rs +++ b/scylla-macros/src/from_user_type.rs @@ -3,10 +3,10 @@ use quote::{quote, quote_spanned}; use syn::{spanned::Spanned, DeriveInput}; /// #[derive(FromUserType)] allows to parse a struct as User Defined Type -pub fn from_user_type_derive(tokens_input: TokenStream) -> TokenStream { - let item = syn::parse::(tokens_input).expect("No DeriveInput"); - let path = crate::parser::get_path(&item).expect("Couldn't get path to the scylla crate"); - let struct_fields = crate::parser::parse_named_fields(&item, "FromUserType"); +pub fn from_user_type_derive(tokens_input: TokenStream) -> Result { + let item = syn::parse::(tokens_input)?; + let path = crate::parser::get_path(&item)?; + let struct_fields = crate::parser::parse_named_fields(&item, "FromUserType")?; let struct_name = &item.ident; let (impl_generics, ty_generics, where_clause) = item.generics.split_for_impl(); @@ -74,5 +74,5 @@ pub fn from_user_type_derive(tokens_input: TokenStream) -> TokenStream { } }; - TokenStream::from(generated) + Ok(TokenStream::from(generated)) } diff --git a/scylla-macros/src/into_user_type.rs b/scylla-macros/src/into_user_type.rs index 4adfaebd18..2880eb2b02 100644 --- a/scylla-macros/src/into_user_type.rs +++ b/scylla-macros/src/into_user_type.rs @@ -3,10 +3,10 @@ use quote::{quote, quote_spanned}; use syn::{spanned::Spanned, DeriveInput}; /// #[derive(IntoUserType)] allows to parse a struct as User Defined Type -pub fn into_user_type_derive(tokens_input: TokenStream) -> TokenStream { - let item = syn::parse::(tokens_input).expect("No DeriveInput"); - let path = crate::parser::get_path(&item).expect("No path"); - let struct_fields = crate::parser::parse_named_fields(&item, "IntoUserType"); +pub fn into_user_type_derive(tokens_input: TokenStream) -> Result { + let item = syn::parse::(tokens_input)?; + let path = crate::parser::get_path(&item)?; + let struct_fields = crate::parser::parse_named_fields(&item, "IntoUserType")?; let struct_name = &item.ident; let (impl_generics, ty_generics, where_clause) = item.generics.split_for_impl(); @@ -45,5 +45,5 @@ pub fn into_user_type_derive(tokens_input: TokenStream) -> TokenStream { } }; - TokenStream::from(generated) + Ok(TokenStream::from(generated)) } diff --git a/scylla-macros/src/lib.rs b/scylla-macros/src/lib.rs index f5ad28a26d..59300a0020 100644 --- a/scylla-macros/src/lib.rs +++ b/scylla-macros/src/lib.rs @@ -10,26 +10,30 @@ mod value_list; /// Works only on simple structs without generics etc #[proc_macro_derive(FromRow, attributes(scylla_crate))] pub fn from_row_derive(tokens_input: TokenStream) -> TokenStream { - from_row::from_row_derive(tokens_input) + let res = from_row::from_row_derive(tokens_input); + res.unwrap_or_else(|e| e.into_compile_error().into()) } /// #[derive(FromUserType)] allows to parse a struct as User Defined Type /// Works only on simple structs without generics etc #[proc_macro_derive(FromUserType, attributes(scylla_crate))] pub fn from_user_type_derive(tokens_input: TokenStream) -> TokenStream { - from_user_type::from_user_type_derive(tokens_input) + let res = from_user_type::from_user_type_derive(tokens_input); + res.unwrap_or_else(|e| e.into_compile_error().into()) } /// #[derive(IntoUserType)] allows to parse a struct as User Defined Type /// Works only on simple structs without generics etc #[proc_macro_derive(IntoUserType, attributes(scylla_crate))] pub fn into_user_type_derive(tokens_input: TokenStream) -> TokenStream { - into_user_type::into_user_type_derive(tokens_input) + let res = into_user_type::into_user_type_derive(tokens_input); + res.unwrap_or_else(|e| e.into_compile_error().into()) } /// #[derive(ValueList)] derives ValueList for struct /// Works only on simple structs without generics etc #[proc_macro_derive(ValueList, attributes(scylla_crate))] pub fn value_list_derive(tokens_input: TokenStream) -> TokenStream { - value_list::value_list_derive(tokens_input) + let res = value_list::value_list_derive(tokens_input); + res.unwrap_or_else(|e| e.into_compile_error().into()) } diff --git a/scylla-macros/src/parser.rs b/scylla-macros/src/parser.rs index 7b88cb60f3..6c33fba0c3 100644 --- a/scylla-macros/src/parser.rs +++ b/scylla-macros/src/parser.rs @@ -6,16 +6,32 @@ use syn::{Expr, Meta}; pub(crate) fn parse_named_fields<'a>( input: &'a DeriveInput, current_derive: &str, -) -> &'a FieldsNamed { +) -> Result<&'a FieldsNamed, syn::Error> { match &input.data { Data::Struct(data) => match &data.fields { - Fields::Named(named_fields) => named_fields, - _ => panic!( - "derive({}) works only for structs with named fields. Tuples don't need derive.", + Fields::Named(named_fields) => Ok(named_fields), + _ => Err(syn::Error::new_spanned( + data.struct_token, + format!( + "derive({}) works only for structs with named fields", + current_derive + ), + )), + }, + Data::Enum(e) => Err(syn::Error::new_spanned( + e.enum_token, + format!( + "derive({}) works only for structs with named fields", current_derive ), - }, - _ => panic!("derive({}) works only on structs!", current_derive), + )), + Data::Union(u) => Err(syn::Error::new_spanned( + u.union_token, + format!( + "derive({}) works only for structs with named fields", + current_derive + ), + )), } } diff --git a/scylla-macros/src/value_list.rs b/scylla-macros/src/value_list.rs index 4b19083944..bf6fc38e9d 100644 --- a/scylla-macros/src/value_list.rs +++ b/scylla-macros/src/value_list.rs @@ -4,10 +4,10 @@ use syn::DeriveInput; /// #[derive(ValueList)] allows to parse a struct as a list of values, /// which can be fed to the query directly. -pub fn value_list_derive(tokens_input: TokenStream) -> TokenStream { - let item = syn::parse::(tokens_input).expect("No DeriveInput"); - let path = crate::parser::get_path(&item).expect("No path"); - let struct_fields = crate::parser::parse_named_fields(&item, "ValueList"); +pub fn value_list_derive(tokens_input: TokenStream) -> Result { + let item = syn::parse::(tokens_input)?; + let path = crate::parser::get_path(&item)?; + let struct_fields = crate::parser::parse_named_fields(&item, "ValueList")?; let struct_name = &item.ident; let (impl_generics, ty_generics, where_clause) = item.generics.split_for_impl(); @@ -27,5 +27,5 @@ pub fn value_list_derive(tokens_input: TokenStream) -> TokenStream { } }; - TokenStream::from(generated) + Ok(TokenStream::from(generated)) }