From b8779400986c6dc56ac1955e47556abbce27cfe7 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Tue, 2 Jul 2024 09:59:20 +0200 Subject: [PATCH 1/8] Support _variant in outer level enum formatting for Display --- .github/workflows/ci.yml | 2 +- CHANGELOG.md | 4 +- Cargo.toml | 2 +- README.md | 4 +- clippy.toml | 2 +- impl/Cargo.toml | 2 +- impl/doc/display.md | 22 +- impl/src/fmt/display.rs | 150 ++- impl/src/fmt/mod.rs | 4 +- impl/src/fmt/parsing.rs | 1005 ++++++++--------- .../shared_format_positional_placeholders.rs | 19 + ...ared_format_positional_placeholders.stderr | 17 + .../display/shared_format_unclosed_brace.rs | 7 + .../shared_format_unclosed_brace.stderr | 9 + .../display/shared_format_variant_spec.rs | 7 + .../display/shared_format_variant_spec.stderr | 5 + tests/display.rs | 125 ++ 17 files changed, 845 insertions(+), 541 deletions(-) create mode 100644 tests/compile_fail/display/shared_format_positional_placeholders.rs create mode 100644 tests/compile_fail/display/shared_format_positional_placeholders.stderr create mode 100644 tests/compile_fail/display/shared_format_unclosed_brace.rs create mode 100644 tests/compile_fail/display/shared_format_unclosed_brace.stderr create mode 100644 tests/compile_fail/display/shared_format_variant_spec.rs create mode 100644 tests/compile_fail/display/shared_format_variant_spec.stderr diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 657f25c0..13439b8a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -54,7 +54,7 @@ jobs: strategy: fail-fast: false matrix: - msrv: ["1.65.0"] + msrv: ["1.70.0"] os: - ubuntu - macOS diff --git a/CHANGELOG.md b/CHANGELOG.md index a9805322..95d036f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Breaking changes -- The minimum supported Rust version (MSRV) is now Rust 1.65. +- The minimum supported Rust version (MSRV) is now Rust 1.70. - Add the `std` feature which should be disabled in `no_std` environments. - All Cargo features, except `std`, are now disabled by default. The `full` feature can be used to get the old behavior of supporting all possible @@ -47,6 +47,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). [#294](https://github.com/JelteF/derive_more/pull/294)) - The `as_mut` feature is removed, and the `AsMut` derive is now gated by the `as_ref` feature. ([#295](https://github.com/JelteF/derive_more/pull/295)) +- A top level `#[display("...")]` attribute on an enum now requires the usage + of `{_variant}` to include the variant instead of including it at `{}`. ([#377](https://github.com/JelteF/derive_more/pull/377)) ### Added diff --git a/Cargo.toml b/Cargo.toml index 420f60b3..c734182e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,7 +2,7 @@ name = "derive_more" version = "1.0.0-beta.6" edition = "2021" -rust-version = "1.65.0" +rust-version = "1.70.0" description = "Adds #[derive(x)] macros for more traits" authors = ["Jelte Fennema "] license = "MIT" diff --git a/README.md b/README.md index 383f773d..4ce210fc 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ [![Latest Version](https://img.shields.io/crates/v/derive_more.svg)](https://crates.io/crates/derive_more) [![Rust Documentation](https://docs.rs/derive_more/badge.svg)](https://docs.rs/derive_more) [![GitHub license](https://img.shields.io/badge/license-MIT-blue.svg)](https://raw.githubusercontent.com/JelteF/derive_more/master/LICENSE) -[![Rust 1.65+](https://img.shields.io/badge/rustc-1.65+-lightgray.svg)](https://blog.rust-lang.org/2022/11/03/Rust-1.65.0.html) +[![Rust 1.70+](https://img.shields.io/badge/rustc-1.70+-lightgray.svg)](https://blog.rust-lang.org/2022/11/03/Rust-1.70.0.html) [![Unsafe Forbidden](https://img.shields.io/badge/unsafe-forbidden-success.svg)](https://github.com/rust-secure-code/safety-dance) Rust has lots of builtin traits that are implemented for its basic types, such @@ -215,7 +215,7 @@ extern crate derive_more; ## [MSRV] policy -This library requires Rust 1.65 or higher. +This library requires Rust 1.70 or higher. Changing [MSRV] (minimum supported Rust version) of this crate is treated as a **minor version change** in terms of [Semantic Versioning]. - So, if [MSRV] changes are **NOT concerning** for your project, just use the default [caret requirement]: diff --git a/clippy.toml b/clippy.toml index 0c697c31..ca697fd7 100644 --- a/clippy.toml +++ b/clippy.toml @@ -2,7 +2,7 @@ # See full lints list at: # https://rust-lang.github.io/rust-clippy/master/index.html -msrv = "1.65.0" +msrv = "1.70.0" # Ensures consistent bracing for macro calls in the codebase. # Extends default settings: diff --git a/impl/Cargo.toml b/impl/Cargo.toml index 8252fdfb..dc6bcf53 100644 --- a/impl/Cargo.toml +++ b/impl/Cargo.toml @@ -2,7 +2,7 @@ name = "derive_more-impl" version = "1.0.0-beta.6" edition = "2021" -rust-version = "1.65.0" +rust-version = "1.70.0" description = "Internal implementation of `derive_more` crate" authors = ["Jelte Fennema "] license = "MIT" diff --git a/impl/doc/display.md b/impl/doc/display.md index 5c242bdf..6a7139e6 100644 --- a/impl/doc/display.md +++ b/impl/doc/display.md @@ -26,6 +26,12 @@ The variables available in the arguments is `self` and each member of the varian with members of tuple structs being named with a leading underscore and their index, i.e. `_0`, `_1`, `_2`, etc. +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 +customized per variant by including the special `{_variant}` placeholder in +this shared format, which is then replaced by the format string that's provided +on the variant. + ### Other formatting traits @@ -175,6 +181,7 @@ struct Point2D { } #[derive(Display)] +#[display("Enum E: {_variant}")] enum E { Uint(u32), #[display("I am B {:b}", i)] @@ -185,6 +192,13 @@ enum E { Path(PathBuf), } +#[derive(Display)] +#[display("Enum E2: {_0:?}")] +enum E2 { + Uint(u32), + String(&'static str, &'static str), +} + #[derive(Display)] #[display("Hello there!")] union U { @@ -223,9 +237,11 @@ impl PositiveOrNegative { assert_eq!(MyInt(-2).to_string(), "-2"); assert_eq!(Point2D { x: 3, y: 4 }.to_string(), "(3, 4)"); -assert_eq!(E::Uint(2).to_string(), "2"); -assert_eq!(E::Binary { i: -2 }.to_string(), "I am B 11111110"); -assert_eq!(E::Path("abc".into()).to_string(), "I am C abc"); +assert_eq!(E::Uint(2).to_string(), "Enum E: 2"); +assert_eq!(E::Binary { i: -2 }.to_string(), "Enum E: I am B 11111110"); +assert_eq!(E::Path("abc".into()).to_string(), "Enum E: I am C abc"); +assert_eq!(E2::Uint(2).to_string(), "Enum E2: 2"); +assert_eq!(E2::String("shown", "ignored").to_string(), "Enum E2: \"shown\""); assert_eq!(U { i: 2 }.to_string(), "Hello there!"); assert_eq!(format!("{:o}", S), "7"); assert_eq!(format!("{:X}", UH), "UpperHex"); diff --git a/impl/src/fmt/display.rs b/impl/src/fmt/display.rs index e41dcbd1..f70c99c5 100644 --- a/impl/src/fmt/display.rs +++ b/impl/src/fmt/display.rs @@ -9,7 +9,7 @@ use syn::{parse_quote, spanned::Spanned as _}; use crate::utils::{attr::ParseMultiple as _, Spanning}; -use super::{trait_name_to_attribute_name, ContainerAttributes}; +use super::{parsing, trait_name_to_attribute_name, ContainerAttributes, FmtAttribute}; /// Expands a [`fmt::Display`]-like derive macro. /// @@ -80,6 +80,7 @@ fn expand_struct( (attrs, ident, trait_ident, _): ExpansionCtx<'_>, ) -> syn::Result<(Vec, TokenStream)> { let s = Expansion { + shared_format: None, attrs, fields: &s.fields, trait_ident, @@ -110,12 +111,8 @@ fn expand_struct( /// Expands a [`fmt`]-like derive macro for the provided enum. fn expand_enum( e: &syn::DataEnum, - (attrs, _, trait_ident, attr_name): ExpansionCtx<'_>, + (shared_attrs, _, trait_ident, attr_name): ExpansionCtx<'_>, ) -> syn::Result<(Vec, TokenStream)> { - if attrs.fmt.is_some() { - todo!("https://github.com/JelteF/derive_more/issues/142"); - } - let (bounds, match_arms) = e.variants.iter().try_fold( (Vec::new(), TokenStream::new()), |(mut bounds, mut arms), variant| { @@ -138,6 +135,7 @@ fn expand_enum( } let v = Expansion { + shared_format: shared_attrs.fmt.as_ref(), attrs: &attrs, fields: &variant.fields, trait_ident, @@ -198,6 +196,9 @@ fn expand_union( /// [`Display::fmt()`]: fmt::Display::fmt() #[derive(Debug)] struct Expansion<'a> { + /// Format shared between all variants of an enum. + shared_format: Option<&'a FmtAttribute>, + /// Derive macro [`ContainerAttributes`]. attrs: &'a ContainerAttributes, @@ -226,6 +227,81 @@ impl<'a> Expansion<'a> { /// [`Display::fmt()`]: fmt::Display::fmt() /// [`FmtAttribute`]: super::FmtAttribute fn generate_body(&self) -> syn::Result { + if self.shared_format.is_none() { + return self.generate_body_impl(); + } + let shared_format = self.shared_format.as_ref().unwrap(); + if !shared_format.args.is_empty() { + return Err(syn::Error::new( + shared_format.args.span(), + "shared format string does not support positional placeholders, use named placeholders instead", + )); + } + let mut tokens = TokenStream::new(); + let mut maybe_body = None; + let mut current_format = String::new(); + let fmt_string = shared_format.lit.value(); + let maybe_format_string = parsing::format_string(&fmt_string); + let Some(format_string) = maybe_format_string else { + // If we could not parse the format string, we just use the original string so + // we get a nice error message. We also panic as a safety precaution in case our + // parsing fails to parse something that write! allows. + return Ok(quote! { + derive_more::core::write!(__derive_more_f, #shared_format); + unreachable!("derive_more could not parse shared format string, but rust could: {:?}", #fmt_string); + }); + }; + for part in format_string.elements { + match part { + parsing::MaybeFormat::Text(s) => { + current_format.push_str(s); + } + parsing::MaybeFormat::Format { raw, format } => { + if format.arg == Some(parsing::Argument::Identifier("_variant")) { + if format.spec.is_some() { + return Err(syn::Error::new( + shared_format.span(), + "shared format _variant placeholder cannot contain format specifiers", + )); + } + if !current_format.is_empty() { + tokens.extend(quote! { derive_more::core::write!(__derive_more_f, #current_format)?; }); + current_format.clear(); + } + if maybe_body.is_none() { + maybe_body = Some(self.generate_body_impl()?); + } + let body = maybe_body.as_ref().unwrap(); + tokens.extend(quote! { #body?; }); + } else { + if format.arg.is_none() + || matches!(format.arg, Some(parsing::Argument::Integer(_))) + { + return Err(syn::Error::new( + shared_format.span(), + "shared format string cannot contain positional placeholders, use named placeholders instead", + )); + } + current_format.push_str(raw); + } + } + }; + } + if !current_format.is_empty() { + tokens.extend( + quote! { derive_more::core::write!(__derive_more_f, #current_format) }, + ) + } else { + tokens.extend(quote! { Ok(()) }); + } + Ok(tokens) + } + + /// Generates [`Display::fmt()`] implementation for a struct or an enum variant + /// without considering `shared_format`. + /// + /// [`Display::fmt()`]: fmt::Display::fmt() + fn generate_body_impl(&self) -> syn::Result { match &self.attrs.fmt { Some(fmt) => { Ok(if let Some((expr, trait_ident)) = fmt.transparent_call() { @@ -267,27 +343,55 @@ impl<'a> Expansion<'a> { /// Generates trait bounds for a struct or an enum variant. fn generate_bounds(&self) -> Vec { + let mut bounds: Vec = + if let Some(shared_format) = self.shared_format { + let shared_bounds = shared_format + .bounded_types(self.fields) + .map(|(ty, trait_name)| { + let trait_ident = format_ident!("{trait_name}"); + + parse_quote! { #ty: derive_more::core::fmt::#trait_ident } + }) + .chain(self.attrs.bounds.0.clone()) + .collect(); + // If it doesn't contain _variant we don't need to add any other bounds + if !parsing::format_string_formats(&shared_format.lit.value()) + .into_iter() + .flatten() + .any(|f| f.arg == Some(parsing::Argument::Identifier("_variant"))) + { + return shared_bounds; + } + shared_bounds + } else { + Vec::new() + }; + let Some(fmt) = &self.attrs.fmt else { - return self - .fields - .iter() - .next() - .map(|f| { - let ty = &f.ty; - let trait_ident = &self.trait_ident; - vec![parse_quote! { #ty: derive_more::core::fmt::#trait_ident }] - }) - .unwrap_or_default(); + bounds.extend( + self.fields + .iter() + .next() + .map(|f| { + let ty = &f.ty; + let trait_ident = &self.trait_ident; + vec![parse_quote! { #ty: derive_more::core::fmt::#trait_ident }] + }) + .unwrap_or_default(), + ); + return bounds; }; - fmt.bounded_types(self.fields) - .map(|(ty, trait_name)| { - let trait_ident = format_ident!("{trait_name}"); + bounds.extend( + fmt.bounded_types(self.fields) + .map(|(ty, trait_name)| { + let trait_ident = format_ident!("{trait_name}"); - parse_quote! { #ty: derive_more::core::fmt::#trait_ident } - }) - .chain(self.attrs.bounds.0.clone()) - .collect() + parse_quote! { #ty: derive_more::core::fmt::#trait_ident } + }) + .chain(self.attrs.bounds.0.clone()), + ); + bounds } } diff --git a/impl/src/fmt/mod.rs b/impl/src/fmt/mod.rs index 324e574c..b28a773f 100644 --- a/impl/src/fmt/mod.rs +++ b/impl/src/fmt/mod.rs @@ -372,9 +372,9 @@ impl Placeholder { /// Parses [`Placeholder`]s from the provided formatting string. fn parse_fmt_string(s: &str) -> Vec { let mut n = 0; - parsing::format_string(s) + parsing::format_string_formats(s) .into_iter() - .flat_map(|f| f.formats) + .flatten() .map(|format| { let (maybe_arg, ty) = ( format.arg, diff --git a/impl/src/fmt/parsing.rs b/impl/src/fmt/parsing.rs index 8a5d7527..74e053fa 100644 --- a/impl/src/fmt/parsing.rs +++ b/impl/src/fmt/parsing.rs @@ -9,7 +9,7 @@ use unicode_xid::UnicodeXID as XID; /// Output of the [`format_string`] parser. #[derive(Clone, Debug, Eq, PartialEq)] pub(crate) struct FormatString<'a> { - pub(crate) formats: Vec>, + pub(crate) elements: Vec>, } /// Output of the [`format`] parser. @@ -150,7 +150,11 @@ type Fill = char; type Width<'a> = Count<'a>; /// Output of the [`maybe_format`] parser. -type MaybeFormat<'a> = Option>; +#[derive(Debug, Clone, Copy, Eq, PartialEq)] +pub(crate) enum MaybeFormat<'a> { + Format { raw: &'a str, format: Format<'a> }, + Text(&'a str), +} /// Output of the [`identifier`] parser. type Identifier<'a> = &'a str; @@ -188,23 +192,32 @@ type LeftToParse<'a> = &'a str; /// parsers). /// /// [0]: std::fmt#syntax -pub(crate) fn format_string(input: &str) -> Option> { - let (mut input, _) = optional_result(text)(input); - - let formats = iter::repeat(()) +pub(crate) fn format_string(mut input: &str) -> Option> { + let elements = iter::repeat(()) .scan(&mut input, |input, _| { - let (curr, format) = - alt(&mut [&mut maybe_format, &mut map(text, |(i, _)| (i, None))])( - input, - )?; + let (curr, format) = alt(&mut [ + &mut maybe_format, + &mut map(text, |(i, x)| (i, MaybeFormat::Text(x))), + ])(input)?; **input = curr; Some(format) }) - .flatten() .collect(); - // Should consume all tokens for a successful parse. - input.is_empty().then_some(FormatString { formats }) + input.is_empty().then_some(FormatString { elements }) +} + +// Same as `format_string` but returns only the `Format` parts of the string. +pub(crate) fn format_string_formats(input: &str) -> Option> { + format_string(input).map(|f| { + f.elements + .into_iter() + .filter_map(|e| match e { + MaybeFormat::Format { format, .. } => Some(format), + _ => None, + }) + .collect() + }) } /// Parses a `maybe_format` as defined in the [grammar spec][0]. @@ -227,9 +240,12 @@ pub(crate) fn format_string(input: &str) -> Option> { /// [0]: std::fmt#syntax fn maybe_format(input: &str) -> Option<(LeftToParse<'_>, MaybeFormat<'_>)> { alt(&mut [ - &mut map(str("{{"), |i| (i, None)), - &mut map(str("}}"), |i| (i, None)), - &mut map(format, |(i, format)| (i, Some(format))), + &mut map(str("{{"), |i| (i, MaybeFormat::Text("{{"))), + &mut map(str("}}"), |i| (i, MaybeFormat::Text("}}"))), + &mut map(format, |(i, format)| { + let raw = &input[..input.len() - i.len()]; + (i, MaybeFormat::Format { raw, format }) + }), ])(input) } @@ -720,592 +736,569 @@ mod tests { #[test] fn text() { - assert_eq!(format_string(""), Some(FormatString { formats: vec![] })); - assert_eq!( - format_string("test"), - Some(FormatString { formats: vec![] }), - ); - assert_eq!( - format_string("Минск"), - Some(FormatString { formats: vec![] }), - ); - assert_eq!(format_string("🦀"), Some(FormatString { formats: vec![] })); + assert_eq!(format_string_formats(""), Some(vec![])); + assert_eq!(format_string_formats("test"), Some(vec![]),); + assert_eq!(format_string_formats("Минск"), Some(vec![]),); + assert_eq!(format_string_formats("🦀"), Some(vec![])); } #[test] fn argument() { assert_eq!( - format_string("{}"), - Some(FormatString { - formats: vec![Format { - arg: None, - spec: None, - }], - }), + format_string_formats("{}"), + Some(vec![Format { + arg: None, + spec: None, + }],), ); assert_eq!( - format_string("{0}"), - Some(FormatString { - formats: vec![Format { - arg: Some(Argument::Integer(0)), - spec: None, - }], - }), + format_string_formats("{0}"), + Some(vec![Format { + arg: Some(Argument::Integer(0)), + spec: None, + }],), ); assert_eq!( - format_string("{par}"), - Some(FormatString { - formats: vec![Format { - arg: Some(Argument::Identifier("par")), - spec: None, - }], - }), + format_string_formats("{par}"), + Some(vec![Format { + arg: Some(Argument::Identifier("par")), + spec: None, + }],), ); assert_eq!( - format_string("{Минск}"), - Some(FormatString { - formats: vec![Format { - arg: Some(Argument::Identifier("Минск")), - spec: None, - }], - }), + format_string_formats("{Минск}"), + Some(vec![Format { + arg: Some(Argument::Identifier("Минск")), + spec: None, + }],), ); } #[test] fn spec() { assert_eq!( - format_string("{:}"), - Some(FormatString { - formats: vec![Format { - arg: None, - spec: Some(FormatSpec { - align: None, - sign: None, - alternate: None, - zero_padding: None, - width: None, - precision: None, - ty: Type::Display, - }), - }], - }), + format_string_formats("{:}"), + Some(vec![Format { + arg: None, + spec: Some(FormatSpec { + align: None, + sign: None, + alternate: None, + zero_padding: None, + width: None, + precision: None, + ty: Type::Display, + }), + }],), ); assert_eq!( - format_string("{:^}"), - Some(FormatString { - formats: vec![Format { - arg: None, - spec: Some(FormatSpec { - align: Some((None, Align::Center)), - sign: None, - alternate: None, - zero_padding: None, - width: None, - precision: None, - ty: Type::Display, - }), - }], - }), + format_string_formats("{:^}"), + Some(vec![Format { + arg: None, + spec: Some(FormatSpec { + align: Some((None, Align::Center)), + sign: None, + alternate: None, + zero_padding: None, + width: None, + precision: None, + ty: Type::Display, + }), + }],), ); assert_eq!( - format_string("{:-<}"), - Some(FormatString { - formats: vec![Format { - arg: None, - spec: Some(FormatSpec { - align: Some((Some('-'), Align::Left)), - sign: None, - alternate: None, - zero_padding: None, - width: None, - precision: None, - ty: Type::Display, - }), - }], - }), + format_string_formats("{:-<}"), + Some(vec![Format { + arg: None, + spec: Some(FormatSpec { + align: Some((Some('-'), Align::Left)), + sign: None, + alternate: None, + zero_padding: None, + width: None, + precision: None, + ty: Type::Display, + }), + }],), ); assert_eq!( - format_string("{: <}"), - Some(FormatString { - formats: vec![Format { - arg: None, - spec: Some(FormatSpec { - align: Some((Some(' '), Align::Left)), - sign: None, - alternate: None, - zero_padding: None, - width: None, - precision: None, - ty: Type::Display, - }), - }], - }), + format_string_formats("{: <}"), + Some(vec![Format { + arg: None, + spec: Some(FormatSpec { + align: Some((Some(' '), Align::Left)), + sign: None, + alternate: None, + zero_padding: None, + width: None, + precision: None, + ty: Type::Display, + }), + }],), ); assert_eq!( - format_string("{:^<}"), - Some(FormatString { - formats: vec![Format { - arg: None, - spec: Some(FormatSpec { - align: Some((Some('^'), Align::Left)), - sign: None, - alternate: None, - zero_padding: None, - width: None, - precision: None, - ty: Type::Display, - }), - }], - }), + format_string_formats("{:^<}"), + Some(vec![Format { + arg: None, + spec: Some(FormatSpec { + align: Some((Some('^'), Align::Left)), + sign: None, + alternate: None, + zero_padding: None, + width: None, + precision: None, + ty: Type::Display, + }), + }],), ); assert_eq!( - format_string("{:+}"), - Some(FormatString { - formats: vec![Format { - arg: None, - spec: Some(FormatSpec { - align: None, - sign: Some(Sign::Plus), - alternate: None, - zero_padding: None, - width: None, - precision: None, - ty: Type::Display, - }), - }], - }), + format_string_formats("{:+}"), + Some(vec![Format { + arg: None, + spec: Some(FormatSpec { + align: None, + sign: Some(Sign::Plus), + alternate: None, + zero_padding: None, + width: None, + precision: None, + ty: Type::Display, + }), + }],), ); assert_eq!( - format_string("{:^<-}"), - Some(FormatString { - formats: vec![Format { - arg: None, - spec: Some(FormatSpec { - align: Some((Some('^'), Align::Left)), - sign: Some(Sign::Minus), - alternate: None, - zero_padding: None, - width: None, - precision: None, - ty: Type::Display, - }), - }], - }), + format_string_formats("{:^<-}"), + Some(vec![Format { + arg: None, + spec: Some(FormatSpec { + align: Some((Some('^'), Align::Left)), + sign: Some(Sign::Minus), + alternate: None, + zero_padding: None, + width: None, + precision: None, + ty: Type::Display, + }), + }],), ); assert_eq!( - format_string("{:#}"), - Some(FormatString { - formats: vec![Format { - arg: None, - spec: Some(FormatSpec { - align: None, - sign: None, - alternate: Some(Alternate), - zero_padding: None, - width: None, - precision: None, - ty: Type::Display, - }), - }], - }), + format_string_formats("{:#}"), + Some(vec![Format { + arg: None, + spec: Some(FormatSpec { + align: None, + sign: None, + alternate: Some(Alternate), + zero_padding: None, + width: None, + precision: None, + ty: Type::Display, + }), + }],), ); assert_eq!( - format_string("{:+#}"), - Some(FormatString { - formats: vec![Format { - arg: None, - spec: Some(FormatSpec { - align: None, - sign: Some(Sign::Plus), - alternate: Some(Alternate), - zero_padding: None, - width: None, - precision: None, - ty: Type::Display, - }), - }], - }), + format_string_formats("{:+#}"), + Some(vec![Format { + arg: None, + spec: Some(FormatSpec { + align: None, + sign: Some(Sign::Plus), + alternate: Some(Alternate), + zero_padding: None, + width: None, + precision: None, + ty: Type::Display, + }), + }],), ); assert_eq!( - format_string("{:-<#}"), - Some(FormatString { - formats: vec![Format { - arg: None, - spec: Some(FormatSpec { - align: Some((Some('-'), Align::Left)), - sign: None, - alternate: Some(Alternate), - zero_padding: None, - width: None, - precision: None, - ty: Type::Display, - }), - }], - }), + format_string_formats("{:-<#}"), + Some(vec![Format { + arg: None, + spec: Some(FormatSpec { + align: Some((Some('-'), Align::Left)), + sign: None, + alternate: Some(Alternate), + zero_padding: None, + width: None, + precision: None, + ty: Type::Display, + }), + }],), ); assert_eq!( - format_string("{:^<-#}"), - Some(FormatString { - formats: vec![Format { - arg: None, - spec: Some(FormatSpec { - align: Some((Some('^'), Align::Left)), - sign: Some(Sign::Minus), - alternate: Some(Alternate), - zero_padding: None, - width: None, - precision: None, - ty: Type::Display, - }), - }], - }), + format_string_formats("{:^<-#}"), + Some(vec![Format { + arg: None, + spec: Some(FormatSpec { + align: Some((Some('^'), Align::Left)), + sign: Some(Sign::Minus), + alternate: Some(Alternate), + zero_padding: None, + width: None, + precision: None, + ty: Type::Display, + }), + }],), ); assert_eq!( - format_string("{:0}"), - Some(FormatString { - formats: vec![Format { - arg: None, - spec: Some(FormatSpec { - align: None, - sign: None, - alternate: None, - zero_padding: Some(ZeroPadding), - width: None, - precision: None, - ty: Type::Display, - }), - }], - }), + format_string_formats("{:0}"), + Some(vec![Format { + arg: None, + spec: Some(FormatSpec { + align: None, + sign: None, + alternate: None, + zero_padding: Some(ZeroPadding), + width: None, + precision: None, + ty: Type::Display, + }), + }],), ); assert_eq!( - format_string("{:#0}"), - Some(FormatString { - formats: vec![Format { - arg: None, - spec: Some(FormatSpec { - align: None, - sign: None, - alternate: Some(Alternate), - zero_padding: Some(ZeroPadding), - width: None, - precision: None, - ty: Type::Display, - }), - }], - }), + format_string_formats("{:#0}"), + Some(vec![Format { + arg: None, + spec: Some(FormatSpec { + align: None, + sign: None, + alternate: Some(Alternate), + zero_padding: Some(ZeroPadding), + width: None, + precision: None, + ty: Type::Display, + }), + }],), ); assert_eq!( - format_string("{:-0}"), - Some(FormatString { - formats: vec![Format { - arg: None, - spec: Some(FormatSpec { - align: None, - sign: Some(Sign::Minus), - alternate: None, - zero_padding: Some(ZeroPadding), - width: None, - precision: None, - ty: Type::Display, - }), - }], - }), + format_string_formats("{:-0}"), + Some(vec![Format { + arg: None, + spec: Some(FormatSpec { + align: None, + sign: Some(Sign::Minus), + alternate: None, + zero_padding: Some(ZeroPadding), + width: None, + precision: None, + ty: Type::Display, + }), + }],), ); assert_eq!( - format_string("{:^<0}"), - Some(FormatString { - formats: vec![Format { - arg: None, - spec: Some(FormatSpec { - align: Some((Some('^'), Align::Left)), - sign: None, - alternate: None, - zero_padding: Some(ZeroPadding), - width: None, - precision: None, - ty: Type::Display, - }), - }], - }), + format_string_formats("{:^<0}"), + Some(vec![Format { + arg: None, + spec: Some(FormatSpec { + align: Some((Some('^'), Align::Left)), + sign: None, + alternate: None, + zero_padding: Some(ZeroPadding), + width: None, + precision: None, + ty: Type::Display, + }), + }],), ); assert_eq!( - format_string("{:^<+#0}"), - Some(FormatString { - formats: vec![Format { - arg: None, - spec: Some(FormatSpec { - align: Some((Some('^'), Align::Left)), - sign: Some(Sign::Plus), - alternate: Some(Alternate), - zero_padding: Some(ZeroPadding), - width: None, - precision: None, - ty: Type::Display, - }), - }], - }), + format_string_formats("{:^<+#0}"), + Some(vec![Format { + arg: None, + spec: Some(FormatSpec { + align: Some((Some('^'), Align::Left)), + sign: Some(Sign::Plus), + alternate: Some(Alternate), + zero_padding: Some(ZeroPadding), + width: None, + precision: None, + ty: Type::Display, + }), + }],), ); assert_eq!( - format_string("{:1}"), - Some(FormatString { - formats: vec![Format { - arg: None, - spec: Some(FormatSpec { - align: None, - sign: None, - alternate: None, - zero_padding: None, - width: Some(Count::Integer(1)), - precision: None, - ty: Type::Display, - }), - }], - }), + format_string_formats("{:1}"), + Some(vec![Format { + arg: None, + spec: Some(FormatSpec { + align: None, + sign: None, + alternate: None, + zero_padding: None, + width: Some(Count::Integer(1)), + precision: None, + ty: Type::Display, + }), + }],), ); assert_eq!( - format_string("{:1$}"), - Some(FormatString { - formats: vec![Format { - arg: None, - spec: Some(FormatSpec { - align: None, - sign: None, - alternate: None, - zero_padding: None, - width: Some(Count::Parameter(Argument::Integer(1))), - precision: None, - ty: Type::Display, - }), - }], - }), + format_string_formats("{:1$}"), + Some(vec![Format { + arg: None, + spec: Some(FormatSpec { + align: None, + sign: None, + alternate: None, + zero_padding: None, + width: Some(Count::Parameter(Argument::Integer(1))), + precision: None, + ty: Type::Display, + }), + }],), ); assert_eq!( - format_string("{:par$}"), - Some(FormatString { - formats: vec![Format { - arg: None, - spec: Some(FormatSpec { - align: None, - sign: None, - alternate: None, - zero_padding: None, - width: Some(Count::Parameter(Argument::Identifier("par"))), - precision: None, - ty: Type::Display, - }), - }], - }), + format_string_formats("{:par$}"), + Some(vec![Format { + arg: None, + spec: Some(FormatSpec { + align: None, + sign: None, + alternate: None, + zero_padding: None, + width: Some(Count::Parameter(Argument::Identifier("par"))), + precision: None, + ty: Type::Display, + }), + }],), ); assert_eq!( - format_string("{:-^-#0Минск$}"), - Some(FormatString { - formats: vec![Format { - arg: None, - spec: Some(FormatSpec { - align: Some((Some('-'), Align::Center)), - sign: Some(Sign::Minus), - alternate: Some(Alternate), - zero_padding: Some(ZeroPadding), - width: Some(Count::Parameter(Argument::Identifier("Минск"))), - precision: None, - ty: Type::Display, - }), - }], - }), + format_string_formats("{:-^-#0Минск$}"), + Some(vec![Format { + arg: None, + spec: Some(FormatSpec { + align: Some((Some('-'), Align::Center)), + sign: Some(Sign::Minus), + alternate: Some(Alternate), + zero_padding: Some(ZeroPadding), + width: Some(Count::Parameter(Argument::Identifier("Минск"))), + precision: None, + ty: Type::Display, + }), + }],), ); assert_eq!( - format_string("{:.*}"), - Some(FormatString { - formats: vec![Format { - arg: None, - spec: Some(FormatSpec { - align: None, - sign: None, - alternate: None, - zero_padding: None, - width: None, - precision: Some(Precision::Star), - ty: Type::Display, - }), - }], - }), + format_string_formats("{:.*}"), + Some(vec![Format { + arg: None, + spec: Some(FormatSpec { + align: None, + sign: None, + alternate: None, + zero_padding: None, + width: None, + precision: Some(Precision::Star), + ty: Type::Display, + }), + }],), ); assert_eq!( - format_string("{:.0}"), - Some(FormatString { - formats: vec![Format { - arg: None, - spec: Some(FormatSpec { - align: None, - sign: None, - alternate: None, - zero_padding: None, - width: None, - precision: Some(Precision::Count(Count::Integer(0))), - ty: Type::Display, - }), - }], - }), + format_string_formats("{:.0}"), + Some(vec![Format { + arg: None, + spec: Some(FormatSpec { + align: None, + sign: None, + alternate: None, + zero_padding: None, + width: None, + precision: Some(Precision::Count(Count::Integer(0))), + ty: Type::Display, + }), + }],), ); assert_eq!( - format_string("{:.0$}"), - Some(FormatString { - formats: vec![Format { - arg: None, - spec: Some(FormatSpec { - align: None, - sign: None, - alternate: None, - zero_padding: None, - width: None, - precision: Some(Precision::Count(Count::Parameter( - Argument::Integer(0), - ))), - ty: Type::Display, - }), - }], - }), + format_string_formats("{:.0$}"), + Some(vec![Format { + arg: None, + spec: Some(FormatSpec { + align: None, + sign: None, + alternate: None, + zero_padding: None, + width: None, + precision: Some(Precision::Count(Count::Parameter( + Argument::Integer(0), + ))), + ty: Type::Display, + }), + }],), ); assert_eq!( - format_string("{:.par$}"), - Some(FormatString { - formats: vec![Format { - arg: None, - spec: Some(FormatSpec { - align: None, - sign: None, - alternate: None, - zero_padding: None, - width: None, - precision: Some(Precision::Count(Count::Parameter( - Argument::Identifier("par"), - ))), - ty: Type::Display, - }), - }], - }), + format_string_formats("{:.par$}"), + Some(vec![Format { + arg: None, + spec: Some(FormatSpec { + align: None, + sign: None, + alternate: None, + zero_padding: None, + width: None, + precision: Some(Precision::Count(Count::Parameter( + Argument::Identifier("par"), + ))), + ty: Type::Display, + }), + }],), ); assert_eq!( - format_string("{: >+#2$.par$}"), - Some(FormatString { - formats: vec![Format { - arg: None, - spec: Some(FormatSpec { - align: Some((Some(' '), Align::Right)), - sign: Some(Sign::Plus), - alternate: Some(Alternate), - zero_padding: None, - width: Some(Count::Parameter(Argument::Integer(2))), - precision: Some(Precision::Count(Count::Parameter( - Argument::Identifier("par"), - ))), - ty: Type::Display, - }), - }], - }), + format_string_formats("{: >+#2$.par$}"), + Some(vec![Format { + arg: None, + spec: Some(FormatSpec { + align: Some((Some(' '), Align::Right)), + sign: Some(Sign::Plus), + alternate: Some(Alternate), + zero_padding: None, + width: Some(Count::Parameter(Argument::Integer(2))), + precision: Some(Precision::Count(Count::Parameter( + Argument::Identifier("par"), + ))), + ty: Type::Display, + }), + }],), ); assert_eq!( - format_string("{:x?}"), - Some(FormatString { - formats: vec![Format { - arg: None, - spec: Some(FormatSpec { - align: None, - sign: None, - alternate: None, - zero_padding: None, - width: None, - precision: None, - ty: Type::LowerDebug, - }), - }], - }), + format_string_formats("{:x?}"), + Some(vec![Format { + arg: None, + spec: Some(FormatSpec { + align: None, + sign: None, + alternate: None, + zero_padding: None, + width: None, + precision: None, + ty: Type::LowerDebug, + }), + }],), ); assert_eq!( - format_string("{:E}"), - Some(FormatString { - formats: vec![Format { - arg: None, + format_string_formats("{:E}"), + Some(vec![Format { + arg: None, + spec: Some(FormatSpec { + align: None, + sign: None, + alternate: None, + zero_padding: None, + width: None, + precision: None, + ty: Type::UpperExp, + }), + }],), + ); + assert_eq!( + format_string_formats("{: >+#par$.par$X?}"), + Some(vec![Format { + arg: None, + spec: Some(FormatSpec { + align: Some((Some(' '), Align::Right)), + sign: Some(Sign::Plus), + alternate: Some(Alternate), + zero_padding: None, + width: Some(Count::Parameter(Argument::Identifier("par"))), + precision: Some(Precision::Count(Count::Parameter( + Argument::Identifier("par"), + ))), + ty: Type::UpperDebug, + }), + }],), + ); + } + + #[test] + fn full_format() { + assert_eq!( + format_string_formats("prefix{{{0:#?}postfix{par:-^par$.a$}}}"), + Some(vec![ + Format { + arg: Some(Argument::Integer(0)), spec: Some(FormatSpec { align: None, sign: None, - alternate: None, + alternate: Some(Alternate), zero_padding: None, width: None, precision: None, - ty: Type::UpperExp, + ty: Type::Debug, }), - }], - }), - ); - assert_eq!( - format_string("{: >+#par$.par$X?}"), - Some(FormatString { - formats: vec![Format { - arg: None, + }, + Format { + arg: Some(Argument::Identifier("par")), spec: Some(FormatSpec { - align: Some((Some(' '), Align::Right)), - sign: Some(Sign::Plus), - alternate: Some(Alternate), + align: Some((Some('-'), Align::Center)), + sign: None, + alternate: None, zero_padding: None, width: Some(Count::Parameter(Argument::Identifier("par"))), precision: Some(Precision::Count(Count::Parameter( - Argument::Identifier("par"), + Argument::Identifier("a"), ))), - ty: Type::UpperDebug, + ty: Type::Display, }), - }], - }), + }, + ],), ); } #[test] - fn full() { + fn full_parts() { assert_eq!( format_string("prefix{{{0:#?}postfix{par:-^par$.a$}}}"), Some(FormatString { - formats: vec![ - Format { - arg: Some(Argument::Integer(0)), - spec: Some(FormatSpec { - align: None, - sign: None, - alternate: Some(Alternate), - zero_padding: None, - width: None, - precision: None, - ty: Type::Debug, - }), + elements: vec![ + MaybeFormat::Text("prefix"), + MaybeFormat::Text("{{"), + MaybeFormat::Format { + raw: "{0:#?}", + format: Format { + arg: Some(Argument::Integer(0)), + spec: Some(FormatSpec { + align: None, + sign: None, + alternate: Some(Alternate), + zero_padding: None, + width: None, + precision: None, + ty: Type::Debug, + }), + } }, - Format { - arg: Some(Argument::Identifier("par")), - spec: Some(FormatSpec { - align: Some((Some('-'), Align::Center)), - sign: None, - alternate: None, - zero_padding: None, - width: Some(Count::Parameter(Argument::Identifier("par"))), - precision: Some(Precision::Count(Count::Parameter( - Argument::Identifier("a"), - ))), - ty: Type::Display, - }), + MaybeFormat::Text("postfix"), + MaybeFormat::Format { + raw: "{par:-^par$.a$}", + format: Format { + arg: Some(Argument::Identifier("par")), + spec: Some(FormatSpec { + align: Some((Some('-'), Align::Center)), + sign: None, + alternate: None, + zero_padding: None, + width: Some(Count::Parameter(Argument::Identifier( + "par" + ))), + precision: Some(Precision::Count(Count::Parameter( + Argument::Identifier("a"), + ))), + ty: Type::Display, + }), + } }, - ], + MaybeFormat::Text("}}"), + ] }), ); } #[test] fn error() { - assert_eq!(format_string("{"), None); - assert_eq!(format_string("}"), None); - assert_eq!(format_string("{{}"), None); - assert_eq!(format_string("{:x?"), None); - assert_eq!(format_string("{:.}"), None); - assert_eq!(format_string("{:q}"), None); - assert_eq!(format_string("{:par}"), None); - assert_eq!(format_string("{⚙️}"), None); + assert_eq!(format_string_formats("{"), None); + assert_eq!(format_string_formats("}"), None); + assert_eq!(format_string_formats("{{}"), None); + assert_eq!(format_string_formats("{:x?"), None); + assert_eq!(format_string_formats("{:.}"), None); + assert_eq!(format_string_formats("{:q}"), None); + assert_eq!(format_string_formats("{:par}"), None); + assert_eq!(format_string_formats("{⚙️}"), None); } } diff --git a/tests/compile_fail/display/shared_format_positional_placeholders.rs b/tests/compile_fail/display/shared_format_positional_placeholders.rs new file mode 100644 index 00000000..8d2a0ff2 --- /dev/null +++ b/tests/compile_fail/display/shared_format_positional_placeholders.rs @@ -0,0 +1,19 @@ +#[derive(derive_more::Display)] +#[display("Stuff({})")] +enum Foo { + A, +} + +#[derive(derive_more::Display)] +#[display("Stuff({0})")] +enum Foo2 { + A, +} + +#[derive(derive_more::Display)] +#[display("Stuff()", _0, _2)] +enum Foo3 { + A, +} + +fn main() {} diff --git a/tests/compile_fail/display/shared_format_positional_placeholders.stderr b/tests/compile_fail/display/shared_format_positional_placeholders.stderr new file mode 100644 index 00000000..d9f2d523 --- /dev/null +++ b/tests/compile_fail/display/shared_format_positional_placeholders.stderr @@ -0,0 +1,17 @@ +error: shared format string cannot contain positional placeholders, use named placeholders instead + --> tests/compile_fail/display/shared_format_positional_placeholders.rs:2:11 + | +2 | #[display("Stuff({})")] + | ^^^^^^^^^^^ + +error: shared format string cannot contain positional placeholders, use named placeholders instead + --> tests/compile_fail/display/shared_format_positional_placeholders.rs:8:11 + | +8 | #[display("Stuff({0})")] + | ^^^^^^^^^^^^ + +error: shared format string does not support positional placeholders, use named placeholders instead + --> tests/compile_fail/display/shared_format_positional_placeholders.rs:14:22 + | +14 | #[display("Stuff()", _0, _2)] + | ^^ diff --git a/tests/compile_fail/display/shared_format_unclosed_brace.rs b/tests/compile_fail/display/shared_format_unclosed_brace.rs new file mode 100644 index 00000000..47134e25 --- /dev/null +++ b/tests/compile_fail/display/shared_format_unclosed_brace.rs @@ -0,0 +1,7 @@ +#[derive(derive_more::Display)] +#[display("Stuff({)")] +enum Foo { + A, +} + +fn main() {} diff --git a/tests/compile_fail/display/shared_format_unclosed_brace.stderr b/tests/compile_fail/display/shared_format_unclosed_brace.stderr new file mode 100644 index 00000000..4d4861f3 --- /dev/null +++ b/tests/compile_fail/display/shared_format_unclosed_brace.stderr @@ -0,0 +1,9 @@ +error: invalid format string: expected `'}'`, found `')'` + --> tests/compile_fail/display/shared_format_unclosed_brace.rs:2:19 + | +2 | #[display("Stuff({)")] + | -^ expected `'}'` in format string + | | + | because of this opening brace + | + = note: if you intended to print `{`, you can escape it using `{{` diff --git a/tests/compile_fail/display/shared_format_variant_spec.rs b/tests/compile_fail/display/shared_format_variant_spec.rs new file mode 100644 index 00000000..9ad647ad --- /dev/null +++ b/tests/compile_fail/display/shared_format_variant_spec.rs @@ -0,0 +1,7 @@ +#[derive(derive_more::Display)] +#[display("Stuff({_variant:?})")] +enum Foo { + A, +} + +fn main() {} diff --git a/tests/compile_fail/display/shared_format_variant_spec.stderr b/tests/compile_fail/display/shared_format_variant_spec.stderr new file mode 100644 index 00000000..d95676fc --- /dev/null +++ b/tests/compile_fail/display/shared_format_variant_spec.stderr @@ -0,0 +1,5 @@ +error: shared format _variant placeholder cannot contain format specifiers + --> tests/compile_fail/display/shared_format_variant_spec.rs:2:11 + | +2 | #[display("Stuff({_variant:?})")] + | ^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/display.rs b/tests/display.rs index 1e7ddfc1..ebc0012e 100644 --- a/tests/display.rs +++ b/tests/display.rs @@ -1283,6 +1283,131 @@ mod enums { } } } + + mod shared_format { + use super::*; + mod single { + use super::*; + + #[derive(Display)] + #[display("Variant: {_variant}")] + enum Enum { + #[display("A {_0}")] + A(i32), + #[display("B {}", field)] + B { + field: i32, + }, + C, + } + + #[test] + fn assert() { + assert_eq!(Enum::A(1).to_string(), "Variant: A 1"); + assert_eq!(Enum::B { field: 2 }.to_string(), "Variant: B 2",); + assert_eq!(Enum::C.to_string(), "Variant: C",); + } + } + + mod multiple { + use super::*; + + #[derive(Display)] + #[display("{_variant} Variant: {_variant} {_variant}")] + enum Enum { + #[display("A {_0}")] + A(i32), + #[display("B {}", field)] + B { + field: i32, + }, + C, + } + + #[test] + fn assert() { + assert_eq!(Enum::A(1).to_string(), "A 1 Variant: A 1 A 1"); + assert_eq!( + Enum::B { field: 2 }.to_string(), + "B 2 Variant: B 2 B 2", + ); + assert_eq!(Enum::C.to_string(), "C Variant: C C",); + } + } + + mod none { + use super::*; + + /// Make sure that variant specific bounds are not added if _variant is + /// not used. + struct NoDisplay; + + #[derive(Display)] + #[display("Variant")] + enum Enum { + #[display("A {_0}")] + A(i32), + #[display("B {}", field)] + B { + field: i32, + }, + C, + D(T), + } + + #[test] + fn assert() { + assert_eq!(Enum::::A(1).to_string(), "Variant"); + assert_eq!( + Enum::::B { field: 2 }.to_string(), + "Variant", + ); + assert_eq!(Enum::::C.to_string(), "Variant",); + assert_eq!(Enum::::D(NoDisplay).to_string(), "Variant",); + } + } + + mod use_field { + use super::*; + + #[derive(Display)] + #[display("Variant {_0}")] + enum Enum { + A(i32), + B(&'static str), + C(T), + } + + #[test] + fn assert() { + assert_eq!(Enum::::A(1).to_string(), "Variant 1"); + assert_eq!(Enum::::B("abc").to_string(), "Variant abc",); + assert_eq!(Enum::::C(9).to_string(), "Variant 9",); + } + } + + mod use_field_and_variant { + use super::*; + + #[derive(Display)] + #[display("Variant {_variant} {_0}")] + enum Enum { + #[display("A")] + A(i32), + #[display("B")] + B(&'static str), + #[display("C")] + C(T), + } + + #[test] + fn assert() { + assert_eq!(Enum::::A(1).to_string(), "Variant A 1"); + assert_eq!(Enum::::B("abc").to_string(), "Variant B abc",); + assert_eq!(Enum::::C(9).to_string(), "Variant C 9",); + } + } + } } } From 5d7386425980abf84f94bbb415a2e506d3e2e9e2 Mon Sep 17 00:00:00 2001 From: tyranron Date: Fri, 19 Jul 2024 15:27:58 +0300 Subject: [PATCH 2/8] Some fixes and tests improvement [skip ci] --- README.md | 2 +- clippy.toml | 2 -- impl/src/fmt/display.rs | 21 +++++++++++++-------- tests/display.rs | 12 ++++++------ 4 files changed, 20 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 4ce210fc..d92e8f88 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ [![Latest Version](https://img.shields.io/crates/v/derive_more.svg)](https://crates.io/crates/derive_more) [![Rust Documentation](https://docs.rs/derive_more/badge.svg)](https://docs.rs/derive_more) [![GitHub license](https://img.shields.io/badge/license-MIT-blue.svg)](https://raw.githubusercontent.com/JelteF/derive_more/master/LICENSE) -[![Rust 1.70+](https://img.shields.io/badge/rustc-1.70+-lightgray.svg)](https://blog.rust-lang.org/2022/11/03/Rust-1.70.0.html) +[![Rust 1.70+](https://img.shields.io/badge/rustc-1.70+-lightgray.svg)](https://blog.rust-lang.org/2023/06/01/Rust-1.70.0.html) [![Unsafe Forbidden](https://img.shields.io/badge/unsafe-forbidden-success.svg)](https://github.com/rust-secure-code/safety-dance) Rust has lots of builtin traits that are implemented for its basic types, such diff --git a/clippy.toml b/clippy.toml index ca697fd7..2487d144 100644 --- a/clippy.toml +++ b/clippy.toml @@ -2,8 +2,6 @@ # See full lints list at: # https://rust-lang.github.io/rust-clippy/master/index.html -msrv = "1.70.0" - # Ensures consistent bracing for macro calls in the codebase. # Extends default settings: # https://github.com/rust-lang/rust-clippy/blob/master/clippy_lints/src/nonstandard_macro_braces.rs#L143-L184 diff --git a/impl/src/fmt/display.rs b/impl/src/fmt/display.rs index f70c99c5..089f44dc 100644 --- a/impl/src/fmt/display.rs +++ b/impl/src/fmt/display.rs @@ -225,7 +225,6 @@ impl<'a> Expansion<'a> { /// greater than 1. /// /// [`Display::fmt()`]: fmt::Display::fmt() - /// [`FmtAttribute`]: super::FmtAttribute fn generate_body(&self) -> syn::Result { if self.shared_format.is_none() { return self.generate_body_impl(); @@ -234,7 +233,8 @@ impl<'a> Expansion<'a> { if !shared_format.args.is_empty() { return Err(syn::Error::new( shared_format.args.span(), - "shared format string does not support positional placeholders, use named placeholders instead", + "shared format string does not support positional placeholders, use named \ + placeholders instead", )); } let mut tokens = TokenStream::new(); @@ -243,12 +243,15 @@ impl<'a> Expansion<'a> { let fmt_string = shared_format.lit.value(); let maybe_format_string = parsing::format_string(&fmt_string); let Some(format_string) = maybe_format_string else { - // If we could not parse the format string, we just use the original string so - // we get a nice error message. We also panic as a safety precaution in case our - // parsing fails to parse something that write! allows. + // If we could not parse the format string, we just use the original string, so we get + // a nice error message. We also panic as a safety precaution in case our parsing fails + // to parse something that `write!()` allows. return Ok(quote! { derive_more::core::write!(__derive_more_f, #shared_format); - unreachable!("derive_more could not parse shared format string, but rust could: {:?}", #fmt_string); + unreachable!( + "`derive_more` could not parse shared format string, but Rust could: {:?}", + #fmt_string, + ); }); }; for part in format_string.elements { @@ -261,7 +264,8 @@ impl<'a> Expansion<'a> { if format.spec.is_some() { return Err(syn::Error::new( shared_format.span(), - "shared format _variant placeholder cannot contain format specifiers", + "shared format `_variant` placeholder cannot contain format \ + specifiers", )); } if !current_format.is_empty() { @@ -279,7 +283,8 @@ impl<'a> Expansion<'a> { { return Err(syn::Error::new( shared_format.span(), - "shared format string cannot contain positional placeholders, use named placeholders instead", + "shared format string cannot contain positional placeholders, use \ + named placeholders instead", )); } current_format.push_str(raw); diff --git a/tests/display.rs b/tests/display.rs index 9a91afbd..e5f6fa8e 100644 --- a/tests/display.rs +++ b/tests/display.rs @@ -1281,6 +1281,7 @@ mod enums { mod shared_format { use super::*; + mod single { use super::*; @@ -1299,8 +1300,8 @@ mod enums { #[test] fn assert() { assert_eq!(Enum::A(1).to_string(), "Variant: A 1"); - assert_eq!(Enum::B { field: 2 }.to_string(), "Variant: B 2",); - assert_eq!(Enum::C.to_string(), "Variant: C",); + assert_eq!(Enum::B { field: 2 }.to_string(), "Variant: B 2"); + assert_eq!(Enum::C.to_string(), "Variant: C"); } } @@ -1308,7 +1309,7 @@ mod enums { use super::*; #[derive(Display)] - #[display("{_variant} Variant: {_variant} {_variant}")] + #[display("{} Variant: {} {}", _variant)] enum Enum { #[display("A {_0}")] A(i32), @@ -1326,15 +1327,14 @@ mod enums { Enum::B { field: 2 }.to_string(), "B 2 Variant: B 2 B 2", ); - assert_eq!(Enum::C.to_string(), "C Variant: C C",); + assert_eq!(Enum::C.to_string(), "C Variant: C C"); } } mod none { use super::*; - /// Make sure that variant specific bounds are not added if _variant is - /// not used. + /// Make sure that variant-specific bounds are not added if `_variant` is not used. struct NoDisplay; #[derive(Display)] From e30790832624adadf722aa41fc2e6dd7f34634e2 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Fri, 19 Jul 2024 18:46:43 +0200 Subject: [PATCH 3/8] Fix tests --- tests/compile_fail/display/shared_format_variant_spec.stderr | 2 +- tests/display.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/compile_fail/display/shared_format_variant_spec.stderr b/tests/compile_fail/display/shared_format_variant_spec.stderr index d95676fc..2fb6028a 100644 --- a/tests/compile_fail/display/shared_format_variant_spec.stderr +++ b/tests/compile_fail/display/shared_format_variant_spec.stderr @@ -1,4 +1,4 @@ -error: shared format _variant placeholder cannot contain format specifiers +error: shared format `_variant` placeholder cannot contain format specifiers --> tests/compile_fail/display/shared_format_variant_spec.rs:2:11 | 2 | #[display("Stuff({_variant:?})")] diff --git a/tests/display.rs b/tests/display.rs index e5f6fa8e..e67b1e38 100644 --- a/tests/display.rs +++ b/tests/display.rs @@ -1309,7 +1309,7 @@ mod enums { use super::*; #[derive(Display)] - #[display("{} Variant: {} {}", _variant)] + #[display("{_variant} Variant: {_variant} {_variant}")] enum Enum { #[display("A {_0}")] A(i32), From dc223cfae071e31b8df54e825ad982d9630dc6cc Mon Sep 17 00:00:00 2001 From: tyranron Date: Wed, 24 Jul 2024 19:57:50 +0300 Subject: [PATCH 4/8] Re-impl, vol.1 --- .github/workflows/ci.yml | 2 +- CHANGELOG.md | 2 +- Cargo.toml | 2 +- README.md | 4 +- impl/Cargo.toml | 2 +- impl/doc/display.md | 2 +- impl/src/fmt/display.rs | 258 ++--- impl/src/fmt/mod.rs | 34 +- impl/src/fmt/parsing.rs | 1005 +++++++++-------- ...ared_format_positional_placeholders.stderr | 33 +- tests/display.rs | 16 +- 11 files changed, 690 insertions(+), 670 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 13439b8a..657f25c0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -54,7 +54,7 @@ jobs: strategy: fail-fast: false matrix: - msrv: ["1.70.0"] + msrv: ["1.65.0"] os: - ubuntu - macOS diff --git a/CHANGELOG.md b/CHANGELOG.md index b0ba4f43..73d17971 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Breaking changes -- The minimum supported Rust version (MSRV) is now Rust 1.70. +- The minimum supported Rust version (MSRV) is now Rust 1.65. - Add the `std` feature which should be disabled in `no_std` environments. - All Cargo features, except `std`, are now disabled by default. The `full` feature can be used to get the old behavior of supporting all possible diff --git a/Cargo.toml b/Cargo.toml index c734182e..420f60b3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,7 +2,7 @@ name = "derive_more" version = "1.0.0-beta.6" edition = "2021" -rust-version = "1.70.0" +rust-version = "1.65.0" description = "Adds #[derive(x)] macros for more traits" authors = ["Jelte Fennema "] license = "MIT" diff --git a/README.md b/README.md index d92e8f88..383f773d 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ [![Latest Version](https://img.shields.io/crates/v/derive_more.svg)](https://crates.io/crates/derive_more) [![Rust Documentation](https://docs.rs/derive_more/badge.svg)](https://docs.rs/derive_more) [![GitHub license](https://img.shields.io/badge/license-MIT-blue.svg)](https://raw.githubusercontent.com/JelteF/derive_more/master/LICENSE) -[![Rust 1.70+](https://img.shields.io/badge/rustc-1.70+-lightgray.svg)](https://blog.rust-lang.org/2023/06/01/Rust-1.70.0.html) +[![Rust 1.65+](https://img.shields.io/badge/rustc-1.65+-lightgray.svg)](https://blog.rust-lang.org/2022/11/03/Rust-1.65.0.html) [![Unsafe Forbidden](https://img.shields.io/badge/unsafe-forbidden-success.svg)](https://github.com/rust-secure-code/safety-dance) Rust has lots of builtin traits that are implemented for its basic types, such @@ -215,7 +215,7 @@ extern crate derive_more; ## [MSRV] policy -This library requires Rust 1.70 or higher. +This library requires Rust 1.65 or higher. Changing [MSRV] (minimum supported Rust version) of this crate is treated as a **minor version change** in terms of [Semantic Versioning]. - So, if [MSRV] changes are **NOT concerning** for your project, just use the default [caret requirement]: diff --git a/impl/Cargo.toml b/impl/Cargo.toml index 7e69500d..cdce885d 100644 --- a/impl/Cargo.toml +++ b/impl/Cargo.toml @@ -2,7 +2,7 @@ name = "derive_more-impl" version = "1.0.0-beta.6" edition = "2021" -rust-version = "1.70.0" +rust-version = "1.65.0" description = "Internal implementation of `derive_more` crate" authors = ["Jelte Fennema "] license = "MIT" diff --git a/impl/doc/display.md b/impl/doc/display.md index 6a7139e6..b83ce703 100644 --- a/impl/doc/display.md +++ b/impl/doc/display.md @@ -193,7 +193,7 @@ enum E { } #[derive(Display)] -#[display("Enum E2: {_0:?}")] +#[display("Enum E2: {:?}", _0)] enum E2 { Uint(u32), String(&'static str, &'static str), diff --git a/impl/src/fmt/display.rs b/impl/src/fmt/display.rs index 089f44dc..f6ae05f4 100644 --- a/impl/src/fmt/display.rs +++ b/impl/src/fmt/display.rs @@ -9,7 +9,7 @@ use syn::{parse_quote, spanned::Spanned as _}; use crate::utils::{attr::ParseMultiple as _, Spanning}; -use super::{parsing, trait_name_to_attribute_name, ContainerAttributes, FmtAttribute}; +use super::{trait_name_to_attribute_name, ContainerAttributes, FmtAttribute}; /// Expands a [`fmt::Display`]-like derive macro. /// @@ -80,7 +80,7 @@ fn expand_struct( (attrs, ident, trait_ident, _): ExpansionCtx<'_>, ) -> syn::Result<(Vec, TokenStream)> { let s = Expansion { - shared_format: None, + shared_attr: None, attrs, fields: &s.fields, trait_ident, @@ -111,8 +111,14 @@ 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<'_>, + (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!() + } + }*/ + let (bounds, match_arms) = e.variants.iter().try_fold( (Vec::new(), TokenStream::new()), |(mut bounds, mut arms), variant| { @@ -135,7 +141,7 @@ fn expand_enum( } let v = Expansion { - shared_format: shared_attrs.fmt.as_ref(), + shared_attr: container_attrs.fmt.as_ref(), attrs: &attrs, fields: &variant.fields, trait_ident, @@ -196,8 +202,10 @@ fn expand_union( /// [`Display::fmt()`]: fmt::Display::fmt() #[derive(Debug)] struct Expansion<'a> { - /// Format shared between all variants of an enum. - shared_format: Option<&'a FmtAttribute>, + /// [`FmtAttribute`] shared between all variants of an enum. + /// + /// [`None`] for a struct. + shared_attr: Option<&'a FmtAttribute>, /// Derive macro [`ContainerAttributes`]. attrs: &'a ContainerAttributes, @@ -226,101 +234,33 @@ impl<'a> Expansion<'a> { /// /// [`Display::fmt()`]: fmt::Display::fmt() fn generate_body(&self) -> syn::Result { - if self.shared_format.is_none() { - return self.generate_body_impl(); - } - let shared_format = self.shared_format.as_ref().unwrap(); - if !shared_format.args.is_empty() { - return Err(syn::Error::new( - shared_format.args.span(), - "shared format string does not support positional placeholders, use named \ - placeholders instead", - )); - } - let mut tokens = TokenStream::new(); - let mut maybe_body = None; - let mut current_format = String::new(); - let fmt_string = shared_format.lit.value(); - let maybe_format_string = parsing::format_string(&fmt_string); - let Some(format_string) = maybe_format_string else { - // If we could not parse the format string, we just use the original string, so we get - // a nice error message. We also panic as a safety precaution in case our parsing fails - // to parse something that `write!()` allows. - return Ok(quote! { - derive_more::core::write!(__derive_more_f, #shared_format); - unreachable!( - "`derive_more` could not parse shared format string, but Rust could: {:?}", - #fmt_string, - ); - }); - }; - for part in format_string.elements { - match part { - parsing::MaybeFormat::Text(s) => { - current_format.push_str(s); - } - parsing::MaybeFormat::Format { raw, format } => { - if format.arg == Some(parsing::Argument::Identifier("_variant")) { - if format.spec.is_some() { - return Err(syn::Error::new( - shared_format.span(), - "shared format `_variant` placeholder cannot contain format \ - specifiers", - )); - } - if !current_format.is_empty() { - tokens.extend(quote! { derive_more::core::write!(__derive_more_f, #current_format)?; }); - current_format.clear(); - } - if maybe_body.is_none() { - maybe_body = Some(self.generate_body_impl()?); - } - let body = maybe_body.as_ref().unwrap(); - tokens.extend(quote! { #body?; }); + let mut body = match &self.attrs.fmt { + Some(fmt) => { + if let Some((expr, trait_ident)) = fmt.transparent_call() { + if self.shared_attr.is_some() { + let placeholder = + trait_name_to_default_placeholder_literal(&trait_ident); + + quote! { derive_more::core::format_args!(#placeholder, #expr) } } else { - if format.arg.is_none() - || matches!(format.arg, Some(parsing::Argument::Integer(_))) - { - return Err(syn::Error::new( - shared_format.span(), - "shared format string cannot contain positional placeholders, use \ - named placeholders instead", - )); + quote! { + derive_more::core::fmt::#trait_ident::fmt(&(#expr), __derive_more_f) } - current_format.push_str(raw); } - } - }; - } - if !current_format.is_empty() { - tokens.extend( - quote! { derive_more::core::write!(__derive_more_f, #current_format) }, - ) - } else { - tokens.extend(quote! { Ok(()) }); - } - Ok(tokens) - } - - /// Generates [`Display::fmt()`] implementation for a struct or an enum variant - /// without considering `shared_format`. - /// - /// [`Display::fmt()`]: fmt::Display::fmt() - fn generate_body_impl(&self) -> syn::Result { - match &self.attrs.fmt { - Some(fmt) => { - Ok(if let Some((expr, trait_ident)) = fmt.transparent_call() { - quote! { derive_more::core::fmt::#trait_ident::fmt(&(#expr), __derive_more_f) } + } else if self.shared_attr.is_some() { + quote! { derive_more::core::format_args!(#fmt) } } else { quote! { derive_more::core::write!(__derive_more_f, #fmt) } - }) + } } None if self.fields.is_empty() => { let ident_str = self.ident.to_string(); - Ok(quote! { - derive_more::core::write!(__derive_more_f, #ident_str) - }) + if self.shared_attr.is_some() { + quote! { #ident_str } + } else { + quote! { derive_more::core::write!(__derive_more_f, #ident_str) } + } } None if self.fields.len() == 1 => { let field = self @@ -331,71 +271,85 @@ impl<'a> Expansion<'a> { let ident = field.ident.clone().unwrap_or_else(|| format_ident!("_0")); let trait_ident = self.trait_ident; - Ok(quote! { - derive_more::core::fmt::#trait_ident::fmt(#ident, __derive_more_f) - }) + if self.shared_attr.is_some() { + let placeholder = + trait_name_to_default_placeholder_literal(trait_ident); + + quote! { derive_more::core::format_args!(#placeholder, #ident) } + } else { + quote! { derive_more::core::fmt::#trait_ident::fmt(#ident, __derive_more_f) } + } } - _ => Err(syn::Error::new( - self.fields.span(), - format!( - "struct or enum variant with more than 1 field must have \ + _ => { + return Err(syn::Error::new( + self.fields.span(), + format!( + "struct or enum variant with more than 1 field must have \ `#[{}(\"...\", ...)]` attribute", - trait_name_to_attribute_name(self.trait_ident), - ), - )), + trait_name_to_attribute_name(self.trait_ident), + ), + )) + } + }; + + if let Some(shared_fmt) = &self.shared_attr { + let shared_body = if let Some((shared_expr, shared_trait_ident)) = + shared_fmt.transparent_call() + { + quote! { + derive_more::core::fmt::#shared_trait_ident::fmt(#shared_expr, __derive_more_f) + } + } else { + quote! { derive_more::core::write!(__derive_more_f, #shared_fmt) } + }; + + body = if shared_fmt.contains_parameter("_variant") { + quote! { match #body { _variant => #shared_body } } + } else { + shared_body + }; } + + Ok(body) } /// Generates trait bounds for a struct or an enum variant. fn generate_bounds(&self) -> Vec { - let mut bounds: Vec = - if let Some(shared_format) = self.shared_format { - let shared_bounds = shared_format - .bounded_types(self.fields) - .map(|(ty, trait_name)| { - let trait_ident = format_ident!("{trait_name}"); - - parse_quote! { #ty: derive_more::core::fmt::#trait_ident } - }) - .chain(self.attrs.bounds.0.clone()) - .collect(); - // If it doesn't contain _variant we don't need to add any other bounds - if !parsing::format_string_formats(&shared_format.lit.value()) - .into_iter() - .flatten() - .any(|f| f.arg == Some(parsing::Argument::Identifier("_variant"))) - { - return shared_bounds; - } - shared_bounds + let mut bounds = vec![]; + + if self + .shared_attr + .map_or(true, |a| a.contains_parameter("_variant")) + { + if let Some(fmt) = &self.attrs.fmt { + bounds.extend( + fmt.bounded_types(self.fields) + .map(|(ty, trait_name)| { + let trait_ident = format_ident!("{trait_name}"); + + parse_quote! { #ty: derive_more::core::fmt::#trait_ident } + }) + .chain(self.attrs.bounds.0.clone()), + ); } else { - Vec::new() + bounds.extend(self.fields.iter().next().map(|f| { + let ty = &f.ty; + let trait_ident = &self.trait_ident; + parse_quote! { #ty: derive_more::core::fmt::#trait_ident } + })) }; + } - let Some(fmt) = &self.attrs.fmt else { - bounds.extend( - self.fields - .iter() - .next() - .map(|f| { - let ty = &f.ty; - let trait_ident = &self.trait_ident; - vec![parse_quote! { #ty: derive_more::core::fmt::#trait_ident }] - }) - .unwrap_or_default(), - ); - return bounds; - }; - - bounds.extend( - fmt.bounded_types(self.fields) - .map(|(ty, trait_name)| { + if let Some(shared_fmt) = &self.shared_attr { + bounds.extend(shared_fmt.bounded_types(self.fields).map( + |(ty, trait_name)| { let trait_ident = format_ident!("{trait_name}"); parse_quote! { #ty: derive_more::core::fmt::#trait_ident } - }) - .chain(self.attrs.bounds.0.clone()), - ); + }, + )); + } + bounds } } @@ -414,3 +368,19 @@ fn normalize_trait_name(name: &str) -> &'static str { _ => unimplemented!(), } } + +/// Matches the provided [`fmt`] trait `name` to its default formatting placeholder. +fn trait_name_to_default_placeholder_literal(name: &syn::Ident) -> &'static str { + match () { + _ if name == "Binary" => "{:b}", + _ if name == "Debug" => "{:?}", + _ if name == "Display" => "{}", + _ if name == "LowerExp" => "{:e}", + _ if name == "LowerHex" => "{:x}", + _ if name == "Octal" => "{:o}", + _ if name == "Pointer" => "{:p}", + _ if name == "UpperExp" => "{:E}", + _ if name == "UpperHex" => "{:X}", + _ => unimplemented!(), + } +} diff --git a/impl/src/fmt/mod.rs b/impl/src/fmt/mod.rs index b28a773f..87e33d7f 100644 --- a/impl/src/fmt/mod.rs +++ b/impl/src/fmt/mod.rs @@ -235,6 +235,33 @@ 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 { + 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) + } + /// Errors in case legacy syntax is encountered: `fmt = "...", (arg),*`. fn check_legacy_fmt(input: ParseStream<'_>) -> syn::Result<()> { let fork = input.fork(); @@ -278,8 +305,7 @@ impl FmtAttribute { } } -/// Representation of a [named parameter][1] (`identifier '=' expression`) in -/// in a [`FmtAttribute`]. +/// Representation of a [named parameter][1] (`identifier '=' expression`) in a [`FmtAttribute`]. /// /// [1]: https://doc.rust-lang.org/stable/std/fmt/index.html#named-parameters #[derive(Debug)] @@ -372,9 +398,9 @@ impl Placeholder { /// Parses [`Placeholder`]s from the provided formatting string. fn parse_fmt_string(s: &str) -> Vec { let mut n = 0; - parsing::format_string_formats(s) + parsing::format_string(s) .into_iter() - .flatten() + .flat_map(|f| f.formats) .map(|format| { let (maybe_arg, ty) = ( format.arg, diff --git a/impl/src/fmt/parsing.rs b/impl/src/fmt/parsing.rs index 74e053fa..8a5d7527 100644 --- a/impl/src/fmt/parsing.rs +++ b/impl/src/fmt/parsing.rs @@ -9,7 +9,7 @@ use unicode_xid::UnicodeXID as XID; /// Output of the [`format_string`] parser. #[derive(Clone, Debug, Eq, PartialEq)] pub(crate) struct FormatString<'a> { - pub(crate) elements: Vec>, + pub(crate) formats: Vec>, } /// Output of the [`format`] parser. @@ -150,11 +150,7 @@ type Fill = char; type Width<'a> = Count<'a>; /// Output of the [`maybe_format`] parser. -#[derive(Debug, Clone, Copy, Eq, PartialEq)] -pub(crate) enum MaybeFormat<'a> { - Format { raw: &'a str, format: Format<'a> }, - Text(&'a str), -} +type MaybeFormat<'a> = Option>; /// Output of the [`identifier`] parser. type Identifier<'a> = &'a str; @@ -192,32 +188,23 @@ type LeftToParse<'a> = &'a str; /// parsers). /// /// [0]: std::fmt#syntax -pub(crate) fn format_string(mut input: &str) -> Option> { - let elements = iter::repeat(()) +pub(crate) fn format_string(input: &str) -> Option> { + let (mut input, _) = optional_result(text)(input); + + let formats = iter::repeat(()) .scan(&mut input, |input, _| { - let (curr, format) = alt(&mut [ - &mut maybe_format, - &mut map(text, |(i, x)| (i, MaybeFormat::Text(x))), - ])(input)?; + let (curr, format) = + alt(&mut [&mut maybe_format, &mut map(text, |(i, _)| (i, None))])( + input, + )?; **input = curr; Some(format) }) + .flatten() .collect(); - // Should consume all tokens for a successful parse. - input.is_empty().then_some(FormatString { elements }) -} -// Same as `format_string` but returns only the `Format` parts of the string. -pub(crate) fn format_string_formats(input: &str) -> Option> { - format_string(input).map(|f| { - f.elements - .into_iter() - .filter_map(|e| match e { - MaybeFormat::Format { format, .. } => Some(format), - _ => None, - }) - .collect() - }) + // Should consume all tokens for a successful parse. + input.is_empty().then_some(FormatString { formats }) } /// Parses a `maybe_format` as defined in the [grammar spec][0]. @@ -240,12 +227,9 @@ pub(crate) fn format_string_formats(input: &str) -> Option> { /// [0]: std::fmt#syntax fn maybe_format(input: &str) -> Option<(LeftToParse<'_>, MaybeFormat<'_>)> { alt(&mut [ - &mut map(str("{{"), |i| (i, MaybeFormat::Text("{{"))), - &mut map(str("}}"), |i| (i, MaybeFormat::Text("}}"))), - &mut map(format, |(i, format)| { - let raw = &input[..input.len() - i.len()]; - (i, MaybeFormat::Format { raw, format }) - }), + &mut map(str("{{"), |i| (i, None)), + &mut map(str("}}"), |i| (i, None)), + &mut map(format, |(i, format)| (i, Some(format))), ])(input) } @@ -736,569 +720,592 @@ mod tests { #[test] fn text() { - assert_eq!(format_string_formats(""), Some(vec![])); - assert_eq!(format_string_formats("test"), Some(vec![]),); - assert_eq!(format_string_formats("Минск"), Some(vec![]),); - assert_eq!(format_string_formats("🦀"), Some(vec![])); + assert_eq!(format_string(""), Some(FormatString { formats: vec![] })); + assert_eq!( + format_string("test"), + Some(FormatString { formats: vec![] }), + ); + assert_eq!( + format_string("Минск"), + Some(FormatString { formats: vec![] }), + ); + assert_eq!(format_string("🦀"), Some(FormatString { formats: vec![] })); } #[test] fn argument() { assert_eq!( - format_string_formats("{}"), - Some(vec![Format { - arg: None, - spec: None, - }],), + format_string("{}"), + Some(FormatString { + formats: vec![Format { + arg: None, + spec: None, + }], + }), ); assert_eq!( - format_string_formats("{0}"), - Some(vec![Format { - arg: Some(Argument::Integer(0)), - spec: None, - }],), + format_string("{0}"), + Some(FormatString { + formats: vec![Format { + arg: Some(Argument::Integer(0)), + spec: None, + }], + }), ); assert_eq!( - format_string_formats("{par}"), - Some(vec![Format { - arg: Some(Argument::Identifier("par")), - spec: None, - }],), + format_string("{par}"), + Some(FormatString { + formats: vec![Format { + arg: Some(Argument::Identifier("par")), + spec: None, + }], + }), ); assert_eq!( - format_string_formats("{Минск}"), - Some(vec![Format { - arg: Some(Argument::Identifier("Минск")), - spec: None, - }],), + format_string("{Минск}"), + Some(FormatString { + formats: vec![Format { + arg: Some(Argument::Identifier("Минск")), + spec: None, + }], + }), ); } #[test] fn spec() { assert_eq!( - format_string_formats("{:}"), - Some(vec![Format { - arg: None, - spec: Some(FormatSpec { - align: None, - sign: None, - alternate: None, - zero_padding: None, - width: None, - precision: None, - ty: Type::Display, - }), - }],), - ); - assert_eq!( - format_string_formats("{:^}"), - Some(vec![Format { - arg: None, - spec: Some(FormatSpec { - align: Some((None, Align::Center)), - sign: None, - alternate: None, - zero_padding: None, - width: None, - precision: None, - ty: Type::Display, - }), - }],), - ); - assert_eq!( - format_string_formats("{:-<}"), - Some(vec![Format { - arg: None, - spec: Some(FormatSpec { - align: Some((Some('-'), Align::Left)), - sign: None, - alternate: None, - zero_padding: None, - width: None, - precision: None, - ty: Type::Display, - }), - }],), - ); - assert_eq!( - format_string_formats("{: <}"), - Some(vec![Format { - arg: None, - spec: Some(FormatSpec { - align: Some((Some(' '), Align::Left)), - sign: None, - alternate: None, - zero_padding: None, - width: None, - precision: None, - ty: Type::Display, - }), - }],), + format_string("{:}"), + Some(FormatString { + formats: vec![Format { + arg: None, + spec: Some(FormatSpec { + align: None, + sign: None, + alternate: None, + zero_padding: None, + width: None, + precision: None, + ty: Type::Display, + }), + }], + }), ); assert_eq!( - format_string_formats("{:^<}"), - Some(vec![Format { - arg: None, - spec: Some(FormatSpec { - align: Some((Some('^'), Align::Left)), - sign: None, - alternate: None, - zero_padding: None, - width: None, - precision: None, - ty: Type::Display, - }), - }],), + format_string("{:^}"), + Some(FormatString { + formats: vec![Format { + arg: None, + spec: Some(FormatSpec { + align: Some((None, Align::Center)), + sign: None, + alternate: None, + zero_padding: None, + width: None, + precision: None, + ty: Type::Display, + }), + }], + }), ); assert_eq!( - format_string_formats("{:+}"), - Some(vec![Format { - arg: None, - spec: Some(FormatSpec { - align: None, - sign: Some(Sign::Plus), - alternate: None, - zero_padding: None, - width: None, - precision: None, - ty: Type::Display, - }), - }],), + format_string("{:-<}"), + Some(FormatString { + formats: vec![Format { + arg: None, + spec: Some(FormatSpec { + align: Some((Some('-'), Align::Left)), + sign: None, + alternate: None, + zero_padding: None, + width: None, + precision: None, + ty: Type::Display, + }), + }], + }), ); assert_eq!( - format_string_formats("{:^<-}"), - Some(vec![Format { - arg: None, - spec: Some(FormatSpec { - align: Some((Some('^'), Align::Left)), - sign: Some(Sign::Minus), - alternate: None, - zero_padding: None, - width: None, - precision: None, - ty: Type::Display, - }), - }],), + format_string("{: <}"), + Some(FormatString { + formats: vec![Format { + arg: None, + spec: Some(FormatSpec { + align: Some((Some(' '), Align::Left)), + sign: None, + alternate: None, + zero_padding: None, + width: None, + precision: None, + ty: Type::Display, + }), + }], + }), ); assert_eq!( - format_string_formats("{:#}"), - Some(vec![Format { - arg: None, - spec: Some(FormatSpec { - align: None, - sign: None, - alternate: Some(Alternate), - zero_padding: None, - width: None, - precision: None, - ty: Type::Display, - }), - }],), + format_string("{:^<}"), + Some(FormatString { + formats: vec![Format { + arg: None, + spec: Some(FormatSpec { + align: Some((Some('^'), Align::Left)), + sign: None, + alternate: None, + zero_padding: None, + width: None, + precision: None, + ty: Type::Display, + }), + }], + }), ); assert_eq!( - format_string_formats("{:+#}"), - Some(vec![Format { - arg: None, - spec: Some(FormatSpec { - align: None, - sign: Some(Sign::Plus), - alternate: Some(Alternate), - zero_padding: None, - width: None, - precision: None, - ty: Type::Display, - }), - }],), + format_string("{:+}"), + Some(FormatString { + formats: vec![Format { + arg: None, + spec: Some(FormatSpec { + align: None, + sign: Some(Sign::Plus), + alternate: None, + zero_padding: None, + width: None, + precision: None, + ty: Type::Display, + }), + }], + }), ); assert_eq!( - format_string_formats("{:-<#}"), - Some(vec![Format { - arg: None, - spec: Some(FormatSpec { - align: Some((Some('-'), Align::Left)), - sign: None, - alternate: Some(Alternate), - zero_padding: None, - width: None, - precision: None, - ty: Type::Display, - }), - }],), + format_string("{:^<-}"), + Some(FormatString { + formats: vec![Format { + arg: None, + spec: Some(FormatSpec { + align: Some((Some('^'), Align::Left)), + sign: Some(Sign::Minus), + alternate: None, + zero_padding: None, + width: None, + precision: None, + ty: Type::Display, + }), + }], + }), ); assert_eq!( - format_string_formats("{:^<-#}"), - Some(vec![Format { - arg: None, - spec: Some(FormatSpec { - align: Some((Some('^'), Align::Left)), - sign: Some(Sign::Minus), - alternate: Some(Alternate), - zero_padding: None, - width: None, - precision: None, - ty: Type::Display, - }), - }],), + format_string("{:#}"), + Some(FormatString { + formats: vec![Format { + arg: None, + spec: Some(FormatSpec { + align: None, + sign: None, + alternate: Some(Alternate), + zero_padding: None, + width: None, + precision: None, + ty: Type::Display, + }), + }], + }), ); assert_eq!( - format_string_formats("{:0}"), - Some(vec![Format { - arg: None, - spec: Some(FormatSpec { - align: None, - sign: None, - alternate: None, - zero_padding: Some(ZeroPadding), - width: None, - precision: None, - ty: Type::Display, - }), - }],), + format_string("{:+#}"), + Some(FormatString { + formats: vec![Format { + arg: None, + spec: Some(FormatSpec { + align: None, + sign: Some(Sign::Plus), + alternate: Some(Alternate), + zero_padding: None, + width: None, + precision: None, + ty: Type::Display, + }), + }], + }), ); assert_eq!( - format_string_formats("{:#0}"), - Some(vec![Format { - arg: None, - spec: Some(FormatSpec { - align: None, - sign: None, - alternate: Some(Alternate), - zero_padding: Some(ZeroPadding), - width: None, - precision: None, - ty: Type::Display, - }), - }],), + format_string("{:-<#}"), + Some(FormatString { + formats: vec![Format { + arg: None, + spec: Some(FormatSpec { + align: Some((Some('-'), Align::Left)), + sign: None, + alternate: Some(Alternate), + zero_padding: None, + width: None, + precision: None, + ty: Type::Display, + }), + }], + }), ); assert_eq!( - format_string_formats("{:-0}"), - Some(vec![Format { - arg: None, - spec: Some(FormatSpec { - align: None, - sign: Some(Sign::Minus), - alternate: None, - zero_padding: Some(ZeroPadding), - width: None, - precision: None, - ty: Type::Display, - }), - }],), + format_string("{:^<-#}"), + Some(FormatString { + formats: vec![Format { + arg: None, + spec: Some(FormatSpec { + align: Some((Some('^'), Align::Left)), + sign: Some(Sign::Minus), + alternate: Some(Alternate), + zero_padding: None, + width: None, + precision: None, + ty: Type::Display, + }), + }], + }), ); assert_eq!( - format_string_formats("{:^<0}"), - Some(vec![Format { - arg: None, - spec: Some(FormatSpec { - align: Some((Some('^'), Align::Left)), - sign: None, - alternate: None, - zero_padding: Some(ZeroPadding), - width: None, - precision: None, - ty: Type::Display, - }), - }],), + format_string("{:0}"), + Some(FormatString { + formats: vec![Format { + arg: None, + spec: Some(FormatSpec { + align: None, + sign: None, + alternate: None, + zero_padding: Some(ZeroPadding), + width: None, + precision: None, + ty: Type::Display, + }), + }], + }), ); assert_eq!( - format_string_formats("{:^<+#0}"), - Some(vec![Format { - arg: None, - spec: Some(FormatSpec { - align: Some((Some('^'), Align::Left)), - sign: Some(Sign::Plus), - alternate: Some(Alternate), - zero_padding: Some(ZeroPadding), - width: None, - precision: None, - ty: Type::Display, - }), - }],), + format_string("{:#0}"), + Some(FormatString { + formats: vec![Format { + arg: None, + spec: Some(FormatSpec { + align: None, + sign: None, + alternate: Some(Alternate), + zero_padding: Some(ZeroPadding), + width: None, + precision: None, + ty: Type::Display, + }), + }], + }), ); assert_eq!( - format_string_formats("{:1}"), - Some(vec![Format { - arg: None, - spec: Some(FormatSpec { - align: None, - sign: None, - alternate: None, - zero_padding: None, - width: Some(Count::Integer(1)), - precision: None, - ty: Type::Display, - }), - }],), + format_string("{:-0}"), + Some(FormatString { + formats: vec![Format { + arg: None, + spec: Some(FormatSpec { + align: None, + sign: Some(Sign::Minus), + alternate: None, + zero_padding: Some(ZeroPadding), + width: None, + precision: None, + ty: Type::Display, + }), + }], + }), ); assert_eq!( - format_string_formats("{:1$}"), - Some(vec![Format { - arg: None, - spec: Some(FormatSpec { - align: None, - sign: None, - alternate: None, - zero_padding: None, - width: Some(Count::Parameter(Argument::Integer(1))), - precision: None, - ty: Type::Display, - }), - }],), + format_string("{:^<0}"), + Some(FormatString { + formats: vec![Format { + arg: None, + spec: Some(FormatSpec { + align: Some((Some('^'), Align::Left)), + sign: None, + alternate: None, + zero_padding: Some(ZeroPadding), + width: None, + precision: None, + ty: Type::Display, + }), + }], + }), ); assert_eq!( - format_string_formats("{:par$}"), - Some(vec![Format { - arg: None, - spec: Some(FormatSpec { - align: None, - sign: None, - alternate: None, - zero_padding: None, - width: Some(Count::Parameter(Argument::Identifier("par"))), - precision: None, - ty: Type::Display, - }), - }],), + format_string("{:^<+#0}"), + Some(FormatString { + formats: vec![Format { + arg: None, + spec: Some(FormatSpec { + align: Some((Some('^'), Align::Left)), + sign: Some(Sign::Plus), + alternate: Some(Alternate), + zero_padding: Some(ZeroPadding), + width: None, + precision: None, + ty: Type::Display, + }), + }], + }), ); assert_eq!( - format_string_formats("{:-^-#0Минск$}"), - Some(vec![Format { - arg: None, - spec: Some(FormatSpec { - align: Some((Some('-'), Align::Center)), - sign: Some(Sign::Minus), - alternate: Some(Alternate), - zero_padding: Some(ZeroPadding), - width: Some(Count::Parameter(Argument::Identifier("Минск"))), - precision: None, - ty: Type::Display, - }), - }],), + format_string("{:1}"), + Some(FormatString { + formats: vec![Format { + arg: None, + spec: Some(FormatSpec { + align: None, + sign: None, + alternate: None, + zero_padding: None, + width: Some(Count::Integer(1)), + precision: None, + ty: Type::Display, + }), + }], + }), ); assert_eq!( - format_string_formats("{:.*}"), - Some(vec![Format { - arg: None, - spec: Some(FormatSpec { - align: None, - sign: None, - alternate: None, - zero_padding: None, - width: None, - precision: Some(Precision::Star), - ty: Type::Display, - }), - }],), + format_string("{:1$}"), + Some(FormatString { + formats: vec![Format { + arg: None, + spec: Some(FormatSpec { + align: None, + sign: None, + alternate: None, + zero_padding: None, + width: Some(Count::Parameter(Argument::Integer(1))), + precision: None, + ty: Type::Display, + }), + }], + }), ); assert_eq!( - format_string_formats("{:.0}"), - Some(vec![Format { - arg: None, - spec: Some(FormatSpec { - align: None, - sign: None, - alternate: None, - zero_padding: None, - width: None, - precision: Some(Precision::Count(Count::Integer(0))), - ty: Type::Display, - }), - }],), + format_string("{:par$}"), + Some(FormatString { + formats: vec![Format { + arg: None, + spec: Some(FormatSpec { + align: None, + sign: None, + alternate: None, + zero_padding: None, + width: Some(Count::Parameter(Argument::Identifier("par"))), + precision: None, + ty: Type::Display, + }), + }], + }), ); assert_eq!( - format_string_formats("{:.0$}"), - Some(vec![Format { - arg: None, - spec: Some(FormatSpec { - align: None, - sign: None, - alternate: None, - zero_padding: None, - width: None, - precision: Some(Precision::Count(Count::Parameter( - Argument::Integer(0), - ))), - ty: Type::Display, - }), - }],), + format_string("{:-^-#0Минск$}"), + Some(FormatString { + formats: vec![Format { + arg: None, + spec: Some(FormatSpec { + align: Some((Some('-'), Align::Center)), + sign: Some(Sign::Minus), + alternate: Some(Alternate), + zero_padding: Some(ZeroPadding), + width: Some(Count::Parameter(Argument::Identifier("Минск"))), + precision: None, + ty: Type::Display, + }), + }], + }), ); assert_eq!( - format_string_formats("{:.par$}"), - Some(vec![Format { - arg: None, - spec: Some(FormatSpec { - align: None, - sign: None, - alternate: None, - zero_padding: None, - width: None, - precision: Some(Precision::Count(Count::Parameter( - Argument::Identifier("par"), - ))), - ty: Type::Display, - }), - }],), + format_string("{:.*}"), + Some(FormatString { + formats: vec![Format { + arg: None, + spec: Some(FormatSpec { + align: None, + sign: None, + alternate: None, + zero_padding: None, + width: None, + precision: Some(Precision::Star), + ty: Type::Display, + }), + }], + }), ); assert_eq!( - format_string_formats("{: >+#2$.par$}"), - Some(vec![Format { - arg: None, - spec: Some(FormatSpec { - align: Some((Some(' '), Align::Right)), - sign: Some(Sign::Plus), - alternate: Some(Alternate), - zero_padding: None, - width: Some(Count::Parameter(Argument::Integer(2))), - precision: Some(Precision::Count(Count::Parameter( - Argument::Identifier("par"), - ))), - ty: Type::Display, - }), - }],), + format_string("{:.0}"), + Some(FormatString { + formats: vec![Format { + arg: None, + spec: Some(FormatSpec { + align: None, + sign: None, + alternate: None, + zero_padding: None, + width: None, + precision: Some(Precision::Count(Count::Integer(0))), + ty: Type::Display, + }), + }], + }), ); assert_eq!( - format_string_formats("{:x?}"), - Some(vec![Format { - arg: None, - spec: Some(FormatSpec { - align: None, - sign: None, - alternate: None, - zero_padding: None, - width: None, - precision: None, - ty: Type::LowerDebug, - }), - }],), + format_string("{:.0$}"), + Some(FormatString { + formats: vec![Format { + arg: None, + spec: Some(FormatSpec { + align: None, + sign: None, + alternate: None, + zero_padding: None, + width: None, + precision: Some(Precision::Count(Count::Parameter( + Argument::Integer(0), + ))), + ty: Type::Display, + }), + }], + }), ); assert_eq!( - format_string_formats("{:E}"), - Some(vec![Format { - arg: None, - spec: Some(FormatSpec { - align: None, - sign: None, - alternate: None, - zero_padding: None, - width: None, - precision: None, - ty: Type::UpperExp, - }), - }],), + format_string("{:.par$}"), + Some(FormatString { + formats: vec![Format { + arg: None, + spec: Some(FormatSpec { + align: None, + sign: None, + alternate: None, + zero_padding: None, + width: None, + precision: Some(Precision::Count(Count::Parameter( + Argument::Identifier("par"), + ))), + ty: Type::Display, + }), + }], + }), ); assert_eq!( - format_string_formats("{: >+#par$.par$X?}"), - Some(vec![Format { - arg: None, - spec: Some(FormatSpec { - align: Some((Some(' '), Align::Right)), - sign: Some(Sign::Plus), - alternate: Some(Alternate), - zero_padding: None, - width: Some(Count::Parameter(Argument::Identifier("par"))), - precision: Some(Precision::Count(Count::Parameter( - Argument::Identifier("par"), - ))), - ty: Type::UpperDebug, - }), - }],), + format_string("{: >+#2$.par$}"), + Some(FormatString { + formats: vec![Format { + arg: None, + spec: Some(FormatSpec { + align: Some((Some(' '), Align::Right)), + sign: Some(Sign::Plus), + alternate: Some(Alternate), + zero_padding: None, + width: Some(Count::Parameter(Argument::Integer(2))), + precision: Some(Precision::Count(Count::Parameter( + Argument::Identifier("par"), + ))), + ty: Type::Display, + }), + }], + }), ); - } - - #[test] - fn full_format() { assert_eq!( - format_string_formats("prefix{{{0:#?}postfix{par:-^par$.a$}}}"), - Some(vec![ - Format { - arg: Some(Argument::Integer(0)), + format_string("{:x?}"), + Some(FormatString { + formats: vec![Format { + arg: None, spec: Some(FormatSpec { align: None, sign: None, - alternate: Some(Alternate), + alternate: None, zero_padding: None, width: None, precision: None, - ty: Type::Debug, + ty: Type::LowerDebug, }), - }, - Format { - arg: Some(Argument::Identifier("par")), + }], + }), + ); + assert_eq!( + format_string("{:E}"), + Some(FormatString { + formats: vec![Format { + arg: None, spec: Some(FormatSpec { - align: Some((Some('-'), Align::Center)), + align: None, sign: None, alternate: None, zero_padding: None, + width: None, + precision: None, + ty: Type::UpperExp, + }), + }], + }), + ); + assert_eq!( + format_string("{: >+#par$.par$X?}"), + Some(FormatString { + formats: vec![Format { + arg: None, + spec: Some(FormatSpec { + align: Some((Some(' '), Align::Right)), + sign: Some(Sign::Plus), + alternate: Some(Alternate), + zero_padding: None, width: Some(Count::Parameter(Argument::Identifier("par"))), precision: Some(Precision::Count(Count::Parameter( - Argument::Identifier("a"), + Argument::Identifier("par"), ))), - ty: Type::Display, + ty: Type::UpperDebug, }), - }, - ],), + }], + }), ); } #[test] - fn full_parts() { + fn full() { assert_eq!( format_string("prefix{{{0:#?}postfix{par:-^par$.a$}}}"), Some(FormatString { - elements: vec![ - MaybeFormat::Text("prefix"), - MaybeFormat::Text("{{"), - MaybeFormat::Format { - raw: "{0:#?}", - format: Format { - arg: Some(Argument::Integer(0)), - spec: Some(FormatSpec { - align: None, - sign: None, - alternate: Some(Alternate), - zero_padding: None, - width: None, - precision: None, - ty: Type::Debug, - }), - } + formats: vec![ + Format { + arg: Some(Argument::Integer(0)), + spec: Some(FormatSpec { + align: None, + sign: None, + alternate: Some(Alternate), + zero_padding: None, + width: None, + precision: None, + ty: Type::Debug, + }), }, - MaybeFormat::Text("postfix"), - MaybeFormat::Format { - raw: "{par:-^par$.a$}", - format: Format { - arg: Some(Argument::Identifier("par")), - spec: Some(FormatSpec { - align: Some((Some('-'), Align::Center)), - sign: None, - alternate: None, - zero_padding: None, - width: Some(Count::Parameter(Argument::Identifier( - "par" - ))), - precision: Some(Precision::Count(Count::Parameter( - Argument::Identifier("a"), - ))), - ty: Type::Display, - }), - } + Format { + arg: Some(Argument::Identifier("par")), + spec: Some(FormatSpec { + align: Some((Some('-'), Align::Center)), + sign: None, + alternate: None, + zero_padding: None, + width: Some(Count::Parameter(Argument::Identifier("par"))), + precision: Some(Precision::Count(Count::Parameter( + Argument::Identifier("a"), + ))), + ty: Type::Display, + }), }, - MaybeFormat::Text("}}"), - ] + ], }), ); } #[test] fn error() { - assert_eq!(format_string_formats("{"), None); - assert_eq!(format_string_formats("}"), None); - assert_eq!(format_string_formats("{{}"), None); - assert_eq!(format_string_formats("{:x?"), None); - assert_eq!(format_string_formats("{:.}"), None); - assert_eq!(format_string_formats("{:q}"), None); - assert_eq!(format_string_formats("{:par}"), None); - assert_eq!(format_string_formats("{⚙️}"), None); + assert_eq!(format_string("{"), None); + assert_eq!(format_string("}"), None); + assert_eq!(format_string("{{}"), None); + assert_eq!(format_string("{:x?"), None); + assert_eq!(format_string("{:.}"), None); + assert_eq!(format_string("{:q}"), None); + assert_eq!(format_string("{:par}"), None); + assert_eq!(format_string("{⚙️}"), None); } } diff --git a/tests/compile_fail/display/shared_format_positional_placeholders.stderr b/tests/compile_fail/display/shared_format_positional_placeholders.stderr index d9f2d523..e33f7c38 100644 --- a/tests/compile_fail/display/shared_format_positional_placeholders.stderr +++ b/tests/compile_fail/display/shared_format_positional_placeholders.stderr @@ -1,17 +1,34 @@ -error: shared format string cannot contain positional placeholders, use named placeholders instead - --> tests/compile_fail/display/shared_format_positional_placeholders.rs:2:11 +error: 1 positional argument in format string, but no arguments were given + --> tests/compile_fail/display/shared_format_positional_placeholders.rs:2:18 | 2 | #[display("Stuff({})")] - | ^^^^^^^^^^^ + | ^^ -error: shared format string cannot contain positional placeholders, use named placeholders instead - --> tests/compile_fail/display/shared_format_positional_placeholders.rs:8:11 +error: invalid reference to positional argument 0 (no arguments were given) + --> tests/compile_fail/display/shared_format_positional_placeholders.rs:8:19 | 8 | #[display("Stuff({0})")] - | ^^^^^^^^^^^^ + | ^ + | + = note: positional arguments are zero-based + +error: multiple unused formatting arguments + --> tests/compile_fail/display/shared_format_positional_placeholders.rs:14:22 + | +14 | #[display("Stuff()", _0, _2)] + | --------- ^^ ^^ argument never used + | | | + | | argument never used + | multiple missing formatting specifiers -error: shared format string does not support positional placeholders, use named placeholders instead +error[E0425]: cannot find value `_0` in this scope --> tests/compile_fail/display/shared_format_positional_placeholders.rs:14:22 | 14 | #[display("Stuff()", _0, _2)] - | ^^ + | ^^ not found in this scope + +error[E0425]: cannot find value `_2` in this scope + --> tests/compile_fail/display/shared_format_positional_placeholders.rs:14:26 + | +14 | #[display("Stuff()", _0, _2)] + | ^^ not found in this scope diff --git a/tests/display.rs b/tests/display.rs index e67b1e38..bd0f45ae 100644 --- a/tests/display.rs +++ b/tests/display.rs @@ -1309,7 +1309,7 @@ mod enums { use super::*; #[derive(Display)] - #[display("{_variant} Variant: {_variant} {_variant}")] + #[display("{_variant} Variant: {_variant} {}", _variant)] enum Enum { #[display("A {_0}")] A(i32), @@ -1357,8 +1357,8 @@ mod enums { Enum::::B { field: 2 }.to_string(), "Variant", ); - assert_eq!(Enum::::C.to_string(), "Variant",); - assert_eq!(Enum::::D(NoDisplay).to_string(), "Variant",); + assert_eq!(Enum::::C.to_string(), "Variant"); + assert_eq!(Enum::::D(NoDisplay).to_string(), "Variant"); } } @@ -1376,8 +1376,8 @@ mod enums { #[test] fn assert() { assert_eq!(Enum::::A(1).to_string(), "Variant 1"); - assert_eq!(Enum::::B("abc").to_string(), "Variant abc",); - assert_eq!(Enum::::C(9).to_string(), "Variant 9",); + assert_eq!(Enum::::B("abc").to_string(), "Variant abc"); + assert_eq!(Enum::::C(9).to_string(), "Variant 9"); } } @@ -1385,7 +1385,7 @@ mod enums { use super::*; #[derive(Display)] - #[display("Variant {_variant} {_0}")] + #[display("Variant {_variant} {}", _0)] enum Enum { #[display("A")] A(i32), @@ -1398,8 +1398,8 @@ mod enums { #[test] fn assert() { assert_eq!(Enum::::A(1).to_string(), "Variant A 1"); - assert_eq!(Enum::::B("abc").to_string(), "Variant B abc",); - assert_eq!(Enum::::C(9).to_string(), "Variant C 9",); + assert_eq!(Enum::::B("abc").to_string(), "Variant B abc"); + assert_eq!(Enum::::C(9).to_string(), "Variant C 9"); } } } From 4a506c45d2218c453074d755199c58358dab8d7b Mon Sep 17 00:00:00 2001 From: tyranron Date: Wed, 24 Jul 2024 20:28:20 +0300 Subject: [PATCH 5/8] Re-impl, vol.2 --- impl/src/fmt/display.rs | 21 +++++--- impl/src/fmt/mod.rs | 112 ++++++++++++++++++++-------------------- 2 files changed, 71 insertions(+), 62 deletions(-) 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", }, ], From b30e7176693072fb022199eeef4fcde9836eab00 Mon Sep 17 00:00:00 2001 From: tyranron Date: Wed, 24 Jul 2024 21:13:33 +0300 Subject: [PATCH 6/8] Fix --- impl/src/fmt/display.rs | 112 ++++++++++++++++++++++------------------ 1 file changed, 61 insertions(+), 51 deletions(-) diff --git a/impl/src/fmt/display.rs b/impl/src/fmt/display.rs index 22f69dde..0610a145 100644 --- a/impl/src/fmt/display.rs +++ b/impl/src/fmt/display.rs @@ -243,63 +243,73 @@ impl<'a> Expansion<'a> { /// /// [`Display::fmt()`]: fmt::Display::fmt() fn generate_body(&self) -> syn::Result { - let mut body = match &self.attrs.fmt { - Some(fmt) => { - if let Some((expr, trait_ident)) = fmt.transparent_call() { + let mut body = TokenStream::new(); + + if self + .shared_attr + .map_or(true, |a| a.contains_arg("_variant")) + { + body = match &self.attrs.fmt { + Some(fmt) => { + if let Some((expr, trait_ident)) = fmt.transparent_call() { + if self.shared_attr.is_some() { + let placeholder = + trait_name_to_default_placeholder_literal(&trait_ident); + + quote! { derive_more::core::format_args!(#placeholder, #expr) } + } else { + quote! { + derive_more::core::fmt::#trait_ident::fmt(&(#expr), __derive_more_f) + } + } + } else if self.shared_attr.is_some() { + quote! { derive_more::core::format_args!(#fmt) } + } else { + quote! { derive_more::core::write!(__derive_more_f, #fmt) } + } + } + None if self.fields.is_empty() => { + let ident_str = self.ident.to_string(); + + if self.shared_attr.is_some() { + quote! { #ident_str } + } else { + quote! { derive_more::core::write!(__derive_more_f, #ident_str) } + } + } + None if self.fields.len() == 1 => { + let field = self + .fields + .iter() + .next() + .unwrap_or_else(|| unreachable!("count() == 1")); + let ident = + field.ident.clone().unwrap_or_else(|| format_ident!("_0")); + let trait_ident = self.trait_ident; + if self.shared_attr.is_some() { let placeholder = - trait_name_to_default_placeholder_literal(&trait_ident); + trait_name_to_default_placeholder_literal(trait_ident); - quote! { derive_more::core::format_args!(#placeholder, #expr) } + quote! { derive_more::core::format_args!(#placeholder, #ident) } } else { quote! { - derive_more::core::fmt::#trait_ident::fmt(&(#expr), __derive_more_f) + derive_more::core::fmt::#trait_ident::fmt(#ident, __derive_more_f) } } - } else if self.shared_attr.is_some() { - quote! { derive_more::core::format_args!(#fmt) } - } else { - quote! { derive_more::core::write!(__derive_more_f, #fmt) } - } - } - None if self.fields.is_empty() => { - let ident_str = self.ident.to_string(); - - if self.shared_attr.is_some() { - quote! { #ident_str } - } else { - quote! { derive_more::core::write!(__derive_more_f, #ident_str) } - } - } - None if self.fields.len() == 1 => { - let field = self - .fields - .iter() - .next() - .unwrap_or_else(|| unreachable!("count() == 1")); - let ident = field.ident.clone().unwrap_or_else(|| format_ident!("_0")); - let trait_ident = self.trait_ident; - - if self.shared_attr.is_some() { - let placeholder = - trait_name_to_default_placeholder_literal(trait_ident); - - quote! { derive_more::core::format_args!(#placeholder, #ident) } - } else { - quote! { derive_more::core::fmt::#trait_ident::fmt(#ident, __derive_more_f) } } - } - _ => { - return Err(syn::Error::new( - self.fields.span(), - format!( - "struct or enum variant with more than 1 field must have \ + _ => { + return Err(syn::Error::new( + self.fields.span(), + format!( + "struct or enum variant with more than 1 field must have \ `#[{}(\"...\", ...)]` attribute", - trait_name_to_attribute_name(self.trait_ident), - ), - )) - } - }; + trait_name_to_attribute_name(self.trait_ident), + ), + )) + } + }; + } if let Some(shared_fmt) = &self.shared_attr { let shared_body = if let Some((shared_expr, shared_trait_ident)) = @@ -312,10 +322,10 @@ impl<'a> Expansion<'a> { quote! { derive_more::core::write!(__derive_more_f, #shared_fmt) } }; - body = if shared_fmt.contains_arg("_variant") { - quote! { match #body { _variant => #shared_body } } - } else { + body = if body.is_empty() { shared_body + } else { + quote! { match #body { _variant => #shared_body } } }; } From ba743eabfe102f33de85c8f341ddf51cf52b2064 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Sat, 20 Jul 2024 13:09:09 +0200 Subject: [PATCH 7/8] 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 | 159 ++++++++++++++++++++++++++++++++--- tests/display.rs | 181 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 327 insertions(+), 13 deletions(-) diff --git a/impl/src/fmt/display.rs b/impl/src/fmt/display.rs index 0610a145..cb664cdd 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_attr: None, attrs, fields: &s.fields, + type_params, trait_ident, ident, }; @@ -111,7 +124,7 @@ fn expand_struct( /// Expands a [`fmt`]-like derive macro for the provided enum. fn expand_enum( e: &syn::DataEnum, - (container_attrs, _, trait_ident, attr_name): ExpansionCtx<'_>, + (container_attrs, type_params, _, trait_ident, attr_name): ExpansionCtx<'_>, ) -> syn::Result<(Vec, TokenStream)> { if let Some(shared_fmt) = &container_attrs.fmt { if shared_fmt @@ -153,6 +166,7 @@ fn expand_enum( shared_attr: container_attrs.fmt.as_ref(), attrs: &attrs, fields: &variant.fields, + type_params, trait_ident, ident, }; @@ -190,7 +204,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( @@ -227,6 +241,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 @@ -343,34 +360,150 @@ impl<'a> Expansion<'a> { if let Some(fmt) = &self.attrs.fmt { 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()), ); } else { - bounds.extend(self.fields.iter().next().map(|f| { - let ty = &f.ty; - let trait_ident = &self.trait_ident; - parse_quote! { #ty: derive_more::core::fmt::#trait_ident } - })) + bounds.extend( + self.fields + .iter() + .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 }] + }) + .unwrap_or_default(), + ); }; } if let Some(shared_fmt) = &self.shared_attr { - bounds.extend(shared_fmt.bounded_types(self.fields).map( + bounds.extend(shared_fmt.bounded_types(self.fields).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 }) }, )); } 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 bd0f45ae..493206e5 100644 --- a/tests/display.rs +++ b/tests/display.rs @@ -2279,3 +2279,184 @@ 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}; + #[cfg(feature = "std")] + pub use std::{boxed::Box, format, vec::Vec}; + } + + use our_alloc::{format, Box}; + + // We want Vec in scope to test that code generation works if it is + #[allow(unused_imports)] + use our_alloc::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 } })", + ) + } +} From 378b6fca9a1bffccde6510d2c64cfe70aa4c1e9b Mon Sep 17 00:00:00 2001 From: tyranron Date: Thu, 25 Jul 2024 16:41:58 +0300 Subject: [PATCH 8/8] Extract similar code to `fmt` module --- impl/src/fmt/debug.rs | 114 +++------------------------------ impl/src/fmt/display.rs | 136 +++++----------------------------------- impl/src/fmt/mod.rs | 115 +++++++++++++++++++++++++++++++++ tests/display.rs | 12 ++-- 4 files changed, 145 insertions(+), 232 deletions(-) diff --git a/impl/src/fmt/debug.rs b/impl/src/fmt/debug.rs index 524f0218..212860d3 100644 --- a/impl/src/fmt/debug.rs +++ b/impl/src/fmt/debug.rs @@ -11,7 +11,10 @@ use crate::utils::{ Either, Spanning, }; -use super::{trait_name_to_attribute_name, ContainerAttributes, FmtAttribute}; +use super::{ + trait_name_to_attribute_name, ContainerAttributes, ContainsGenericsExt as _, + FmtAttribute, +}; /// Expands a [`fmt::Debug`] derive macro. /// @@ -24,7 +27,7 @@ pub fn expand(input: &syn::DeriveInput, _: &str) -> syn::Result { .unwrap_or_default(); let ident = &input.ident; - let type_params: Vec<_> = input + let type_params = input .generics .params .iter() @@ -32,7 +35,7 @@ pub fn expand(input: &syn::DeriveInput, _: &str) -> syn::Result { syn::GenericParam::Type(t) => Some(&t.ident), syn::GenericParam::Const(..) | syn::GenericParam::Lifetime(..) => None, }) - .collect(); + .collect::>(); let (bounds, body) = match &input.data { syn::Data::Struct(s) => { @@ -355,7 +358,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_generic_param(ty) { + if !ty.contains_generics(self.type_params) { return None; } @@ -369,7 +372,7 @@ impl<'a> Expansion<'a> { self.fields.iter().try_fold(out, |mut out, field| { let ty = &field.ty; - if !self.contains_generic_param(ty) { + if !ty.contains_generics(self.type_params) { return Ok(out); } @@ -392,105 +395,4 @@ 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() - .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", - ), - } - } } diff --git a/impl/src/fmt/display.rs b/impl/src/fmt/display.rs index 3bd58dff..429e78b7 100644 --- a/impl/src/fmt/display.rs +++ b/impl/src/fmt/display.rs @@ -9,7 +9,10 @@ use syn::{ext::IdentExt as _, parse_quote, spanned::Spanned as _}; use crate::utils::{attr::ParseMultiple as _, Spanning}; -use super::{trait_name_to_attribute_name, ContainerAttributes, FmtAttribute}; +use super::{ + trait_name_to_attribute_name, ContainerAttributes, ContainsGenericsExt as _, + FmtAttribute, +}; /// Expands a [`fmt::Display`]-like derive macro. /// @@ -32,7 +35,7 @@ pub fn expand(input: &syn::DeriveInput, trait_name: &str) -> syn::Result = input + let type_params = input .generics .params .iter() @@ -40,7 +43,7 @@ pub fn expand(input: &syn::DeriveInput, trait_name: &str) -> syn::Result Some(&t.ident), syn::GenericParam::Const(..) | syn::GenericParam::Lifetime(..) => None, }) - .collect(); + .collect::>(); let ctx: ExpansionCtx = (&attrs, &type_params, ident, &trait_ident, &attr_name); let (bounds, body) = match &input.data { @@ -356,7 +359,7 @@ impl<'a> Expansion<'a> { bounds.extend( fmt.bounded_types(self.fields) .filter_map(|(ty, trait_name)| { - if !self.contains_generic_param(ty) { + if !ty.contains_generics(self.type_params) { return None; } let trait_ident = format_ident!("{trait_name}"); @@ -366,27 +369,21 @@ impl<'a> Expansion<'a> { .chain(self.attrs.bounds.0.clone()), ); } else { - bounds.extend( - self.fields - .iter() - .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 }] - }) - .unwrap_or_default(), - ); + bounds.extend(self.fields.iter().next().and_then(|f| { + let ty = &f.ty; + if !ty.contains_generics(self.type_params) { + return None; + } + let trait_ident = &self.trait_ident; + Some(parse_quote! { #ty: derive_more::core::fmt::#trait_ident }) + })); }; } if let Some(shared_fmt) = &self.shared_attr { bounds.extend(shared_fmt.bounded_types(self.fields).filter_map( |(ty, trait_name)| { - if !self.contains_generic_param(ty) { + if !ty.contains_generics(self.type_params) { return None; } let trait_ident = format_ident!("{trait_name}"); @@ -398,107 +395,6 @@ impl<'a> Expansion<'a> { 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/impl/src/fmt/mod.rs b/impl/src/fmt/mod.rs index bd559c35..62b57f09 100644 --- a/impl/src/fmt/mod.rs +++ b/impl/src/fmt/mod.rs @@ -523,6 +523,121 @@ where } } +trait ContainsGenericsExt { + /// Checks whether this definition contains any of the provided `type_params`. + fn contains_generics(&self, type_params: &[&syn::Ident]) -> bool; +} + +impl ContainsGenericsExt for syn::Type { + fn contains_generics(&self, type_params: &[&syn::Ident]) -> bool { + if type_params.is_empty() { + return false; + } + match self { + Self::Path(syn::TypePath { qself, path }) => { + if let Some(qself) = qself { + if qself.ty.contains_generics(type_params) { + return true; + } + } + + if let Some(ident) = path.get_ident() { + type_params.iter().any(|param| *param == ident) + } else { + path.contains_generics(type_params) + } + } + + Self::Array(syn::TypeArray { elem, .. }) + | Self::Group(syn::TypeGroup { elem, .. }) + | Self::Paren(syn::TypeParen { elem, .. }) + | Self::Ptr(syn::TypePtr { elem, .. }) + | Self::Reference(syn::TypeReference { elem, .. }) + | Self::Slice(syn::TypeSlice { elem, .. }) => { + elem.contains_generics(type_params) + } + + Self::BareFn(syn::TypeBareFn { inputs, output, .. }) => { + inputs + .iter() + .any(|arg| arg.ty.contains_generics(type_params)) + || match output { + syn::ReturnType::Default => false, + syn::ReturnType::Type(_, ty) => { + ty.contains_generics(type_params) + } + } + } + + Self::Tuple(syn::TypeTuple { elems, .. }) => { + elems.iter().any(|ty| ty.contains_generics(type_params)) + } + + Self::TraitObject(syn::TypeTraitObject { bounds, .. }) => { + bounds.iter().any(|bound| match bound { + syn::TypeParamBound::Trait(syn::TraitBound { path, .. }) => { + path.contains_generics(type_params) + } + syn::TypeParamBound::Lifetime(..) + | syn::TypeParamBound::Verbatim(..) => false, + _ => unimplemented!( + "syntax is not supported by `derive_more`, please report a bug", + ), + }) + } + + Self::ImplTrait(..) + | Self::Infer(..) + | Self::Macro(..) + | Self::Never(..) + | Self::Verbatim(..) => false, + _ => unimplemented!( + "syntax is not supported by `derive_more`, please report a bug", + ), + } + } +} + +impl ContainsGenericsExt for syn::Path { + fn contains_generics(&self, type_params: &[&syn::Ident]) -> bool { + if type_params.is_empty() { + return false; + } + self.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, .. }) => { + ty.contains_generics(type_params) + } + + 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| ty.contains_generics(type_params)) + || match output { + syn::ReturnType::Default => false, + syn::ReturnType::Type(_, ty) => { + ty.contains_generics(type_params) + } + } + } + }) + } +} + #[cfg(test)] mod fmt_attribute_spec { use itertools::Itertools as _; diff --git a/tests/display.rs b/tests/display.rs index fc1f9e82..60de8182 100644 --- a/tests/display.rs +++ b/tests/display.rs @@ -2452,9 +2452,9 @@ mod type_variables { ItemStruct { next: OptionalBox { inner: Some(Box::new(ItemStruct { - next: OptionalBox { inner: None } - })) - } + next: OptionalBox { inner: None }, + })), + }, }, ), "Some(ItemStruct { next: OptionalBox { inner: None } })", @@ -2464,7 +2464,7 @@ mod type_variables { format!( "{}", ItemTuple(OptionalBox { - inner: Some(Box::new(ItemTuple(OptionalBox { inner: None }))) + inner: Some(Box::new(ItemTuple(OptionalBox { inner: None }))), }), ), "Some(ItemTuple(OptionalBox { inner: None }))", @@ -2475,8 +2475,8 @@ mod type_variables { "{}", ItemTupleContainerFmt(OptionalBox { inner: Some(Box::new(ItemTupleContainerFmt(OptionalBox { - inner: None - }))) + inner: None, + }))), }), ), "Item(Some(ItemTupleContainerFmt(OptionalBox { inner: None })))",