From 1905af07efa87215c2b6ecc8b0002fbb87776dad Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Tue, 19 Sep 2023 16:23:14 -0700 Subject: [PATCH] Add default strategy for selecting primary label. Change attribute syntax to #[label(primary)]. --- miette-derive/src/label.rs | 63 +++++++++++++++++---------- src/handlers/graphical.rs | 5 ++- src/miette_diagnostic.rs | 2 +- src/protocol.rs | 4 +- tests/graphical.rs | 51 ++++++++++++---------- tests/test_diagnostic_source_macro.rs | 2 +- 6 files changed, 76 insertions(+), 51 deletions(-) diff --git a/miette-derive/src/label.rs b/miette-derive/src/label.rs index 232925e..dd5ec69 100644 --- a/miette-derive/src/label.rs +++ b/miette-derive/src/label.rs @@ -25,6 +25,7 @@ struct Label { struct LabelAttr { label: Option, + primary: bool, } impl Parse for LabelAttr { @@ -41,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() { @@ -57,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 }) } } @@ -91,16 +109,7 @@ impl Labels { let mut labels = Vec::new(); for (i, field) in fields.iter().enumerate() { for attr in &field.attrs { - let is_label = attr.path().is_ident("label"); - let is_primary_label = attr.path().is_ident("primary_label"); - if is_label || is_primary_label { - if is_primary_label && labels.iter().any(|l: &Label| l.primary) { - return Err(syn::Error::new( - field.span(), - "Cannot have more than one primary label.", - )); - } - + if attr.path().is_ident("label") { let span = if let Some(ident) = field.ident.clone() { syn::Member::Named(ident) } else { @@ -110,13 +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: is_primary_label, + primary, }); } } diff --git a/src/handlers/graphical.rs b/src/handlers/graphical.rs index 0bdf6c1..06aaff6 100644 --- a/src/handlers/graphical.rs +++ b/src/handlers/graphical.rs @@ -391,7 +391,10 @@ impl GraphicalReportHandler { ) -> fmt::Result { let (contents, lines) = self.get_lines(source, context.inner())?; - let primary_label = labels.iter().find(|label| label.primary()); + let primary_label = labels + .iter() + .find(|label| label.primary()) + .or_else(|| labels.first()); // sorting is your friend let labels = labels diff --git a/src/miette_diagnostic.rs b/src/miette_diagnostic.rs index 4135103..9863e88 100644 --- a/src/miette_diagnostic.rs +++ b/src/miette_diagnostic.rs @@ -252,7 +252,7 @@ impl MietteDiagnostic { /// ``` pub fn and_labels(mut self, labels: impl IntoIterator) -> Self { let mut all_labels = self.labels.unwrap_or_default(); - all_labels.extend(labels.into_iter()); + all_labels.extend(labels); self.labels = Some(all_labels); self } diff --git a/src/protocol.rs b/src/protocol.rs index fd032bd..be313b1 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -377,7 +377,7 @@ fn test_serialize_labeled_span() { json!({ "label": "label", "span": { "offset": 0, "length": 0, }, - "primary": false, + "primary": false, }) ) } @@ -397,7 +397,7 @@ fn test_deserialize_labeled_span() { let span: LabeledSpan = serde_json::from_value(json!({ "span": { "offset": 0, "length": 0, }, - "primary": false + "primary": false })) .unwrap(); assert_eq!(span, LabeledSpan::new(None, 0, 0)); diff --git a/tests/graphical.rs b/tests/graphical.rs index 87a6c21..887e454 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 · ▲ @@ -1220,22 +1220,27 @@ fn primary_label() { struct MyBad { #[source_code] src: NamedSource, - #[primary_label("The root cause")] - bad_bit: SourceSpan, + #[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"), - bad_bit: (24, 4).into(), + 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 · ──┬─ - · ╰── The root cause + · ╰── nope ╰──── "# .to_string(); diff --git a/tests/test_diagnostic_source_macro.rs b/tests/test_diagnostic_source_macro.rs index 536aedf..4466ec5 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