From 178267c701c233542078c09fe6b19802f9642dbd Mon Sep 17 00:00:00 2001 From: HampusM Date: Mon, 30 Jan 2023 21:29:21 +0100 Subject: feat: improve macro error messages --- macros/src/injectable/dependency.rs | 149 +++++++++++++-------- macros/src/injectable/implementation.rs | 229 ++++++++++++++++++++++++++++---- macros/src/injectable/macro_args.rs | 135 +++++++++++++------ 3 files changed, 392 insertions(+), 121 deletions(-) (limited to 'macros/src/injectable') diff --git a/macros/src/injectable/dependency.rs b/macros/src/injectable/dependency.rs index d9d904e..6ff0697 100644 --- a/macros/src/injectable/dependency.rs +++ b/macros/src/injectable/dependency.rs @@ -1,7 +1,9 @@ -use proc_macro2::Ident; +use proc_macro2::{Ident, Span}; +use syn::spanned::Spanned; use syn::{parse2, FnArg, GenericArgument, LitStr, PathArguments, Type}; use crate::injectable::named_attr_input::NamedAttrInput; +use crate::util::error::diagnostic_error_enum; use crate::util::syn_path::SynPathExt; /// Interface for a representation of a dependency of a injectable type. @@ -40,29 +42,40 @@ impl IDependency for Dependency { let typed_new_method_arg = match new_method_arg { FnArg::Typed(typed_arg) => Ok(typed_arg), - FnArg::Receiver(_) => Err(DependencyError::UnexpectedSelfArgument), + FnArg::Receiver(receiver_arg) => Err(DependencyError::UnexpectedSelf { + self_token_span: receiver_arg.self_token.span, + }), }?; let dependency_type_path = match typed_new_method_arg.ty.as_ref() { Type::Path(arg_type_path) => Ok(arg_type_path), Type::Reference(ref_type_path) => match ref_type_path.elem.as_ref() { Type::Path(arg_type_path) => Ok(arg_type_path), - &_ => Err(DependencyError::TypeNotPath), + other_type => Err(DependencyError::InvalidType { + type_span: other_type.span(), + }), }, - &_ => Err(DependencyError::TypeNotPath), + other_type => Err(DependencyError::InvalidType { + type_span: other_type.span(), + }), }?; - let ptr_path_segment = dependency_type_path - .path - .segments - .last() - .map_or_else(|| Err(DependencyError::PtrTypePathEmpty), Ok)?; + let ptr_path_segment = dependency_type_path.path.segments.last().map_or_else( + || { + Err(DependencyError::MissingType { + arg_span: typed_new_method_arg.span(), + }) + }, + Ok, + )?; let ptr_ident = ptr_path_segment.ident.clone(); - let ptr_generic_args = &match &ptr_path_segment.arguments { + let ptr_generic_args = match ptr_path_segment.arguments.clone() { PathArguments::AngleBracketed(generic_args) => Ok(generic_args), - &_ => Err(DependencyError::PtrTypeNoGenerics), + _ => Err(DependencyError::DependencyTypeMissingGenerics { + ptr_ident_span: ptr_ident.span(), + }), }? .args; @@ -70,7 +83,9 @@ impl IDependency for Dependency if let Some(GenericArgument::Type(interface)) = ptr_generic_args.first() { Ok(interface.clone()) } else { - Err(DependencyError::PtrTypeNoGenerics) + Err(DependencyError::DependencyTypeMissingGenerics { + ptr_ident_span: ptr_ident.span(), + }) }?; let arg_attrs = &typed_new_method_arg.attrs; @@ -84,15 +99,17 @@ impl IDependency for Dependency let opt_named_attr_tokens = opt_named_attr.map(|attr| &attr.tokens); - let opt_named_attr_input = if let Some(named_attr_tokens) = opt_named_attr_tokens - { - Some( - parse2::(named_attr_tokens.clone()) - .map_err(DependencyError::ParseNamedAttrInputFailed)?, - ) - } else { - None - }; + let opt_named_attr_input = + if let Some(named_attr_tokens) = opt_named_attr_tokens { + Some(parse2::(named_attr_tokens.clone()).map_err( + |err| DependencyError::InvalidNamedAttrInput { + arg_span: typed_new_method_arg.span(), + err, + }, + )?) + } else { + None + }; Ok(Self { interface, @@ -117,23 +134,43 @@ impl IDependency for Dependency } } -#[derive(Debug, thiserror::Error)] +diagnostic_error_enum! { pub enum DependencyError { - #[error("Unexpected 'self' argument in 'new' method")] - UnexpectedSelfArgument, - - #[error("Argument type must either be a path or a path reference")] - TypeNotPath, - - #[error("Expected pointer type path to not be empty")] - PtrTypePathEmpty, - - #[error("Expected pointer type to take generic arguments")] - PtrTypeNoGenerics, - - #[error("Failed to parse the input to a 'named' attribute")] - ParseNamedAttrInputFailed(#[source] syn::Error), + #[error("Unexpected 'self' parameter"), span = self_token_span] + #[help("Remove the 'self' parameter"), span = self_token_span] + UnexpectedSelf { + self_token_span: Span + }, + + #[ + error("Dependency type must either be a path or a path reference"), + span = type_span + ] + InvalidType { + type_span: Span + }, + + #[error("Dependency is missing a type"), span = arg_span] + MissingType { + arg_span: Span + }, + + #[ + error("Expected dependency type to take generic parameters"), + span = ptr_ident_span + ] + DependencyTypeMissingGenerics { + ptr_ident_span: Span + }, + + #[error("Dependency has a 'named' attribute given invalid input"), span = arg_span] + #[source(err)] + InvalidNamedAttrInput { + arg_span: Span, + err: syn::Error + }, +} } #[cfg(test)] @@ -162,9 +199,9 @@ mod tests use crate::test_utils; #[test] - fn can_build_dependency() -> Result<(), Box> + fn can_build_dependency() { - assert_eq!( + assert!(matches!( Dependency::build(&FnArg::Typed(PatType { attrs: vec![], pat: Box::new(Pat::Verbatim(TokenStream::default().into())), @@ -177,17 +214,17 @@ mod tests ]))] ), ]))) - }))?, - Dependency { + })), + Ok(dependency) if dependency == Dependency { interface: test_utils::create_type(test_utils::create_path(&[ PathSegment::from(format_ident!("Foo")) ])), ptr: format_ident!("TransientPtr"), name: None } - ); + )); - assert_eq!( + assert!(matches!( Dependency::build(&FnArg::Typed(PatType { attrs: vec![], pat: Box::new(Pat::Verbatim(TokenStream::default().into())), @@ -202,23 +239,21 @@ mod tests ]))] ), ]))) - }))?, - Dependency { + })), + Ok(dependency) if dependency == Dependency { interface: test_utils::create_type(test_utils::create_path(&[ PathSegment::from(format_ident!("Bar")) ])), ptr: format_ident!("SingletonPtr"), name: None } - ); - - Ok(()) + )); } #[test] - fn can_build_dependency_with_name() -> Result<(), Box> + fn can_build_dependency_with_name() { - assert_eq!( + assert!(matches!( Dependency::build(&FnArg::Typed(PatType { attrs: vec![Attribute { pound_token: Pound::default(), @@ -240,17 +275,17 @@ mod tests ]))] ), ]))) - }))?, - Dependency { + })), + Ok(dependency) if dependency == Dependency { interface: test_utils::create_type(test_utils::create_path(&[ PathSegment::from(format_ident!("Foo")) ])), ptr: format_ident!("TransientPtr"), name: Some(LitStr::new("cool", Span::call_site())) } - ); + )); - assert_eq!( + assert!(matches!( Dependency::build(&FnArg::Typed(PatType { attrs: vec![Attribute { pound_token: Pound::default(), @@ -274,17 +309,15 @@ mod tests ]))] ), ]))) - }))?, - Dependency { + })), + Ok(dependency) if dependency == Dependency { interface: test_utils::create_type(test_utils::create_path(&[ PathSegment::from(format_ident!("Bar")) ])), ptr: format_ident!("FactoryPtr"), name: Some(LitStr::new("awesome", Span::call_site())) } - ); - - Ok(()) + )); } #[test] diff --git a/macros/src/injectable/implementation.rs b/macros/src/injectable/implementation.rs index 9b7236c..0fd73de 100644 --- a/macros/src/injectable/implementation.rs +++ b/macros/src/injectable/implementation.rs @@ -1,11 +1,22 @@ use std::error::Error; -use proc_macro2::Ident; +use proc_macro2::{Ident, Span, TokenStream}; use quote::{format_ident, quote, ToTokens}; -use syn::parse::{Parse, ParseStream}; -use syn::{parse_str, ExprMethodCall, FnArg, Generics, ImplItemMethod, ItemImpl, Type}; - -use crate::injectable::dependency::IDependency; +use syn::spanned::Spanned; +use syn::{ + parse2, + parse_str, + ExprMethodCall, + FnArg, + Generics, + ImplItemMethod, + ItemImpl, + ReturnType, + Type, +}; + +use crate::injectable::dependency::{DependencyError, IDependency}; +use crate::util::error::diagnostic_error_enum; use crate::util::item_impl::find_impl_method_by_name_mut; use crate::util::string::camelcase_to_snakecase; use crate::util::syn_path::SynPathExt; @@ -19,36 +30,107 @@ pub struct InjectableImpl pub self_type: Type, pub generics: Generics, pub original_impl: ItemImpl, + + new_method: ImplItemMethod, } -impl Parse for InjectableImpl +impl InjectableImpl { #[cfg(not(tarpaulin_include))] - fn parse(input: ParseStream) -> syn::Result + pub fn parse(input: TokenStream) -> Result { - let input_fork = input.fork(); + let mut item_impl = parse2::(input).map_err(|err| { + InjectableImplError::NotAImplementation { + err_span: err.span(), + } + })?; - let mut item_impl = input.parse::()?; + if let Some((_, trait_path, _)) = item_impl.trait_ { + return Err(InjectableImplError::TraitImpl { + trait_path_span: trait_path.span(), + }); + } - let new_method = find_impl_method_by_name_mut(&mut item_impl, "new") - .map_or_else(|| Err(input_fork.error("Missing a 'new' method")), Ok)?; + let item_impl_span = item_impl.self_ty.span(); - let dependencies = - Self::build_dependencies(new_method).map_err(|err| input.error(err))?; + let new_method = find_impl_method_by_name_mut(&mut item_impl, "new").ok_or( + InjectableImplError::MissingNewMethod { + implementation_span: item_impl_span, + }, + )?; + + let dependencies = Self::build_dependencies(new_method).map_err(|err| { + InjectableImplError::ContainsAInvalidDependency { + implementation_span: item_impl_span, + err, + } + })?; Self::remove_method_argument_attrs(new_method); + let new_method = new_method.clone(); + Ok(Self { dependencies, self_type: item_impl.self_ty.as_ref().clone(), generics: item_impl.generics.clone(), original_impl: item_impl, + new_method, }) } -} -impl InjectableImpl -{ + pub fn validate(&self) -> Result<(), InjectableImplError> + { + if matches!(self.new_method.sig.output, ReturnType::Default) { + return Err(InjectableImplError::InvalidNewMethodReturnType { + new_method_output_span: self.new_method.sig.output.span(), + expected: "Self".to_string(), + found: "()".to_string(), + }); + } + + if let ReturnType::Type(_, ret_type) = &self.new_method.sig.output { + if let Type::Path(path_type) = ret_type.as_ref() { + if path_type + .path + .get_ident() + .map_or_else(|| true, |ident| *ident != "Self") + { + return Err(InjectableImplError::InvalidNewMethodReturnType { + new_method_output_span: self.new_method.sig.output.span(), + expected: "Self".to_string(), + found: ret_type.to_token_stream().to_string(), + }); + } + } else { + return Err(InjectableImplError::InvalidNewMethodReturnType { + new_method_output_span: self.new_method.sig.output.span(), + expected: "Self".to_string(), + found: ret_type.to_token_stream().to_string(), + }); + } + } + + if let Some(unsafety) = self.new_method.sig.unsafety { + return Err(InjectableImplError::NewMethodUnsafe { + unsafety_span: unsafety.span, + }); + } + + if let Some(asyncness) = self.new_method.sig.asyncness { + return Err(InjectableImplError::NewMethodAsync { + asyncness_span: asyncness.span, + }); + } + + if !self.new_method.sig.generics.params.is_empty() { + return Err(InjectableImplError::NewMethodGeneric { + generics_span: self.new_method.sig.generics.span(), + }); + } + Ok(()) + } + #[cfg(not(tarpaulin_include))] pub fn expand(&self, no_doc_hidden: bool, is_async: bool) -> proc_macro2::TokenStream @@ -216,6 +298,36 @@ impl InjectableImpl } } + #[cfg(not(tarpaulin_include))] + pub fn expand_dummy_blocking_impl(&self) -> proc_macro2::TokenStream + { + let generics = &self.generics; + let self_type = &self.self_type; + + let di_container_var = format_ident!("{}", DI_CONTAINER_VAR_NAME); + let dependency_history_var = format_ident!("{}", DEPENDENCY_HISTORY_VAR_NAME); + + quote! { + impl #generics syrette::interfaces::injectable::Injectable< + syrette::di_container::blocking::DIContainer, + syrette::dependency_history::DependencyHistory + > for #self_type + { + fn resolve( + #di_container_var: &std::rc::Rc< + syrette::di_container::blocking::DIContainer + >, + mut #dependency_history_var: syrette::dependency_history::DependencyHistory + ) -> Result< + syrette::ptr::TransientPtr, + syrette::errors::injectable::InjectableError> + { + unimplemented!(); + } + } + } + } + fn create_get_dep_method_calls( dependencies: &[Dep], is_async: bool, @@ -288,8 +400,9 @@ impl InjectableImpl }) } - fn build_dependencies(new_method: &ImplItemMethod) - -> Result, Box> + fn build_dependencies( + new_method: &ImplItemMethod, + ) -> Result, DependencyError> { let new_method_args = &new_method.sig.inputs; @@ -338,6 +451,74 @@ impl InjectableImpl } } +diagnostic_error_enum! { +pub enum InjectableImplError +{ + #[ + error("The 'injectable' attribute must be placed on a implementation"), + span = err_span + ] + NotAImplementation + { + err_span: Span + }, + + #[ + error("The 'injectable' attribute cannot be placed on a trait implementation"), + span = trait_path_span + ] + TraitImpl + { + trait_path_span: Span + }, + + #[error("Missing a 'new' method"), span = implementation_span] + #[note("Required by the 'injectable' attribute macro")] + MissingNewMethod { + implementation_span: Span + }, + + #[ + error(concat!( + "Invalid 'new' method return type. Expected it to be '{}'. ", + "Found '{}'" + ), expected, found), + span = new_method_output_span + ] + InvalidNewMethodReturnType + { + new_method_output_span: Span, + expected: String, + found: String + }, + + #[error("'new' method is not allowed to be unsafe"), span = unsafety_span] + #[note("Required by the 'injectable' attribute macro")] + NewMethodUnsafe { + unsafety_span: Span + }, + + #[error("'new' method is not allowed to be async"), span = asyncness_span] + #[note("Required by the 'injectable' attribute macro")] + NewMethodAsync { + asyncness_span: Span + }, + + #[error("'new' method is not allowed to have generics"), span = generics_span] + #[note("Required by the 'injectable' attribute macro")] + NewMethodGeneric { + generics_span: Span + }, + + #[error("Has a invalid dependency"), span = implementation_span] + #[source(err)] + ContainsAInvalidDependency { + implementation_span: Span, + err: DependencyError + }, +} +} + #[cfg(test)] mod tests { @@ -436,8 +617,8 @@ mod tests .expect() .returning(|_| Ok(MockIDependency::new())); - let dependencies = - InjectableImpl::::build_dependencies(&method)?; + let dependencies = InjectableImpl::::build_dependencies(&method) + .expect("Expected Ok"); assert_eq!(dependencies.len(), 2); @@ -445,7 +626,7 @@ mod tests } #[test] - fn can_build_named_dependencies() -> Result<(), Box> + fn can_build_named_dependencies() { let method = ImplItemMethod { attrs: vec![], @@ -517,12 +698,10 @@ mod tests .returning(|_| Ok(MockIDependency::new())) .times(2); - let dependencies = - InjectableImpl::::build_dependencies(&method)?; + let dependencies = InjectableImpl::::build_dependencies(&method) + .expect("Expected Ok"); assert_eq!(dependencies.len(), 2); - - Ok(()) } #[test] diff --git a/macros/src/injectable/macro_args.rs b/macros/src/injectable/macro_args.rs index d730e0d..6582cc6 100644 --- a/macros/src/injectable/macro_args.rs +++ b/macros/src/injectable/macro_args.rs @@ -1,8 +1,10 @@ +use proc_macro2::Span; use syn::parse::{Parse, ParseStream}; use syn::punctuated::Punctuated; -use syn::{Token, TypePath}; +use syn::{Ident, Token, TypePath}; use crate::macro_flag::MacroFlag; +use crate::util::error::diagnostic_error_enum; use crate::util::iterator_ext::IteratorExt; pub const INJECTABLE_MACRO_FLAGS: &[&str] = @@ -14,9 +16,34 @@ pub struct InjectableMacroArgs pub flags: Punctuated, } +impl InjectableMacroArgs +{ + pub fn check_flags(&self) -> Result<(), InjectableMacroArgsError> + { + for flag in &self.flags { + if !INJECTABLE_MACRO_FLAGS.contains(&flag.flag.to_string().as_str()) { + return Err(InjectableMacroArgsError::UnknownFlag { + flag_ident: flag.flag.clone(), + }); + } + } + + if let Some((dupe_flag_first, dupe_flag_second)) = + self.flags.iter().find_duplicate() + { + return Err(InjectableMacroArgsError::DuplicateFlag { + first_flag_ident: dupe_flag_first.flag.clone(), + last_flag_span: dupe_flag_second.flag.span(), + }); + } + + Ok(()) + } +} + impl Parse for InjectableMacroArgs { - fn parse(input: ParseStream) -> syn::Result + fn parse(input: ParseStream) -> Result { let input_fork = input.fork(); @@ -50,31 +77,33 @@ impl Parse for InjectableMacroArgs let flags = Punctuated::::parse_terminated(input)?; - for flag in &flags { - let flag_str = flag.flag.to_string(); - - if !INJECTABLE_MACRO_FLAGS.contains(&flag_str.as_str()) { - return Err(input.error(format!( - "Unknown flag '{}'. Expected one of [ {} ]", - flag_str, - INJECTABLE_MACRO_FLAGS.join(",") - ))); - } - } - - let flag_names = flags - .iter() - .map(|flag| flag.flag.to_string()) - .collect::>(); - - if let Some(dupe_flag_name) = flag_names.iter().find_duplicate() { - return Err(input.error(format!("Duplicate flag '{dupe_flag_name}'"))); - } - Ok(Self { interface, flags }) } } +diagnostic_error_enum! { +pub enum InjectableMacroArgsError +{ + #[error("Unknown flag '{flag_ident}'"), span = flag_ident.span()] + #[ + help("Expected one of: {}", INJECTABLE_MACRO_FLAGS.join(", ")), + span = flag_ident.span() + ] + UnknownFlag + { + flag_ident: Ident + }, + + #[error("Duplicate flag '{first_flag_ident}'"), span = first_flag_ident.span()] + #[note("Previously mentioned here"), span = last_flag_span] + DuplicateFlag + { + first_flag_ident: Ident, + last_flag_span: Span + }, +} +} + #[cfg(test)] mod tests { @@ -188,30 +217,60 @@ mod tests } #[test] - fn cannot_parse_with_invalid_flag() + fn can_parse_with_unknown_flag() -> Result<(), Box> { let input_args = quote! { IFoo, haha = true, async = false }; - assert!(matches!(parse2::(input_args), Err(_))); + assert!(parse2::(input_args).is_ok()); + + Ok(()) } #[test] - fn cannot_parse_with_duplicate_flag() + fn can_parse_with_duplicate_flag() { - assert!(matches!( - parse2::(quote! { - IFoo, async = false, no_doc_hidden = true, async = false - }), - Err(_) - )); + assert!(parse2::(quote! { + IFoo, async = false, no_doc_hidden = true, async = false + }) + .is_ok()); + + assert!(parse2::(quote! { + IFoo, async = true , no_doc_hidden = true, async = false + }) + .is_ok()); + } - assert!(matches!( - parse2::(quote! { - IFoo, async = true , no_doc_hidden = true, async = false - }), - Err(_) - )); + #[test] + fn check_flags_fail_with_unknown_flag() -> Result<(), Box> + { + let input_args = quote! { + IFoo, haha = true, async = false + }; + + let injectable_macro_args = parse2::(input_args)?; + + assert!(injectable_macro_args.check_flags().is_err()); + + Ok(()) + } + + #[test] + fn check_flags_fail_with_duplicate_flag() -> Result<(), Box> + { + let macro_args = parse2::(quote! { + IFoo, async = false, no_doc_hidden = true, async = false + })?; + + assert!(macro_args.check_flags().is_err()); + + let macro_args_two = parse2::(quote! { + IFoo, async = true , no_doc_hidden = true, async = false + })?; + + assert!(macro_args_two.check_flags().is_err()); + + Ok(()) } } -- cgit v1.2.3-18-g5258