From db0b7e403a5ae52ae360991b6508490d8c579886 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Wed, 20 Sep 2023 15:36:33 -0700 Subject: [PATCH] feat(labels): Add support for primary label in specifying line/col information (#291) --- miette-derive/src/label.rs | 79 +++++++++++++++++++++------ src/handlers/graphical.rs | 27 +++++++-- src/miette_diagnostic.rs | 12 ++-- src/protocol.rs | 32 +++++++++-- tests/graphical.rs | 73 ++++++++++++++++++------- tests/test_diagnostic_source_macro.rs | 2 +- 6 files changed, 174 insertions(+), 51 deletions(-) diff --git a/miette-derive/src/label.rs b/miette-derive/src/label.rs index e0bc70ac..dd5ec693 100644 --- a/miette-derive/src/label.rs +++ b/miette-derive/src/label.rs @@ -20,10 +20,12 @@ struct Label { label: Option, ty: syn::Type, span: syn::Member, + primary: bool, } struct LabelAttr { label: Option, + primary: bool, } impl Parse for LabelAttr { @@ -40,10 +42,22 @@ impl Parse for LabelAttr { } }); let la = input.lookahead1(); - let label = if la.peek(syn::token::Paren) { - // #[label("{}", x)] + let (primary, label) = if la.peek(syn::token::Paren) { + // #[label(primary?, "{}", x)] let content; parenthesized!(content in input); + + let primary = if content.peek(syn::Ident) { + let ident: syn::Ident = content.parse()?; + if ident != "primary" { + return Err(syn::Error::new(input.span(), "Invalid argument to label() attribute. The argument must be a literal string or the keyword `primary`.")); + } + let _ = content.parse::(); + true + } else { + false + }; + if content.peek(syn::LitStr) { let fmt = content.parse()?; let args = if content.is_empty() { @@ -56,22 +70,27 @@ impl Parse for LabelAttr { args, has_bonus_display: false, }; - Some(display) + (primary, Some(display)) + } else if !primary { + return Err(syn::Error::new(input.span(), "Invalid argument to label() attribute. The argument must be a literal string or the keyword `primary`.")); } else { - return Err(syn::Error::new(input.span(), "Invalid argument to label() attribute. The first argument must be a literal string.")); + (primary, None) } } else if la.peek(Token![=]) { // #[label = "blabla"] input.parse::()?; - 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 }) + Ok(LabelAttr { label, primary }) } } @@ -100,12 +119,21 @@ impl Labels { }) }; use quote::ToTokens; - let LabelAttr { label } = + let LabelAttr { label, primary } = syn::parse2::(attr.meta.to_token_stream())?; + + if primary && labels.iter().any(|l: &Label| l.primary) { + return Err(syn::Error::new( + field.span(), + "Cannot have more than one primary label.", + )); + } + labels.push(Label { label, span, ty: field.ty.clone(), + primary, }); } } @@ -120,13 +148,23 @@ impl Labels { pub(crate) fn gen_struct(&self, fields: &syn::Fields) -> Option { let (display_pat, display_members) = display_pat_members(fields); let labels = self.0.iter().map(|highlight| { - let Label { span, label, ty } = highlight; + let Label { + span, + label, + ty, + primary, + } = highlight; let var = quote! { __miette_internal_var }; + let ctor = if *primary { + quote! { miette::LabeledSpan::new_primary_with_span } + } else { + quote! { miette::LabeledSpan::new_with_span } + }; if let Some(display) = label { let (fmt, args) = display.expand_shorthand_cloned(&display_members); quote! { miette::macro_helpers::OptionalWrapper::<#ty>::new().to_option(&self.#span) - .map(|#var| miette::LabeledSpan::new_with_span( + .map(|#var| #ctor( std::option::Option::Some(format!(#fmt #args)), #var.clone(), )) @@ -134,7 +172,7 @@ impl Labels { } else { quote! { miette::macro_helpers::OptionalWrapper::<#ty>::new().to_option(&self.#span) - .map(|#var| miette::LabeledSpan::new_with_span( + .map(|#var| #ctor( std::option::Option::None, #var.clone(), )) @@ -161,7 +199,7 @@ impl 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, ty } = label; + let Label { span, label, ty, primary } = label; let field = match &span { syn::Member::Named(ident) => ident.clone(), syn::Member::Unnamed(syn::Index { index, .. }) => { @@ -169,11 +207,16 @@ impl Labels { } }; let var = quote! { __miette_internal_var }; + let ctor = if *primary { + quote! { miette::LabeledSpan::new_primary_with_span } + } else { + quote! { miette::LabeledSpan::new_with_span } + }; if let Some(display) = label { let (fmt, args) = display.expand_shorthand_cloned(&display_members); quote! { miette::macro_helpers::OptionalWrapper::<#ty>::new().to_option(#field) - .map(|#var| miette::LabeledSpan::new_with_span( + .map(|#var| #ctor( std::option::Option::Some(format!(#fmt #args)), #var.clone(), )) @@ -181,7 +224,7 @@ impl Labels { } else { quote! { miette::macro_helpers::OptionalWrapper::<#ty>::new().to_option(#field) - .map(|#var| miette::LabeledSpan::new_with_span( + .map(|#var| #ctor( std::option::Option::None, #var.clone(), )) diff --git a/src/handlers/graphical.rs b/src/handlers/graphical.rs index e61f5951..a7399fd8 100644 --- a/src/handlers/graphical.rs +++ b/src/handlers/graphical.rs @@ -391,6 +391,11 @@ impl GraphicalReportHandler { ) -> fmt::Result { let (contents, lines) = self.get_lines(source, context.inner())?; + let primary_label = labels + .iter() + .find(|label| label.primary()) + .or_else(|| labels.first()); + // sorting is your friend let labels = labels .iter() @@ -431,19 +436,33 @@ impl GraphicalReportHandler { self.theme.characters.hbar, )?; - if let Some(source_name) = contents.name() { + // If there is a primary label, then use its span + // as the reference point for line/column information. + let primary_contents = match primary_label { + Some(label) => source + .read_span(label.inner(), 0, 0) + .map_err(|_| fmt::Error)?, + None => contents, + }; + + if let Some(source_name) = primary_contents.name() { let source_name = source_name.style(self.theme.styles.link); writeln!( f, "[{}:{}:{}]", source_name, - contents.line() + 1, - contents.column() + 1 + primary_contents.line() + 1, + primary_contents.column() + 1 )?; } else if lines.len() <= 1 { writeln!(f, "{}", self.theme.characters.hbar.to_string().repeat(3))?; } else { - writeln!(f, "[{}:{}]", contents.line() + 1, contents.column() + 1)?; + writeln!( + f, + "[{}:{}]", + primary_contents.line() + 1, + primary_contents.column() + 1 + )?; } // Now it's time for the fun part--actually rendering everything! diff --git a/src/miette_diagnostic.rs b/src/miette_diagnostic.rs index dc0468e4..9863e881 100644 --- a/src/miette_diagnostic.rs +++ b/src/miette_diagnostic.rs @@ -292,14 +292,16 @@ fn test_serialize_miette_diagnostic() { "offset": 0, "length": 0 }, - "label": "label1" + "label": "label1", + "primary": false }, { "span": { "offset": 1, "length": 2 }, - "label": "label2" + "label": "label2", + "primary": false } ] }); @@ -350,14 +352,16 @@ fn test_deserialize_miette_diagnostic() { "offset": 0, "length": 0 }, - "label": "label1" + "label": "label1", + "primary": false }, { "span": { "offset": 1, "length": 2 }, - "label": "label2" + "label": "label2", + "primary": false } ] }); diff --git a/src/protocol.rs b/src/protocol.rs index 36c35398..be313b13 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -249,6 +249,7 @@ pub struct LabeledSpan { #[cfg_attr(feature = "serde", serde(skip_serializing_if = "Option::is_none"))] label: Option, span: SourceSpan, + primary: bool, } impl LabeledSpan { @@ -257,6 +258,7 @@ impl LabeledSpan { Self { label, span: SourceSpan::new(SourceOffset(offset), SourceOffset(len)), + primary: false, } } @@ -265,6 +267,16 @@ impl LabeledSpan { Self { label, span: span.into(), + primary: false, + } + } + + /// Makes a new labeled primary span using an existing span. + pub fn new_primary_with_span(label: Option, span: impl Into) -> Self { + Self { + label, + span: span.into(), + primary: true, } } @@ -340,6 +352,11 @@ impl LabeledSpan { pub const fn is_empty(&self) -> bool { self.span.is_empty() } + + /// True if this `LabeledSpan` is a primary span. + pub const fn primary(&self) -> bool { + self.primary + } } #[cfg(feature = "serde")] @@ -350,7 +367,8 @@ fn test_serialize_labeled_span() { assert_eq!( json!(LabeledSpan::new(None, 0, 0)), json!({ - "span": { "offset": 0, "length": 0 } + "span": { "offset": 0, "length": 0, }, + "primary": false, }) ); @@ -358,7 +376,8 @@ fn test_serialize_labeled_span() { json!(LabeledSpan::new(Some("label".to_string()), 0, 0)), json!({ "label": "label", - "span": { "offset": 0, "length": 0 } + "span": { "offset": 0, "length": 0, }, + "primary": false, }) ) } @@ -370,20 +389,23 @@ fn test_deserialize_labeled_span() { let span: LabeledSpan = serde_json::from_value(json!({ "label": null, - "span": { "offset": 0, "length": 0 } + "span": { "offset": 0, "length": 0, }, + "primary": false, })) .unwrap(); assert_eq!(span, LabeledSpan::new(None, 0, 0)); let span: LabeledSpan = serde_json::from_value(json!({ - "span": { "offset": 0, "length": 0 } + "span": { "offset": 0, "length": 0, }, + "primary": false })) .unwrap(); assert_eq!(span, LabeledSpan::new(None, 0, 0)); let span: LabeledSpan = serde_json::from_value(json!({ "label": "label", - "span": { "offset": 0, "length": 0 } + "span": { "offset": 0, "length": 0, }, + "primary": false })) .unwrap(); assert_eq!(span, LabeledSpan::new(Some("label".to_string()), 0, 0)) diff --git a/tests/graphical.rs b/tests/graphical.rs index 0c69470c..887e4548 100644 --- a/tests/graphical.rs +++ b/tests/graphical.rs @@ -86,7 +86,7 @@ fn single_line_highlight_span_full_line() { println!("Error: {}", out); let expected = r#" × oops! - ╭─[issue:1:1] + ╭─[issue:2:1] 1 │ source 2 │ text · ──┬─ @@ -120,7 +120,7 @@ fn single_line_with_wide_char() -> Result<(), MietteError> { let expected = r#"oops::my::bad × oops! - ╭─[bad_file.rs:1:1] + ╭─[bad_file.rs:2:7] 1 │ source 2 │ 👼🏼text · ───┬── @@ -159,7 +159,7 @@ fn single_line_with_two_tabs() -> Result<(), MietteError> { let expected = r#"oops::my::bad × oops! - ╭─[bad_file.rs:1:1] + ╭─[bad_file.rs:2:3] 1 │ source 2 │ text · ──┬─ @@ -198,7 +198,7 @@ fn single_line_with_tab_in_middle() -> Result<(), MietteError> { let expected = r#"oops::my::bad × oops! - ╭─[bad_file.rs:1:1] + ╭─[bad_file.rs:2:8] 1 │ source 2 │ text = text · ──┬─ @@ -235,7 +235,7 @@ fn single_line_highlight() -> Result<(), MietteError> { let expected = r#"oops::my::bad × oops! - ╭─[bad_file.rs:1:1] + ╭─[bad_file.rs:2:3] 1 │ source 2 │ text · ──┬─ @@ -270,7 +270,7 @@ fn external_source() -> Result<(), MietteError> { let expected = r#"oops::my::bad × oops! - ╭─[bad_file.rs:1:1] + ╭─[bad_file.rs:2:3] 1 │ source 2 │ text · ──┬─ @@ -343,7 +343,7 @@ fn single_line_highlight_offset_end_of_line() -> Result<(), MietteError> { let expected = r#"oops::my::bad × oops! - ╭─[bad_file.rs:1:1] + ╭─[bad_file.rs:1:7] 1 │ source · ▲ · ╰── this bit here @@ -379,7 +379,7 @@ fn single_line_highlight_include_end_of_line() -> Result<(), MietteError> { let expected = r#"oops::my::bad × oops! - ╭─[bad_file.rs:1:1] + ╭─[bad_file.rs:2:3] 1 │ source 2 │ text · ──┬── @@ -416,7 +416,7 @@ fn single_line_highlight_include_end_of_line_crlf() -> Result<(), MietteError> { let expected = r#"oops::my::bad × oops! - ╭─[bad_file.rs:1:1] + ╭─[bad_file.rs:2:3] 1 │ source 2 │ text · ──┬── @@ -453,7 +453,7 @@ fn single_line_highlight_with_empty_span() -> Result<(), MietteError> { let expected = r#"oops::my::bad × oops! - ╭─[bad_file.rs:1:1] + ╭─[bad_file.rs:2:3] 1 │ source 2 │ text · ▲ @@ -490,7 +490,7 @@ fn single_line_highlight_no_label() -> Result<(), MietteError> { let expected = r#"oops::my::bad × oops! - ╭─[bad_file.rs:1:1] + ╭─[bad_file.rs:2:3] 1 │ source 2 │ text · ──── @@ -526,7 +526,7 @@ fn single_line_highlight_at_line_start() -> Result<(), MietteError> { let expected = r#"oops::my::bad × oops! - ╭─[bad_file.rs:1:1] + ╭─[bad_file.rs:2:1] 1 │ source 2 │ text · ──┬─ @@ -569,7 +569,7 @@ fn multiple_same_line_highlights() -> Result<(), MietteError> { let expected = r#"oops::my::bad × oops! - ╭─[bad_file.rs:1:1] + ╭─[bad_file.rs:2:3] 1 │ source 2 │ text text text text text · ──┬─ ──┬─ ──┬─ @@ -616,7 +616,7 @@ fn multiple_same_line_highlights_with_tabs_in_middle() -> Result<(), MietteError let expected = r#"oops::my::bad × oops! - ╭─[bad_file.rs:1:1] + ╭─[bad_file.rs:2:3] 1 │ source 2 │ text text text text text · ──┬─ ──┬─ ──┬─ @@ -655,7 +655,7 @@ fn multiline_highlight_adjacent() -> Result<(), MietteError> { let expected = r#"oops::my::bad × oops! - ╭─[bad_file.rs:1:1] + ╭─[bad_file.rs:2:3] 1 │ source 2 │ ╭─▶ text 3 │ ├─▶ here @@ -969,7 +969,7 @@ fn related() -> Result<(), MietteError> { let expected = r#"oops::my::bad × oops! - ╭─[bad_file.rs:1:1] + ╭─[bad_file.rs:2:3] 1 │ source 2 │ text · ──┬─ @@ -1031,7 +1031,7 @@ fn related_source_code_propagation() -> Result<(), MietteError> { let expected = r#"oops::my::bad × oops! - ╭─[bad_file.rs:1:1] + ╭─[bad_file.rs:2:3] 1 │ source 2 │ text · ──┬─ @@ -1136,7 +1136,7 @@ fn related_severity() -> Result<(), MietteError> { let expected = r#"oops::my::bad × oops! - ╭─[bad_file.rs:1:1] + ╭─[bad_file.rs:2:3] 1 │ source 2 │ text · ──┬─ @@ -1201,7 +1201,7 @@ fn zero_length_eol_span() { println!("Error: {}", out); let expected = r#" × oops! - ╭─[issue:1:1] + ╭─[issue:2:1] 1 │ this is the first line 2 │ this is the second line · ▲ @@ -1212,3 +1212,38 @@ fn zero_length_eol_span() { assert_eq!(expected, out); } + +#[test] +fn primary_label() { + #[derive(Error, Debug, Diagnostic)] + #[error("oops!")] + struct MyBad { + #[source_code] + src: NamedSource, + #[label] + first_label: SourceSpan, + #[label(primary, "nope")] + second_label: SourceSpan, + } + let err = MyBad { + src: NamedSource::new("issue", "this is the first line\nthis is the second line"), + first_label: (2, 4).into(), + second_label: (24, 4).into(), + }; + let out = fmt_report(err.into()); + println!("Error: {}", out); + + // line 2 should be the primary, not line 1 + let expected = r#" × oops! + ╭─[issue:2:2] + 1 │ this is the first line + · ──── + 2 │ this is the second line + · ──┬─ + · ╰── nope + ╰──── +"# + .to_string(); + + assert_eq!(expected, out); +} diff --git a/tests/test_diagnostic_source_macro.rs b/tests/test_diagnostic_source_macro.rs index df30b2e9..0ca396a1 100644 --- a/tests/test_diagnostic_source_macro.rs +++ b/tests/test_diagnostic_source_macro.rs @@ -91,7 +91,7 @@ fn test_diagnostic_source_pass_extra_info() { println!("Error: {}", out); let expected = r#" × TestError ╰─▶ × A complex error happened - ╭─[1:1] + ╭─[1:2] 1 │ Hello · ──┬─ · ╰── here