Repository: zkat/miette (rust)

Issue:
Derive macro breaks on types with `'a` lifetime parameters
```rust
#[derive(Debug, Diagnostic, Error)]
#[error("foo")]
#[diagnostic(help("foo"))]
enum Foo<'a> {
    Bar {
        bar: &'a str
    }
}
```

Gives the following error at compile time

```
error[E0496]: lifetime name `'a` shadows a lifetime name that is already in scope
   --> tests/derive.rs:611:21
    |
611 |     #[derive(Debug, Diagnostic, Error)]
    |                     ^^^^^^^^^^ lifetime `'a` already in scope
...
614 |     enum Foo<'a> {
    |              -- first declared here
    |
    = note: this error originates in the derive macro `Diagnostic` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0308]: method not compatible with trait
   --> tests/derive.rs:611:21
    |
611 |     #[derive(Debug, Diagnostic, Error)]
    |                     ^^^^^^^^^^ lifetime mismatch
    |
    = note: expected fn pointer `fn(&'a enum_lifetime_parameter::Foo<'a>) -> Option<Box<(dyn std::fmt::Display + 'a)>>`
               found fn pointer `fn(&'a enum_lifetime_parameter::Foo<'a>) -> Option<Box<(dyn std::fmt::Display + 'a)>>`
note: the lifetime `'a` as defined here...
   --> tests/derive.rs:611:21
    |
611 |     #[derive(Debug, Diagnostic, Error)]
    |                     ^^^^^^^^^^
note: ...does not necessarily outlive the lifetime `'a` as defined here
   --> tests/derive.rs:614:14
    |
614 |     enum Foo<'a> {
    |              ^^
    = note: this error originates in the derive macro `Diagnostic` (in Nightly builds, run with -Z macro-backtrace for more info)
```

I'm pretty sure this is caused by the use of explicit `'a` lifetimes at several points in the derive macro generated code. Shadowing generic parameters in an impl block like this isn't allowed, so these probably need to either be elided or replaced with a gensym named lifetime parameter.

I tried a first pass at fixing this myself, but didn't get far. I was able to confirm that replacing the named lifetimes in the derive output with `'b` fixed this specific instance, so I'm on the right track for what the underlying problem is. I had trouble implementing the fully generic solution though, since I'm not very familiar with this part of the codebase.

Pull request:
Elide lifetimes in derived functions
Closes #211.

The bug was caused by user structs and enums using the same lifetime (`'a`) that was used by some of the derived functions (`help`, `url`, `code`) in their signatures. So for the following example:

```rust
use miette::Diagnostic;
use thiserror::Error;

#[derive(Debug, Diagnostic, Error)]
#[error("foo")]
#[diagnostic(help("foo"))]
pub enum Foo<'a> {
    Bar { bar: &'a str },
}
```

 you would get derived code that looks like this:

```rust
use miette::Diagnostic;
use thiserror::Error;
#[error("foo")]
#[diagnostic(help("foo"))]
pub enum Foo<'a> {
    Bar { bar: &'a str },
}

impl<'a> miette::Diagnostic for Foo<'a> {
    fn help<'a>(
        &'a self,
    ) -> std::option::Option<std::boxed::Box<dyn std::fmt::Display + 'a>> {
        #[allow(unused_variables, deprecated)]
        match self {
            Self::Bar { bar } => {
                std::option::Option::Some(
                    std::boxed::Box::new({
                        let res = ::alloc::fmt::format(
                            ::core::fmt::Arguments::new_v1(&["foo"], &[]),
                        );
                        res
                    }),
                )
            }
            _ => std::option::Option::None,
        }
    }
}
```
Which is a compile error because 'a is defined twice. However, it seems like you can just elide the explicit lifetime in the generated functions, which gives you code that looks like:

