diff --git a/CHANGELOG.md b/CHANGELOG.md index 73d17971..f8900dc0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -104,6 +104,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Hygiene of macro expansions in presence of custom `core` crate. ([#327](https://github.com/JelteF/derive_more/pull/327)) - Fix documentation of generated methods in `IsVariant` derive. +- Make `{field:p}` do the expected thing in format strings for `Display` and + `Debug`. Also document weirdness around `Pointer` formatting when using + expressions, due to field variables being references. ## 0.99.10 - 2020-09-11 diff --git a/impl/doc/debug.md b/impl/doc/debug.md index 3d95b90a..be0ad8bd 100644 --- a/impl/doc/debug.md +++ b/impl/doc/debug.md @@ -14,8 +14,27 @@ This derive macro is a clever superset of `Debug` from standard library. Additio You supply a format by placing an attribute on a struct or enum variant, or its particular field: `#[debug("...", args...)]`. The format is exactly like in [`format!()`] or any other [`format_args!()`]-based macros. -The variables available in the arguments is `self` and each member of the struct or enum variant, with members of tuple -structs being named with a leading underscore and their index, i.e. `_0`, `_1`, `_2`, etc. +The variables available in the arguments is `self` and each member of the +struct or enum variant, with members of tuple structs being named with a +leading underscore and their index, i.e. `_0`, `_1`, `_2`, etc. Due to +ownership/lifetime limitations the member variables are all references to the +fields, except when used directly in the format string. For most purposes this +detail doesn't matter, but it is quite important when using `Pointer` +formatting. If you don't use the `{field:p}` syntax, you have to dereference +once to get the address of the field itself, instead of the reference to the +field: + + +```rust +#[derive(derive_more::Debug)] +#[debug("{field:p} {:p}", *field)] +struct RefInt<'a> { + field: &'a i32, +} + +let a = &123; +assert_eq!(format!("{:?}", RefInt{field: &a}), format!("{a:p} {:p}", a)); +``` ### Generic data types diff --git a/impl/doc/display.md b/impl/doc/display.md index 6a7139e6..3b88a797 100644 --- a/impl/doc/display.md +++ b/impl/doc/display.md @@ -22,9 +22,28 @@ inner variable. If there is no such variable, or there is more than 1, an error You supply a format by attaching an attribute of the syntax: `#[display("...", args...)]`. The format supplied is passed verbatim to `write!`. -The variables available in the arguments is `self` and each member of the variant, -with members of tuple structs being named with a leading underscore and their index, -i.e. `_0`, `_1`, `_2`, etc. +The variables available in the arguments is `self` and each member of the +variant, with members of tuple structs being named with a leading underscore +and their index, i.e. `_0`, `_1`, `_2`, etc. Due to ownership/lifetime +limitations the member variables are all references to the fields, except when +used directly in the format string. For most purposes this detail doesn't +matter, but it is quite important when using `Pointer` formatting. If you don't +use the `{field:p}` syntax, you have to dereference once to get the address of +the field itself, instead of the reference to the field: + + +```rust +# use derive_more::Display; +# +#[derive(Display)] +#[display("{field:p} {:p}", *field)] +struct RefInt<'a> { + field: &'a i32, +} + +let a = &123; +assert_eq!(format!("{}", RefInt{field: &a}), format!("{a:p} {:p}", a)); +``` For enums you can also specify a shared format on the enum itself instead of the variant. This format is used for each of the variants, and can be @@ -55,7 +74,7 @@ E.g., for a structure `Foo` defined like this: # trait Trait { type Type; } # #[derive(Display)] -#[display("{} {} {:?} {:p}", a, b, c, d)] +#[display("{a} {b} {c:?} {d:p}")] struct Foo<'a, T1, T2: Trait, T3> { a: T1, b: ::Type, diff --git a/impl/src/fmt/debug.rs b/impl/src/fmt/debug.rs index 212860d3..837f193a 100644 --- a/impl/src/fmt/debug.rs +++ b/impl/src/fmt/debug.rs @@ -244,15 +244,49 @@ impl<'a> Expansion<'a> { Ok(()) } + fn field_idents(&self) -> impl Iterator + '_ { + self.fields + .iter() + .enumerate() + .map(|(i, f)| f.ident.clone().unwrap_or_else(|| format_ident!("_{i}"))) + } + + fn field_params<'selff, 's>( + &'selff self, + fmt: &'a FmtAttribute, + ) -> impl Iterator + 's + where + 'a: 's, + 'selff: 's, + { + let used_arguments: Vec = fmt.placeholder_names().collect(); + self.field_idents().filter_map(move |var| { + if used_arguments.contains(&var.to_string()) { + Some(quote! { #var = *#var }) + } else { + None + } + }) + } + /// Generates [`Debug::fmt()`] implementation for a struct or an enum variant. /// /// [`Debug::fmt()`]: std::fmt::Debug::fmt() fn generate_body(&self) -> syn::Result { if let Some(fmt) = &self.attr.fmt { return Ok(if let Some((expr, trait_ident)) = fmt.transparent_call() { - quote! { derive_more::core::fmt::#trait_ident::fmt(&(#expr), __derive_more_f) } + let expr = if self.field_idents().any(|var| { + fmt.placeholder_names() + .any(|arg| arg == var.to_string().as_str()) + }) { + quote! { #expr } + } else { + quote! { &(#expr) } + }; + quote! { derive_more::core::fmt::#trait_ident::fmt(#expr, __derive_more_f) } } else { - quote! { derive_more::core::write!(__derive_more_f, #fmt) } + let field_params = self.field_params(fmt); + quote! { derive_more::core::write!(__derive_more_f, #fmt, #(#field_params),*) } }); }; @@ -288,12 +322,14 @@ impl<'a> Expansion<'a> { exhaustive = false; Ok::<_, syn::Error>(out) } - Some(FieldAttribute::Right(fmt_attr)) => Ok(quote! { - derive_more::__private::DebugTuple::field( - #out, - &derive_more::core::format_args!(#fmt_attr), + Some(FieldAttribute::Right(fmt_attr)) => { + let field_params = self.field_params(&fmt_attr); + Ok(quote! { + derive_more::__private::DebugTuple::field( + #out, + &derive_more::core::format_args!(#fmt_attr, #(#field_params),*), ) - }), + })}, None => { let ident = format_ident!("_{i}"); Ok(quote! { @@ -330,13 +366,15 @@ impl<'a> Expansion<'a> { exhaustive = false; Ok::<_, syn::Error>(out) } - Some(FieldAttribute::Right(fmt_attr)) => Ok(quote! { + Some(FieldAttribute::Right(fmt_attr)) => { + let field_params = self.field_params(&fmt_attr); + Ok(quote! { derive_more::core::fmt::DebugStruct::field( #out, #field_str, - &derive_more::core::format_args!(#fmt_attr), + &derive_more::core::format_args!(#fmt_attr, #(#field_params),*), ) - }), + })}, None => Ok(quote! { derive_more::core::fmt::DebugStruct::field(#out, #field_str, &#field_ident) }), diff --git a/impl/src/fmt/display.rs b/impl/src/fmt/display.rs index 429e78b7..55e91eca 100644 --- a/impl/src/fmt/display.rs +++ b/impl/src/fmt/display.rs @@ -5,7 +5,7 @@ use std::fmt; use proc_macro2::TokenStream; use quote::{format_ident, quote}; -use syn::{ext::IdentExt as _, parse_quote, spanned::Spanned as _}; +use syn::{ext::IdentExt as _, parse_quote, spanned::Spanned as _, Ident}; use crate::utils::{attr::ParseMultiple as _, Spanning}; @@ -254,6 +254,31 @@ struct Expansion<'a> { } impl<'a> Expansion<'a> { + fn field_idents(&self) -> impl Iterator + '_ { + self.fields + .iter() + .enumerate() + .map(|(i, f)| f.ident.clone().unwrap_or_else(|| format_ident!("_{i}"))) + } + + fn field_params<'selff, 's>( + &'selff self, + fmt: &'a FmtAttribute, + ) -> impl Iterator + 's + where + 'a: 's, + 'selff: 's, + { + let used_arguments: Vec = fmt.placeholder_names().collect(); + self.field_idents().filter_map(move |var| { + if used_arguments.contains(&var.to_string()) { + Some(quote! { #var = *#var }) + } else { + None + } + }) + } + /// Generates [`Display::fmt()`] implementation for a struct or an enum variant. /// /// # Errors @@ -278,13 +303,23 @@ impl<'a> Expansion<'a> { body = match &self.attrs.fmt { Some(fmt) => { if has_shared_attr { - quote! { &derive_more::core::format_args!(#fmt) } + let field_params = self.field_params(fmt); + quote! { &derive_more::core::format_args!(#fmt, #(#field_params),*) } } else if let Some((expr, trait_ident)) = fmt.transparent_call() { + let expr = if self.field_idents().any(|var| { + fmt.placeholder_names() + .any(|arg| arg == var.to_string().as_str()) + }) { + quote! { #expr } + } else { + quote! { &(#expr) } + }; quote! { - derive_more::core::fmt::#trait_ident::fmt(&(#expr), __derive_more_f) + derive_more::core::fmt::#trait_ident::fmt(#expr, __derive_more_f) } } else { - quote! { derive_more::core::write!(__derive_more_f, #fmt) } + let field_params = self.field_params(fmt); + quote! { derive_more::core::write!(__derive_more_f, #fmt, #(#field_params),*) } } } None if self.fields.is_empty() => { @@ -332,8 +367,9 @@ impl<'a> Expansion<'a> { if has_shared_attr { if let Some(shared_fmt) = &self.shared_attr { + let field_params = self.field_params(shared_fmt); let shared_body = quote! { - derive_more::core::write!(__derive_more_f, #shared_fmt) + derive_more::core::write!(__derive_more_f, #shared_fmt, #(#field_params),*) }; body = if body.is_empty() { diff --git a/impl/src/fmt/mod.rs b/impl/src/fmt/mod.rs index 62b57f09..befa8529 100644 --- a/impl/src/fmt/mod.rs +++ b/impl/src/fmt/mod.rs @@ -113,14 +113,16 @@ impl Parse for FmtAttribute { fn parse(input: ParseStream<'_>) -> syn::Result { Self::check_legacy_fmt(input)?; - Ok(Self { + let mut parsed = Self { lit: input.parse()?, comma: input .peek(token::Comma) .then(|| input.parse()) .transpose()?, args: input.parse_terminated(FmtArgument::parse, token::Comma)?, - }) + }; + parsed.args.pop_punct(); + Ok(parsed) } } @@ -273,6 +275,15 @@ impl FmtAttribute { }) } + fn placeholder_names(&self) -> impl Iterator { + Placeholder::parse_fmt_string(&self.lit.value()) + .into_iter() + .filter_map(|placeholder| match placeholder.arg { + Parameter::Named(name) => Some(name), + _ => None, + }) + } + /// Errors in case legacy syntax is encountered: `fmt = "...", (arg),*`. fn check_legacy_fmt(input: ParseStream<'_>) -> syn::Result<()> { let fork = input.fork(); diff --git a/tests/debug.rs b/tests/debug.rs index 8944bd42..1c483ef2 100644 --- a/tests/debug.rs +++ b/tests/debug.rs @@ -262,6 +262,16 @@ mod structs { field: &'a i32, } + #[derive(Debug)] + #[debug("{_0:p}")] + struct TupleTransparent<'a>(&'a i32); + + #[derive(Debug)] + #[debug("{field:p}")] + struct StructTransparent<'a> { + field: &'a i32, + } + #[test] fn assert() { let a = 42; @@ -273,6 +283,14 @@ mod structs { format!("{:?}", Struct { field: &a }), format!("Struct {{ field: {0:p}.{0:p} }}", &a), ); + assert_eq!( + format!("{:?}", TupleTransparent(&a)), + format!("{0:p}", &a), + ); + assert_eq!( + format!("{:?}", StructTransparent { field: &a }), + format!("{0:p}", &a), + ); } } } @@ -564,11 +582,11 @@ mod structs { use derive_more::Debug; #[derive(Debug)] - #[debug("{_0:p} * {_1:p}", _0 = self.0)] + #[debug("{_0:p} * {_1:p}")] struct Tuple<'a, 'b>(&'a u8, &'b bool); #[derive(Debug)] - #[debug("{a:p} * {b:p}", a = self.a)] + #[debug("{a:p} * {b:p}")] struct Struct<'a, 'b> { a: &'a u8, b: &'b bool, @@ -1305,7 +1323,7 @@ mod generic { #[derive(Debug)] struct AliasedFieldNamedGenericStruct { - #[debug("{field1}", field1 = field2)] + #[debug("{field3}", field3 = field2)] field1: T, field2: i32, } @@ -1423,7 +1441,7 @@ mod generic { } #[derive(Debug)] - struct AliasedFieldUnnamedGenericStruct(#[debug("{_0}", _0 = _1)] T, i32); + struct AliasedFieldUnnamedGenericStruct(#[debug("{_2}", _2 = _1)] T, i32); #[test] fn aliased_field_unnamed_generic_struct() { assert_eq!( diff --git a/tests/display.rs b/tests/display.rs index 0626fabc..f0d480db 100644 --- a/tests/display.rs +++ b/tests/display.rs @@ -447,7 +447,7 @@ mod structs { } #[derive(Display)] - #[display("{}", format_args!("{field:p}"))] + #[display("{}", format_args!("{field:p}", field = *field))] struct StructPointer<'a> { field: &'a i32, } @@ -566,15 +566,15 @@ mod structs { #[derive(Display)] #[display( - "{_0} {ident} {_1} {} {}", - _1, _0 + _1, ident = 123, _1 = _0, + "{_0} {ident} {_2} {} {}", + _1, _0 + _1, ident = 123, _2 = _0, )] struct Tuple(i32, i32); #[derive(Display)] #[display( - "{field1} {ident} {field2} {} {}", - field2, field1 + field2, ident = 123, field2 = field1, + "{field1} {ident} {field3} {} {}", + field2, field1 + field2, ident = 123, field3 = field1, )] struct Struct { field1: i32, @@ -1010,7 +1010,7 @@ mod enums { #[derive(Display)] enum Pointer<'a> { - #[display("{:p}", _0)] + #[display("{:p}", *_0)] A(&'a i32), #[display("{field:p}")] B { field: &'a u8 }, @@ -1121,9 +1121,9 @@ mod enums { #[derive(Display)] enum Pointer<'a> { - #[display("{}", format_args!("{:p}", _0))] + #[display("{}", format_args!("{:p}", *_0))] A(&'a i32), - #[display("{}", format_args!("{field:p}"))] + #[display("{}", format_args!("{field:p}", field = *field))] B { field: &'a u8 }, } @@ -1166,13 +1166,13 @@ mod enums { #[display("named")] StrNamed { field1: i32, field2: i32 }, #[display( - "{_0} {ident} {_1} {} {}", - _1, _0 + _1, ident = 123, _1 = _0, + "{_0} {ident} {_2} {} {}", + _1, _0 + _1, ident = 123, _2 = _0, )] InterpolatedUnnamed(i32, i32), #[display( - "{field1} {ident} {field2} {} {}", - field2, field1 + field2, ident = 123, field2 = field1, + "{field1} {ident} {field3} {} {}", + field2, field1 + field2, ident = 123, field3 = field1, )] InterpolatedNamed { field1: i32, field2: i32 }, } @@ -1271,7 +1271,7 @@ mod enums { #[derive(Display)] enum Pointer<'a> { - #[display("{:p}", _1)] + #[display("{:p}", *_1)] A(&'a f64, &'a f32), #[display("{a:p}")] B { a: &'a f64, b: &'a f32 }, @@ -1466,6 +1466,31 @@ mod enums { assert_eq!(Enum::::C(9).to_string(), "Variant C 9"); } } + + mod pointer { + use super::*; + + #[derive(Display)] + #[display("Pointer {_0:p} {_variant} {_0:p}")] + enum Pointer<'a> { + #[display("A")] + A(&'a f64), + #[display("B")] + B(&'a f32), + } + #[test] + fn assert() { + let (a, b) = (8.3, 42.1); + assert_eq!( + format!("{}", Pointer::A(&a)), + format!("Pointer {0:p} A {0:p}", &a), + ); + assert_eq!( + format!("{}", Pointer::B(&b)), + format!("Pointer {0:p} B {0:p}", &b), + ); + } + } } } } @@ -2106,7 +2131,7 @@ mod generic { struct TupleUpperExp(T); #[derive(Display)] - #[display("{:p}", _0)] + #[display("{_0:p}")] struct TuplePointer(T); #[derive(Display)] @@ -2154,7 +2179,7 @@ mod generic { } #[derive(Display)] - #[display("{:p}", field)] + #[display("{field:p}")] struct StructPointer { field: T, } @@ -2217,7 +2242,7 @@ mod generic { #[derive(Display)] enum EnumPointer { - #[display("{:p}", _0)] + #[display("{_0:p}")] A(A), #[display("{field:p}")] B { field: B },