From 16713df62ef4fd7ca7f68bae503ebea3a6a0e73e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustav=20S=C3=B6rn=C3=A4s?= Date: Tue, 18 Jun 2024 17:53:07 +0200 Subject: [PATCH 1/4] Only put Debug-like bounds on type variables --- impl/src/fmt/debug.rs | 133 ++++++++++++++++++++++++++++++++++++++++-- tests/debug.rs | 67 +++++++++++++++++++++ 2 files changed, 194 insertions(+), 6 deletions(-) diff --git a/impl/src/fmt/debug.rs b/impl/src/fmt/debug.rs index 68e873e1..ddcd44ed 100644 --- a/impl/src/fmt/debug.rs +++ b/impl/src/fmt/debug.rs @@ -24,9 +24,22 @@ pub fn expand(input: &syn::DeriveInput, _: &str) -> syn::Result { .unwrap_or_default(); let ident = &input.ident; + let type_variables = input + .generics + .params + .iter() + .filter_map(|param| match param { + syn::GenericParam::Lifetime(_) => None, + syn::GenericParam::Type(ty) => Some(&ty.ident), + syn::GenericParam::Const(_) => None, + }) + .collect(); + let (bounds, body) = match &input.data { - syn::Data::Struct(s) => expand_struct(attrs, ident, s, &attr_name), - syn::Data::Enum(e) => expand_enum(attrs, e, &attr_name), + syn::Data::Struct(s) => { + expand_struct(attrs, ident, s, type_variables, &attr_name) + } + syn::Data::Enum(e) => expand_enum(attrs, e, type_variables, &attr_name), syn::Data::Union(_) => { return Err(syn::Error::new( input.span(), @@ -64,11 +77,13 @@ fn expand_struct( attrs: ContainerAttributes, ident: &Ident, s: &syn::DataStruct, + type_variables: Vec<&syn::Ident>, attr_name: &syn::Ident, ) -> syn::Result<(Vec, TokenStream)> { let s = Expansion { attr: &attrs, fields: &s.fields, + type_variables: &type_variables, ident, attr_name, }; @@ -99,6 +114,7 @@ fn expand_struct( fn expand_enum( mut attrs: ContainerAttributes, e: &syn::DataEnum, + type_variables: Vec<&syn::Ident>, attr_name: &syn::Ident, ) -> syn::Result<(Vec, TokenStream)> { if let Some(enum_fmt) = attrs.fmt.as_ref() { @@ -136,6 +152,7 @@ fn expand_enum( let v = Expansion { attr: &attrs, fields: &variant.fields, + type_variables: &type_variables, ident, attr_name, }; @@ -195,6 +212,9 @@ struct Expansion<'a> { /// Struct or enum [`syn::Fields`]. fields: &'a syn::Fields, + /// Type variables in this struct or enum. + type_variables: &'a [&'a syn::Ident], + /// Name of the attributes, considered by this macro. attr_name: &'a syn::Ident, } @@ -334,15 +354,26 @@ impl<'a> Expansion<'a> { let mut out = self.attr.bounds.0.clone().into_iter().collect::>(); if let Some(fmt) = self.attr.fmt.as_ref() { - out.extend(fmt.bounded_types(self.fields).map(|(ty, trait_name)| { - let trait_ident = format_ident!("{trait_name}"); + out.extend(fmt.bounded_types(self.fields).filter_map( + |(ty, trait_name)| { + if !self.contains_type_variable(ty) { + return None; + } - parse_quote! { #ty: derive_more::core::fmt::#trait_ident } - })); + let trait_ident = format_ident!("{trait_name}"); + + Some(parse_quote! { #ty: derive_more::core::fmt::#trait_ident }) + }, + )); Ok(out) } else { self.fields.iter().try_fold(out, |mut out, field| { let ty = &field.ty; + + if !self.contains_type_variable(ty) { + return Ok(out); + } + match FieldAttribute::parse_attrs(&field.attrs, self.attr_name)? .map(Spanning::into_inner) { @@ -362,4 +393,94 @@ impl<'a> Expansion<'a> { }) } } + + fn contains_type_variable(&self, ty: &syn::Type) -> bool { + match ty { + syn::Type::Path(syn::TypePath { qself, path }) => { + if let Some(qself) = qself { + if self.contains_type_variable(&qself.ty) { + return true; + } + } + + if let Some(ident) = path.get_ident() { + self.type_variables + .iter() + .any(|type_var| *type_var == ident) + } else { + path.segments.iter().any(|segment| { + self.type_variables + .iter() + .any(|type_var| *type_var == &segment.ident) + || match &segment.arguments { + syn::PathArguments::None => false, + syn::PathArguments::AngleBracketed( + syn::AngleBracketedGenericArguments { + args, .. + }, + ) => args.iter().any(|generic| match generic { + syn::GenericArgument::Type(ty) + | syn::GenericArgument::AssocType( + syn::AssocType { ty, .. }, + ) => self.contains_type_variable(ty), + + syn::GenericArgument::Lifetime(_) + | syn::GenericArgument::Const(_) + | syn::GenericArgument::AssocConst(_) + | syn::GenericArgument::Constraint(_) => false, + _ => false, + }), + syn::PathArguments::Parenthesized( + syn::ParenthesizedGenericArguments { + inputs, + output, + .. + }, + ) => { + inputs + .iter() + .any(|ty| self.contains_type_variable(ty)) + || match output { + syn::ReturnType::Default => false, + syn::ReturnType::Type(_, ty) => { + self.contains_type_variable(ty) + } + } + } + } + }) + } + } + + syn::Type::Array(syn::TypeArray { elem, .. }) + | syn::Type::Group(syn::TypeGroup { elem, .. }) + | syn::Type::Paren(syn::TypeParen { elem, .. }) + | syn::Type::Ptr(syn::TypePtr { elem, .. }) + | syn::Type::Reference(syn::TypeReference { elem, .. }) + | syn::Type::Slice(syn::TypeSlice { elem, .. }) => { + self.contains_type_variable(elem) + } + + syn::Type::BareFn(syn::TypeBareFn { inputs, output, .. }) => { + inputs + .iter() + .any(|arg| self.contains_type_variable(&arg.ty)) + || match output { + syn::ReturnType::Default => false, + syn::ReturnType::Type(_, ty) => self.contains_type_variable(ty), + } + } + syn::Type::Tuple(syn::TypeTuple { elems, .. }) => { + elems.iter().any(|ty| self.contains_type_variable(ty)) + } + + syn::Type::ImplTrait(_) => false, + syn::Type::Infer(_) => false, + syn::Type::Macro(_) => false, + syn::Type::Never(_) => false, + syn::Type::TraitObject(_) => false, + syn::Type::Verbatim(_) => false, + _ => false, + } + } } diff --git a/tests/debug.rs b/tests/debug.rs index 15e7f12f..43e00091 100644 --- a/tests/debug.rs +++ b/tests/debug.rs @@ -1932,3 +1932,70 @@ mod complex_enum_syntax { assert_eq!(format!("{:?}", Enum::A), "A"); } } + +// See: https://github.com/JelteF/derive_more/issues/363 +mod type_variables { + #[cfg(not(feature = "std"))] + use alloc::{boxed::Box, format, vec, vec::Vec}; + + use derive_more::Debug; + + #[derive(Debug)] + struct ItemStruct { + next: Option>, + } + + #[derive(Debug)] + struct ItemTuple(Option>); + + #[derive(Debug)] + #[debug("Item({_0:?})")] + struct ItemTupleContainerFmt(Option>); + + #[derive(Debug)] + enum ItemEnum { + Node { children: Vec, inner: i32 }, + Leaf { inner: i32 }, + } + + #[test] + fn assert() { + assert_eq!( + format!( + "{:?}", + ItemStruct { + next: Some(Box::new(ItemStruct { next: None })) + } + ), + "ItemStruct { next: Some(ItemStruct { next: None }) }" + ); + + assert_eq!( + format!("{:?}", ItemTuple(Some(Box::new(ItemTuple(None)))),), + "ItemTuple(Some(ItemTuple(None)))" + ); + + assert_eq!( + format!( + "{:?}", + ItemTupleContainerFmt(Some(Box::new(ItemTupleContainerFmt(None)))), + ), + "Item(Some(Item(None)))" + ); + + let item = ItemEnum::Node { + children: vec![ + ItemEnum::Node { + children: vec![], + inner: 0, + }, + ItemEnum::Leaf { inner: 1 }, + ], + inner: 2, + }; + assert_eq!( + format!("{item:?}"), + "Node { children: [Node { children: [], inner: 0 }, Leaf { inner: 1 }], inner: 2 }", + ) + } +} From 6131444ce31864256fd2218df759c8fbc1f75a12 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Fri, 5 Jul 2024 21:20:06 +0200 Subject: [PATCH 2/4] Add more tests and support trait objects --- impl/src/fmt/debug.rs | 137 ++++++++++++++++++++++-------------------- tests/debug.rs | 73 +++++++++++++++++++++- 2 files changed, 142 insertions(+), 68 deletions(-) diff --git a/impl/src/fmt/debug.rs b/impl/src/fmt/debug.rs index ddcd44ed..2178fb2b 100644 --- a/impl/src/fmt/debug.rs +++ b/impl/src/fmt/debug.rs @@ -24,22 +24,21 @@ pub fn expand(input: &syn::DeriveInput, _: &str) -> syn::Result { .unwrap_or_default(); let ident = &input.ident; - let type_variables = input + let type_params: Vec<_> = input .generics .params .iter() .filter_map(|param| match param { - syn::GenericParam::Lifetime(_) => None, - syn::GenericParam::Type(ty) => Some(&ty.ident), - syn::GenericParam::Const(_) => None, + syn::GenericParam::Type(param) => Some(¶m.ident), + _ => None, }) .collect(); let (bounds, body) = match &input.data { syn::Data::Struct(s) => { - expand_struct(attrs, ident, s, type_variables, &attr_name) + expand_struct(attrs, ident, s, &type_params, &attr_name) } - syn::Data::Enum(e) => expand_enum(attrs, e, type_variables, &attr_name), + syn::Data::Enum(e) => expand_enum(attrs, e, &type_params, &attr_name), syn::Data::Union(_) => { return Err(syn::Error::new( input.span(), @@ -77,13 +76,13 @@ fn expand_struct( attrs: ContainerAttributes, ident: &Ident, s: &syn::DataStruct, - type_variables: Vec<&syn::Ident>, + type_params: &[&syn::Ident], attr_name: &syn::Ident, ) -> syn::Result<(Vec, TokenStream)> { let s = Expansion { attr: &attrs, fields: &s.fields, - type_variables: &type_variables, + type_params, ident, attr_name, }; @@ -114,7 +113,7 @@ fn expand_struct( fn expand_enum( mut attrs: ContainerAttributes, e: &syn::DataEnum, - type_variables: Vec<&syn::Ident>, + type_params: &[&syn::Ident], attr_name: &syn::Ident, ) -> syn::Result<(Vec, TokenStream)> { if let Some(enum_fmt) = attrs.fmt.as_ref() { @@ -152,7 +151,7 @@ fn expand_enum( let v = Expansion { attr: &attrs, fields: &variant.fields, - type_variables: &type_variables, + type_params, ident, attr_name, }; @@ -212,8 +211,8 @@ struct Expansion<'a> { /// Struct or enum [`syn::Fields`]. fields: &'a syn::Fields, - /// Type variables in this struct or enum. - type_variables: &'a [&'a syn::Ident], + /// Type parameters in this struct or enum. + type_params: &'a [&'a syn::Ident], /// Name of the attributes, considered by this macro. attr_name: &'a syn::Ident, @@ -356,7 +355,7 @@ impl<'a> Expansion<'a> { if let Some(fmt) = self.attr.fmt.as_ref() { out.extend(fmt.bounded_types(self.fields).filter_map( |(ty, trait_name)| { - if !self.contains_type_variable(ty) { + if !self.contains_generic_param(ty) { return None; } @@ -370,7 +369,7 @@ impl<'a> Expansion<'a> { self.fields.iter().try_fold(out, |mut out, field| { let ty = &field.ty; - if !self.contains_type_variable(ty) { + if !self.contains_generic_param(ty) { return Ok(out); } @@ -394,61 +393,54 @@ impl<'a> Expansion<'a> { } } - fn contains_type_variable(&self, ty: &syn::Type) -> bool { + fn path_contains_generic_param(&self, path: &syn::Path) -> bool { + path.segments + .iter() + .any(|segment| match &segment.arguments { + syn::PathArguments::None => false, + syn::PathArguments::AngleBracketed( + syn::AngleBracketedGenericArguments { args, .. }, + ) => args.iter().any(|generic| match generic { + syn::GenericArgument::Type(ty) + | syn::GenericArgument::AssocType(syn::AssocType { ty, .. }) => { + self.contains_generic_param(ty) + } + + syn::GenericArgument::Lifetime(_) + | syn::GenericArgument::Const(_) + | syn::GenericArgument::AssocConst(_) + | syn::GenericArgument::Constraint(_) => false, + _ => unimplemented!( + "syntax is not supported by derive_more, please report a bug" + ), + }), + syn::PathArguments::Parenthesized( + syn::ParenthesizedGenericArguments { inputs, output, .. }, + ) => { + inputs.iter().any(|ty| self.contains_generic_param(ty)) + || match output { + syn::ReturnType::Default => false, + syn::ReturnType::Type(_, ty) => { + self.contains_generic_param(ty) + } + } + } + }) + } + + fn contains_generic_param(&self, ty: &syn::Type) -> bool { match ty { syn::Type::Path(syn::TypePath { qself, path }) => { if let Some(qself) = qself { - if self.contains_type_variable(&qself.ty) { + if self.contains_generic_param(&qself.ty) { return true; } } if let Some(ident) = path.get_ident() { - self.type_variables - .iter() - .any(|type_var| *type_var == ident) + self.type_params.iter().any(|param| *param == ident) } else { - path.segments.iter().any(|segment| { - self.type_variables - .iter() - .any(|type_var| *type_var == &segment.ident) - || match &segment.arguments { - syn::PathArguments::None => false, - syn::PathArguments::AngleBracketed( - syn::AngleBracketedGenericArguments { - args, .. - }, - ) => args.iter().any(|generic| match generic { - syn::GenericArgument::Type(ty) - | syn::GenericArgument::AssocType( - syn::AssocType { ty, .. }, - ) => self.contains_type_variable(ty), - - syn::GenericArgument::Lifetime(_) - | syn::GenericArgument::Const(_) - | syn::GenericArgument::AssocConst(_) - | syn::GenericArgument::Constraint(_) => false, - _ => false, - }), - syn::PathArguments::Parenthesized( - syn::ParenthesizedGenericArguments { - inputs, - output, - .. - }, - ) => { - inputs - .iter() - .any(|ty| self.contains_type_variable(ty)) - || match output { - syn::ReturnType::Default => false, - syn::ReturnType::Type(_, ty) => { - self.contains_type_variable(ty) - } - } - } - } - }) + self.path_contains_generic_param(path) } } @@ -458,29 +450,42 @@ impl<'a> Expansion<'a> { | syn::Type::Ptr(syn::TypePtr { elem, .. }) | syn::Type::Reference(syn::TypeReference { elem, .. }) | syn::Type::Slice(syn::TypeSlice { elem, .. }) => { - self.contains_type_variable(elem) + self.contains_generic_param(elem) } syn::Type::BareFn(syn::TypeBareFn { inputs, output, .. }) => { inputs .iter() - .any(|arg| self.contains_type_variable(&arg.ty)) + .any(|arg| self.contains_generic_param(&arg.ty)) || match output { syn::ReturnType::Default => false, - syn::ReturnType::Type(_, ty) => self.contains_type_variable(ty), + syn::ReturnType::Type(_, ty) => self.contains_generic_param(ty), } } syn::Type::Tuple(syn::TypeTuple { elems, .. }) => { - elems.iter().any(|ty| self.contains_type_variable(ty)) + elems.iter().any(|ty| self.contains_generic_param(ty)) } syn::Type::ImplTrait(_) => false, syn::Type::Infer(_) => false, syn::Type::Macro(_) => false, syn::Type::Never(_) => false, - syn::Type::TraitObject(_) => false, + syn::Type::TraitObject(syn::TypeTraitObject { bounds, .. }) => { + bounds.iter().any(|bound| match bound { + syn::TypeParamBound::Trait(syn::TraitBound { path, .. }) => { + self.path_contains_generic_param(path) + } + syn::TypeParamBound::Lifetime(_) => false, + syn::TypeParamBound::Verbatim(_) => false, + _ => unimplemented!( + "syntax is not supported by derive_more, please report a bug" + ), + }) + } syn::Type::Verbatim(_) => false, - _ => false, + _ => unimplemented!( + "syntax is not supported by derive_more, please report a bug" + ), } } } diff --git a/tests/debug.rs b/tests/debug.rs index 43e00091..7d566ee6 100644 --- a/tests/debug.rs +++ b/tests/debug.rs @@ -1935,8 +1935,14 @@ mod complex_enum_syntax { // See: https://github.com/JelteF/derive_more/issues/363 mod type_variables { - #[cfg(not(feature = "std"))] - use alloc::{boxed::Box, format, vec, vec::Vec}; + mod our_alloc { + #[cfg(not(feature = "std"))] + pub use alloc::{boxed::Box, format, vec, vec::Vec}; + #[cfg(feature = "std")] + pub use std::{boxed::Box, format, vec, vec::Vec}; + } + + use our_alloc::{format, vec, Box, Vec}; use derive_more::Debug; @@ -1958,6 +1964,69 @@ mod type_variables { Leaf { inner: i32 }, } + #[derive(Debug)] + struct VecMeansDifferent { + next: our_alloc::Vec, + real: Vec, + } + + #[derive(Debug)] + struct Array { + #[debug("{t}")] + t: [T; 10], + } + + mod parens { + #![allow(unused_parens)] // test that type is found even in parentheses + use derive_more::Debug; + #[derive(Debug)] + struct Paren { + t: (T), + } + } + + #[derive(Debug)] + struct ParenthesizedGenericArgumentsInput { + t: dyn Fn(T) -> i32, + } + + #[derive(Debug)] + struct ParenthesizedGenericArgumentsOutput { + t: dyn Fn(i32) -> T, + } + + #[derive(Debug)] + struct Ptr { + t: *const T, + } + + #[derive(Debug)] + struct Reference<'a, T> { + t: &'a T, + } + + #[derive(Debug)] + struct Slice<'a, T> { + t: &'a [T], + } + + #[derive(Debug)] + struct BareFn { + t: Box T>, + } + + #[derive(Debug)] + struct Tuple { + t: Box<(T, T)>, + } + + trait MyTrait {} + + #[derive(Debug)] + struct TraitObject { + t: Box>, + } + #[test] fn assert() { assert_eq!( From 5424185b60a85c85647ee60420b013381f086fe7 Mon Sep 17 00:00:00 2001 From: tyranron Date: Fri, 19 Jul 2024 13:00:47 +0300 Subject: [PATCH 3/4] Corrections and improvements --- impl/src/fmt/debug.rs | 17 +++++++++++------ tests/debug.rs | 12 +++++++----- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/impl/src/fmt/debug.rs b/impl/src/fmt/debug.rs index 113eb501..524f0218 100644 --- a/impl/src/fmt/debug.rs +++ b/impl/src/fmt/debug.rs @@ -28,9 +28,9 @@ pub fn expand(input: &syn::DeriveInput, _: &str) -> syn::Result { .generics .params .iter() - .filter_map(|param| match param { - syn::GenericParam::Type(param) => Some(¶m.ident), - _ => None, + .filter_map(|p| match p { + syn::GenericParam::Type(t) => Some(&t.ident), + syn::GenericParam::Const(..) | syn::GenericParam::Lifetime(..) => None, }) .collect(); @@ -393,6 +393,7 @@ impl<'a> Expansion<'a> { } } + /// Checks whether the provided [`syn::Path`] contains any of these [`Expansion::type_params`]. fn path_contains_generic_param(&self, path: &syn::Path) -> bool { path.segments .iter() @@ -411,7 +412,7 @@ impl<'a> Expansion<'a> { | syn::GenericArgument::AssocConst(_) | syn::GenericArgument::Constraint(_) => false, _ => unimplemented!( - "syntax is not supported by derive_more, please report a bug" + "syntax is not supported by `derive_more`, please report a bug", ), }), syn::PathArguments::Parenthesized( @@ -428,7 +429,11 @@ impl<'a> Expansion<'a> { }) } + /// Checks whether the provided [`syn::Type`] contains any of these [`Expansion::type_params`]. fn contains_generic_param(&self, ty: &syn::Type) -> bool { + if self.type_params.is_empty() { + return false; + } match ty { syn::Type::Path(syn::TypePath { qself, path }) => { if let Some(qself) = qself { @@ -478,13 +483,13 @@ impl<'a> Expansion<'a> { syn::TypeParamBound::Lifetime(_) => false, syn::TypeParamBound::Verbatim(_) => false, _ => unimplemented!( - "syntax is not supported by derive_more, please report a bug" + "syntax is not supported by `derive_more`, please report a bug", ), }) } syn::Type::Verbatim(_) => false, _ => unimplemented!( - "syntax is not supported by derive_more, please report a bug" + "syntax is not supported by `derive_more`, please report a bug", ), } } diff --git a/tests/debug.rs b/tests/debug.rs index 0f40a26f..0c4b180a 100644 --- a/tests/debug.rs +++ b/tests/debug.rs @@ -1978,7 +1978,9 @@ mod type_variables { mod parens { #![allow(unused_parens)] // test that type is found even in parentheses + use derive_more::Debug; + #[derive(Debug)] struct Paren { t: (T), @@ -2034,14 +2036,14 @@ mod type_variables { "{:?}", ItemStruct { next: Some(Box::new(ItemStruct { next: None })) - } + }, ), - "ItemStruct { next: Some(ItemStruct { next: None }) }" + "ItemStruct { next: Some(ItemStruct { next: None }) }", ); assert_eq!( - format!("{:?}", ItemTuple(Some(Box::new(ItemTuple(None)))),), - "ItemTuple(Some(ItemTuple(None)))" + format!("{:?}", ItemTuple(Some(Box::new(ItemTuple(None))))), + "ItemTuple(Some(ItemTuple(None)))", ); assert_eq!( @@ -2049,7 +2051,7 @@ mod type_variables { "{:?}", ItemTupleContainerFmt(Some(Box::new(ItemTupleContainerFmt(None)))), ), - "Item(Some(Item(None)))" + "Item(Some(Item(None)))", ); let item = ItemEnum::Node { From 022fdfb2a0eb4af3990e206a9cb207098c7fed5d Mon Sep 17 00:00:00 2001 From: tyranron Date: Fri, 19 Jul 2024 13:04:29 +0300 Subject: [PATCH 4/4] Fix dat `sum` tests --- tests/sum.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/sum.rs b/tests/sum.rs index a273bf80..503d6b4f 100644 --- a/tests/sum.rs +++ b/tests/sum.rs @@ -1,4 +1,5 @@ #![cfg_attr(not(feature = "std"), no_std)] +#![allow(dead_code)] // some code is tested for type checking only use derive_more::Sum;