Repository: zkat/miette
Pull request #291 (2023-09-19 19:43:46)

Add support for primary label in specifying line/col information
As discussed in #91 and #208, it is sometimes desirable for the line/column numbers in a diagnostic to reference a label as opposed to the start of the context. However, there has to be a way of identifying which label to reference.

This PR implements one possible mechanism: a `label(primary)` attribute which acts just like `label`, except it marks a single label as being "primary". For example, this program:

```rust
use miette::{Diagnostic, SourceSpan};
use thiserror::Error;

#[derive(Error, Debug, Diagnostic)]
#[error("Primary span error")]
struct PrimarySpanError {
    #[source_code]
    src: String,
    #[label(primary)] // note the new attribute parameter
    span: SourceSpan,
}

fn main() -> miette::Result<()> {
    Err(PrimarySpanError {
        src: String::from("Hello\nworld"),
        span: (7..10).into(),
    }
    .into())
}
```

Produces this output:

```
Error:   × Primary span error
   ╭─[2:2]
 1 │ Hello
 2 │ world
   ·  ───
   ╰────
```

Note that the line/column info is `2:2` rather than `1:1` which you would get with a normal `#[label]`.

The core technical changes are:
* Adding a `primary: bool` field to `LabeledSpan`.
* Adding a method `LabeledSpan::new_primary_with_span` that constructs a `LabeledSpan` with `primary: true`.
* Extending the derive macro to support `label(primary)`.
* Modifying the `GraphicalHandler` to look for a primary label and use it if possible.
* Added a test to `graphical.rs` to check for this behavior.

These changes were specifically made to be fully backwards compatible.

This is my first time in the miette codebase, so I may have missed a code path. Let me know if I should change anything.

zkat: I think this kind of change is a long time coming and I really appreciate you taking the time to do this!

As far as specifics go: instead of having to add a new attribute, how would you feel about an API where, by default, the first attribute tagged with `#[label]` is treated as the primary one, and if you want a subsequent attribute to be the primary instead, you do `#[label(primary)]`? I think 99.9999% of the time, people don't really want the current "viewport" to be what shows up in that `[line:col]` display, and it's been an ongoing issue. I think a lot of people would be very happy if, without having to make any changes, that display just magically pointed to one of their spans, which will for the most part be the only one to begin with.
willcrichton: I would be happy with that design as well. Two questions:
1. When you say "an API", do you mean a change to the default API everyone uses, or a distinct new part of the miette API surface?
2. If a user wants a format string in a primary label, should the expected syntax be `#[label(primary, "{}", arg)]`?
zkat: 1. Change `#[label]` itself so it starts marking things as "primary"
2. Yes. Basically in https://github.com/zkat/miette/pull/291/files#diff-a775e9b52316667cfcabc8d12a083cba8d18759534d48b7a5da6cd647bc6fe6dR394, to `.or_else(|| labels.first())` (or whatever)
willcrichton: Ok I just updated the code and the tests. How does it look?

Patch:
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 4fafac3..912bfcc 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -10,14 +10,12 @@ jobs:
     name: Check fmt & build docs
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v1
+      - uses: actions/checkout@v4
       - name: Install Rust
-        uses: actions-rs/toolchain@v1
+        uses: dtolnay/rust-toolchain@master
         with:
-          profile: minimal
           toolchain: stable
           components: rustfmt
-          override: true
       - name: rustfmt
         run: cargo fmt --all -- --check
       - name: docs
@@ -32,14 +30,15 @@ jobs:
         os: [ubuntu-latest, macOS-latest, windows-latest]
 
     steps:
-      - uses: actions/checkout@v1
+      - uses: actions/checkout@v4
       - name: Install Rust
-        uses: actions-rs/toolchain@v1
+        uses: dtolnay/rust-toolchain@master
         with:
-          profile: minimal
           toolchain: ${{ matrix.rust }}
           components: clippy
-          override: true
+      - name: Force older version of is-terminal for MSRV builds
+        if: matrix.rust == '1.56.0'
+        run:  cargo update -p is-terminal --precise 0.4.7
       - name: Clippy
         run: cargo clippy --all -- -D warnings
       - name: Run tests
