From 4f336943b41cff49e6663b40712cb477b8c117b7 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Sat, 20 Jul 2024 13:09:09 +0200 Subject: [PATCH] Only put Display-like bounds on type variables Resolves #363 Related to #371 Requires #377 This makes sure we don't add unnecessary bounds for Display-like derives. It does so in the same way as #371 did for the Debug derive: By only adding bounds when the type contains a type variable. --- impl/src/fmt/display.rs | 144 ++++++++++++++++++++++++++++++-- tests/display.rs | 177 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 313 insertions(+), 8 deletions(-) diff --git a/impl/src/fmt/display.rs b/impl/src/fmt/display.rs index 089f44dc..4afb4e65 100644 --- a/impl/src/fmt/display.rs +++ b/impl/src/fmt/display.rs @@ -32,7 +32,17 @@ pub fn expand(input: &syn::DeriveInput, trait_name: &str) -> syn::Result = input + .generics + .params + .iter() + .filter_map(|p| match p { + syn::GenericParam::Type(t) => Some(&t.ident), + syn::GenericParam::Const(..) | syn::GenericParam::Lifetime(..) => None, + }) + .collect(); + + let ctx: ExpansionCtx = (&attrs, &type_params, ident, &trait_ident, &attr_name); let (bounds, body) = match &input.data { syn::Data::Struct(s) => expand_struct(s, ctx), syn::Data::Enum(e) => expand_enum(e, ctx), @@ -62,6 +72,7 @@ pub fn expand(input: &syn::DeriveInput, trait_name: &str) -> syn::Result syn::Result = ( &'a ContainerAttributes, + &'a [&'a syn::Ident], &'a syn::Ident, &'a syn::Ident, &'a syn::Ident, @@ -77,12 +89,13 @@ type ExpansionCtx<'a> = ( /// Expands a [`fmt::Display`]-like derive macro for the provided struct. fn expand_struct( s: &syn::DataStruct, - (attrs, ident, trait_ident, _): ExpansionCtx<'_>, + (attrs, type_params, ident, trait_ident, _): ExpansionCtx<'_>, ) -> syn::Result<(Vec, TokenStream)> { let s = Expansion { shared_format: None, attrs, fields: &s.fields, + type_params, trait_ident, ident, }; @@ -111,7 +124,8 @@ fn expand_struct( /// Expands a [`fmt`]-like derive macro for the provided enum. fn expand_enum( e: &syn::DataEnum, - (shared_attrs, _, trait_ident, attr_name): ExpansionCtx<'_>, + + (shared_attrs, type_params, _, trait_ident, attr_name): ExpansionCtx<'_>, ) -> syn::Result<(Vec, TokenStream)> { let (bounds, match_arms) = e.variants.iter().try_fold( (Vec::new(), TokenStream::new()), @@ -138,6 +152,7 @@ fn expand_enum( shared_format: shared_attrs.fmt.as_ref(), attrs: &attrs, fields: &variant.fields, + type_params, trait_ident, ident, }; @@ -175,7 +190,7 @@ fn expand_enum( /// Expands a [`fmt::Display`]-like derive macro for the provided union. fn expand_union( u: &syn::DataUnion, - (attrs, _, _, attr_name): ExpansionCtx<'_>, + (attrs, _, _, _, attr_name): ExpansionCtx<'_>, ) -> syn::Result<(Vec, TokenStream)> { let fmt = &attrs.fmt.as_ref().ok_or_else(|| { syn::Error::new( @@ -210,6 +225,9 @@ struct Expansion<'a> { /// Struct or enum [`syn::Fields`]. fields: &'a syn::Fields, + /// Type parameters in this struct or enum. + type_params: &'a [&'a syn::Ident], + /// [`fmt`] trait [`syn::Ident`]. /// /// [`syn::Ident`]: struct@syn::Ident @@ -352,10 +370,13 @@ impl<'a> Expansion<'a> { if let Some(shared_format) = self.shared_format { let shared_bounds = shared_format .bounded_types(self.fields) - .map(|(ty, trait_name)| { + .filter_map(|(ty, trait_name)| { + if !self.contains_generic_param(ty) { + return None; + } let trait_ident = format_ident!("{trait_name}"); - parse_quote! { #ty: derive_more::core::fmt::#trait_ident } + Some(parse_quote! { #ty: derive_more::core::fmt::#trait_ident }) }) .chain(self.attrs.bounds.0.clone()) .collect(); @@ -379,6 +400,9 @@ impl<'a> Expansion<'a> { .next() .map(|f| { let ty = &f.ty; + if !self.contains_generic_param(ty) { + return vec![]; + } let trait_ident = &self.trait_ident; vec![parse_quote! { #ty: derive_more::core::fmt::#trait_ident }] }) @@ -389,15 +413,119 @@ impl<'a> Expansion<'a> { bounds.extend( fmt.bounded_types(self.fields) - .map(|(ty, trait_name)| { + .filter_map(|(ty, trait_name)| { + if !self.contains_generic_param(ty) { + return None; + } let trait_ident = format_ident!("{trait_name}"); - parse_quote! { #ty: derive_more::core::fmt::#trait_ident } + Some(parse_quote! { #ty: derive_more::core::fmt::#trait_ident }) }) .chain(self.attrs.bounds.0.clone()), ); bounds } + + /// 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() + .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) + } + } + } + }) + } + + /// 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 { + if self.contains_generic_param(&qself.ty) { + return true; + } + } + + if let Some(ident) = path.get_ident() { + self.type_params.iter().any(|param| *param == ident) + } else { + self.path_contains_generic_param(path) + } + } + + 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_generic_param(elem) + } + + syn::Type::BareFn(syn::TypeBareFn { inputs, output, .. }) => { + inputs + .iter() + .any(|arg| self.contains_generic_param(&arg.ty)) + || match output { + syn::ReturnType::Default => false, + syn::ReturnType::Type(_, ty) => self.contains_generic_param(ty), + } + } + syn::Type::Tuple(syn::TypeTuple { elems, .. }) => { + 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(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, + _ => unimplemented!( + "syntax is not supported by `derive_more`, please report a bug", + ), + } + } } /// Matches the provided derive macro `name` to appropriate actual trait name. diff --git a/tests/display.rs b/tests/display.rs index e67b1e38..4eaee235 100644 --- a/tests/display.rs +++ b/tests/display.rs @@ -2279,3 +2279,180 @@ mod generic { } } } + +// See: https://github.com/JelteF/derive_more/issues/363 +mod type_variables { + 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::Display; + + #[derive(Display, Debug)] + #[display("{inner:?}")] + #[display(bounds(T: Display))] + struct OptionalBox { + inner: Option>, + } + + #[derive(Display, Debug)] + #[display("{next}")] + struct ItemStruct { + next: OptionalBox, + } + + #[derive(Display)] + #[derive(Debug)] + struct ItemTuple(OptionalBox); + + #[derive(Display)] + #[derive(Debug)] + #[display("Item({_0})")] + struct ItemTupleContainerFmt(OptionalBox); + + #[derive(Display, Debug)] + #[display("{next}")] + enum ItemEnumOuterFormat { + Variant1 { + next: OptionalBox, + }, + Variant2 { + next: OptionalBox, + }, + } + + #[derive(Display, Debug)] + enum ItemEnumInnerFormat { + #[display("{next} {inner}")] + Node { + next: OptionalBox, + inner: i32, + }, + #[display("{inner}")] + Leaf { inner: i32 }, + } + + #[derive(Display)] + #[derive(Debug)] + #[display("{next:?}, {real:?}")] + struct VecMeansDifferent { + next: our_alloc::Vec, + real: Vec, + } + + #[derive(Display)] + #[derive(Debug)] + #[display("{t:?}")] + struct Array { + t: [T; 10], + } + + mod parens { + #![allow(unused_parens)] // test that type is found even in parentheses + + use derive_more::Display; + + #[derive(Display)] + struct Paren { + t: (T), + } + } + + #[derive(Display)] + struct ParenthesizedGenericArgumentsInput { + t: dyn Fn(T) -> i32, + } + + #[derive(Display)] + struct ParenthesizedGenericArgumentsOutput { + t: dyn Fn(i32) -> T, + } + + #[derive(Display)] + struct Ptr { + t: *const T, + } + + #[derive(Display)] + struct Reference<'a, T> { + t: &'a T, + } + + #[derive(Display)] + struct Slice<'a, T> { + t: &'a [T], + } + + #[derive(Display)] + struct BareFn { + t: Box T>, + } + + #[derive(Display)] + struct Tuple { + t: Box<(T, T)>, + } + + trait MyTrait {} + + #[derive(Display)] + struct TraitObject { + t: Box>, + } + + #[test] + fn assert() { + assert_eq!( + format!( + "{}", + ItemStruct { + next: OptionalBox { + inner: Some(Box::new(ItemStruct { + next: OptionalBox { inner: None } + })) + } + }, + ), + "Some(ItemStruct { next: OptionalBox { inner: None } })", + ); + + assert_eq!( + format!( + "{}", + ItemTuple(OptionalBox { + inner: Some(Box::new(ItemTuple(OptionalBox { inner: None }))) + }), + ), + "Some(ItemTuple(OptionalBox { inner: None }))", + ); + + assert_eq!( + format!( + "{}", + ItemTupleContainerFmt(OptionalBox { + inner: Some(Box::new(ItemTupleContainerFmt(OptionalBox { + inner: None + }))) + }), + ), + "Item(Some(ItemTupleContainerFmt(OptionalBox { inner: None })))", + ); + + let item = ItemEnumOuterFormat::Variant1 { + next: OptionalBox { + inner: Some(Box::new(ItemEnumOuterFormat::Variant2 { + next: OptionalBox { inner: None }, + })), + }, + }; + assert_eq!( + format!("{item}"), + "Some(Variant2 { next: OptionalBox { inner: None } })", + ) + } +}