From 9da62cd05d777f8bd962f1fe94a75c47b11ee07e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kat=20March=C3=A1n?= <kzm@zkat.tech>
Date: Sun, 17 Apr 2022 22:35:14 -0700
Subject: [PATCH] feat(label): use macro magic instead of optional flag for
 optional labels

---
 miette-derive/src/label.rs | 158 +++++++++++++------------------------
 src/lib.rs                 |   8 +-
 src/macro_helpers.rs       |  10 ++-
 tests/derive.rs            |  16 ++--
 4 files changed, 73 insertions(+), 119 deletions(-)

diff --git a/miette-derive/src/label.rs b/miette-derive/src/label.rs
index 1d45d7b..c63f589 100644
--- a/miette-derive/src/label.rs
+++ b/miette-derive/src/label.rs
@@ -18,35 +18,21 @@ pub struct Labels(Vec<Label>);
 
 struct Label {
     label: Option<Display>,
-    optional: bool,
+    ty: syn::Type,
     span: syn::Member,
 }
 
 struct LabelAttr {
     label: Option<Display>,
-    optional: bool,
 }
 
 impl Parse for LabelAttr {
     fn parse(input: ParseStream) -> syn::Result<Self> {
         let la = input.lookahead1();
-        let (label, optional) = if la.peek(syn::token::Paren) {
+        let label = if la.peek(syn::token::Paren) {
             // #[label("{}", x)]
             let content;
             parenthesized!(content in input);
-            let optional = if content.peek(syn::Ident) {
-                let ident = content.parse::<syn::Ident>()?;
-                if ident == "optional" {
-                    if content.peek(syn::Token![,]) {
-                        content.parse::<syn::Token![,]>()?;
-                    }
-                    true
-                } else {
-                    false
-                }
-            } else {
-                false
-            };
             if content.peek(syn::LitStr) {
                 let fmt = content.parse()?;
                 let args = if content.is_empty() {
@@ -59,27 +45,22 @@ impl Parse for LabelAttr {
                     args,
                     has_bonus_display: false,
                 };
-                (Some(display), optional)
-            } else if optional {
-                (None, optional)
+                Some(display)
             } else {
-                return Err(syn::Error::new(input.span(), "Invalid argument to label() attribute. The first argument must be a literal string or the identifier `optional`"));
+                return Err(syn::Error::new(input.span(), "Invalid argument to label() attribute. The first argument must be a literal string."));
             }
         } else if la.peek(Token![=]) {
             // #[label = "blabla"]
             input.parse::<Token![=]>()?;
-            (
-                Some(Display {
-                    fmt: input.parse()?,
-                    args: TokenStream::new(),
-                    has_bonus_display: false,
-                }),
-                false,
-            )
+            Some(Display {
+                fmt: input.parse()?,
+                args: TokenStream::new(),
+                has_bonus_display: false,
+            })
         } else {
-            (None, false)
+            None
         };
-        Ok(LabelAttr { label, optional })
+        Ok(LabelAttr { label })
     }
 }
 
@@ -107,12 +88,11 @@ impl Labels {
                             span: field.span(),
                         })
                     };
-                    let LabelAttr { label, optional } =
-                        syn::parse2::<LabelAttr>(attr.tokens.clone())?;
+                    let LabelAttr { label } = syn::parse2::<LabelAttr>(attr.tokens.clone())?;
                     labels.push(Label {
                         label,
                         span,
-                        optional,
+                        ty: field.ty.clone(),
                     });
                 }
             }
