Repository: brendanzab/codespan (rust)

Issue:
File column/line should consistently point to the first primary label instead of the first label
Currently when you have a diagnostic, and that diagnostic contains 2 labels, say, one secondary at `1:1` and one primary at `6:3`, the column/line at the top of the diagnostic on rich rendering shows `1:1`, this should instead point to the first primary label, which seems to be the case for short rendering. When you use short rendering it points to `6:3`, this should be made consistent across rendering types.

Pull request description:
fix locus calculation
Changed the multiline_overlap test to cover this case too (and documented why the labels are ordered differently for the future). The output should be unchanged although the ordering of the labels in the struct has been changed.

This should resolve #259 again.

Patch:
diff --git a/codespan-lsp/Cargo.toml b/codespan-lsp/Cargo.toml
index eff879c..a2296a3 100644
--- a/codespan-lsp/Cargo.toml
+++ b/codespan-lsp/Cargo.toml
@@ -16,5 +16,5 @@ codespan-reporting = { version = "0.9.5", path = "../codespan-reporting" }
 # will be valid for all the versions in this range. Getting this range wrong
 # could potentially break down-stream builds on a `cargo update`. This is an
 # absolute no-no, breaking much of what we enjoy about Cargo!
-lsp-types = ">=0.70, <0.81"
+lsp-types = ">=0.70, <0.80"
 url = "2"
