From d76973dd104190b108ddc784c8ca78b0bee9e6c8 Mon Sep 17 00:00:00 2001 From: Roland Fredenhagen Date: Mon, 3 Feb 2025 00:35:44 +0100 Subject: [PATCH] improve debug --- ci/test_all_features.sh | 2 +- impl/Cargo.toml | 2 +- impl/src/fmt/debug.rs | 8 +- impl/src/fmt/display.rs | 4 +- impl/src/fmt/mod.rs | 87 ++++++++++++++----- .../as_mut/renamed_generic.stderr | 3 + .../as_ref/renamed_generic.stderr | 3 + .../debug/unknown_attribute.stderr | 2 +- 8 files changed, 80 insertions(+), 31 deletions(-) diff --git a/ci/test_all_features.sh b/ci/test_all_features.sh index 0e76e299..9cf7fee7 100755 --- a/ci/test_all_features.sh +++ b/ci/test_all_features.sh @@ -7,6 +7,6 @@ fi set -euxo pipefail -for feature in $(tomljson Cargo.toml | jq --raw-output '.features | keys[]' | grep -v 'default\|std\|testing-helpers'); do +for feature in debug display; do RUSTFLAGS='-D warnings' cargo +nightly test -p derive_more --tests --no-default-features --features "$feature$std,testing-helpers" done diff --git a/impl/Cargo.toml b/impl/Cargo.toml index a1d309ab..330dd8a8 100644 --- a/impl/Cargo.toml +++ b/impl/Cargo.toml @@ -53,7 +53,7 @@ add = [] add_assign = [] as_ref = ["syn/extra-traits", "syn/visit"] constructor = [] -debug = ["syn/extra-traits", "dep:unicode-xid", "dep:convert_case"] +debug = ["syn/extra-traits", "dep:unicode-xid"] deref = [] deref_mut = [] display = ["syn/extra-traits", "dep:unicode-xid", "dep:convert_case"] diff --git a/impl/src/fmt/debug.rs b/impl/src/fmt/debug.rs index e2c8bc83..95cd0d55 100644 --- a/impl/src/fmt/debug.rs +++ b/impl/src/fmt/debug.rs @@ -2,6 +2,8 @@ //! //! [`fmt::Debug`]: std::fmt::Debug +use std::convert::Infallible; + use proc_macro2::TokenStream; use quote::{format_ident, quote}; use syn::{ext::IdentExt as _, parse_quote, spanned::Spanned as _}; @@ -77,7 +79,7 @@ pub fn expand(input: &syn::DeriveInput, _: &str) -> syn::Result { /// /// [`fmt::Debug`]: std::fmt::Debug fn expand_struct( - attrs: ContainerAttributes, + attrs: ContainerAttributes, ident: &syn::Ident, s: &syn::DataStruct, type_params: &[&syn::Ident], @@ -115,7 +117,7 @@ fn expand_struct( /// /// [`fmt::Debug`]: std::fmt::Debug fn expand_enum( - mut attrs: ContainerAttributes, + mut attrs: ContainerAttributes, e: &syn::DataEnum, type_params: &[&syn::Ident], attr_name: &syn::Ident, @@ -207,7 +209,7 @@ type FieldAttribute = Either; /// [`Debug::fmt()`]: std::fmt::Debug::fmt() #[derive(Debug)] struct Expansion<'a> { - attr: &'a ContainerAttributes, + attr: &'a ContainerAttributes, /// Struct or enum [`Ident`](struct@syn::Ident). ident: &'a syn::Ident, diff --git a/impl/src/fmt/display.rs b/impl/src/fmt/display.rs index 3a9a9d0c..d6d023fd 100644 --- a/impl/src/fmt/display.rs +++ b/impl/src/fmt/display.rs @@ -87,7 +87,7 @@ pub fn expand(input: &syn::DeriveInput, trait_name: &str) -> syn::Result = ( - &'a ContainerAttributes, + &'a ContainerAttributes, &'a [&'a syn::Ident], &'a syn::Ident, &'a syn::Ident, @@ -250,7 +250,7 @@ struct Expansion<'a> { rename_all: Option, /// Derive macro [`ContainerAttributes`]. - attrs: &'a ContainerAttributes, + attrs: &'a ContainerAttributes, /// Struct or enum [`syn::Ident`]. /// diff --git a/impl/src/fmt/mod.rs b/impl/src/fmt/mod.rs index 14e2c26f..596e2026 100644 --- a/impl/src/fmt/mod.rs +++ b/impl/src/fmt/mod.rs @@ -8,8 +8,7 @@ pub(crate) mod debug; pub(crate) mod display; mod parsing; -use std::fmt::Display; - +#[cfg(feature = "display")] use convert_case::{Case, Casing as _}; use proc_macro2::TokenStream; use quote::{format_ident, quote, ToTokens}; @@ -19,7 +18,7 @@ use syn::{ parse_quote, punctuated::Punctuated, spanned::Spanned as _, - token, LitStr, Token, + token, }; use crate::{ @@ -106,15 +105,18 @@ impl BoundsAttribute { /// - `SCREAMING_SNAKE_CASE` /// - `kebab-case` /// - `SCREAMING-KEBAB-CASE` +#[cfg(feature = "display")] #[derive(Debug, Clone, Copy)] -struct RenameAllAttribute(Case); +struct RenameAllAttribute(#[allow(unused)] Case); +#[cfg(feature = "display")] impl RenameAllAttribute { fn convert_case(&self, ident: &syn::Ident) -> String { ident.unraw().to_string().to_case(self.0) } } +#[cfg(feature = "display")] impl Parse for RenameAllAttribute { fn parse(input: ParseStream<'_>) -> syn::Result { let _ = input.parse::().and_then(|p| { @@ -128,9 +130,9 @@ impl Parse for RenameAllAttribute { } })?; - input.parse::()?; + input.parse::()?; - let value: LitStr = input.parse()?; + let value: syn::LitStr = input.parse()?; // TODO should we really do a case insensitive comparision here? Ok(Self(match value.value().replace(['-', '_'], "").to_lowercase().as_str() { @@ -568,8 +570,8 @@ impl Placeholder { /// are allowed. /// /// [`fmt::Display`]: std::fmt::Display -#[derive(Debug, Default)] -struct ContainerAttributes { +#[derive(Debug)] +struct ContainerAttributes { /// Interpolation [`FmtAttribute`]. fmt: Option, @@ -577,11 +579,25 @@ struct ContainerAttributes { bounds: BoundsAttribute, /// Rename unit enum variants following a similar behavior as [`serde`](https://serde.rs/container-attrs.html#rename_all). - rename_all: Option, + rename_all: Option, } -impl Spanning { - fn validate_for_struct(&self, attr_name: impl Display) -> syn::Result<()> { +impl std::default::Default for ContainerAttributes { + fn default() -> Self { + Self { + fmt: None, + bounds: BoundsAttribute::default(), + rename_all: None, + } + } +} + +#[cfg(feature = "display")] +impl Spanning> { + fn validate_for_struct( + &self, + attr_name: impl std::fmt::Display, + ) -> syn::Result<()> { if self.rename_all.is_some() { Err(syn::Error::new( self.span, @@ -593,21 +609,43 @@ impl Spanning { } } -mod kw { - use syn::custom_keyword; - - custom_keyword!(rename_all); - custom_keyword!(bounds); - custom_keyword!(bound); +#[cfg(feature = "debug")] +impl Parse for ContainerAttributes { + fn parse(input: ParseStream<'_>) -> syn::Result { + // We do check `FmtAttribute::check_legacy_fmt` eagerly here, because `Either` will swallow + // any error of the `Either::Left` if the `Either::Right` succeeds. + FmtAttribute::check_legacy_fmt(input)?; + >::parse(input).map(|v| match v { + Either::Left(fmt) => Self { + bounds: BoundsAttribute::default(), + fmt: Some(fmt), + rename_all: None, + }, + Either::Right(bounds) => Self { + bounds, + fmt: None, + rename_all: None, + }, + }) + } } -impl Parse for ContainerAttributes { +#[cfg(feature = "display")] +impl Parse for ContainerAttributes { fn parse(input: ParseStream<'_>) -> syn::Result { + mod kw { + use syn::custom_keyword; + + custom_keyword!(rename_all); + custom_keyword!(bounds); + custom_keyword!(bound); + } + // We do check `FmtAttribute::check_legacy_fmt` eagerly here, because `Either` will swallow // any error of the `Either::Left` if the `Either::Right` succeeds. FmtAttribute::check_legacy_fmt(input)?; let lookahead = input.lookahead1(); - Ok(if lookahead.peek(LitStr) { + Ok(if lookahead.peek(syn::LitStr) { Self { fmt: Some(input.parse()?), bounds: BoundsAttribute::default(), @@ -616,7 +654,7 @@ impl Parse for ContainerAttributes { } else if lookahead.peek(kw::rename_all) || lookahead.peek(kw::bounds) || lookahead.peek(kw::bound) - || lookahead.peek(Token![where]) + || lookahead.peek(syn::Token![where]) { let mut bounds = BoundsAttribute::default(); let mut rename_all = None; @@ -633,14 +671,14 @@ impl Parse for ContainerAttributes { } } else if lookahead.peek(kw::bounds) || lookahead.peek(kw::bound) - || lookahead.peek(Token![where]) + || lookahead.peek(syn::Token![where]) { bounds.0.extend(input.parse::()?.0) } else { return Err(lookahead.error()); } if !input.is_empty() { - input.parse::()?; + input.parse::()?; } } Self { @@ -654,7 +692,10 @@ impl Parse for ContainerAttributes { } } -impl attr::ParseMultiple for ContainerAttributes { +impl attr::ParseMultiple for ContainerAttributes +where + ContainerAttributes: Parse, +{ fn merge_attrs( prev: Spanning, new: Spanning, diff --git a/tests/compile_fail/as_mut/renamed_generic.stderr b/tests/compile_fail/as_mut/renamed_generic.stderr index 190faaed..08e2cb40 100644 --- a/tests/compile_fail/as_mut/renamed_generic.stderr +++ b/tests/compile_fail/as_mut/renamed_generic.stderr @@ -13,6 +13,9 @@ error[E0599]: the method `as_mut` exists for struct `Baz`, but its trait bo = note: trait bound `Foo: AsMut>` was not satisfied note: the trait `AsMut` must be implemented --> $RUST/core/src/convert/mod.rs + | + | pub trait AsMut { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ = help: items from traits can only be used if the trait is implemented and in scope = note: the following trait defines an item `as_mut`, perhaps you need to implement it: candidate #1: `AsMut` diff --git a/tests/compile_fail/as_ref/renamed_generic.stderr b/tests/compile_fail/as_ref/renamed_generic.stderr index 07311720..445e89af 100644 --- a/tests/compile_fail/as_ref/renamed_generic.stderr +++ b/tests/compile_fail/as_ref/renamed_generic.stderr @@ -13,6 +13,9 @@ error[E0599]: the method `as_ref` exists for struct `Baz`, but its trait bo = note: trait bound `Foo: AsRef>` was not satisfied note: the trait `AsRef` must be implemented --> $RUST/core/src/convert/mod.rs + | + | pub trait AsRef { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ = help: items from traits can only be used if the trait is implemented and in scope = note: the following trait defines an item `as_ref`, perhaps you need to implement it: candidate #1: `AsRef` diff --git a/tests/compile_fail/debug/unknown_attribute.stderr b/tests/compile_fail/debug/unknown_attribute.stderr index c634447a..1820df7b 100644 --- a/tests/compile_fail/debug/unknown_attribute.stderr +++ b/tests/compile_fail/debug/unknown_attribute.stderr @@ -1,4 +1,4 @@ -error: expected one of: string literal, `rename_all`, `bounds`, `bound`, `where` +error: unknown attribute argument, expected `bound(...)` --> tests/compile_fail/debug/unknown_attribute.rs:2:9 | 2 | #[debug(unknown = "unknown")]