From e547d242a3cff1f951a7ca01aab339c053760d94 Mon Sep 17 00:00:00 2001 From: Valentin Tolmer Date: Sun, 9 Oct 2022 12:49:12 +0200 Subject: [PATCH] Rename DisplayArgs to EventArgs, clean up --- tracing-attributes/src/attr.rs | 150 +++++++++++++++++-------------- tracing-attributes/src/expand.rs | 21 ++--- tracing-attributes/tests/err.rs | 20 +++++ tracing-attributes/tests/ret.rs | 4 +- 4 files changed, 112 insertions(+), 83 deletions(-) diff --git a/tracing-attributes/src/attr.rs b/tracing-attributes/src/attr.rs index 0ca5e1dd99..93afd5ab3e 100644 --- a/tracing-attributes/src/attr.rs +++ b/tracing-attributes/src/attr.rs @@ -7,29 +7,29 @@ use syn::ext::IdentExt as _; use syn::parse::{Parse, ParseStream}; #[derive(Clone, Default, Debug)] -pub(crate) struct DisplayArgs { - pub(crate) level: Option, +pub(crate) struct EventArgs { + level: Option, pub(crate) mode: FormatMode, } #[derive(Clone, Default, Debug)] pub(crate) struct InstrumentArgs { - pub(crate) level: Option, + level: Option, pub(crate) name: Option, target: Option, pub(crate) parent: Option, pub(crate) follows_from: Option, pub(crate) skips: HashSet, pub(crate) fields: Option, - pub(crate) err_args: Option, - pub(crate) ret_args: Option, + pub(crate) err_args: Option, + pub(crate) ret_args: Option, /// Errors describing any unrecognized parse inputs that we skipped. parse_warnings: Vec, } impl InstrumentArgs { - pub(crate) fn level(&self) -> impl ToTokens { - level_to_tokens(&self.level, false) + pub(crate) fn level(&self) -> Level { + self.level.clone().unwrap_or(Level::Info) } pub(crate) fn target(&self) -> impl ToTokens { @@ -124,11 +124,11 @@ impl Parse for InstrumentArgs { args.fields = Some(input.parse()?); } else if lookahead.peek(kw::err) { let _ = input.parse::(); - let err_args = DisplayArgs::parse(input)?; + let err_args = EventArgs::parse(input)?; args.err_args = Some(err_args); } else if lookahead.peek(kw::ret) { let _ = input.parse::()?; - let ret_args = DisplayArgs::parse(input)?; + let ret_args = EventArgs::parse(input)?; args.ret_args = Some(ret_args); } else if lookahead.peek(Token![,]) { let _ = input.parse::()?; @@ -147,7 +147,13 @@ impl Parse for InstrumentArgs { } } -impl Parse for DisplayArgs { +impl EventArgs { + pub(crate) fn level(&self, default: Level) -> Level { + self.level.clone().unwrap_or(default) + } +} + +impl Parse for EventArgs { fn parse(input: ParseStream<'_>) -> syn::Result { if !input.peek(syn::token::Paren) { return Ok(Self::default()); @@ -162,21 +168,17 @@ impl Parse for DisplayArgs { return Err(content.error("expected only a single `level` argument")); } result.level = Some(content.parse()?); - } else { - if result.mode != FormatMode::default() { - return Err(content.error("expected only a single format argument")); - } - let maybe_mode: Option = content.parse()?; - if let Some(ident) = maybe_mode { - match ident.to_string().as_str() { - "Debug" => result.mode = FormatMode::Debug, - "Display" => result.mode = FormatMode::Display, - _ => { - return Err(syn::Error::new( - ident.span(), - "unknown error mode, must be Debug or Display", - )) - } + } else if result.mode != FormatMode::default() { + return Err(content.error("expected only a single format argument")); + } else if let Some(ident) = content.parse::>()? { + match ident.to_string().as_str() { + "Debug" => result.mode = FormatMode::Debug, + "Display" => result.mode = FormatMode::Display, + _ => { + return Err(syn::Error::new( + ident.span(), + "unknown error mode, must be Debug or Display", + )) } } } @@ -361,8 +363,11 @@ impl ToTokens for FieldKind { #[derive(Clone, Debug)] pub(crate) enum Level { - Str(LitStr), - Int(LitInt), + Trace, + Debug, + Info, + Warn, + Error, Path(Path), } @@ -372,9 +377,47 @@ impl Parse for Level { let _ = input.parse::()?; let lookahead = input.lookahead1(); if lookahead.peek(LitStr) { - Ok(Self::Str(input.parse()?)) + let str: LitStr = input.parse()?; + if str.value().eq_ignore_ascii_case("trace") { + Ok(Level::Trace) + } else if str.value().eq_ignore_ascii_case("debug") { + Ok(Level::Debug) + } else if str.value().eq_ignore_ascii_case("info") { + Ok(Level::Info) + } else if str.value().eq_ignore_ascii_case("warn") { + Ok(Level::Warn) + } else if str.value().eq_ignore_ascii_case("error") { + Ok(Level::Error) + } else { + Err(input.error( + "unknown verbosity level, expected one of \"trace\", \ + \"debug\", \"info\", \"warn\", or \"error\", or a number 1-5", + )) + } } else if lookahead.peek(LitInt) { - Ok(Self::Int(input.parse()?)) + fn is_level(lit: &LitInt, expected: u64) -> bool { + match lit.base10_parse::() { + Ok(value) => value == expected, + Err(_) => false, + } + } + let int: LitInt = input.parse()?; + if is_level(&int, 1) { + Ok(Level::Trace) + } else if is_level(&int, 2) { + Ok(Level::Debug) + } else if is_level(&int, 3) { + Ok(Level::Info) + } else if is_level(&int, 4) { + Ok(Level::Warn) + } else if is_level(&int, 5) { + Ok(Level::Error) + } else { + Err(input.error( + "unknown verbosity level, expected one of \"trace\", \ + \"debug\", \"info\", \"warn\", or \"error\", or a number 1-5", + )) + } } else if lookahead.peek(Ident) { Ok(Self::Path(input.parse()?)) } else { @@ -383,47 +426,16 @@ impl Parse for Level { } } -pub(crate) fn level_to_tokens(level: &Option, default_error: bool) -> impl ToTokens { - fn is_level(lit: &LitInt, expected: u64) -> bool { - match lit.base10_parse::() { - Ok(value) => value == expected, - Err(_) => false, - } - } - - match level { - Some(Level::Str(ref lit)) if lit.value().eq_ignore_ascii_case("trace") => { - quote!(tracing::Level::TRACE) - } - Some(Level::Str(ref lit)) if lit.value().eq_ignore_ascii_case("debug") => { - quote!(tracing::Level::DEBUG) - } - Some(Level::Str(ref lit)) if lit.value().eq_ignore_ascii_case("info") => { - quote!(tracing::Level::INFO) - } - Some(Level::Str(ref lit)) if lit.value().eq_ignore_ascii_case("warn") => { - quote!(tracing::Level::WARN) - } - Some(Level::Str(ref lit)) if lit.value().eq_ignore_ascii_case("error") => { - quote!(tracing::Level::ERROR) +impl ToTokens for Level { + fn to_tokens(&self, tokens: &mut TokenStream) { + match self { + Level::Trace => tokens.extend(quote!(tracing::Level::TRACE)), + Level::Debug => tokens.extend(quote!(tracing::Level::DEBUG)), + Level::Info => tokens.extend(quote!(tracing::Level::INFO)), + Level::Warn => tokens.extend(quote!(tracing::Level::WARN)), + Level::Error => tokens.extend(quote!(tracing::Level::ERROR)), + Level::Path(ref pat) => tokens.extend(quote!(#pat)), } - Some(Level::Int(ref lit)) if is_level(lit, 1) => quote!(tracing::Level::TRACE), - Some(Level::Int(ref lit)) if is_level(lit, 2) => quote!(tracing::Level::DEBUG), - Some(Level::Int(ref lit)) if is_level(lit, 3) => quote!(tracing::Level::INFO), - Some(Level::Int(ref lit)) if is_level(lit, 4) => quote!(tracing::Level::WARN), - Some(Level::Int(ref lit)) if is_level(lit, 5) => quote!(tracing::Level::ERROR), - Some(Level::Path(ref pat)) => quote!(#pat), - Some(_) => quote! { - compile_error!( - "unknown verbosity level, expected one of \"trace\", \ - \"debug\", \"info\", \"warn\", or \"error\", or a number 1-5" - ) - }, - None => if default_error { - quote!(tracing::Level::ERROR) - } else { - quote!(tracing::Level::INFO) - }, } } diff --git a/tracing-attributes/src/expand.rs b/tracing-attributes/src/expand.rs index 437f35f266..b1e6b25269 100644 --- a/tracing-attributes/src/expand.rs +++ b/tracing-attributes/src/expand.rs @@ -10,7 +10,7 @@ use syn::{ }; use crate::{ - attr::{level_to_tokens, DisplayArgs, Field, Fields, FormatMode, InstrumentArgs}, + attr::{Level, Field, Fields, FormatMode, InstrumentArgs}, MaybeItemFn, MaybeItemFnRef, }; @@ -116,7 +116,8 @@ fn gen_block( .map(|name| quote!(#name)) .unwrap_or_else(|| quote!(#instrumented_function_name)); - let level = args.level(); + let args_level = args.level(); + let level = args_level.clone(); let follows_from = args.follows_from.iter(); let follows_from = quote! { @@ -233,9 +234,9 @@ fn gen_block( let target = args.target(); let err_event = match args.err_args { - Some(DisplayArgs { level, mode }) => { - let level_tokens = level_to_tokens(&level, true); - match mode { + Some(event_args) => { + let level_tokens = event_args.level(Level::Error); + match event_args.mode { FormatMode::Default | FormatMode::Display => Some(quote!( tracing::event!(target: #target, #level_tokens, error = %e) )), @@ -248,13 +249,9 @@ fn gen_block( }; let ret_event = match args.ret_args { - Some(DisplayArgs { - level: ret_level, - mode, - }) => { - let ret_level = ret_level.or(args.level); - let level_tokens = level_to_tokens(&ret_level, false); - match mode { + Some(event_args) => { + let level_tokens = event_args.level(args_level); + match event_args.mode { FormatMode::Display => Some(quote!( tracing::event!(target: #target, #level_tokens, return = %x) )), diff --git a/tracing-attributes/tests/err.rs b/tracing-attributes/tests/err.rs index f5aa3b3514..842a6e0e3a 100644 --- a/tracing-attributes/tests/err.rs +++ b/tracing-attributes/tests/err.rs @@ -301,3 +301,23 @@ fn test_err_dbg_info() { with_default(collector, || err_dbg_info().ok()); handle.assert_finished(); } + +#[instrument(level = "warn", err(level = "info"))] +fn err_warn_info() -> Result { + u8::try_from(1234) +} + +#[test] +fn test_err_warn_info() { + let span = span::mock().named("err_warn_info").at_level(Level::WARN); + let (collector, handle) = collector::mock() + .new_span(span.clone()) + .enter(span.clone()) + .event(event::mock().at_level(Level::INFO)) + .exit(span.clone()) + .drop_span(span) + .done() + .run_with_handle(); + with_default(collector, || err_warn_info().ok()); + handle.assert_finished(); +} diff --git a/tracing-attributes/tests/ret.rs b/tracing-attributes/tests/ret.rs index 5650c429d5..2511e0a520 100644 --- a/tracing-attributes/tests/ret.rs +++ b/tracing-attributes/tests/ret.rs @@ -261,7 +261,7 @@ fn ret_warn_info() -> i32 { #[test] fn test_warn_info() { - let span = span::mock().named("ret_warn_info"); + let span = span::mock().named("ret_warn_info").at_level(Level::WARN); let (collector, handle) = collector::mock() .new_span(span.clone()) .enter(span.clone()) @@ -286,7 +286,7 @@ fn ret_dbg_warn() -> i32 { #[test] fn test_dbg_warn() { - let span = span::mock().named("ret_dbg_warn"); + let span = span::mock().named("ret_dbg_warn").at_level(Level::INFO); let (collector, handle) = collector::mock() .new_span(span.clone()) .enter(span.clone())