@@ -54,14 +53,12 @@ jobs:
     runs-on: ubuntu-latest
 
     steps:
-      - uses: actions/checkout@v1
+      - uses: actions/checkout@v4
       - name: Install Rust
-        uses: actions-rs/toolchain@v1
+        uses: dtolnay/rust-toolchain@master
         with:
-          profile: minimal
           toolchain: nightly
           components: miri,rust-src
-          override: true
       - name: Run tests with miri
         env:
           MIRIFLAGS: -Zmiri-disable-isolation -Zmiri-strict-provenance
@@ -75,13 +72,11 @@ jobs:
         os: [ubuntu-latest, macOS-latest, windows-latest]
 
     steps:
-      - uses: actions/checkout@v1
+      - uses: actions/checkout@v4
       - name: Install Rust
-        uses: actions-rs/toolchain@v1
+        uses: dtolnay/rust-toolchain@master
         with:
-          profile: minimal
           toolchain: nightly
-          override: true
       - name: Run minimal version build
         run: cargo build -Z minimal-versions --all-features
 
diff --git a/miette-derive/Cargo.toml b/miette-derive/Cargo.toml
index becdc17..b5a7032 100644
--- a/miette-derive/Cargo.toml
+++ b/miette-derive/Cargo.toml
@@ -11,6 +11,6 @@ repository = "https://github.com/zkat/miette"
 proc-macro = true
 
 [dependencies]
-proc-macro2 = "1.0"
+proc-macro2 = "1.0.60"
 quote = "1.0"
 syn = "2.0.11"
diff --git a/miette-derive/src/label.rs b/miette-derive/src/label.rs
index e0bc70a..dd5ec69 100644
--- a/miette-derive/src/label.rs
+++ b/miette-derive/src/label.rs
@@ -20,10 +20,12 @@ struct Label {
     label: Option<Display>,
     ty: syn::Type,
     span: syn::Member,
+    primary: bool,
 }
 
 struct LabelAttr {
     label: Option<Display>,
+    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::<Token![,]>();
+                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::<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 })
+        Ok(LabelAttr { label, primary })
     }
 }
 
@@ -100,12 +119,21 @@ impl Labels {
                         })
                     };
                     use quote::ToTokens;
-                    let LabelAttr { label } =
+                    let LabelAttr { label, primary } =
                         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 {
                         label,
                         span,
                         ty: field.ty.clone(),
+                        primary,
                     });
                 }
             }
@@ -120,13 +148,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, 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 b5dd754..a7399fd 100644
--- a/src/handlers/graphical.rs
+++ b/src/handlers/graphical.rs
@@ -382,15 +382,20 @@ impl GraphicalReportHandler {
         Ok(())
     }
 
-    fn render_context<'a>(
+    fn render_context(
         &self,
         f: &mut impl fmt::Write,
-        source: &'a dyn SourceCode,
+        source: &dyn SourceCode,
         context: &LabeledSpan,
         labels: &[LabeledSpan],
     ) -> 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 67b75d0..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<Item = LabeledSpan>) -> 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
     }
@@ -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 36c3539..be313b1 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<String>,
     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<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 {
         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))


