Repository: JelteF/derive_more
Pull request #247 (2023-03-08 07:07:51)

Clarify legacy attribute syntax errors for `Dispaly`-like and `Debug` macros
Resolves https://github.com/JelteF/derive_more/issues/244#issuecomment-1458625976




## Synopsis

> Regarding the error message for the old syntax. It is too ambiguous in my opinion: It saids use `"{name}: {source}"` instead but the old syntax already has `"{name}: {source}"`! It took me a while to realize that I need to remove `fmt =`. The error would be better if it tells user what to remove instead of what to include.




## Solution

Change error from 

```
error: legacy syntax, use `"Stuff({}): {}", bar` instead
 --> tests/compile_fail/display/legacy_fmt_syntax.rs:2:10
  |
2 | #[display(fmt = "Stuff({}): {}", "bar")]
  |          ^
```

to

```
error: legacy syntax, remove `fmt =` and use `"Stuff({}): {}", bar` instead
 --> tests/compile_fail/display/legacy_fmt_syntax.rs:2:10
  |
2 | #[display(fmt = "Stuff({}): {}", "bar")]
  |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
```


## Checklist

- [x] Documentation is updated (if required)
- [x] Tests are added/updated (if required)
- [x] [CHANGELOG entry][l:1] is added (if required)




[l:1]: /CHANGELOG.md



Patch:
diff --git a/impl/src/fmt/debug.rs b/impl/src/fmt/debug.rs
index ecb4ed1..fbfdc1f 100644
--- a/impl/src/fmt/debug.rs
+++ b/impl/src/fmt/debug.rs
@@ -165,10 +165,12 @@ impl ContainerAttributes {
 
 impl Parse for ContainerAttributes {
     fn parse(input: ParseStream) -> Result<Self> {
-        let error_span = input.span();
+        use proc_macro2::Delimiter::Parenthesis;
 
+        let error_span = input.cursor().group(Parenthesis).map(|(_, span, _)| span);
         let content;
         syn::parenthesized!(content in input);
+        let error_span = error_span.unwrap_or_else(|| unreachable!());
 
         BoundsAttribute::check_legacy_fmt(&content, error_span)?;
 
@@ -218,10 +220,12 @@ impl FieldAttribute {
 
 impl Parse for FieldAttribute {
     fn parse(input: ParseStream) -> Result<Self> {
-        let error_span = input.span();
+        use proc_macro2::Delimiter::Parenthesis;
 
+        let error_span = input.cursor().group(Parenthesis).map(|(_, span, _)| span);
         let content;
         syn::parenthesized!(content in input);
+        let error_span = error_span.unwrap_or_else(|| unreachable!());
 
         FmtAttribute::check_legacy_fmt(&content, error_span)?;
 
diff --git a/impl/src/fmt/display.rs b/impl/src/fmt/display.rs
index d2a8d54..7df1d33 100644
--- a/impl/src/fmt/display.rs
+++ b/impl/src/fmt/display.rs
@@ -252,10 +252,12 @@ enum Attribute {
 
 impl Parse for Attribute {
     fn parse(input: ParseStream) -> Result<Self> {
-        let error_span = input.span();
+        use proc_macro2::Delimiter::Parenthesis;
 
+        let error_span = input.cursor().group(Parenthesis).map(|(_, span, _)| span);
         let content;
         syn::parenthesized!(content in input);
+        let error_span = error_span.unwrap_or_else(|| unreachable!());
 
         BoundsAttribute::check_legacy_fmt(&content, error_span)?;
         FmtAttribute::check_legacy_fmt(&content, error_span)?;
diff --git a/impl/src/fmt/mod.rs b/impl/src/fmt/mod.rs
index 4b42f86..6891362 100644
--- a/impl/src/fmt/mod.rs
+++ b/impl/src/fmt/mod.rs
@@ -158,7 +158,10 @@ impl FmtAttribute {
             .map_or(Ok(()), |fmt| {
                 Err(Error::new(
                     error_span,
-                    format!("legacy syntax, use `{}` instead", fmt.join(", ")),
+                    format!(
+                        "legacy syntax, remove `fmt =` and use `{}` instead",
+                        fmt.join(", "),
+                    ),
                 ))
             }),
             Ok(_) | Err(_) => Ok(()),


Test patch:
diff --git a/tests/compile_fail/debug/legacy_bound_syntax.stderr b/tests/compile_fail/debug/legacy_bound_syntax.stderr
index b5ff58e..a182fcf 100644
--- a/tests/compile_fail/debug/legacy_bound_syntax.stderr
+++ b/tests/compile_fail/debug/legacy_bound_syntax.stderr
@@ -2,4 +2,4 @@ error: legacy syntax, use `bound(String: std::fmt::Display)` instead
  --> tests/compile_fail/debug/legacy_bound_syntax.rs:2:8
   |
 2 | #[debug(bound = "String: std::fmt::Display")]
-  |        ^
+  |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/tests/compile_fail/debug/legacy_fmt_syntax.stderr b/tests/compile_fail/debug/legacy_fmt_syntax.stderr
index 3d182ca..57e5c78 100644
--- a/tests/compile_fail/debug/legacy_fmt_syntax.stderr
+++ b/tests/compile_fail/debug/legacy_fmt_syntax.stderr
@@ -1,5 +1,5 @@
-error: legacy syntax, use `"Stuff({}): {}", bar` instead
+error: legacy syntax, remove `fmt =` and use `"Stuff({}): {}", bar` instead
  --> tests/compile_fail/debug/legacy_fmt_syntax.rs:3:12
   |
 3 |     #[debug(fmt = "Stuff({}): {}", "bar")]
-  |            ^
+  |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/tests/compile_fail/display/legacy_bound_syntax.stderr b/tests/compile_fail/display/legacy_bound_syntax.stderr
index 6d0daef..55b724c 100644
--- a/tests/compile_fail/display/legacy_bound_syntax.stderr
+++ b/tests/compile_fail/display/legacy_bound_syntax.stderr
@@ -2,4 +2,4 @@ error: legacy syntax, use `bound(String: std::fmt::Display)` instead
  --> tests/compile_fail/display/legacy_bound_syntax.rs:2:10
   |
 2 | #[display(bound = "String: std::fmt::Display")]
-  |          ^
+  |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/tests/compile_fail/display/legacy_fmt_syntax.stderr b/tests/compile_fail/display/legacy_fmt_syntax.stderr
index 0cbb253..68e427b 100644
--- a/tests/compile_fail/display/legacy_fmt_syntax.stderr
+++ b/tests/compile_fail/display/legacy_fmt_syntax.stderr
@@ -1,5 +1,5 @@
-error: legacy syntax, use `"Stuff({}): {}", bar` instead
+error: legacy syntax, remove `fmt =` and use `"Stuff({}): {}", bar` instead
  --> tests/compile_fail/display/legacy_fmt_syntax.rs:2:10
   |
 2 | #[display(fmt = "Stuff({}): {}", "bar")]
-  |          ^
+  |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^