@@ -127,43 +107,23 @@ impl Labels {
     pub(crate) fn gen_struct(&self, fields: &syn::Fields) -> Option<TokenStream> {
         let (display_pat, display_members) = display_pat_members(fields);
         let labels = self.0.iter().map(|highlight| {
-            let Label {
-                span,
-                label,
-                optional,
-            } = highlight;
+            let Label { span, label, ty } = highlight;
             let var = quote! { __miette_internal_var };
             if let Some(display) = label {
                 let (fmt, args) = display.expand_shorthand_cloned(&display_members);
-                if *optional {
-                    quote! {
-                        self.#span.clone().map(|#var|
-                            miette::LabeledSpan::new_with_span(
-                                std::option::Option::Some(format!(#fmt #args)),
-                                #var,
-                        ))
-                    }
-                } else {
-                    quote! {
-                        Some(miette::LabeledSpan::new_with_span(
-                            std::option::Option::Some(format!(#fmt #args)),
-                            self.#span.clone(),
-                        ))
-                    }
-                }
-            } else if *optional {
                 quote! {
-                    self.#span.clone().map(|#var|
-                        miette::LabeledSpan::new_with_span(
-                            std::option::Option::None,
-                            #var,
+                    miette::macro_helpers::OptionalWrapper::<#ty>::new().to_option(&self.#span)
+                    .map(|#var| miette::LabeledSpan::new_with_span(
+                        std::option::Option::Some(format!(#fmt #args)),
+                        #var.clone(),
                     ))
                 }
             } else {
                 quote! {
-                    Some(miette::LabeledSpan::new_with_span(
+                    miette::macro_helpers::OptionalWrapper::<#ty>::new().to_option(&self.#span)
+                    .map(|#var| miette::LabeledSpan::new_with_span(
                         std::option::Option::None,
-                        self.#span.clone(),
+                        #var.clone(),
                     ))
                 }
             }
@@ -171,6 +131,7 @@ impl Labels {
         Some(quote! {
             #[allow(unused_variables)]
             fn labels(&self) -> std::option::Option<std::boxed::Box<dyn std::iter::Iterator<Item = miette::LabeledSpan> + '_>> {
+                use miette::macro_helpers::ToOption;
                 let Self #display_pat = self;
                 std::option::Option::Some(Box::new(vec![
                     #(#labels),*
@@ -186,59 +147,46 @@ impl Labels {
             |ident, fields, DiagnosticConcreteArgs { labels, .. }| {
                 let (display_pat, display_members) = display_pat_members(fields);
                 labels.as_ref().and_then(|labels| {
-                let variant_labels = labels.0.iter().map(|label| {
-                    let Label { span, label, optional } = label;
-                    let field = match &span {
-                        syn::Member::Named(ident) => ident.clone(),
-                        syn::Member::Unnamed(syn::Index { index, .. }) => {
-                            format_ident!("_{}", index)
-                        }
-                    };
-                    let var = quote! { __miette_internal_var };
-                    if let Some(display) = label {
-                        let (fmt, args) = display.expand_shorthand_cloned(&display_members);
-                        if *optional {
+                    let variant_labels = labels.0.iter().map(|label| {
+                        let Label { span, label, ty } = label;
+                        let field = match &span {
+                            syn::Member::Named(ident) => ident.clone(),
+                            syn::Member::Unnamed(syn::Index { index, .. }) => {
+                                format_ident!("_{}", index)
+                            }
+                        };
+                        let var = quote! { __miette_internal_var };
+                        if let Some(display) = label {
+                            let (fmt, args) = display.expand_shorthand_cloned(&display_members);
                             quote! {
-                                #field.clone().map(|#var|
-                                    miette::LabeledSpan::new_with_span(
-                                        std::option::Option::Some(format!(#fmt #args)),
-                                        #var,
+                                miette::macro_helpers::OptionalWrapper::<#ty>::new().to_option(#field)
+                                .map(|#var| miette::LabeledSpan::new_with_span(
+                                    std::option::Option::Some(format!(#fmt #args)),
+                                    #var.clone(),
                                 ))
                             }
                         } else {
                             quote! {
-                                Some(miette::LabeledSpan::new_with_span(
-                                    std::option::Option::Some(format!(#fmt #args)),
-                                    #field.clone(),
+                                miette::macro_helpers::OptionalWrapper::<#ty>::new().to_option(#field)
+                                .map(|#var| miette::LabeledSpan::new_with_span(
+                                    std::option::Option::None,
+                                    #var.clone(),
                                 ))
                             }
                         }
-                    } else if *optional {
-                        quote! {
-                            #field.clone().map(|#var|
-                                miette::LabeledSpan::new_with_span(
-                                    std::option::Option::None,
-                                    #var,
-                            ))
-                        }
-                    } else {
-                        quote! {
-                            Some(miette::LabeledSpan::new_with_span(
-                                std::option::Option::None,
-                                #field.clone(),
-                            ))
-                        }
+                    });
+                    let variant_name = ident.clone();
+                    match &fields {
+                        syn::Fields::Unit => None,
+                        _ => Some(quote! {
+                            Self::#variant_name #display_pat => {
+                                use miette::macro_helpers::ToOption;
+                                std::option::Option::Some(std::boxed::Box::new(vec![
+                                    #(#variant_labels),*
+                                ].into_iter().filter(Option::is_some).map(Option::unwrap)))
+                            }
+                        }),
                     }
-                });
-                let variant_name = ident.clone();
-                match &fields {
-                    syn::Fields::Unit => None,
-                    _ => Some(quote! {
-                        Self::#variant_name #display_pat => std::option::Option::Some(std::boxed::Box::new(vec![
-                            #(#variant_labels),*
-                        ].into_iter().filter(Option::is_some).map(Option::unwrap))),
-                    }),
-                }
                 })
             },
         )
diff --git a/src/lib.rs b/src/lib.rs
index 95d9d42..d76f805 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -369,12 +369,12 @@
 //!     #[label("This is bad")]
 //!     snip2: (usize, usize), // `(usize, usize)` is `Into<SourceSpan>`!
 //!
-//!     // Snippets can be optional, by tagging them as such:
-//!     #[label(optional, "some text")]
+//!     // Snippets can be optional, by using Option:
+//!     #[label("some text")]
 //!     snip3: Option<SourceSpan>,
 //!
 //!     // with or without label text
-//!     #[label(optional)]
+//!     #[label]
 //!     snip4: Option<SourceSpan>,
 //! }
 //! ```
@@ -407,7 +407,7 @@
 //! #[diagnostic()]
 //! struct Foo {
 //!     #[help]
-//!     advice: Option<String>,
+//!     advice: Option<String>, // Can also just be `String`
 //! }
 //!
 //! let err = Foo {
diff --git a/src/macro_helpers.rs b/src/macro_helpers.rs
index 39a3960..5520899 100644
--- a/src/macro_helpers.rs
+++ b/src/macro_helpers.rs
@@ -1,6 +1,9 @@
+// Huge thanks to @jam1gamer for this hack:
+// https://twitter.com/jam1garner/status/1515887996444323840
+
 #[doc(hidden)]
 pub trait IsOption {}
-impl <T> IsOption for Option<T> {}
+impl<T> IsOption for Option<T> {}
 
 #[doc(hidden)]
 #[derive(Debug, Default)]
@@ -18,7 +21,10 @@ pub trait ToOption {
     fn to_option<T>(self, value: T) -> Option<T>;
 }
 
-impl<T> OptionalWrapper<T> where T: IsOption {
+impl<T> OptionalWrapper<T>
+where
+    T: IsOption,
+{
     #[doc(hidden)]
     pub fn to_option(self, value: &T) -> &T {
         value
diff --git a/tests/derive.rs b/tests/derive.rs
index 6394abd..e0ee980 100644
--- a/tests/derive.rs
+++ b/tests/derive.rs
@@ -319,9 +319,9 @@ fn test_snippet_named_struct() {
         var2: (usize, usize),
         #[label]
         var3: (usize, usize),
-        #[label(optional, "var 4")]
+        #[label("var 4")]
         var4: Option<(usize, usize)>,
-        #[label(optional)]
+        #[label]
         var5: Option<(usize, usize)>,
     }
 }
@@ -336,8 +336,8 @@ fn test_snippet_unnamed_struct() {
         #[label("{0}")] SourceSpan,
         #[label = "idk"] SourceSpan,
         #[label] SourceSpan,
-        #[label(optional, "foo")] Option<SourceSpan>,
-        #[label(optional)] Option<SourceSpan>,
+        #[label("foo")] Option<SourceSpan>,
+        #[label] Option<SourceSpan>,
     );
 }
 
@@ -358,9 +358,9 @@ fn test_snippet_enum() {
             var1: SourceSpan,
             #[label]
             var2: SourceSpan,
-            #[label(optional, "var 3")]
+            #[label("var 3")]
             var3: Option<(usize, usize)>,
-            #[label(optional)]
+            #[label]
             var4: Option<(usize, usize)>,
         },
         #[diagnostic(code(foo::b))]
@@ -370,8 +370,8 @@ fn test_snippet_enum() {
             #[label("{1}")] SourceSpan,
             #[label = "blorp"] SourceSpan,
             #[label] SourceSpan,
-            #[label(optional, "foo")] Option<SourceSpan>,
-            #[label(optional)] Option<SourceSpan>,
+            #[label("foo")] Option<SourceSpan>,
+            #[label] Option<SourceSpan>,
         ),
     }
 }