diff --git a/impl/src/fmt/display.rs b/impl/src/fmt/display.rs index f6ae05f4..22f69dde 100644 --- a/impl/src/fmt/display.rs +++ b/impl/src/fmt/display.rs @@ -113,11 +113,20 @@ fn expand_enum( e: &syn::DataEnum, (container_attrs, _, trait_ident, attr_name): ExpansionCtx<'_>, ) -> syn::Result<(Vec, TokenStream)> { - /*if let Some(shared_attr) = &container_attrs.fmt { - if shared_attr.placeholders_by_name("_variant").any(|p| false) { - todo!() + if let Some(shared_fmt) = &container_attrs.fmt { + if shared_fmt + .placeholders_by_arg("_variant") + .any(|p| p.has_modifiers || p.trait_name != "Display") + { + // TODO: This limitation can be lifted, by analyzing the `shared_fmt` deeper and using + // `&dyn fmt::TraitName` for transparency instead of just `format_args!()` in the + // expansion. + return Err(syn::Error::new( + shared_fmt.span(), + "shared format `_variant` placeholder cannot contain format specifiers", + )); } - }*/ + } let (bounds, match_arms) = e.variants.iter().try_fold( (Vec::new(), TokenStream::new()), @@ -303,7 +312,7 @@ impl<'a> Expansion<'a> { quote! { derive_more::core::write!(__derive_more_f, #shared_fmt) } }; - body = if shared_fmt.contains_parameter("_variant") { + body = if shared_fmt.contains_arg("_variant") { quote! { match #body { _variant => #shared_body } } } else { shared_body @@ -319,7 +328,7 @@ impl<'a> Expansion<'a> { if self .shared_attr - .map_or(true, |a| a.contains_parameter("_variant")) + .map_or(true, |a| a.contains_arg("_variant")) { if let Some(fmt) = &self.attrs.fmt { bounds.extend( diff --git a/impl/src/fmt/mod.rs b/impl/src/fmt/mod.rs index 87e33d7f..bd559c35 100644 --- a/impl/src/fmt/mod.rs +++ b/impl/src/fmt/mod.rs @@ -235,31 +235,42 @@ impl FmtAttribute { }) } - /// Checks whether this [`FmtAttribute`] contains an argument with the provided `name`, either - /// in its direct [`FmtArgument`]s or inside [`Placeholder`]s. - fn contains_parameter(&self, name: &str) -> bool { + #[cfg(feature = "display")] + /// Checks whether this [`FmtAttribute`] contains an argument with the provided `name` (either + /// in its direct [`FmtArgument`]s or inside [`Placeholder`]s). + fn contains_arg(&self, name: &str) -> bool { + self.placeholders_by_arg(name).next().is_some() + } + + #[cfg(feature = "display")] + /// Returns an [`Iterator`] over [`Placeholder`]s using an argument with the provided `name` + /// (either in its direct [`FmtArgument`]s of this [`FmtAttribute`] or inside the + /// [`Placeholder`] itself). + fn placeholders_by_arg<'a>( + &'a self, + name: &'a str, + ) -> impl Iterator + 'a { let placeholders = Placeholder::parse_fmt_string(&self.lit.value()); - placeholders - .into_iter() - .filter_map(move |placeholder| { - match placeholder.arg { - Parameter::Named(name) => self - .args - .iter() - .find_map(|a| (a.alias()? == &name).then_some(&a.expr)) - .map_or(Some(name), |expr| { - expr.ident().map(ToString::to_string) - }), - Parameter::Positional(i) => self - .args - .iter() - .nth(i) - .and_then(|a| a.expr.ident().filter(|_| a.alias.is_none())) - .map(ToString::to_string), - } - }) - .any(|arg_name| arg_name == name) + placeholders.into_iter().filter(move |placeholder| { + match &placeholder.arg { + Parameter::Named(name) => self + .args + .iter() + .find_map(|a| (a.alias()? == name).then_some(&a.expr)) + .map_or(Some(name.clone()), |expr| { + expr.ident().map(ToString::to_string) + }), + Parameter::Positional(i) => self + .args + .iter() + .nth(*i) + .and_then(|a| a.expr.ident().filter(|_| a.alias.is_none())) + .map(ToString::to_string), + } + .as_deref() + == Some(name) + }) } /// Errors in case legacy syntax is encountered: `fmt = "...", (arg),*`. @@ -377,20 +388,13 @@ impl<'a> From> for Parameter { /// Representation of a formatting placeholder. #[derive(Debug, Eq, PartialEq)] struct Placeholder { - /// Formatting argument (either named or positional) to be used by this placeholder. + /// Formatting argument (either named or positional) to be used by this [`Placeholder`]. arg: Parameter, - /// [Width parameter][1], if present. - /// - /// [1]: https://doc.rust-lang.org/stable/std/fmt/index.html#width - width: Option, + /// Indicator whether this [`Placeholder`] has any formatting modifiers. + has_modifiers: bool, - /// [Precision parameter][1], if present. - /// - /// [1]: https://doc.rust-lang.org/stable/std/fmt/index.html#precision - precision: Option, - - /// Name of [`std::fmt`] trait to be used for rendering this placeholder. + /// Name of [`std::fmt`] trait to be used for rendering this [`Placeholder`]. trait_name: &'static str, } @@ -415,16 +419,18 @@ impl Placeholder { Self { arg: position, - width: format.spec.and_then(|s| match s.width { - Some(parsing::Count::Parameter(arg)) => Some(arg.into()), - _ => None, - }), - precision: format.spec.and_then(|s| match s.precision { - Some(parsing::Precision::Count(parsing::Count::Parameter( - arg, - ))) => Some(arg.into()), - _ => None, - }), + has_modifiers: format + .spec + .map(|s| { + s.align.is_some() + || s.sign.is_some() + || s.alternate.is_some() + || s.zero_padding.is_some() + || s.width.is_some() + || s.precision.is_some() + || !s.ty.is_trivial() + }) + .unwrap_or_default(), trait_name: ty.trait_name(), } }) @@ -603,38 +609,32 @@ mod placeholder_parse_fmt_string_spec { vec![ Placeholder { arg: Parameter::Positional(0), - width: None, - precision: None, + has_modifiers: false, trait_name: "Display", }, Placeholder { arg: Parameter::Positional(1), - width: None, - precision: None, + has_modifiers: false, trait_name: "Debug", }, Placeholder { arg: Parameter::Positional(1), - width: Some(Parameter::Positional(0)), - precision: None, + has_modifiers: true, trait_name: "Display", }, Placeholder { arg: Parameter::Positional(2), - width: None, - precision: Some(Parameter::Positional(1)), + has_modifiers: true, trait_name: "LowerHex", }, Placeholder { arg: Parameter::Named("par".to_owned()), - width: None, - precision: None, + has_modifiers: true, trait_name: "Debug", }, Placeholder { arg: Parameter::Positional(2), - width: Some(Parameter::Named("width".to_owned())), - precision: None, + has_modifiers: true, trait_name: "Display", }, ],