```rust
impl<'a> miette::Diagnostic for Foo<'a> {
    fn help(&self) -> std::option::Option<std::boxed::Box<dyn std::fmt::Display + '_>> {
        #[allow(unused_variables, deprecated)]
        match self {
            Self::Bar { bar } => {
                std::option::Option::Some(
                    std::boxed::Box::new({
                        let res = ::alloc::fmt::format(
                            ::core::fmt::Arguments::new_v1(&["foo"], &[]),
                        );
                        res
                    }),
                )
            }
            _ => std::option::Option::None,
        }
    }
}
```
So all appears to be well. I changed most of the derived tests to use a `'a` lifetime while trying to work out where it was breaking. It seems like this is a case that should be covered but maybe in its own specialised unit test rather than scattered through the others?

Patch:
diff --git a/miette-derive/src/code.rs b/miette-derive/src/code.rs
index 59aa436..22dc795 100644
--- a/miette-derive/src/code.rs
+++ b/miette-derive/src/code.rs
@@ -72,7 +72,7 @@ impl Code {
     pub(crate) fn gen_struct(&self) -> Option<TokenStream> {
         let code = &self.0;
         Some(quote! {
-            fn code<'a>(&'a self) -> std::option::Option<std::boxed::Box<dyn std::fmt::Display + 'a>> {
+            fn code(&self) -> std::option::Option<std::boxed::Box<dyn std::fmt::Display + '_>> {
                 std::option::Option::Some(std::boxed::Box::new(#code))
             }
         })
diff --git a/miette-derive/src/forward.rs b/miette-derive/src/forward.rs
index c8757b2..171019a 100644
--- a/miette-derive/src/forward.rs
+++ b/miette-derive/src/forward.rs
@@ -58,13 +58,13 @@ impl WhichFn {
     pub fn signature(&self) -> TokenStream {
         match self {
             Self::Code => quote! {
-                fn code<'a>(&'a self) -> std::option::Option<std::boxed::Box<dyn std::fmt::Display + 'a>>
+                fn code(& self) -> std::option::Option<std::boxed::Box<dyn std::fmt::Display + '_>>
             },
             Self::Help => quote! {
-                fn help<'a>(&'a self) -> std::option::Option<std::boxed::Box<dyn std::fmt::Display + 'a>>
+                fn help(& self) -> std::option::Option<std::boxed::Box<dyn std::fmt::Display + '_>>
             },
             Self::Url => quote! {
-                fn url<'a>(&'a self) -> std::option::Option<std::boxed::Box<dyn std::fmt::Display + 'a>>
+                fn url(& self) -> std::option::Option<std::boxed::Box<dyn std::fmt::Display + '_>>
             },
             Self::Severity => quote! {
                 fn severity(&self) -> std::option::Option<miette::Severity>
diff --git a/miette-derive/src/help.rs b/miette-derive/src/help.rs
index c5eedd1..ae2c5d8 100644
--- a/miette-derive/src/help.rs
+++ b/miette-derive/src/help.rs
@@ -108,7 +108,7 @@ impl Help {
                         Some(quote! {
                             Self::#ident #display_pat => {
                                 use miette::macro_helpers::ToOption;
-                                miette::macro_helpers::OptionalWrapper::<#ty>::new().to_option(&#help).as_ref().map(|#var| -> std::boxed::Box<dyn std::fmt::Display + 'a> { std::boxed::Box::new(format!("{}", #var)) })
+                                miette::macro_helpers::OptionalWrapper::<#ty>::new().to_option(&#help).as_ref().map(|#var| -> std::boxed::Box<dyn std::fmt::Display + '_> { std::boxed::Box::new(format!("{}", #var)) })
                             },
                         })
                     }
@@ -123,7 +123,7 @@ impl Help {
             Help::Display(display) => {
                 let (fmt, args) = display.expand_shorthand_cloned(&display_members);
                 Some(quote! {
-                    fn help<'a>(&'a self) -> std::option::Option<std::boxed::Box<dyn std::fmt::Display + 'a>> {
+                    fn help(&self) -> std::option::Option<std::boxed::Box<dyn std::fmt::Display + '_>> {
                         #[allow(unused_variables, deprecated)]
                         let Self #display_pat = self;
                         std::option::Option::Some(std::boxed::Box::new(format!(#fmt #args)))
@@ -133,11 +133,11 @@ impl Help {
             Help::Field(member, ty) => {
                 let var = quote! { __miette_internal_var };
                 Some(quote! {
-                    fn help<'a>(&'a self) -> std::option::Option<std::boxed::Box<dyn std::fmt::Display + 'a>> {
+                    fn help(&self) -> std::option::Option<std::boxed::Box<dyn std::fmt::Display + '_>> {
                         #[allow(unused_variables, deprecated)]
                         let Self #display_pat = self;
                         use miette::macro_helpers::ToOption;
-                        miette::macro_helpers::OptionalWrapper::<#ty>::new().to_option(&self.#member).as_ref().map(|#var| -> std::boxed::Box<dyn std::fmt::Display + 'a> { std::boxed::Box::new(format!("{}", #var)) })
+                        miette::macro_helpers::OptionalWrapper::<#ty>::new().to_option(&self.#member).as_ref().map(|#var| -> std::boxed::Box<dyn std::fmt::Display + '_> { std::boxed::Box::new(format!("{}", #var)) })
                     }
                 })
             }
diff --git a/miette-derive/src/url.rs b/miette-derive/src/url.rs
index d2e7adf..734d1a4 100644
--- a/miette-derive/src/url.rs
+++ b/miette-derive/src/url.rs
@@ -129,7 +129,7 @@ impl Url {
             }
         };
         Some(quote! {
-            fn url<'a>(&'a self) -> std::option::Option<std::boxed::Box<dyn std::fmt::Display + 'a>> {
+            fn url(&self) -> std::option::Option<std::boxed::Box<dyn std::fmt::Display + '_>> {
                 #[allow(unused_variables, deprecated)]
                 let Self #pat = self;
                 std::option::Option::Some(std::boxed::Box::new(format!(#fmt #args)))


Test patch:
diff --git a/tests/derive.rs b/tests/derive.rs
index e0ee980..7faae42 100644
--- a/tests/derive.rs
+++ b/tests/derive.rs
@@ -185,7 +185,7 @@ fn fmt_help() {
     #[derive(Debug, Diagnostic, Error)]
     #[error("welp")]
     #[diagnostic(code(foo::bar::baz), help("{} x {0} x {:?}", 1, "2"))]
-    struct FooStruct(String);
+    struct FooStruct<'a>(&'a str);
 
     assert_eq!(
         "1 x hello x \"2\"".to_string(),
@@ -195,8 +195,8 @@ fn fmt_help() {
     #[derive(Debug, Diagnostic, Error)]
     #[error("welp")]
     #[diagnostic(code(foo::bar::baz), help("{} x {my_field} x {:?}", 1, "2"))]
-    struct BarStruct {
-        my_field: String,
+    struct BarStruct<'a> {
+        my_field: &'a str,
     }
 
     assert_eq!(
@@ -211,9 +211,9 @@ fn fmt_help() {
 
     #[derive(Debug, Diagnostic, Error)]
     #[error("welp")]
-    enum FooEnum {
+    enum FooEnum<'a> {
         #[diagnostic(code(foo::x), help("{} x {0} x {:?}", 1, "2"))]
-        X(String),
+        X(&'a str),
 
         #[diagnostic(code(foo::x), help("{} x {len} x {:?}", 1, "2"))]
         Y { len: usize },
@@ -243,9 +243,9 @@ fn help_field() {
     #[derive(Debug, Diagnostic, Error)]
     #[error("welp")]
     #[diagnostic()]
-    struct Foo {
+    struct Foo<'a> {
         #[help]
-        do_this: Option<String>,
+        do_this: Option<&'a str>,
     }
 
     assert_eq!(
@@ -261,11 +261,11 @@ fn help_field() {
     #[derive(Debug, Diagnostic, Error)]
     #[error("welp")]
     #[diagnostic()]
-    enum Bar {
-        A(#[help] Option<String>),
+    enum Bar<'a> {
+        A(#[help] Option<&'a str>),
         B {
             #[help]
-            do_this: Option<String>,
+            do_this: Option<&'a str>,
         },
     }
 
@@ -286,7 +286,7 @@ fn help_field() {
     #[derive(Debug, Diagnostic, Error)]
     #[error("welp")]
     #[diagnostic()]
-    struct Baz(#[help] Option<String>);
+    struct Baz<'a>(#[help] Option<&'a str>);
 
     assert_eq!(
         "x".to_string(),
@@ -296,7 +296,7 @@ fn help_field() {
     #[derive(Debug, Diagnostic, Error)]
     #[error("welp")]
     #[diagnostic()]
-    struct Quux(#[help] String);
+    struct Quux<'a>(#[help] &'a str);
 
     assert_eq!(
         "x".to_string(),
@@ -309,9 +309,9 @@ fn test_snippet_named_struct() {
     #[derive(Debug, Diagnostic, Error)]
     #[error("welp")]
     #[diagnostic(code(foo::bar::baz))]
-    struct Foo {
+    struct Foo<'a> {
         #[source_code]
-        src: String,
+        src: &'a str,
         #[label("var 1")]
         var1: SourceSpan,
         #[label = "var 2"]
@@ -331,8 +331,8 @@ fn test_snippet_unnamed_struct() {
     #[derive(Debug, Diagnostic, Error)]
     #[error("welp")]
     #[diagnostic(code(foo::bar::baz))]
-    struct Foo(
-        #[source_code] String,
+    struct Foo<'a>(
+        #[source_code] &'a str,
         #[label("{0}")] SourceSpan,
         #[label = "idk"] SourceSpan,
         #[label] SourceSpan,
@@ -346,11 +346,11 @@ fn test_snippet_enum() {
     #[derive(Debug, Diagnostic, Error)]
     #[error("welp")]
     #[allow(dead_code)]
-    enum Foo {
+    enum Foo<'a> {
         #[diagnostic(code(foo::a))]
         A {
             #[source_code]
-            src: String,
+            src: &'a str,
             msg: String,
             #[label("hi this is where the thing went wrong ({msg})")]
             var0: SourceSpan,
@@ -516,9 +516,9 @@ fn test_forward_struct_named() {
         help("{help}"),
         forward(span)
     )]
-    struct Struct {
+    struct Struct<'a> {
         span: ForwardsTo,
-        help: &'static str,
+        help: &'a str,
     }
     // Also check the From impl here
     let diag = Struct {
@@ -535,7 +535,7 @@ fn test_forward_struct_unnamed() {
     #[derive(Debug, Diagnostic, Error)]
     #[error("display")]
     #[diagnostic(code(foo::bar::overridden), url("{1}"), forward(0))]
-    struct Struct(ForwardsTo, &'static str);
+    struct Struct<'a>(ForwardsTo, &'a str);
 
     // Also check the From impl here
     let diag = Struct(ForwardsTo::new(), "url here");
@@ -546,12 +546,12 @@ fn test_forward_struct_unnamed() {
 #[test]
 fn test_forward_enum_named() {
     #[derive(Debug, Diagnostic, Error)]
-    enum Enum {
+    enum Enum<'a> {
         #[error("help: {help_text}")]
         #[diagnostic(code(foo::bar::overridden), help("{help_text}"), forward(span))]
         Variant {
             span: ForwardsTo,
-            help_text: &'static str,
+            help_text: &'a str,
         },
     }
     // Also check the From impl here
@@ -569,10 +569,10 @@ fn test_forward_enum_named() {
 #[test]
 fn test_forward_enum_unnamed() {
     #[derive(Debug, Diagnostic, Error)]
-    enum ForwardEnumUnnamed {
+    enum ForwardEnumUnnamed<'a> {
         #[error("help: {1}")]
         #[diagnostic(code(foo::bar::overridden), help("{1}"), forward(0))]
-        Variant(ForwardsTo, &'static str),
+        Variant(ForwardsTo, &'a str),
     }
     // Also check the From impl here
     let variant = ForwardEnumUnnamed::Variant(ForwardsTo::new(), "overridden help please");