feat(labels): Add support for primary label in specifying line/col information (#291)

This commit is contained in:
Will Crichton 2023-09-20 15:36:33 -07:00 committed by GitHub
parent cc81382a60
commit db0b7e403a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 174 additions and 51 deletions

View File

@ -20,10 +20,12 @@ struct Label {
label: Option<Display>, label: Option<Display>,
ty: syn::Type, ty: syn::Type,
span: syn::Member, span: syn::Member,
primary: bool,
} }
struct LabelAttr { struct LabelAttr {
label: Option<Display>, label: Option<Display>,
primary: bool,
} }
impl Parse for LabelAttr { impl Parse for LabelAttr {
@ -40,10 +42,22 @@ impl Parse for LabelAttr {
} }
}); });
let la = input.lookahead1(); let la = input.lookahead1();
let label = if la.peek(syn::token::Paren) { let (primary, label) = if la.peek(syn::token::Paren) {
// #[label("{}", x)] // #[label(primary?, "{}", x)]
let content; let content;
parenthesized!(content in input); 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::<Token![,]>();
true
} else {
false
};
if content.peek(syn::LitStr) { if content.peek(syn::LitStr) {
let fmt = content.parse()?; let fmt = content.parse()?;
let args = if content.is_empty() { let args = if content.is_empty() {
@ -56,22 +70,27 @@ impl Parse for LabelAttr {
args, args,
has_bonus_display: false, 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 { } 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![=]) { } else if la.peek(Token![=]) {
// #[label = "blabla"] // #[label = "blabla"]
input.parse::<Token![=]>()?; input.parse::<Token![=]>()?;
Some(Display { (
fmt: input.parse()?, false,
args: TokenStream::new(), Some(Display {
has_bonus_display: false, fmt: input.parse()?,
}) args: TokenStream::new(),
has_bonus_display: false,
}),
)
} else { } else {
None (false, None)
}; };
Ok(LabelAttr { label }) Ok(LabelAttr { label, primary })
} }
} }
@ -100,12 +119,21 @@ impl Labels {
}) })
}; };
use quote::ToTokens; use quote::ToTokens;
let LabelAttr { label } = let LabelAttr { label, primary } =
syn::parse2::<LabelAttr>(attr.meta.to_token_stream())?; syn::parse2::<LabelAttr>(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 { labels.push(Label {
label, label,
span, span,
ty: field.ty.clone(), ty: field.ty.clone(),
primary,
}); });
} }
} }
@ -120,13 +148,23 @@ impl Labels {
pub(crate) fn gen_struct(&self, fields: &syn::Fields) -> Option<TokenStream> { pub(crate) fn gen_struct(&self, fields: &syn::Fields) -> Option<TokenStream> {
let (display_pat, display_members) = display_pat_members(fields); let (display_pat, display_members) = display_pat_members(fields);
let labels = self.0.iter().map(|highlight| { 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 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 { if let Some(display) = label {
let (fmt, args) = display.expand_shorthand_cloned(&display_members); let (fmt, args) = display.expand_shorthand_cloned(&display_members);
quote! { quote! {
miette::macro_helpers::OptionalWrapper::<#ty>::new().to_option(&self.#span) 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)), std::option::Option::Some(format!(#fmt #args)),
#var.clone(), #var.clone(),
)) ))
@ -134,7 +172,7 @@ impl Labels {
} else { } else {
quote! { quote! {
miette::macro_helpers::OptionalWrapper::<#ty>::new().to_option(&self.#span) miette::macro_helpers::OptionalWrapper::<#ty>::new().to_option(&self.#span)
.map(|#var| miette::LabeledSpan::new_with_span( .map(|#var| #ctor(
std::option::Option::None, std::option::Option::None,
#var.clone(), #var.clone(),
)) ))
@ -161,7 +199,7 @@ impl Labels {
let (display_pat, display_members) = display_pat_members(fields); let (display_pat, display_members) = display_pat_members(fields);
labels.as_ref().and_then(|labels| { labels.as_ref().and_then(|labels| {
let variant_labels = labels.0.iter().map(|label| { 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 { let field = match &span {
syn::Member::Named(ident) => ident.clone(), syn::Member::Named(ident) => ident.clone(),
syn::Member::Unnamed(syn::Index { index, .. }) => { syn::Member::Unnamed(syn::Index { index, .. }) => {
@ -169,11 +207,16 @@ impl Labels {
} }
}; };
let var = quote! { __miette_internal_var }; 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 { if let Some(display) = label {
let (fmt, args) = display.expand_shorthand_cloned(&display_members); let (fmt, args) = display.expand_shorthand_cloned(&display_members);
quote! { quote! {
miette::macro_helpers::OptionalWrapper::<#ty>::new().to_option(#field) 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)), std::option::Option::Some(format!(#fmt #args)),
#var.clone(), #var.clone(),
)) ))
@ -181,7 +224,7 @@ impl Labels {
} else { } else {
quote! { quote! {
miette::macro_helpers::OptionalWrapper::<#ty>::new().to_option(#field) miette::macro_helpers::OptionalWrapper::<#ty>::new().to_option(#field)
.map(|#var| miette::LabeledSpan::new_with_span( .map(|#var| #ctor(
std::option::Option::None, std::option::Option::None,
#var.clone(), #var.clone(),
)) ))

View File

@ -391,6 +391,11 @@ impl GraphicalReportHandler {
) -> fmt::Result { ) -> fmt::Result {
let (contents, lines) = self.get_lines(source, context.inner())?; 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 // sorting is your friend
let labels = labels let labels = labels
.iter() .iter()
@ -431,19 +436,33 @@ impl GraphicalReportHandler {
self.theme.characters.hbar, 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); let source_name = source_name.style(self.theme.styles.link);
writeln!( writeln!(
f, f,
"[{}:{}:{}]", "[{}:{}:{}]",
source_name, source_name,
contents.line() + 1, primary_contents.line() + 1,
contents.column() + 1 primary_contents.column() + 1
)?; )?;
} else if lines.len() <= 1 { } else if lines.len() <= 1 {
writeln!(f, "{}", self.theme.characters.hbar.to_string().repeat(3))?; writeln!(f, "{}", self.theme.characters.hbar.to_string().repeat(3))?;
} else { } 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! // Now it's time for the fun part--actually rendering everything!

View File

@ -292,14 +292,16 @@ fn test_serialize_miette_diagnostic() {
"offset": 0, "offset": 0,
"length": 0 "length": 0
}, },
"label": "label1" "label": "label1",
"primary": false
}, },
{ {
"span": { "span": {
"offset": 1, "offset": 1,
"length": 2 "length": 2
}, },
"label": "label2" "label": "label2",
"primary": false
} }
] ]
}); });
@ -350,14 +352,16 @@ fn test_deserialize_miette_diagnostic() {
"offset": 0, "offset": 0,
"length": 0 "length": 0
}, },
"label": "label1" "label": "label1",
"primary": false
}, },
{ {
"span": { "span": {
"offset": 1, "offset": 1,
"length": 2 "length": 2
}, },
"label": "label2" "label": "label2",
"primary": false
} }
] ]
}); });