Test patch:
diff --git a/tests/derive.rs b/tests/derive.rs
index 7faae42..dbaf7cb 100644
--- a/tests/derive.rs
+++ b/tests/derive.rs
@@ -189,7 +189,7 @@ fn fmt_help() {
 
     assert_eq!(
         "1 x hello x \"2\"".to_string(),
-        FooStruct("hello".into()).help().unwrap().to_string()
+        FooStruct("hello").help().unwrap().to_string()
     );
 
     #[derive(Debug, Diagnostic, Error)]
@@ -201,12 +201,7 @@ fn fmt_help() {
 
     assert_eq!(
         "1 x hello x \"2\"".to_string(),
-        BarStruct {
-            my_field: "hello".into()
-        }
-        .help()
-        .unwrap()
-        .to_string()
+        BarStruct { my_field: "hello" }.help().unwrap().to_string()
     );
 
     #[derive(Debug, Diagnostic, Error)]
@@ -224,7 +219,7 @@ fn fmt_help() {
 
     assert_eq!(
         "1 x bar x \"2\"".to_string(),
-        FooEnum::X("bar".into()).help().unwrap().to_string()
+        FooEnum::X("bar").help().unwrap().to_string()
     );
 
     assert_eq!(
@@ -250,12 +245,7 @@ fn help_field() {
 
     assert_eq!(
         "x".to_string(),
-        Foo {
-            do_this: Some("x".into())
-        }
-        .help()
-        .unwrap()
-        .to_string()
+        Foo { do_this: Some("x") }.help().unwrap().to_string()
     );
 
     #[derive(Debug, Diagnostic, Error)]
@@ -271,16 +261,11 @@ fn help_field() {
 
     assert_eq!(
         "x".to_string(),
-        Bar::A(Some("x".into())).help().unwrap().to_string()
+        Bar::A(Some("x")).help().unwrap().to_string()
     );
     assert_eq!(
         "x".to_string(),
-        Bar::B {
-            do_this: Some("x".into())
-        }
-        .help()
-        .unwrap()
-        .to_string()
+        Bar::B { do_this: Some("x") }.help().unwrap().to_string()
     );
 
     #[derive(Debug, Diagnostic, Error)]
@@ -288,20 +273,14 @@ fn help_field() {
     #[diagnostic()]
     struct Baz<'a>(#[help] Option<&'a str>);
 
-    assert_eq!(
-        "x".to_string(),
-        Baz(Some("x".into())).help().unwrap().to_string()
-    );
+    assert_eq!("x".to_string(), Baz(Some("x")).help().unwrap().to_string());
 
     #[derive(Debug, Diagnostic, Error)]
     #[error("welp")]
     #[diagnostic()]
     struct Quux<'a>(#[help] &'a str);
 
-    assert_eq!(
-        "x".to_string(),
-        Quux("x".into()).help().unwrap().to_string()
-    );
+    assert_eq!("x".to_string(), Quux("x").help().unwrap().to_string());
 }
 
 #[test]
diff --git a/tests/graphical.rs b/tests/graphical.rs
index 0c69470..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
    · ▲
@@ -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 536aedf..0ca396a 100644
--- a/tests/test_diagnostic_source_macro.rs
+++ b/tests/test_diagnostic_source_macro.rs
@@ -79,7 +79,7 @@ fn test_diagnostic_source() {
 fn test_diagnostic_source_pass_extra_info() {
     let diag = TestBoxedError(Box::new(SourceError {
         code: String::from("Hello\nWorld!"),
-        help: format!("Have you tried turning it on and off again?"),
+        help: String::from("Have you tried turning it on and off again?"),
         label: (1, 4),
     }));
     let mut out = String::new();
@@ -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
diff --git a/tests/test_json.rs b/tests/test_json.rs
index 5bd14cb..ae482b8 100644
--- a/tests/test_json.rs
+++ b/tests/test_json.rs
@@ -52,7 +52,6 @@ mod json_report_handler {
             "related": []
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);
@@ -98,7 +97,6 @@ mod json_report_handler {
             "related": []
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);
@@ -144,7 +142,6 @@ mod json_report_handler {
             "related": []
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);
@@ -190,7 +187,6 @@ mod json_report_handler {
             "related": []
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);
@@ -235,7 +231,6 @@ mod json_report_handler {
             "related": []
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);
@@ -281,7 +276,6 @@ mod json_report_handler {
             "related": []
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);
@@ -347,7 +341,6 @@ mod json_report_handler {
             "related": []
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);
@@ -393,7 +386,6 @@ mod json_report_handler {
             "related": []
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);
@@ -456,7 +448,6 @@ mod json_report_handler {
             "related": []
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);
@@ -532,7 +523,6 @@ mod json_report_handler {
             "related": []
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);
@@ -588,7 +578,6 @@ mod json_report_handler {
             "related": []
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);
@@ -644,7 +633,6 @@ mod json_report_handler {
             "related": []
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);
@@ -700,7 +688,6 @@ mod json_report_handler {
             "related": []
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);
@@ -728,7 +715,6 @@ mod json_report_handler {
             "related": []
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);
@@ -822,7 +808,6 @@ mod json_report_handler {
             }]
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);
@@ -920,7 +905,6 @@ mod json_report_handler {
             }]
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);