Merge pull request #383 from dtolnay/both

Support Display and Debug of same path in error message
dtolnay · Nov 10, 2024 · a061beb · a061beb
2 parents 49be39d + 6388293
commit a061beb
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 16 deletions.
diff --git 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;

diff --git 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());
 }