Repository: JelteF/derive_more
Pull request #288 (2023-08-09 14:48:45)

Fix legacy diagnostic for `Display`-like macros
Related to #225




## Synopsis

Diagnostic introduced in #225 regarding old syntax usage, displays incorrect message in case formatting arguments are specified as identifiers (not string literals):

```rust
#[derive(derive_more::Display)]
#[display(fmt = "Stuff({}): {}", _0)]
pub struct Bar(String);
```

```
error: unknown attribute, expected `bound(...)`
 --> tests/compile_fail/display/legacy_fmt_syntax.rs:8:11
  |
8 | #[display(fmt = "Stuff({}): {}", _0)]
  |           ^^^
```

In my experience, this situation happens very often in the wild.



## Solution

Fix the diagnostic to consider ident formatting arguments properly.




## Checklist

- [x] ~~Documentation is updated~~ (not required)
- [x] Tests are added/updated (if required)
- [x] ~~[CHANGELOG entry][l:1] is added~~ (not required)




[l:1]: /CHANGELOG.md



Patch:
diff --git a/impl/src/fmt/mod.rs b/impl/src/fmt/mod.rs
index 0fc7565..de4014d 100644
--- a/impl/src/fmt/mod.rs
+++ b/impl/src/fmt/mod.rs
@@ -17,7 +17,7 @@ use syn::{
     token, Ident,
 };
 
-use crate::parsing::Expr;
+use crate::{parsing::Expr, utils::Either};
 
 /// Representation of a macro attribute expressing additional trait bounds.
 #[derive(Debug, Default)]
@@ -141,16 +141,20 @@ impl FmtAttribute {
         match path {
             Ok(path) if path.is_ident("fmt") => (|| {
                 let args = fork
-                    .parse_terminated(syn::Lit::parse, token::Comma)
+                    .parse_terminated(
+                        <Either<syn::Lit, syn::Ident>>::parse,
+                        token::Comma,
+                    )
                     .ok()?
                     .into_iter()
                     .enumerate()
-                    .filter_map(|(i, lit)| match lit {
-                        syn::Lit::Str(str) => Some(if i == 0 {
+                    .filter_map(|(i, arg)| match arg {
+                        Either::Left(syn::Lit::Str(str)) => Some(if i == 0 {
                             format!("\"{}\"", str.value())
                         } else {
                             str.value()
                         }),
+                        Either::Right(ident) => Some(ident.to_string()),
                         _ => None,
                     })
                     .collect::<Vec<_>>();
@@ -171,7 +175,7 @@ impl FmtAttribute {
 }
 
 impl Parse for FmtAttribute {
-    fn parse(input: ParseStream) -> syn::Result<Self> {
+    fn parse(input: ParseStream<'_>) -> syn::Result<Self> {
         Ok(Self {
             lit: input.parse()?,
             comma: input
diff --git a/impl/src/utils.rs b/impl/src/utils.rs
index 9523591..95c3f91 100644
--- a/impl/src/utils.rs
+++ b/impl/src/utils.rs
@@ -13,8 +13,15 @@ use syn::{
     TypeParamBound, Variant, WhereClause,
 };
 
+#[cfg(any(
+    feature = "debug",
+    feature = "display",
+    feature = "from",
+    feature = "into",
+))]
+pub(crate) use self::either::Either;
 #[cfg(any(feature = "from", feature = "into"))]
-pub(crate) use self::{either::Either, fields_ext::FieldsExt};
+pub(crate) use self::fields_ext::FieldsExt;
 
 #[derive(Clone, Copy, Default)]
 pub struct DeterministicState;
@@ -1347,10 +1354,16 @@ pub fn is_type_parameter_used_in_type(
     }
 }
 
-#[cfg(any(feature = "from", feature = "into"))]
+#[cfg(any(
+    feature = "debug",
+    feature = "display",
+    feature = "from",
+    feature = "into",
+))]
 mod either {
     use proc_macro2::TokenStream;
     use quote::ToTokens;
+    use syn::parse::{Parse, ParseStream};
 
     /// Either [`Left`] or [`Right`].
     ///
@@ -1364,6 +1377,20 @@ mod either {
         Right(R),
     }
 
+    impl<L, R> Parse for Either<L, R>
+    where
+        L: Parse,
+        R: Parse,
+    {
+        fn parse(input: ParseStream<'_>) -> syn::Result<Self> {
+            if L::parse(&input.fork()).is_ok() {
+                L::parse(input).map(Self::Left)
+            } else {
+                R::parse(input).map(Self::Right)
+            }
+        }
+    }
+
     impl<L, R, T> Iterator for Either<L, R>
     where
         L: Iterator<Item = T>,


Test patch:
diff --git a/tests/compile_fail/debug/legacy_fmt_syntax.rs b/tests/compile_fail/debug/legacy_fmt_syntax.rs
index ff49445..15e7821 100644
--- a/tests/compile_fail/debug/legacy_fmt_syntax.rs
+++ b/tests/compile_fail/debug/legacy_fmt_syntax.rs
@@ -4,4 +4,8 @@ pub struct Foo {
     bar: String,
 }
 
+#[derive(derive_more::Debug)]
+#[debug(fmt = "Stuff({}): {}", _0)]
+pub struct Bar(String);
+
 fn main() {}
diff --git a/tests/compile_fail/debug/legacy_fmt_syntax.stderr b/tests/compile_fail/debug/legacy_fmt_syntax.stderr
index 30b9b1e..af04aa6 100644
--- a/tests/compile_fail/debug/legacy_fmt_syntax.stderr
+++ b/tests/compile_fail/debug/legacy_fmt_syntax.stderr
@@ -3,3 +3,9 @@ error: legacy syntax, remove `fmt =` and use `"Stuff({}): {}", bar` instead
   |
 3 |     #[debug(fmt = "Stuff({}): {}", "bar")]
   |             ^^^
+
+error: legacy syntax, remove `fmt =` and use `"Stuff({}): {}", _0` instead
+ --> tests/compile_fail/debug/legacy_fmt_syntax.rs:8:9
+  |
+8 | #[debug(fmt = "Stuff({}): {}", _0)]
+  |         ^^^
diff --git a/tests/compile_fail/display/legacy_fmt_syntax.rs b/tests/compile_fail/display/legacy_fmt_syntax.rs
index 7db9837..e55c523 100644
--- a/tests/compile_fail/display/legacy_fmt_syntax.rs
+++ b/tests/compile_fail/display/legacy_fmt_syntax.rs
@@ -4,4 +4,8 @@ pub struct Foo {
     bar: String,
 }
 
+#[derive(derive_more::Display)]
+#[display(fmt = "Stuff({}): {}", _0)]
+pub struct Bar(String);
+
 fn main() {}
diff --git a/tests/compile_fail/display/legacy_fmt_syntax.stderr b/tests/compile_fail/display/legacy_fmt_syntax.stderr
index f533747..b93c10f 100644
--- a/tests/compile_fail/display/legacy_fmt_syntax.stderr
+++ b/tests/compile_fail/display/legacy_fmt_syntax.stderr
@@ -3,3 +3,9 @@ error: legacy syntax, remove `fmt =` and use `"Stuff({}): {}", bar` instead
   |
 2 | #[display(fmt = "Stuff({}): {}", "bar")]
   |           ^^^
+
+error: legacy syntax, remove `fmt =` and use `"Stuff({}): {}", _0` instead
+ --> tests/compile_fail/display/legacy_fmt_syntax.rs:8:11
+  |
+8 | #[display(fmt = "Stuff({}): {}", _0)]
+  |           ^^^