diff --git a/codespan-reporting/src/diagnostic.rs b/codespan-reporting/src/diagnostic.rs
index 8f017ba..828aff1 100644
--- a/codespan-reporting/src/diagnostic.rs
+++ b/codespan-reporting/src/diagnostic.rs
@@ -113,7 +113,7 @@ impl<FileId> Label<FileId> {
 /// Represents a diagnostic message that can provide information like errors and
 /// warnings to the user.
 ///
-/// The position of a Diagnostic is considered to be the position of the [`Label`] with a style of [`LabelStyle::primary`] that has the smallest start position.
+/// The position of a Diagnostic is considered to be the position of the [`Label`] that has the earliest starting position and has the highest style which appears in all the labels of the diagnostic.
 #[derive(Clone, Debug)]
 #[cfg_attr(feature = "serialization", derive(Serialize, Deserialize))]
 pub struct Diagnostic<FileId> {
diff --git a/codespan-reporting/src/term.rs b/codespan-reporting/src/term.rs
index 122b038..3b90abb 100644
--- a/codespan-reporting/src/term.rs
+++ b/codespan-reporting/src/term.rs
@@ -126,7 +126,7 @@ pub fn emit<'files, F: Files<'files>>(
 
     let mut renderer = Renderer::new(writer, config);
     match config.display_style {
-        DisplayStyle::Rich => RichDiagnostic::new(diagnostic, config).render(files, &mut renderer),
+        DisplayStyle::Rich => RichDiagnostic::new(diagnostic).render(files, &mut renderer),
         DisplayStyle::Short => ShortDiagnostic::new(diagnostic).render(files, &mut renderer),
     }
 }
diff --git a/codespan-reporting/src/term/renderer.rs b/codespan-reporting/src/term/renderer.rs
index a61f93c..cb79443 100644
--- a/codespan-reporting/src/term/renderer.rs
+++ b/codespan-reporting/src/term/renderer.rs
@@ -127,6 +127,16 @@ impl<'writer, 'config> Renderer<'writer, 'config> {
         &self.config.styles
     }
 
+    // FIXME: Architectural smell - we'd prefer not to pass information from the renderer to the view.
+    pub fn start_context_lines(&self) -> usize {
+        self.config.start_context_lines
+    }
+
+    // FIXME: Architectural smell - we'd prefer not to pass information from the renderer to the view.
+    pub fn end_context_lines(&self) -> usize {
+        self.config.end_context_lines
+    }
+
     /// Diagnostic header, with severity, code, and message.
     ///
     /// ```text
diff --git a/codespan-reporting/src/term/views.rs b/codespan-reporting/src/term/views.rs
index b645421..076a237 100644
--- a/codespan-reporting/src/term/views.rs
+++ b/codespan-reporting/src/term/views.rs
@@ -3,7 +3,7 @@ use std::ops::Range;
 use crate::diagnostic::{Diagnostic, LabelStyle};
 use crate::files::{Files, Location};
 use crate::term::renderer::{Locus, MultiLabel, Renderer, SingleLabel};
-use crate::term::{Config, RenderError};
+use crate::term::RenderError;
 
 /// Count the number of decimal digits in `n`.
 fn count_digits(mut n: usize) -> usize {
@@ -16,20 +16,16 @@ fn count_digits(mut n: usize) -> usize {
 }
 
 /// Output a richly formatted diagnostic, with source code previews.
-pub struct RichDiagnostic<'diagnostic, 'config, FileId> {
+pub struct RichDiagnostic<'diagnostic, FileId> {
     diagnostic: &'diagnostic Diagnostic<FileId>,
-    config: &'config Config,
 }
 
-impl<'diagnostic, 'config, FileId> RichDiagnostic<'diagnostic, 'config, FileId>
+impl<'diagnostic, FileId> RichDiagnostic<'diagnostic, FileId>
 where
     FileId: Copy + PartialEq,
 {
-    pub fn new(
-        diagnostic: &'diagnostic Diagnostic<FileId>,
-        config: &'config Config,
-    ) -> RichDiagnostic<'diagnostic, 'config, FileId> {
-        RichDiagnostic { diagnostic, config }
+    pub fn new(diagnostic: &'diagnostic Diagnostic<FileId>) -> RichDiagnostic<'diagnostic, FileId> {
+        RichDiagnostic { diagnostic }
     }
 
     pub fn render<'files>(
@@ -123,10 +119,11 @@ where
             {
                 Some(labeled_file) => {
                     // another diagnostic also referenced this file
-                    if labeled_file.start > label.range.start
-                        && labeled_file.max_label_style >= label.style
+                    if labeled_file.max_label_style > label.style
+                        || (labeled_file.max_label_style == label.style
+                            && labeled_file.start > label.range.start)
                     {
-                        // this label indicates an earlier start and has at least the same level of style
+                        // this label has a higher style or has the same style but starts earlier
                         labeled_file.start = label.range.start;
                         labeled_file.location = files
                             .location(label.file_id, label.range.start)
@@ -274,10 +271,10 @@ where
                     // The line should be rendered to match the configuration of how much context to show.
                     line.must_render |=
                         // Is this line part of the context after the start of the label?
-                        line_index - start_line_index <= self.config.start_context_lines
+                        line_index - start_line_index <= renderer.start_context_lines()
                         ||
                         // Is this line part of the context before the end of the label?
-                        end_line_index - line_index <= self.config.end_context_lines;
+                        end_line_index - line_index <= renderer.end_context_lines();
                 }
 
                 // Last labeled line


Test patch:
diff --git a/codespan-reporting/tests/term.rs b/codespan-reporting/tests/term.rs
index 4a0fa26..8b1bf2d 100644
--- a/codespan-reporting/tests/term.rs
+++ b/codespan-reporting/tests/term.rs
@@ -569,9 +569,10 @@ mod multiline_overlapping {
                     .with_message("match arms have incompatible types")
                     .with_code("E0308")
                     .with_labels(vec![
+                        // this secondary label is before the primary label to test the locus calculation (see issue #259)
+                        Label::secondary((), 89..134).with_message("this is found to be of type `Result<ByteIndex, LineIndexOutOfBoundsError>`"),
                         Label::primary((), 230..351).with_message("expected enum `Result`, found struct `LineIndexOutOfBoundsError`"),
                         Label::secondary((), 8..362).with_message("`match` arms have incompatible types"),
-                        Label::secondary((), 89..134).with_message("this is found to be of type `Result<ByteIndex, LineIndexOutOfBoundsError>`"),
                         Label::secondary((), 167..195).with_message("this is found to be of type `Result<ByteIndex, LineIndexOutOfBoundsError>`"),
                     ])
                     .with_notes(vec![unindent::unindent(