Repository: dtolnay/thiserror
Pull request #383 (2024-11-10 21:00:47)

Support Display and Debug of same path in error message
Previously, for an error containing a Path or PathBuf, `#[error("display:{0} debug:{0:?}")]` would work but `#[error("debug:{0:?} display:{0}")]` would not compile.

```console
error[E0277]: `PathBuf` doesn't implement `std::fmt::Display`
  --> tests/test_path.rs:36:13
   |
32 | #[derive(Error, Debug)]
   |          ----- in this derive macro expansion
...
36 |     #[error("debug:{0:?} display:{0}")]
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^ `PathBuf` cannot be formatted with the default formatter; call `.display()` on it
   |
   = help: the trait `std::fmt::Display` is not implemented for `PathBuf`
   = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
   = note: call `.display()` or `.to_string_lossy()` to safely print paths, as they may contain non-Unicode data
   = help: the trait `std::fmt::Display` is implemented for `Var<'_, T>`
   = note: this error originates in the macro `$crate::format_args` which comes from the expansion of the derive macro `Error` (in Nightly builds, run with -Z macro-backtrace for more info)
```


Patch:
diff --git a/impl/src/fmt.rs b/impl/src/fmt.rs
index e99c695..82fc68e 100644
--- a/impl/src/fmt.rs
+++ b/impl/src/fmt.rs
@@ -4,7 +4,7 @@ use crate::scan_expr::scan_expr;
 use crate::unraw::{IdentUnraw, MemberUnraw};
 use proc_macro2::{Delimiter, TokenStream, TokenTree};
 use quote::{format_ident, quote, quote_spanned};
-use std::collections::{BTreeSet, HashMap, HashSet};
+use std::collections::{BTreeSet, HashMap};
 use std::iter;
 use syn::ext::IdentExt;
 use syn::parse::discouraged::Speculative;
@@ -34,7 +34,7 @@ impl Display<'_> {
         let mut infinite_recursive = false;
         let mut implied_bounds = BTreeSet::new();
         let mut bindings = Vec::new();
-        let mut macro_named_args = HashSet::new();
+        let mut macro_named_args = BTreeSet::new();
 
         self.requires_fmt_machinery = self.requires_fmt_machinery || fmt.contains('}');
 
@@ -85,15 +85,11 @@ impl Display<'_> {
                 }
                 _ => continue,
             };
-            let binding_value = match &member {
-                MemberUnraw::Unnamed(index) => format_ident!("_{}", index),
-                MemberUnraw::Named(ident) => ident.to_local(),
-            };
-            let mut wrapped_binding_value = quote!(::thiserror::__private::Var(#binding_value));
             let end_spec = match read.find('}') {
                 Some(end_spec) => end_spec,
                 None => return Ok(()),
             };
+            let mut bonus_display = false;
             let bound = match read[..end_spec].chars().next_back() {
                 Some('?') => Trait::Debug,
                 Some('o') => Trait::Octal,
@@ -105,10 +101,7 @@ impl Display<'_> {
                 Some('E') => Trait::UpperExp,
                 Some(_) => Trait::Display,
                 None => {
-                    has_bonus_display = true;
-                    wrapped_binding_value = quote_spanned! {span=>
-                        #binding_value.as_display()
-                    };
+                    bonus_display = true;
                     Trait::Display
                 }
             };
@@ -119,19 +112,36 @@ impl Display<'_> {
                 out += &member.to_string();
                 continue;
             }
+            let formatvar_prefix = if bonus_display {
+                "__display"
+            } else {
+                "__field"
+            };
             let mut formatvar = IdentUnraw::new(match &member {
-                MemberUnraw::Unnamed(index) => format_ident!("__field{}", index),
-                MemberUnraw::Named(ident) => format_ident!("__field_{}", ident.to_string()),
+                MemberUnraw::Unnamed(index) => format_ident!("{}{}", formatvar_prefix, index),
+                MemberUnraw::Named(ident) => {
+                    format_ident!("{}_{}", formatvar_prefix, ident.to_string())
+                }
             });
             while user_named_args.contains(&formatvar) {
                 formatvar = IdentUnraw::new(format_ident!("_{}", formatvar.to_string()));
             }
             out += &formatvar.to_string();
-            if macro_named_args.insert(member) {
-                bindings.push((formatvar.to_local(), wrapped_binding_value));
-            } else {
+            if !macro_named_args.insert(formatvar.clone()) {
                 // Already added to bindings by a previous use.
+                continue;
             }
+            let binding_value = match &member {
+                MemberUnraw::Unnamed(index) => format_ident!("_{}", index),
+                MemberUnraw::Named(ident) => ident.to_local(),
+            };
+            let wrapped_binding_value = if bonus_display {
+                quote_spanned!(span=> #binding_value.as_display())
+            } else {
+                quote!(::thiserror::__private::Var(#binding_value))
+            };
+            has_bonus_display |= bonus_display;
+            bindings.push((formatvar.to_local(), wrapped_binding_value));
         }
 
         out += read;


Test patch:
diff --git a/tests/test_path.rs b/tests/test_path.rs
index 16881ae..5bb6972 100644
--- a/tests/test_path.rs
+++ b/tests/test_path.rs
@@ -29,6 +29,14 @@ pub struct UnsizedError {
     pub tail: str,
 }
 
+#[derive(Error, Debug)]
+pub enum BothError {
+    #[error("display:{0} debug:{0:?}")]
+    DisplayDebug(PathBuf),
+    #[error("debug:{0:?} display:{0}")]
+    DebugDisplay(PathBuf),
+}
+
 fn assert<T: Display>(expected: &str, value: T) {
     assert_eq!(expected, value.to_string());
 }