diff --git a/Cargo.lock b/Cargo.lock index ca93d68e91c..5aecd17db3b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3176,10 +3176,11 @@ name = "iroha_telemetry_derive" version = "2.0.0-pre-rc.20" dependencies = [ "iroha_core", - "proc-macro-error", + "iroha_macro_utils", + "manyhow", "proc-macro2", "quote", - "syn 1.0.109", + "syn 2.0.41", "trybuild", ] diff --git a/telemetry/derive/Cargo.toml b/telemetry/derive/Cargo.toml index b2c53970a87..de03e2bb488 100644 --- a/telemetry/derive/Cargo.toml +++ b/telemetry/derive/Cargo.toml @@ -19,10 +19,11 @@ is-it-maintained-open-issues = { repository = "https://github.com/hyperledger/ir maintenance = { status = "actively-developed" } [dependencies] -syn = { workspace = true, features = ["default", "full"] } +syn2 = { workspace = true } quote = { workspace = true } proc-macro2 = { workspace = true } -proc-macro-error = { workspace = true } +manyhow = { workspace = true } +iroha_macro_utils = { workspace = true } [dev-dependencies] iroha_core = { workspace = true } diff --git a/telemetry/derive/src/lib.rs b/telemetry/derive/src/lib.rs index 779b3cdba0b..eff6d509c62 100644 --- a/telemetry/derive/src/lib.rs +++ b/telemetry/derive/src/lib.rs @@ -1,15 +1,11 @@ //! Attribute-like macro for instrumenting `isi` for `prometheus` //! metrics. See [`macro@metrics`] for more details. -use proc_macro::TokenStream; -#[cfg(feature = "metric-instrumentation")] -use proc_macro2::TokenStream as TokenStream2; -use proc_macro_error::{abort, proc_macro_error}; +use iroha_macro_utils::Emitter; +use manyhow::{emit, manyhow, Result}; +use proc_macro2::TokenStream; use quote::{quote, ToTokens}; -use syn::{ - parse::Parse, parse_macro_input, punctuated::Punctuated, token::Comma, FnArg, ItemFn, LitStr, - Path, Type, -}; +use syn2::{parse::Parse, punctuated::Punctuated, token::Comma, FnArg, LitStr, Path, Type}; // TODO: export these as soon as proc-macro crates are able to export // anything other than proc-macros. @@ -43,19 +39,19 @@ fn type_has_metrics_field(ty: &Type) -> bool { /// /// # Errors /// If no argument is of type `WorldStateView`. -fn arg_metrics(input: &Punctuated) -> Result> { +fn arg_metrics(input: &Punctuated) -> Result> { input .iter() .find(|arg| match arg { FnArg::Typed(typ) => match &*typ.ty { - syn::Type::Reference(typ) => type_has_metrics_field(&typ.elem), + syn2::Type::Reference(typ) => type_has_metrics_field(&typ.elem), _ => false, }, _ => false, }) .and_then(|arg| match arg { FnArg::Typed(typ) => match *typ.pat.clone() { - syn::Pat::Ident(ident) => Some(ident.ident), + syn2::Pat::Ident(ident) => Some(ident.ident), _ => None, }, _ => None, @@ -67,7 +63,7 @@ fn arg_metrics(input: &Punctuated) -> Result); // `HashSet` — idiomatic; slow impl Parse for MetricSpecs { - fn parse(input: syn::parse::ParseStream) -> syn::Result { + fn parse(input: syn2::parse::ParseStream) -> syn2::Result { let vars = Punctuated::::parse_terminated(input)?; Ok(Self(vars.into_iter().collect())) } @@ -80,14 +76,14 @@ struct MetricSpec { } impl Parse for MetricSpec { - fn parse(input: syn::parse::ParseStream) -> syn::Result { - let _timing = ::parse(input).is_ok(); - let metric_name_lit = syn::Lit::parse(input)?; + fn parse(input: syn2::parse::ParseStream) -> syn2::Result { + let _timing = ::parse(input).is_ok(); + let metric_name_lit = syn2::Lit::parse(input)?; let metric_name = match metric_name_lit { - syn::Lit::Str(lit_str) => { + syn2::Lit::Str(lit_str) => { if lit_str.value().contains(' ') { - return Err(syn::Error::new( + return Err(syn2::Error::new( proc_macro2::Span::call_site(), "Spaces are not allowed. Use underscores '_'", )); @@ -95,7 +91,7 @@ impl Parse for MetricSpec { lit_str } _ => { - return Err(syn::Error::new( + return Err(syn2::Error::new( proc_macro2::Span::call_site(), "Must be a string literal. Format `[+]\"name_of_metric\"`.", )) @@ -145,54 +141,91 @@ impl ToTokens for MetricSpec { /// Ok(()) /// } /// ``` -#[proc_macro_error] +#[manyhow] #[proc_macro_attribute] pub fn metrics(attr: TokenStream, item: TokenStream) -> TokenStream { - let ItemFn { + let mut emitter = Emitter::new(); + + let Some(func): Option = emitter.handle(syn2::parse2(item)) else { + return emitter.finish_token_stream(); + }; + + // This is a good sanity check. Possibly redundant. + if func.sig.ident != "execute" { + emit!( + emitter, + func.sig.ident, + "Function should be an `impl execute`" + ); + } + + if func.sig.inputs.is_empty() { + emit!( + emitter, + func.sig, + "Function must have at least one argument of type `WorldStateView`." + ); + return emitter.finish_token_stream(); + } + + let Some(metric_specs): Option = emitter.handle(syn2::parse2(attr)) else { + return emitter.finish_token_stream(); + }; + + let result = impl_metrics(&mut emitter, &metric_specs, &func); + + emitter.finish_token_stream_with(result) +} + +fn impl_metrics(emitter: &mut Emitter, _specs: &MetricSpecs, func: &syn2::ItemFn) -> TokenStream { + let syn2::ItemFn { attrs, vis, sig, block, - }: ItemFn = parse_macro_input!(item as ItemFn); + } = func; - // This is a good sanity check. Possibly redundant. - if sig.ident != "execute" { - abort!(sig.ident, "Function should be an `impl execute`"); - } match sig.output.clone() { - syn::ReturnType::Default => abort!( + syn2::ReturnType::Default => emit!( + emitter, sig.output, "`Fn` must return `Result`. Returns nothing instead. " ), #[allow(clippy::string_to_string)] - syn::ReturnType::Type(_, typ) => match *typ { + syn2::ReturnType::Type(_, typ) => match *typ { Type::Path(pth) => { let Path { segments, .. } = pth.path; let type_name = &segments.last().expect("non-empty path").ident; if *type_name != "Result" { - abort!( + emit!( + emitter, type_name, - format!("Should return `Result`. Found {type_name}") + "Should return `Result`. Found {}", + type_name ); } } - _ => abort!( + _ => emit!( + emitter, typ, "Should return `Result`. Non-path result type specification found" ), }, } - if sig.inputs.is_empty() { - abort!( - sig, - "Function must have at least one argument of type `WorldStateView`." - ); - } - let _specs = parse_macro_input!(attr as MetricSpecs); + // Again this may seem fragile, but if we move the metrics from // the `WorldStateView`, we'd need to refactor many things anyway - let _metric_arg_ident = arg_metrics(&sig.inputs) - .unwrap_or_else(|args| abort!(args, "At least one argument must be a `WorldStateView`.")); + let _metric_arg_ident = match arg_metrics(&sig.inputs) { + Ok(ident) => ident, + Err(args) => { + emit!( + emitter, + args, + "At least one argument must be a `WorldStateView`." + ); + return quote!(); + } + }; #[cfg(feature = "metric-instrumentation")] let res = { @@ -210,8 +243,7 @@ pub fn metrics(attr: TokenStream, item: TokenStream) -> TokenStream { #successes }; res - }) - .into(); + }); }; #[cfg(not(feature = "metric-instrumentation"))] @@ -219,8 +251,7 @@ pub fn metrics(attr: TokenStream, item: TokenStream) -> TokenStream { #(#attrs)* #vis #sig { #block } - ) - .into(); + ); res } @@ -228,7 +259,7 @@ pub fn metrics(attr: TokenStream, item: TokenStream) -> TokenStream { fn write_metrics( metric_arg_ident: proc_macro2::Ident, specs: MetricSpecs, -) -> (TokenStream2, TokenStream2, TokenStream2) { +) -> (TokenStream, TokenStream, TokenStream) { let inc_metric = |spec: &MetricSpec, kind: &str| { quote!( #metric_arg_ident @@ -246,17 +277,17 @@ fn write_metrics( .observe((end_time.as_millis() - start_time.as_millis()) as f64); ) }; - let totals: TokenStream2 = specs + let totals: TokenStream = specs .0 .iter() .map(|spec| inc_metric(spec, "total")) .collect(); - let successes: TokenStream2 = specs + let successes: TokenStream = specs .0 .iter() .map(|spec| inc_metric(spec, "success")) .collect(); - let times: TokenStream2 = specs + let times: TokenStream = specs .0 .iter() .filter(|spec| spec.timing) diff --git a/telemetry/derive/tests/ui_fail/not_execute.rs b/telemetry/derive/tests/ui_fail/not_execute.rs index 7a63c17d08f..ddd566862bd 100644 --- a/telemetry/derive/tests/ui_fail/not_execute.rs +++ b/telemetry/derive/tests/ui_fail/not_execute.rs @@ -1,4 +1,5 @@ use iroha_telemetry_derive::metrics; +use iroha_core::prelude::WorldStateView; #[metrics(+"test_query", "another_test_query_without_timing")] fn exequte(wsv: &WorldStateView) -> Result<(), ()> { diff --git a/telemetry/derive/tests/ui_fail/not_execute.stderr b/telemetry/derive/tests/ui_fail/not_execute.stderr index 7f4ab728dae..4146a4ab62b 100644 --- a/telemetry/derive/tests/ui_fail/not_execute.stderr +++ b/telemetry/derive/tests/ui_fail/not_execute.stderr @@ -1,5 +1,5 @@ error: Function should be an `impl execute` - --> tests/ui_fail/not_execute.rs:4:4 + --> tests/ui_fail/not_execute.rs:5:4 | -4 | fn exequte(wsv: &WorldStateView) -> Result<(), ()> { +5 | fn exequte(wsv: &WorldStateView) -> Result<(), ()> { | ^^^^^^^ diff --git a/telemetry/derive/tests/ui_fail/not_return_result.rs b/telemetry/derive/tests/ui_fail/not_return_result.rs index ca779d8e5ec..b5620e99f63 100644 --- a/telemetry/derive/tests/ui_fail/not_return_result.rs +++ b/telemetry/derive/tests/ui_fail/not_return_result.rs @@ -1,8 +1,11 @@ use iroha_telemetry_derive::metrics; +use iroha_core::prelude::WorldStateView; + +type MyNotResult = Option; #[metrics(+"test_query", "another_test_query_without_timing")] -fn execute(_wsv: &WorldStateView) -> iroha_core::RESULT { - Ok(()) +fn execute(_wsv: &WorldStateView) -> MyNotResult { + None } fn main() { diff --git a/telemetry/derive/tests/ui_fail/not_return_result.stderr b/telemetry/derive/tests/ui_fail/not_return_result.stderr index 6652f72014d..09daedb0d51 100644 --- a/telemetry/derive/tests/ui_fail/not_return_result.stderr +++ b/telemetry/derive/tests/ui_fail/not_return_result.stderr @@ -1,5 +1,5 @@ -error: Should return `Result`. Found RESULT - --> tests/ui_fail/not_return_result.rs:4:50 +error: Should return `Result`. Found MyNotResult + --> tests/ui_fail/not_return_result.rs:7:38 | -4 | fn execute(_wsv: &WorldStateView) -> iroha_core::RESULT { - | ^^^^^^ +7 | fn execute(_wsv: &WorldStateView) -> MyNotResult { + | ^^^^^^^^^^^ diff --git a/telemetry/derive/tests/ui_fail/return_nothing.rs b/telemetry/derive/tests/ui_fail/return_nothing.rs index 419325ac0ba..6058f160b19 100644 --- a/telemetry/derive/tests/ui_fail/return_nothing.rs +++ b/telemetry/derive/tests/ui_fail/return_nothing.rs @@ -1,8 +1,9 @@ use iroha_telemetry_derive::metrics; +use iroha_core::prelude::WorldStateView; #[metrics(+"test_query", "another_test_query_without_timing")] fn execute(wsv: &WorldStateView) { - Ok(()) + Ok::<(), ()>(()); } fn main() { diff --git a/telemetry/derive/tests/ui_fail/return_nothing.stderr b/telemetry/derive/tests/ui_fail/return_nothing.stderr index 93385e20c54..f92d23ee205 100644 --- a/telemetry/derive/tests/ui_fail/return_nothing.stderr +++ b/telemetry/derive/tests/ui_fail/return_nothing.stderr @@ -1,7 +1,7 @@ error: `Fn` must return `Result`. Returns nothing instead. - --> tests/ui_fail/return_nothing.rs:3:1 + --> tests/ui_fail/return_nothing.rs:4:1 | -3 | #[metrics(+"test_query", "another_test_query_without_timing")] +4 | #[metrics(+"test_query", "another_test_query_without_timing")] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: this error originates in the attribute macro `metrics` (in Nightly builds, run with -Z macro-backtrace for more info)