diff --git a/tracing-attributes/tests/fields.rs b/tracing-attributes/tests/fields.rs index 7124572d6..b1e58ef1b 100644 --- a/tracing-attributes/tests/fields.rs +++ b/tracing-attributes/tests/fields.rs @@ -77,6 +77,33 @@ impl HasField { fn self_expr_field(&self) {} } +struct Disp { + bar: u64, +} + +impl ::std::fmt::Display for Disp { + fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result { + ::std::write!(f, "Disp.bar={bar}", bar = self.bar) + } +} + +#[derive(Debug)] +struct Deb { + _val: u64, +} + +#[instrument(fields(foo = display(Disp { bar: 6, })))] +fn fn_bare_display() {} + +#[instrument(skip_all, fields(foo = display(param)))] +fn fn_bare_display_param(param: Disp) {} + +#[instrument(fields(foo = debug(Deb { _val: 100, })))] +fn fn_bare_debug() {} + +#[instrument(skip_all, fields(foo = debug(param)))] +fn fn_bare_debug_param(param: Deb) {} + #[test] fn fields() { let span = expect::span().with_fields( @@ -236,6 +263,52 @@ fn clashy_const_field_name() { }); } +#[test] +fn bare_display() { + use tracing::field::display; + + let span = expect::span().with_fields(expect::field("foo").with_value(&display("Disp.bar=6"))); + + run_test(span, || { + fn_bare_display(); + }); +} + +#[test] +fn bare_display_param() { + use tracing::field::display; + + let span = expect::span().with_fields(expect::field("foo").with_value(&display("Disp.bar=6"))); + + run_test(span, || { + fn_bare_display_param(Disp { bar: 6 }); + }); +} + +#[test] +fn bare_debug() { + use tracing::field::debug; + + let span = + expect::span().with_fields(expect::field("foo").with_value(&debug(Deb { _val: 100 }))); + + run_test(span, || { + fn_bare_debug(); + }); +} + +#[test] +fn bare_debug_param() { + use tracing::field::debug; + + let span = + expect::span().with_fields(expect::field("foo").with_value(&debug(Deb { _val: 100 }))); + + run_test(span, || { + fn_bare_debug_param(Deb { _val: 100 }); + }); +} + fn run_test T, T>(span: NewSpan, fun: F) { let (subscriber, handle) = subscriber::mock() .new_span(span) diff --git a/tracing/Cargo.toml b/tracing/Cargo.toml index 4f13c01b6..9b0c0ec82 100644 --- a/tracing/Cargo.toml +++ b/tracing/Cargo.toml @@ -35,6 +35,8 @@ criterion = { version = "0.3.6", default-features = false } futures = { version = "0.3.21", default-features = false } log = "0.4.17" tracing-mock = { path = "../tracing-mock" } +trybuild = "1.0.64" +rustversion = "1.0.9" [target.'cfg(target_arch = "wasm32")'.dev-dependencies] wasm-bindgen-test = "0.3.38" diff --git a/tracing/src/macros.rs b/tracing/src/macros.rs index 6e6bc2f56..a98f72ca6 100644 --- a/tracing/src/macros.rs +++ b/tracing/src/macros.rs @@ -2831,12 +2831,24 @@ macro_rules! valueset { $($rest)* ) }; + (@ { $(,)* $($out:expr),* }, $($k:ident).+ = debug($val:expr), $($rest:tt)*) => { + $crate::valueset!( + @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::debug(&$val) as &dyn $crate::field::Value)) }, + $($rest)* + ) + }; (@ { $(,)* $($out:expr),* }, $($k:ident).+ = %$val:expr, $($rest:tt)*) => { $crate::valueset!( @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::display(&$val) as &dyn $crate::field::Value)) }, $($rest)* ) }; + (@ { $(,)* $($out:expr),* }, $($k:ident).+ = display($val:expr), $($rest:tt)*) => { + $crate::valueset!( + @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::display(&$val) as &dyn $crate::field::Value)) }, + $($rest)* + ) + }; (@ { $(,)* $($out:expr),* }, $($k:ident).+ = $val:expr, $($rest:tt)*) => { $crate::valueset!( @ { $($out),*, ($crate::__macro_support::Option::Some(&$val as &dyn $crate::field::Value)) }, @@ -2866,11 +2878,21 @@ macro_rules! valueset { @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::debug(&$val) as &dyn $crate::field::Value)) }, ) }; + (@ { $(,)* $($out:expr),* }, $($k:ident).+ = debug($val:expr)) => { + $crate::valueset!( + @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::debug(&$val) as &dyn $crate::field::Value)) }, + ) + }; (@ { $(,)* $($out:expr),* }, $($k:ident).+ = %$val:expr) => { $crate::valueset!( @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::display(&$val) as &dyn $crate::field::Value)) }, ) }; + (@ { $(,)* $($out:expr),* }, $($k:ident).+ = display($val:expr)) => { + $crate::valueset!( + @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::display(&$val) as &dyn $crate::field::Value)) }, + ) + }; (@ { $(,)* $($out:expr),* }, $($k:ident).+ = $val:expr) => { $crate::valueset!( @ { $($out),*, ($crate::__macro_support::Option::Some(&$val as &dyn $crate::field::Value)) }, @@ -2899,12 +2921,24 @@ macro_rules! valueset { $($rest)* ) }; + (@ { $(,)* $($out:expr),* }, $k:literal = debug($val:expr), $($rest:tt)*) => { + $crate::valueset!( + @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::debug(&$val) as &dyn $crate::field::Value)) }, + $($rest)* + ) + }; (@ { $(,)* $($out:expr),* }, $k:literal = %$val:expr, $($rest:tt)*) => { $crate::valueset!( @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::display(&$val) as &dyn $crate::field::Value)) }, $($rest)* ) }; + (@ { $(,)* $($out:expr),* }, $k:literal = display($val:expr), $($rest:tt)*) => { + $crate::valueset!( + @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::display(&$val) as &dyn $crate::field::Value)) }, + $($rest)* + ) + }; (@ { $(,)* $($out:expr),* }, $k:literal = $val:expr, $($rest:tt)*) => { $crate::valueset!( @ { $($out),*, ($crate::__macro_support::Option::Some(&$val as &dyn $crate::field::Value)) }, @@ -2916,11 +2950,21 @@ macro_rules! valueset { @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::debug(&$val) as &dyn $crate::field::Value)) }, ) }; + (@ { $(,)* $($out:expr),* }, $k:literal = debug($val:expr)) => { + $crate::valueset!( + @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::debug(&$val) as &dyn $crate::field::Value)) }, + ) + }; (@ { $(,)* $($out:expr),* }, $k:literal = %$val:expr) => { $crate::valueset!( @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::display(&$val) as &dyn $crate::field::Value)) }, ) }; + (@ { $(,)* $($out:expr),* }, $k:literal = display($val:expr)) => { + $crate::valueset!( + @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::display(&$val) as &dyn $crate::field::Value)) }, + ) + }; (@ { $(,)* $($out:expr),* }, $k:literal = $val:expr) => { $crate::valueset!( @ { $($out),*, ($crate::__macro_support::Option::Some(&$val as &dyn $crate::field::Value)) }, @@ -2930,35 +2974,57 @@ macro_rules! valueset { // Handle constant names (@ { $(,)* $($out:expr),* }, { $k:expr } = ?$val:expr, $($rest:tt)*) => { $crate::valueset!( - @ { $($out),*, (Some(&$crate::field::debug(&$val) as &dyn $crate::field::Value)) }, + @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::debug(&$val) as &dyn $crate::field::Value)) }, + $($rest)* + ) + }; + (@ { $(,)* $($out:expr),* }, { $k:expr } = debug($val:expr), $($rest:tt)*) => { + $crate::valueset!( + @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::debug(&$val) as &dyn $crate::field::Value)) }, $($rest)* ) }; (@ { $(,)* $($out:expr),* }, { $k:expr } = %$val:expr, $($rest:tt)*) => { $crate::valueset!( - @ { $($out),*, (Some(&$crate::field::display(&$val) as &dyn $crate::field::Value)) }, + @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::display(&$val) as &dyn $crate::field::Value)) }, + $($rest)* + ) + }; + (@ { $(,)* $($out:expr),* }, { $k:expr } = display($val:expr), $($rest:tt)*) => { + $crate::valueset!( + @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::display(&$val) as &dyn $crate::field::Value)) }, $($rest)* ) }; (@ { $(,)* $($out:expr),* }, { $k:expr } = $val:expr, $($rest:tt)*) => { $crate::valueset!( - @ { $($out),*, (Some(&$val as &dyn $crate::field::Value)) }, + @ { $($out),*, ($crate::__macro_support::Option::Some(&$val as &dyn $crate::field::Value)) }, $($rest)* ) }; (@ { $(,)* $($out:expr),* }, { $k:expr } = ?$val:expr) => { $crate::valueset!( - @ { $($out),*, (Some(&$crate::field::debug(&$val) as &dyn $crate::field::Value)) }, + @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::debug(&$val) as &dyn $crate::field::Value)) }, + ) + }; + (@ { $(,)* $($out:expr),* }, { $k:expr } = debug($val:expr)) => { + $crate::valueset!( + @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::debug(&$val) as &dyn $crate::field::Value)) }, ) }; (@ { $(,)* $($out:expr),* }, { $k:expr } = %$val:expr) => { $crate::valueset!( - @ { $($out),*, (Some(&$crate::field::display(&$val) as &dyn $crate::field::Value)) }, + @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::display(&$val) as &dyn $crate::field::Value)) }, + ) + }; + (@ { $(,)* $($out:expr),* }, { $k:expr } = display($val:expr)) => { + $crate::valueset!( + @ { $($out),*, ($crate::__macro_support::Option::Some(&$crate::field::display(&$val) as &dyn $crate::field::Value)) }, ) }; (@ { $(,)* $($out:expr),* }, { $k:expr } = $val:expr) => { $crate::valueset!( - @ { $($out),*, (Some(&$val as &dyn $crate::field::Value)) }, + @ { $($out),*, ($crate::__macro_support::Option::Some(&$val as &dyn $crate::field::Value)) }, ) }; @@ -2973,7 +3039,10 @@ macro_rules! valueset { #[allow(unused_imports)] // This import statement CANNOT be removed as it will break existing use cases. // See #831, #2332, #3424 for the last times we tried. - use $crate::field::{debug, display, Value}; + // It previously contained `display` and `debug` as well, but those have been + // moved into the macro patterns instead, still no idea how to remove `Value` + // without breaking though. + use $crate::field::Value; $fields.value_set_all($crate::valueset!( @ { }, $($kvs)+ diff --git a/tracing/tests/macros.rs b/tracing/tests/macros.rs index 9fa3bb03e..5356507b0 100644 --- a/tracing/tests/macros.rs +++ b/tracing/tests/macros.rs @@ -20,6 +20,21 @@ impl ::std::fmt::Display for DisplayDebug { } } +struct Disp { + bar: u64, +} + +impl ::std::fmt::Display for Disp { + fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result { + ::std::write!(f, "Disp.bar={bar}", bar = self.bar) + } +} + +#[derive(Debug)] +struct Deb { + _val: u64, +} + // Tests that macros work across various invocation syntax. // // These are quite repetitive, and _could_ be generated by a macro. However, @@ -1398,6 +1413,91 @@ fn borrow_val_spans() { zero.push_str("hello world"); } +#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)] +#[test] +fn bare_display_debug() { + error!(foo = display("foo")); + error!(foo = display(Disp { bar: 4 })); + error!(quux = debug(Deb { _val: 1024 })); + error!(quux = debug(4_u8 as u64)); + + error!(mog = 5, foo = display("foo")); + error!(mog = 5, foo = display(Disp { bar: 4 })); + error!(mog = 5, quux = debug(Deb { _val: 1024 })); + error!(mog = 5, quux = debug(4_u8 as u64)); + + error!(foo = display("foo"), mog = 5); + error!(foo = display(Disp { bar: 4 }), mog = 5); + error!(quux = debug(Deb { _val: 1024 }), mog = 5); + error!(quux = debug(4_u8 as u64), mog = 5); + + error!({ foo = display("foo") }, "gom = {:?}", 8); + error!({ foo = display(Disp { bar: 4 }) }, "gom = {:?}", 8); + error!({ quux = debug(Deb { _val: 1024 }) }, "gom = {:?}", 8); + error!({ quux = debug(4_u8 as u64) }, "gom = {:?}", 8); + + error!("foo" = display("foo")); + error!("foo" = display(Disp { bar: 4 })); + error!("quux" = debug(Deb { _val: 1024 })); + error!("quux" = debug(4_u8 as u64)); + + error!(mog = 5, "foo" = display("foo")); + error!(mog = 5, "foo" = display(Disp { bar: 4 })); + error!(mog = 5, "quux" = debug(Deb { _val: 1024 })); + error!(mog = 5, "quux" = debug(4_u8 as u64)); + + error!("foo" = display("foo"), mog = 5); + error!("foo" = display(Disp { bar: 4 }), mog = 5); + error!("quux" = debug(Deb { _val: 1024 }), mog = 5); + error!("quux" = debug(4_u8 as u64), mog = 5); + + error!({ "foo" = display("foo") }, "gom = {:?}", 8); + error!({ "foo" = display(Disp { bar: 4 }) }, "gom = {:?}", 8); + error!({ "quux" = debug(Deb { _val: 1024 }) }, "gom = {:?}", 8); + error!({ "quux" = debug(4_u8 as u64) }, "gom = {:?}", 8); + + const FOO: &str = "FOO"; + const QUUX: &str = "QUUX"; + error!({ FOO } = display("foo")); + error!({ FOO } = display(Disp { bar: 4 })); + error!({ QUUX } = debug(Deb { _val: 1024 })); + error!({ QUUX } = debug(4_u8 as u64)); + + error!(mog = 5, { FOO } = display("foo")); + error!(mog = 5, { FOO } = display(Disp { bar: 4 })); + error!(mog = 5, { QUUX } = debug(Deb { _val: 1024 })); + error!(mog = 5, { QUUX } = debug(4_u8 as u64)); + + error!({ FOO } = display("foo"), mog = 5); + error!({ FOO } = display(Disp { bar: 4 }), mog = 5); + error!({ QUUX } = debug(Deb { _val: 1024 }), mog = 5); + error!({ QUUX } = debug(4_u8 as u64), mog = 5); + + error!({ {FOO} = display("foo") }, "gom = {:?}", 8); + error!({ {FOO} = display(Disp { bar: 4, }) }, "gom = {:?}", 8); + error!({ {QUUX} = debug(Deb { _val: 1024, }) }, "gom = {:?}", 8); + error!({ {QUUX} = debug(4_u8 as u64) }, "gom = {:?}", 8); + + let display = Disp { bar: 4 }; + let debug = Deb { _val: 96 }; + error!("{debug:?} {display}"); + error!(%display, ?debug); + error!(display = %display, debug = ?debug); + + let display = "str also implements Value"; + let debug = "str also implements Value"; + error!("{debug:?} {display}"); + error!(display, debug); + error!(display = display, debug = debug); +} + +#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)] +#[test] +fn bare_value() { + let num = ::std::option::Option::Some(52); + error!(bar = &num as &dyn Value); +} + #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)] #[test] fn callsite_macro_api() { diff --git a/tracing/tests/ui.rs b/tracing/tests/ui.rs new file mode 100644 index 000000000..7dabf3bbe --- /dev/null +++ b/tracing/tests/ui.rs @@ -0,0 +1,8 @@ +// Only test on stable, since UI tests are bound to change over time + +#[rustversion::stable] +#[test] +fn compile_fail() { + let t = trybuild::TestCases::new(); + t.compile_fail("tests/ui/fail/*.rs"); +} diff --git a/tracing/tests/ui/fail/debug_without_key.rs b/tracing/tests/ui/fail/debug_without_key.rs new file mode 100644 index 000000000..5adc3c8ff --- /dev/null +++ b/tracing/tests/ui/fail/debug_without_key.rs @@ -0,0 +1,9 @@ +//! We support bare `debug` function in macros, but only when a key is +//! specified (e.g. `foo = debug(foo)` is supported but just `debug(foo)` +//! isn't). + +fn main() { + let foo = "foo"; + tracing::info!(debug(foo)); +} + diff --git a/tracing/tests/ui/fail/debug_without_key.stderr b/tracing/tests/ui/fail/debug_without_key.stderr new file mode 100644 index 000000000..00624cdbc --- /dev/null +++ b/tracing/tests/ui/fail/debug_without_key.stderr @@ -0,0 +1,10 @@ +error: format argument must be a string literal + --> tests/ui/fail/debug_without_key.rs:7:20 + | +7 | tracing::info!(debug(foo)); + | ^^^^^^^^^^ + | +help: you might be missing a string literal to format with + | +7 | tracing::info!("{}", debug(foo)); + | +++++ diff --git a/tracing/tests/ui/fail/display_without_key.rs b/tracing/tests/ui/fail/display_without_key.rs new file mode 100644 index 000000000..b6a7ca72e --- /dev/null +++ b/tracing/tests/ui/fail/display_without_key.rs @@ -0,0 +1,8 @@ +//! We support bare `display` function in macros, but only when a key is +//! specified (e.g. `foo = display(foo)` is supported but just `display(foo)` +//! isn't). + +fn main() { + let foo = "foo"; + tracing::info!(display(foo)); +} diff --git a/tracing/tests/ui/fail/display_without_key.stderr b/tracing/tests/ui/fail/display_without_key.stderr new file mode 100644 index 000000000..16a14866d --- /dev/null +++ b/tracing/tests/ui/fail/display_without_key.stderr @@ -0,0 +1,10 @@ +error: format argument must be a string literal + --> tests/ui/fail/display_without_key.rs:7:20 + | +7 | tracing::info!(display(foo)); + | ^^^^^^^^^^^^ + | +help: you might be missing a string literal to format with + | +7 | tracing::info!("{}", display(foo)); + | +++++