From 7f2496ecff86c374368c42977fdcb54716fd928c Mon Sep 17 00:00:00 2001 From: Nahor Date: Fri, 16 Feb 2024 15:53:34 -0800 Subject: [PATCH 1/4] feat(collection): add label(collection) documentation to lib.rs --- src/lib.rs | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index 7674f2be..7891511f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -48,6 +48,7 @@ //! - [... handler options](#-handler-options) //! - [... dynamic diagnostics](#-dynamic-diagnostics) //! - [... syntax highlighting](#-syntax-highlighting) +//! - [... collection of labels](#-collection-of-labels) //! - [Acknowledgements](#acknowledgements) //! - [License](#license) //! @@ -672,6 +673,57 @@ //! [`with_syntax_highlighting`](MietteHandlerOpts::with_syntax_highlighting) //! method. See the [`highlighters`] module docs for more details. //! +//! ### ... collection of labels +//! +//! When the number of labels is unknown, you can use a collection of `SourceSpan` +//! (or any type convertible into `SourceSpan`). For this, add the `collection` +//! parameter to `label` and use any type than can be iterated over for the field. +//! +//! ```rust,ignore +//! #[derive(Debug, Diagnostic, Error)] +//! #[error("oops!")] +//! struct MyError { +//! #[label("main issue")] +//! primary_span: SourceSpan, +//! +//! #[label(collection, "related to this")] +//! other_spans: Vec>, +//! } +//! +//! let report: miette::Report = MyError { +//! primary_span: (6, 9).into(), +//! other_spans: vec![19..26, 30..41], +//! }.into(); +//! +//! println!("{:?}", report.with_source_code("About something or another or yet another ...".to_string())); +//! ``` +//! +//! A collection can also be of `LabeledSpan` if you want to have different text +//! for different labels. Labels with no text will use the one from the `label` +//! attribute +//! +//! ```rust,ignore +//! #[derive(Debug, Diagnostic, Error)] +//! #[error("oops!")] +//! struct MyError { +//! #[label("main issue")] +//! primary_span: SourceSpan, +//! +//! #[label(collection, "related to this")] +//! other_spans: Vec, // LabeledSpan +//! } +//! +//! let report: miette::Report = MyError { +//! primary_span: (6, 9).into(), +//! other_spans: vec![ +//! LabeledSpan::new(None, 19, 7), // Use default text `related to this` +//! LabeledSpan::new(Some("and also this".to_string()), 30, 11), // Use specific text +//! ], +//! }.into(); +//! +//! println!("{:?}", report.with_source_code("About something or another or yet another ...".to_string())); +//! ``` +//! //! ## MSRV //! //! This crate requires rustc 1.70.0 or later. From d144f7a28edc1a1a67940526fef6ab42177177cf Mon Sep 17 00:00:00 2001 From: Nahor Date: Sat, 17 Feb 2024 10:14:54 -0800 Subject: [PATCH 2/4] feat(collection): remove repeated formatting of label text Because of a typo, the label text was being formatted multiple times per label in a collection. With the fix, the text is formatted only once per collection --- miette-derive/src/label.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/miette-derive/src/label.rs b/miette-derive/src/label.rs index cd6994ab..9f16f7f4 100644 --- a/miette-derive/src/label.rs +++ b/miette-derive/src/label.rs @@ -217,8 +217,8 @@ impl Labels { .map(|span| { use miette::macro_helpers::{ToLabelSpanWrapper,ToLabeledSpan}; let mut labeled_span = ToLabelSpanWrapper::to_labeled_span(span.clone()); - if #display.is_some() && labeled_span.label().is_none() { - labeled_span.set_label(#display) + if display.is_some() && labeled_span.label().is_none() { + labeled_span.set_label(display.clone()) } labeled_span }) @@ -306,8 +306,8 @@ impl Labels { .map(|span| { use miette::macro_helpers::{ToLabelSpanWrapper,ToLabeledSpan}; let mut labeled_span = ToLabelSpanWrapper::to_labeled_span(span.clone()); - if #display.is_some() && labeled_span.label().is_none() { - labeled_span.set_label(#display) + if display.is_some() && labeled_span.label().is_none() { + labeled_span.set_label(display.clone()) } labeled_span }) From 85576d7914ca986eafc321fbbfe95dce73fbf826 Mon Sep 17 00:00:00 2001 From: Nahor Date: Sat, 17 Feb 2024 10:29:54 -0800 Subject: [PATCH 3/4] feat(collection): chain iterators rather than extend vector Since we are going to iterate anyway, instead of growing the label vector, chain the iterators. This should be more efficient. In some cases, this also remove a compilation warning about the label vector being unnecessarily mutable. --- miette-derive/src/label.rs | 76 ++++++++++++++++++++------------------ 1 file changed, 40 insertions(+), 36 deletions(-) diff --git a/miette-derive/src/label.rs b/miette-derive/src/label.rs index 9f16f7f4..5e1243a8 100644 --- a/miette-derive/src/label.rs +++ b/miette-derive/src/label.rs @@ -162,7 +162,6 @@ impl Labels { pub(crate) fn gen_struct(&self, fields: &syn::Fields) -> Option { let (display_pat, display_members) = display_pat_members(fields); - let labels_gen_var = quote! { labels }; let labels = self.0.iter().filter_map(|highlight| { let Label { span, @@ -194,7 +193,7 @@ impl Labels { )) }) }); - let collections = self.0.iter().filter_map(|label| { + let collections_chain = self.0.iter().filter_map(|label| { let Label { span, label, @@ -211,18 +210,20 @@ impl Labels { quote! { std::option::Option::None } }; Some(quote! { - let display = #display; - #labels_gen_var.extend(self.#span.iter().map(|label| { - miette::macro_helpers::OptionalWrapper::<#ty>::new().to_option(label) - .map(|span| { - use miette::macro_helpers::{ToLabelSpanWrapper,ToLabeledSpan}; - let mut labeled_span = ToLabelSpanWrapper::to_labeled_span(span.clone()); - if display.is_some() && labeled_span.label().is_none() { - labeled_span.set_label(display.clone()) - } - labeled_span - }) - })); + .chain({ + let display = #display; + self.#span.iter().map(move |label| { + miette::macro_helpers::OptionalWrapper::<#ty>::new().to_option(label) + .map(|span| { + use miette::macro_helpers::{ToLabelSpanWrapper,ToLabeledSpan}; + let mut labeled_span = ToLabelSpanWrapper::to_labeled_span(span.clone()); + if display.is_some() && labeled_span.label().is_none() { + labeled_span.set_label(display.clone()) + } + labeled_span + }) + }) + }) }) }); @@ -232,12 +233,13 @@ impl Labels { use miette::macro_helpers::ToOption; let Self #display_pat = self; - let mut #labels_gen_var = vec![ + let labels_iter = vec![ #(#labels),* - ]; - #(#collections)* + ] + .into_iter() + #(#collections_chain)*; - std::option::Option::Some(Box::new(#labels_gen_var.into_iter().filter(Option::is_some).map(Option::unwrap))) + std::option::Option::Some(Box::new(labels_iter.filter(Option::is_some).map(Option::unwrap))) } }) } @@ -249,7 +251,6 @@ impl Labels { |ident, fields, DiagnosticConcreteArgs { labels, .. }| { let (display_pat, display_members) = display_pat_members(fields); labels.as_ref().and_then(|labels| { - let labels_gen_var = quote! { labels }; let variant_labels = labels.0.iter().filter_map(|label| { let Label { span, label, ty, lbl_ty } = label; if *lbl_ty == LabelType::Collection { @@ -282,7 +283,7 @@ impl Labels { )) }) }); - let collections = labels.0.iter().filter_map(|label| { + let collections_chain = labels.0.iter().filter_map(|label| { let Label { span, label, ty, lbl_ty } = label; if *lbl_ty != LabelType::Collection { return None; @@ -300,18 +301,20 @@ impl Labels { quote! { std::option::Option::None } }; Some(quote! { - let display = #display; - #labels_gen_var.extend(#field.iter().map(|label| { - miette::macro_helpers::OptionalWrapper::<#ty>::new().to_option(label) - .map(|span| { - use miette::macro_helpers::{ToLabelSpanWrapper,ToLabeledSpan}; - let mut labeled_span = ToLabelSpanWrapper::to_labeled_span(span.clone()); - if display.is_some() && labeled_span.label().is_none() { - labeled_span.set_label(display.clone()) - } - labeled_span - }) - })); + .chain({ + let display = #display; + #field.iter().map(move |label| { + miette::macro_helpers::OptionalWrapper::<#ty>::new().to_option(label) + .map(|span| { + use miette::macro_helpers::{ToLabelSpanWrapper,ToLabeledSpan}; + let mut labeled_span = ToLabelSpanWrapper::to_labeled_span(span.clone()); + if display.is_some() && labeled_span.label().is_none() { + labeled_span.set_label(display.clone()); + } + labeled_span + }) + }) + }) }) }); let variant_name = ident.clone(); @@ -320,11 +323,12 @@ impl Labels { _ => Some(quote! { Self::#variant_name #display_pat => { use miette::macro_helpers::ToOption; - let mut #labels_gen_var = vec![ + let labels_iter = vec![ #(#variant_labels),* - ]; - #(#collections)* - std::option::Option::Some(std::boxed::Box::new(#labels_gen_var.into_iter().filter(Option::is_some).map(Option::unwrap))) + ] + .into_iter() + #(#collections_chain)*; + std::option::Option::Some(std::boxed::Box::new(labels_iter.filter(Option::is_some).map(Option::unwrap))) } }), } From 589eacc2c3dfbe89f9fa7287b0efb6e6cde216a1 Mon Sep 17 00:00:00 2001 From: Nahor Date: Sat, 17 Feb 2024 10:55:56 -0800 Subject: [PATCH 4/4] feat(collection): remove unnecessary `OptionalWrapper` - In a label collection, there is no need for a `None` label. One should just not add that label - Code wise it is also incorrect. The wrapper will be for a vector of spans, not for individual spans. It happens to work anyway, but this was not the intended use. --- miette-derive/src/label.rs | 38 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/miette-derive/src/label.rs b/miette-derive/src/label.rs index 5e1243a8..ab2ceac1 100644 --- a/miette-derive/src/label.rs +++ b/miette-derive/src/label.rs @@ -197,7 +197,7 @@ impl Labels { let Label { span, label, - ty, + ty: _, lbl_ty, } = label; if *lbl_ty != LabelType::Collection { @@ -212,16 +212,13 @@ impl Labels { Some(quote! { .chain({ let display = #display; - self.#span.iter().map(move |label| { - miette::macro_helpers::OptionalWrapper::<#ty>::new().to_option(label) - .map(|span| { - use miette::macro_helpers::{ToLabelSpanWrapper,ToLabeledSpan}; - let mut labeled_span = ToLabelSpanWrapper::to_labeled_span(span.clone()); - if display.is_some() && labeled_span.label().is_none() { - labeled_span.set_label(display.clone()) - } - labeled_span - }) + self.#span.iter().map(move |span| { + use miette::macro_helpers::{ToLabelSpanWrapper,ToLabeledSpan}; + let mut labeled_span = ToLabelSpanWrapper::to_labeled_span(span.clone()); + if display.is_some() && labeled_span.label().is_none() { + labeled_span.set_label(display.clone()) + } + Some(labeled_span) }) }) }) @@ -284,7 +281,7 @@ impl Labels { }) }); let collections_chain = labels.0.iter().filter_map(|label| { - let Label { span, label, ty, lbl_ty } = label; + let Label { span, label, ty: _, lbl_ty } = label; if *lbl_ty != LabelType::Collection { return None; } @@ -303,16 +300,13 @@ impl Labels { Some(quote! { .chain({ let display = #display; - #field.iter().map(move |label| { - miette::macro_helpers::OptionalWrapper::<#ty>::new().to_option(label) - .map(|span| { - use miette::macro_helpers::{ToLabelSpanWrapper,ToLabeledSpan}; - let mut labeled_span = ToLabelSpanWrapper::to_labeled_span(span.clone()); - if display.is_some() && labeled_span.label().is_none() { - labeled_span.set_label(display.clone()); - } - labeled_span - }) + #field.iter().map(move |span| { + use miette::macro_helpers::{ToLabelSpanWrapper,ToLabeledSpan}; + let mut labeled_span = ToLabelSpanWrapper::to_labeled_span(span.clone()); + if display.is_some() && labeled_span.label().is_none() { + labeled_span.set_label(display.clone()); + } + Some(labeled_span) }) }) })