View File

@ -249,6 +249,7 @@ pub struct LabeledSpan {
#[cfg_attr(feature = "serde", serde(skip_serializing_if = "Option::is_none"))] #[cfg_attr(feature = "serde", serde(skip_serializing_if = "Option::is_none"))]
label: Option<String>, label: Option<String>,
span: SourceSpan, span: SourceSpan,
primary: bool,
} }
impl LabeledSpan { impl LabeledSpan {
@ -257,6 +258,7 @@ impl LabeledSpan {
Self { Self {
label, label,
span: SourceSpan::new(SourceOffset(offset), SourceOffset(len)), span: SourceSpan::new(SourceOffset(offset), SourceOffset(len)),
primary: false,
} }
} }
@ -265,6 +267,16 @@ impl LabeledSpan {
Self { Self {
label, label,
span: span.into(), span: span.into(),
primary: false,
}
}
/// Makes a new labeled primary span using an existing span.
pub fn new_primary_with_span(label: Option<String>, span: impl Into<SourceSpan>) -> Self {
Self {
label,
span: span.into(),
primary: true,
} }
} }
@ -340,6 +352,11 @@ impl LabeledSpan {
pub const fn is_empty(&self) -> bool { pub const fn is_empty(&self) -> bool {
self.span.is_empty() self.span.is_empty()
} }
/// True if this `LabeledSpan` is a primary span.
pub const fn primary(&self) -> bool {
self.primary
}
} }
#[cfg(feature = "serde")] #[cfg(feature = "serde")]
@ -350,7 +367,8 @@ fn test_serialize_labeled_span() {
assert_eq!( assert_eq!(
json!(LabeledSpan::new(None, 0, 0)), json!(LabeledSpan::new(None, 0, 0)),
json!({ 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!(LabeledSpan::new(Some("label".to_string()), 0, 0)),
json!({ json!({
"label": "label", "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!({ let span: LabeledSpan = serde_json::from_value(json!({
"label": null, "label": null,
"span": { "offset": 0, "length": 0 } "span": { "offset": 0, "length": 0, },
"primary": false,
})) }))
.unwrap(); .unwrap();
assert_eq!(span, LabeledSpan::new(None, 0, 0)); assert_eq!(span, LabeledSpan::new(None, 0, 0));
let span: LabeledSpan = serde_json::from_value(json!({ let span: LabeledSpan = serde_json::from_value(json!({
"span": { "offset": 0, "length": 0 } "span": { "offset": 0, "length": 0, },
"primary": false
})) }))
.unwrap(); .unwrap();
assert_eq!(span, LabeledSpan::new(None, 0, 0)); assert_eq!(span, LabeledSpan::new(None, 0, 0));
let span: LabeledSpan = serde_json::from_value(json!({ let span: LabeledSpan = serde_json::from_value(json!({
"label": "label", "label": "label",
"span": { "offset": 0, "length": 0 } "span": { "offset": 0, "length": 0, },
"primary": false
})) }))
.unwrap(); .unwrap();
assert_eq!(span, LabeledSpan::new(Some("label".to_string()), 0, 0)) assert_eq!(span, LabeledSpan::new(Some("label".to_string()), 0, 0))

View File

@ -86,7 +86,7 @@ fn single_line_highlight_span_full_line() {
println!("Error: {}", out); println!("Error: {}", out);
let expected = r#" × oops! let expected = r#" × oops!
[issue:1:1] [issue:2:1]
1 source 1 source
2 text 2 text
· ·
@ -120,7 +120,7 @@ fn single_line_with_wide_char() -> Result<(), MietteError> {
let expected = r#"oops::my::bad let expected = r#"oops::my::bad
× oops! × oops!
[bad_file.rs:1:1] [bad_file.rs:2:7]
1 source 1 source
2 👼🏼text 2 👼🏼text
· ·
@ -159,7 +159,7 @@ fn single_line_with_two_tabs() -> Result<(), MietteError> {
let expected = r#"oops::my::bad let expected = r#"oops::my::bad
× oops! × oops!
[bad_file.rs:1:1] [bad_file.rs:2:3]
1 source 1 source
2 text 2 text
· ·
@ -198,7 +198,7 @@ fn single_line_with_tab_in_middle() -> Result<(), MietteError> {
let expected = r#"oops::my::bad let expected = r#"oops::my::bad
× oops! × oops!
[bad_file.rs:1:1] [bad_file.rs:2:8]
1 source 1 source
2 text = text 2 text = text
· ·
@ -235,7 +235,7 @@ fn single_line_highlight() -> Result<(), MietteError> {
let expected = r#"oops::my::bad let expected = r#"oops::my::bad
× oops! × oops!
[bad_file.rs:1:1] [bad_file.rs:2:3]
1 source 1 source
2 text 2 text
· ·
@ -270,7 +270,7 @@ fn external_source() -> Result<(), MietteError> {
let expected = r#"oops::my::bad let expected = r#"oops::my::bad
× oops! × oops!
[bad_file.rs:1:1] [bad_file.rs:2:3]
1 source 1 source
2 text 2 text
· ·
@ -343,7 +343,7 @@ fn single_line_highlight_offset_end_of_line() -> Result<(), MietteError> {
let expected = r#"oops::my::bad let expected = r#"oops::my::bad
× oops! × oops!
[bad_file.rs:1:1] [bad_file.rs:1:7]
1 source 1 source
· ·
· this bit here · this bit here
@ -379,7 +379,7 @@ fn single_line_highlight_include_end_of_line() -> Result<(), MietteError> {
let expected = r#"oops::my::bad let expected = r#"oops::my::bad
× oops! × oops!
[bad_file.rs:1:1] [bad_file.rs:2:3]
1 source 1 source
2 text 2 text
· ·
@ -416,7 +416,7 @@ fn single_line_highlight_include_end_of_line_crlf() -> Result<(), MietteError> {
let expected = r#"oops::my::bad let expected = r#"oops::my::bad
× oops! × oops!
[bad_file.rs:1:1] [bad_file.rs:2:3]
1 source 1 source
2 text 2 text
· ·
@ -453,7 +453,7 @@ fn single_line_highlight_with_empty_span() -> Result<(), MietteError> {
let expected = r#"oops::my::bad let expected = r#"oops::my::bad
× oops! × oops!
[bad_file.rs:1:1] [bad_file.rs:2:3]
1 source 1 source
2 text 2 text
· ·
@ -490,7 +490,7 @@ fn single_line_highlight_no_label() -> Result<(), MietteError> {
let expected = r#"oops::my::bad let expected = r#"oops::my::bad
× oops! × oops!
[bad_file.rs:1:1] [bad_file.rs:2:3]
1 source 1 source
2 text 2 text
· ·
@ -526,7 +526,7 @@ fn single_line_highlight_at_line_start() -> Result<(), MietteError> {
let expected = r#"oops::my::bad let expected = r#"oops::my::bad
× oops! × oops!
[bad_file.rs:1:1] [bad_file.rs:2:1]
1 source 1 source
2 text 2 text
· ·
@ -569,7 +569,7 @@ fn multiple_same_line_highlights() -> Result<(), MietteError> {
let expected = r#"oops::my::bad let expected = r#"oops::my::bad
× oops! × oops!
[bad_file.rs:1:1] [bad_file.rs:2:3]
1 source 1 source
2 text text text text text 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 let expected = r#"oops::my::bad
× oops! × oops!
[bad_file.rs:1:1] [bad_file.rs:2:3]
1 source 1 source
2 text text text text text 2 text text text text text
· ·
@ -655,7 +655,7 @@ fn multiline_highlight_adjacent() -> Result<(), MietteError> {
let expected = r#"oops::my::bad let expected = r#"oops::my::bad
× oops! × oops!
[bad_file.rs:1:1] [bad_file.rs:2:3]
1 source 1 source
2 text 2 text
3 here 3 here
@ -969,7 +969,7 @@ fn related() -> Result<(), MietteError> {
let expected = r#"oops::my::bad let expected = r#"oops::my::bad
× oops! × oops!
[bad_file.rs:1:1] [bad_file.rs:2:3]
1 source 1 source
2 text 2 text
· ·
@ -1031,7 +1031,7 @@ fn related_source_code_propagation() -> Result<(), MietteError> {
let expected = r#"oops::my::bad let expected = r#"oops::my::bad
× oops! × oops!
[bad_file.rs:1:1] [bad_file.rs:2:3]
1 source 1 source
2 text 2 text
· ·
@ -1136,7 +1136,7 @@ fn related_severity() -> Result<(), MietteError> {
let expected = r#"oops::my::bad let expected = r#"oops::my::bad
× oops! × oops!
[bad_file.rs:1:1] [bad_file.rs:2:3]
1 source 1 source
2 text 2 text
· ·
@ -1201,7 +1201,7 @@ fn zero_length_eol_span() {
println!("Error: {}", out); println!("Error: {}", out);
let expected = r#" × oops! let expected = r#" × oops!
[issue:1:1] [issue:2:1]
1 this is the first line 1 this is the first line
2 this is the second line 2 this is the second line
· ·
@ -1212,3 +1212,38 @@ fn zero_length_eol_span() {
assert_eq!(expected, out); 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);
}

View File

@ -91,7 +91,7 @@ fn test_diagnostic_source_pass_extra_info() {
println!("Error: {}", out); println!("Error: {}", out);
let expected = r#" × TestError let expected = r#" × TestError
× A complex error happened × A complex error happened
[1:1] [1:2]
1 Hello 1 Hello
· ·
· here · here