From 5c33ea3583fc5f8b5fac809ec41551421f5b07f4 Mon Sep 17 00:00:00 2001 From: Christofer Nolander Date: Wed, 27 Jul 2022 22:54:05 +0200 Subject: [PATCH 1/2] Allow adding labels to notes --- codespan-reporting/examples/custom_files.rs | 8 +- codespan-reporting/examples/peg_calculator.rs | 4 +- codespan-reporting/examples/readme_preview.rs | 6 +- codespan-reporting/examples/term.rs | 18 +- codespan-reporting/src/diagnostic.rs | 31 +- codespan-reporting/src/term/views.rs | 699 ++++++++++-------- .../term__note_with_labels__medium_color.snap | 7 + ...rm__note_with_labels__medium_no_color.snap | 7 + ...note_with_labels__rich_ascii_no_color.snap | 17 + .../term__note_with_labels__rich_color.snap | 17 + ...term__note_with_labels__rich_no_color.snap | 17 + .../term__note_with_labels__short_color.snap | 6 + ...erm__note_with_labels__short_no_color.snap | 6 + codespan-reporting/tests/term.rs | 94 ++- 14 files changed, 578 insertions(+), 359 deletions(-) create mode 100644 codespan-reporting/tests/snapshots/term__note_with_labels__medium_color.snap create mode 100644 codespan-reporting/tests/snapshots/term__note_with_labels__medium_no_color.snap create mode 100644 codespan-reporting/tests/snapshots/term__note_with_labels__rich_ascii_no_color.snap create mode 100644 codespan-reporting/tests/snapshots/term__note_with_labels__rich_color.snap create mode 100644 codespan-reporting/tests/snapshots/term__note_with_labels__rich_no_color.snap create mode 100644 codespan-reporting/tests/snapshots/term__note_with_labels__short_color.snap create mode 100644 codespan-reporting/tests/snapshots/term__note_with_labels__short_no_color.snap diff --git a/codespan-reporting/examples/custom_files.rs b/codespan-reporting/examples/custom_files.rs index ba3bdb36..b423fedf 100644 --- a/codespan-reporting/examples/custom_files.rs +++ b/codespan-reporting/examples/custom_files.rs @@ -9,7 +9,7 @@ //! cargo run --example custom_files //! ``` -use codespan_reporting::diagnostic::{Diagnostic, Label}; +use codespan_reporting::diagnostic::{Diagnostic, Label, Note}; use codespan_reporting::term; use codespan_reporting::term::termcolor::{ColorChoice, StandardStream}; use std::ops::Range; @@ -177,8 +177,8 @@ impl Message { .collect(), ) .with_notes(vec![ - "found greetings!".to_owned(), - "pleas no greetings :(".to_owned(), + Note::new("found greetings!".to_owned()), + Note::new("pleas no greetings :(".to_owned()), ]), Message::OverTheTopExclamations { exclamations } => Diagnostic::error() .with_message("over-the-top exclamations") @@ -190,7 +190,7 @@ impl Message { }) .collect(), ) - .with_notes(vec!["ridiculous!".to_owned()]), + .with_notes(vec![Note::new("ridiculous!".to_owned())]), } } } diff --git a/codespan-reporting/examples/peg_calculator.rs b/codespan-reporting/examples/peg_calculator.rs index 882ce6da..93d838b9 100644 --- a/codespan-reporting/examples/peg_calculator.rs +++ b/codespan-reporting/examples/peg_calculator.rs @@ -7,7 +7,7 @@ //! cargo run --example peg_calculator //! ``` -use codespan_reporting::diagnostic::{Diagnostic, Label}; +use codespan_reporting::diagnostic::{Diagnostic, Label, Note}; use codespan_reporting::files::SimpleFile; use codespan_reporting::term; use codespan_reporting::term::termcolor::{ColorChoice, StandardStream}; @@ -59,7 +59,7 @@ fn main() -> anyhow::Result<()> { .with_labels(vec![ Label::primary((), start..start).with_message("parse error") ]) - .with_notes(vec![format!("expected: {}", error.expected)]); + .with_notes(vec![Note::new(format!("expected: {}", error.expected))]); term::emit(&mut writer.lock(), &config, &file, &diagnostic)?; } diff --git a/codespan-reporting/examples/readme_preview.rs b/codespan-reporting/examples/readme_preview.rs index f5a6468a..fc2c466d 100644 --- a/codespan-reporting/examples/readme_preview.rs +++ b/codespan-reporting/examples/readme_preview.rs @@ -7,7 +7,7 @@ //! cargo run --example readme_preview svg > codespan-reporting/assets/readme_preview.svg //! ``` -use codespan_reporting::diagnostic::{Diagnostic, Label}; +use codespan_reporting::diagnostic::{Diagnostic, Label, Note}; use codespan_reporting::files::SimpleFile; use codespan_reporting::term::termcolor::{Color, ColorSpec, StandardStream, WriteColor}; use codespan_reporting::term::{self, ColorArg}; @@ -69,12 +69,12 @@ fn main() -> anyhow::Result<()> { Label::secondary((), 306..312).with_message("this is found to be of type `String`"), Label::secondary((), 186..192).with_message("expected type `String` found here"), ]) - .with_notes(vec![unindent::unindent( + .with_notes(vec![Note::new(unindent::unindent( " expected type `String` found type `Nat` ", - )])]; + ))])]; // let mut files = SimpleFiles::new(); match Opts::from_args() { diff --git a/codespan-reporting/examples/term.rs b/codespan-reporting/examples/term.rs index ee2c057d..94f84a56 100644 --- a/codespan-reporting/examples/term.rs +++ b/codespan-reporting/examples/term.rs @@ -5,7 +5,7 @@ //! cargo run --example term //! ``` -use codespan_reporting::diagnostic::{Diagnostic, Label}; +use codespan_reporting::diagnostic::{Diagnostic, Label, Note}; use codespan_reporting::files::SimpleFiles; use codespan_reporting::term::termcolor::StandardStream; use codespan_reporting::term::{self, ColorArg}; @@ -99,7 +99,7 @@ fn main() -> anyhow::Result<()> { Label::primary(file_id1, 96..102).with_message("unknown builtin") ]) .with_notes(vec![ - "there is a builtin with a similar name: `NATURAL`".to_owned() + Note::new("there is a builtin with a similar name: `NATURAL`".to_owned()) ]), // Unused parameter warning Diagnostic::warning() @@ -107,7 +107,7 @@ fn main() -> anyhow::Result<()> { .with_labels(vec![ Label::primary(file_id1, 285..289).with_message("unused parameter") ]) - .with_notes(vec!["consider using a wildcard pattern: `_`".to_owned()]), + .with_notes(vec![Note::new("consider using a wildcard pattern: `_`".to_owned())]), // Unexpected type error Diagnostic::error() .with_message("unexpected type in application of `_+_`") @@ -117,12 +117,12 @@ fn main() -> anyhow::Result<()> { Label::secondary(file_id1, 130..155) .with_message("based on the definition of `_+_`"), ]) - .with_notes(vec![unindent::unindent( + .with_notes(vec![Note::new(unindent::unindent( " expected type `Nat` found type `String` ", - )]), + ))]), // Incompatible match clause error Diagnostic::error() .with_message("`case` clauses have incompatible types") @@ -134,12 +134,12 @@ fn main() -> anyhow::Result<()> { Label::secondary(file_id3, 41..47) .with_message("expected type `String` found here"), ]) - .with_notes(vec![unindent::unindent( + .with_notes(vec![Note::new(unindent::unindent( " expected type `String` found type `Nat` ", - )]), + ))]), // Incompatible match clause error Diagnostic::error() .with_message("`case` clauses have incompatible types") @@ -157,12 +157,12 @@ fn main() -> anyhow::Result<()> { Label::secondary(file_id3, 186..192) .with_message("expected type `String` found here"), ]) - .with_notes(vec![unindent::unindent( + .with_notes(vec![Note::new(unindent::unindent( " expected type `String` found type `Nat` ", - )]), + ))]), ]; let writer = StandardStream::stderr(opts.color.into()); diff --git a/codespan-reporting/src/diagnostic.rs b/codespan-reporting/src/diagnostic.rs index 8e3abf4f..4f18c20b 100644 --- a/codespan-reporting/src/diagnostic.rs +++ b/codespan-reporting/src/diagnostic.rs @@ -92,6 +92,33 @@ impl Label { } } +/// A note that is associated with the primary cause of the diagnostic. +#[derive(Clone, Debug, PartialEq, Eq)] +#[cfg_attr(feature = "serialization", derive(Serialize, Deserialize))] +pub struct Note { + /// A message that provides additional information about the diagnostic. + /// This can include line breaks for improved formatting. + pub message: String, + /// An optional label showing the cause of the note. + pub labels: Vec>, +} + +impl Note { + /// Create a new note without a label. + pub fn new(message: String) -> Note { + Note { + message, + labels: Vec::new(), + } + } + + /// Add a label to the note. + pub fn with_labels(mut self, mut labels: Vec>) -> Note { + self.labels.append(&mut labels); + self + } +} + /// Represents a diagnostic message that can provide information like errors and /// warnings to the user. /// @@ -115,7 +142,7 @@ pub struct Diagnostic { pub labels: Vec>, /// Notes that are associated with the primary cause of the diagnostic. /// These can include line breaks for improved formatting. - pub notes: Vec, + pub notes: Vec>, } impl Diagnostic { @@ -184,7 +211,7 @@ impl Diagnostic { } /// Add some notes to the diagnostic. - pub fn with_notes(mut self, mut notes: Vec) -> Diagnostic { + pub fn with_notes(mut self, mut notes: Vec>) -> Diagnostic { self.notes.append(&mut notes); self } diff --git a/codespan-reporting/src/term/views.rs b/codespan-reporting/src/term/views.rs index ebfce733..82dbb353 100644 --- a/codespan-reporting/src/term/views.rs +++ b/codespan-reporting/src/term/views.rs @@ -1,6 +1,7 @@ +use std::collections::BTreeMap; use std::ops::Range; -use crate::diagnostic::{Diagnostic, LabelStyle}; +use crate::diagnostic::{Diagnostic, Label, LabelStyle, Severity}; use crate::files::{Error, Files, Location}; use crate::term::renderer::{Locus, MultiLabel, Renderer, SingleLabel}; use crate::term::Config; @@ -38,8 +39,6 @@ where where FileId: 'files, { - use std::collections::BTreeMap; - struct LabeledFile<'diagnostic, FileId> { file_id: FileId, start: usize, @@ -77,220 +76,390 @@ where must_render: bool, } - // TODO: Make this data structure external, to allow for allocation reuse - let mut labeled_files = Vec::>::new(); - // Keep track of the outer padding to use when rendering the - // snippets of source code. - let mut outer_padding = 0; - - // Group labels by file - for label in &self.diagnostic.labels { - let start_line_index = files.line_index(label.file_id, label.range.start)?; - let start_line_number = files.line_number(label.file_id, start_line_index)?; - let start_line_range = files.line_range(label.file_id, start_line_index)?; - let end_line_index = files.line_index(label.file_id, label.range.end)?; - let end_line_number = files.line_number(label.file_id, end_line_index)?; - let end_line_range = files.line_range(label.file_id, end_line_index)?; - - outer_padding = std::cmp::max(outer_padding, count_digits(start_line_number)); - outer_padding = std::cmp::max(outer_padding, count_digits(end_line_number)); - - // NOTE: This could be made more efficient by using an associative - // data structure like a hashmap or B-tree, but we use a vector to - // preserve the order that unique files appear in the list of labels. - let labeled_file = match labeled_files - .iter_mut() - .find(|labeled_file| label.file_id == labeled_file.file_id) - { - Some(labeled_file) => { - // another diagnostic also referenced this file - if labeled_file.max_label_style > label.style - || (labeled_file.max_label_style == label.style - && labeled_file.start > label.range.start) - { - // 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)?; - labeled_file.max_label_style = label.style; + /// Group labels by the file they occur in + fn group_labels_by_file<'diagnostic, 'files, FileId>( + labels: impl Iterator>, + labeled_files: &mut Vec>, + outer_padding: &mut usize, + config: &Config, + files: &'files impl Files<'files, FileId = FileId>, + ) -> Result<(), Error> + where + FileId: Copy + PartialEq + 'files + 'diagnostic, + { + // Group labels by file + for label in labels { + let start_line_index = files.line_index(label.file_id, label.range.start)?; + let start_line_number = files.line_number(label.file_id, start_line_index)?; + let start_line_range = files.line_range(label.file_id, start_line_index)?; + let end_line_index = files.line_index(label.file_id, label.range.end)?; + let end_line_number = files.line_number(label.file_id, end_line_index)?; + let end_line_range = files.line_range(label.file_id, end_line_index)?; + + *outer_padding = std::cmp::max(*outer_padding, count_digits(start_line_number)); + *outer_padding = std::cmp::max(*outer_padding, count_digits(end_line_number)); + + // NOTE: This could be made more efficient by using an associative + // data structure like a hashmap or B-tree, but we use a vector to + // preserve the order that unique files appear in the list of labels. + let labeled_file = match labeled_files + .iter_mut() + .find(|labeled_file| label.file_id == labeled_file.file_id) + { + Some(labeled_file) => { + // another diagnostic also referenced this file + if labeled_file.max_label_style > label.style + || (labeled_file.max_label_style == label.style + && labeled_file.start > label.range.start) + { + // 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)?; + labeled_file.max_label_style = label.style; + } + labeled_file + } + None => { + // no other diagnostic referenced this file yet + labeled_files.push(LabeledFile { + file_id: label.file_id, + start: label.range.start, + name: files.name(label.file_id)?.to_string(), + location: files.location(label.file_id, label.range.start)?, + num_multi_labels: 0, + lines: BTreeMap::new(), + max_label_style: label.style, + }); + // this unwrap should never fail because we just pushed an element + labeled_files + .last_mut() + .expect("just pushed an element that disappeared") } - labeled_file - } - None => { - // no other diagnostic referenced this file yet - labeled_files.push(LabeledFile { - file_id: label.file_id, - start: label.range.start, - name: files.name(label.file_id)?.to_string(), - location: files.location(label.file_id, label.range.start)?, - num_multi_labels: 0, - lines: BTreeMap::new(), - max_label_style: label.style, - }); - // this unwrap should never fail because we just pushed an element - labeled_files - .last_mut() - .expect("just pushed an element that disappeared") - } - }; - - // insert context lines before label - // start from 1 because 0 would be the start of the label itself - for offset in 1..self.config.before_label_lines + 1 { - let index = if let Some(index) = start_line_index.checked_sub(offset) { - index - } else { - // we are going from smallest to largest offset, so if - // the offset can not be subtracted from the start we - // reached the first line - break; }; - if let Ok(range) = files.line_range(label.file_id, index) { - let line = - labeled_file.get_or_insert_line(index, range, start_line_number - offset); - line.must_render = true; - } else { - break; + // insert context lines before label + // start from 1 because 0 would be the start of the label itself + for offset in 1..config.before_label_lines + 1 { + let index = if let Some(index) = start_line_index.checked_sub(offset) { + index + } else { + // we are going from smallest to largest offset, so if + // the offset can not be subtracted from the start we + // reached the first line + break; + }; + + if let Ok(range) = files.line_range(label.file_id, index) { + let line = labeled_file.get_or_insert_line( + index, + range, + start_line_number - offset, + ); + line.must_render = true; + } else { + break; + } } - } - // insert context lines after label - // start from 1 because 0 would be the end of the label itself - for offset in 1..self.config.after_label_lines + 1 { - let index = end_line_index - .checked_add(offset) - .expect("line index too big"); + // insert context lines after label + // start from 1 because 0 would be the end of the label itself + for offset in 1..config.after_label_lines + 1 { + let index = end_line_index + .checked_add(offset) + .expect("line index too big"); + + if let Ok(range) = files.line_range(label.file_id, index) { + let line = + labeled_file.get_or_insert_line(index, range, end_line_number + offset); + line.must_render = true; + } else { + break; + } + } - if let Ok(range) = files.line_range(label.file_id, index) { - let line = - labeled_file.get_or_insert_line(index, range, end_line_number + offset); + if start_line_index == end_line_index { + // Single line + // + // ```text + // 2 │ (+ test "") + // │ ^^ expected `Int` but found `String` + // ``` + let label_start = label.range.start - start_line_range.start; + // Ensure that we print at least one caret, even when we + // have a zero-length source range. + let label_end = + usize::max(label.range.end - start_line_range.start, label_start + 1); + + let line = labeled_file.get_or_insert_line( + start_line_index, + start_line_range, + start_line_number, + ); + + // Ensure that the single line labels are lexicographically + // sorted by the range of source code that they cover. + let index = match line.single_labels.binary_search_by(|(_, range, _)| { + // `Range` doesn't implement `Ord`, so convert to `(usize, usize)` + // to piggyback off its lexicographic comparison implementation. + (range.start, range.end).cmp(&(label_start, label_end)) + }) { + // If the ranges are the same, order the labels in reverse + // to how they were originally specified in the diagnostic. + // This helps with printing in the renderer. + Ok(index) | Err(index) => index, + }; + + line.single_labels + .insert(index, (label.style, label_start..label_end, &label.message)); + + // If this line is not rendered, the SingleLabel is not visible. line.must_render = true; } else { - break; + // Multiple lines + // + // ```text + // 4 │ fizz₁ num = case (mod num 5) (mod num 3) of + // │ ╭─────────────^ + // 5 │ │ 0 0 => "FizzBuzz" + // 6 │ │ 0 _ => "Fizz" + // 7 │ │ _ 0 => "Buzz" + // 8 │ │ _ _ => num + // │ ╰──────────────^ `case` clauses have incompatible types + // ``` + + let label_index = labeled_file.num_multi_labels; + labeled_file.num_multi_labels += 1; + + // First labeled line + let label_start = label.range.start - start_line_range.start; + + let start_line = labeled_file.get_or_insert_line( + start_line_index, + start_line_range.clone(), + start_line_number, + ); + + start_line.multi_labels.push(( + label_index, + label.style, + MultiLabel::Top(label_start), + )); + + // The first line has to be rendered so the start of the label is visible. + start_line.must_render = true; + + // Marked lines + // + // ```text + // 5 │ │ 0 0 => "FizzBuzz" + // 6 │ │ 0 _ => "Fizz" + // 7 │ │ _ 0 => "Buzz" + // ``` + for line_index in (start_line_index + 1)..end_line_index { + let line_range = files.line_range(label.file_id, line_index)?; + let line_number = files.line_number(label.file_id, line_index)?; + + *outer_padding = std::cmp::max(*outer_padding, count_digits(line_number)); + + let line = + labeled_file.get_or_insert_line(line_index, line_range, line_number); + + line.multi_labels + .push((label_index, label.style, MultiLabel::Left)); + + // 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 <= config.start_context_lines + || + // Is this line part of the context before the end of the label? + end_line_index - line_index <= config.end_context_lines; + } + + // Last labeled line + // + // ```text + // 8 │ │ _ _ => num + // │ ╰──────────────^ `case` clauses have incompatible types + // ``` + let label_end = label.range.end - end_line_range.start; + + let end_line = labeled_file.get_or_insert_line( + end_line_index, + end_line_range, + end_line_number, + ); + + end_line.multi_labels.push(( + label_index, + label.style, + MultiLabel::Bottom(label_end, &label.message), + )); + + // The last line has to be rendered so the end of the label is visible. + end_line.must_render = true; } } - if start_line_index == end_line_index { - // Single line - // - // ```text - // 2 │ (+ test "") - // │ ^^ expected `Int` but found `String` - // ``` - let label_start = label.range.start - start_line_range.start; - // Ensure that we print at least one caret, even when we - // have a zero-length source range. - let label_end = - usize::max(label.range.end - start_line_range.start, label_start + 1); - - let line = labeled_file.get_or_insert_line( - start_line_index, - start_line_range, - start_line_number, - ); - - // Ensure that the single line labels are lexicographically - // sorted by the range of source code that they cover. - let index = match line.single_labels.binary_search_by(|(_, range, _)| { - // `Range` doesn't implement `Ord`, so convert to `(usize, usize)` - // to piggyback off its lexicographic comparison implementation. - (range.start, range.end).cmp(&(label_start, label_end)) - }) { - // If the ranges are the same, order the labels in reverse - // to how they were originally specified in the diagnostic. - // This helps with printing in the renderer. - Ok(index) | Err(index) => index, - }; - - line.single_labels - .insert(index, (label.style, label_start..label_end, &label.message)); - - // If this line is not rendered, the SingleLabel is not visible. - line.must_render = true; - } else { - // Multiple lines - // - // ```text - // 4 │ fizz₁ num = case (mod num 5) (mod num 3) of - // │ ╭─────────────^ - // 5 │ │ 0 0 => "FizzBuzz" - // 6 │ │ 0 _ => "Fizz" - // 7 │ │ _ 0 => "Buzz" - // 8 │ │ _ _ => num - // │ ╰──────────────^ `case` clauses have incompatible types - // ``` - - let label_index = labeled_file.num_multi_labels; - labeled_file.num_multi_labels += 1; - - // First labeled line - let label_start = label.range.start - start_line_range.start; - - let start_line = labeled_file.get_or_insert_line( - start_line_index, - start_line_range.clone(), - start_line_number, - ); - - start_line.multi_labels.push(( - label_index, - label.style, - MultiLabel::Top(label_start), - )); + Ok(()) + } - // The first line has to be rendered so the start of the label is visible. - start_line.must_render = true; + fn render_labels<'diagnostic, 'files, FileId>( + renderer: &mut Renderer<'_, '_>, + labeled_files: Vec>, + outer_padding: usize, + severity: Severity, + final_snippet: bool, + files: &'files impl Files<'files, FileId = FileId>, + ) -> Result<(), Error> + where + FileId: Copy + PartialEq + 'files + 'diagnostic, + { + // Source snippets + // + // ```text + // ┌─ test:2:9 + // │ + // 2 │ (+ test "") + // │ ^^ expected `Int` but found `String` + // │ + // ``` + let mut labeled_files = labeled_files.into_iter().peekable(); + while let Some(labeled_file) = labeled_files.next() { + let source = files.source(labeled_file.file_id)?; + let source = source.as_ref(); - // Marked lines + // Top left border and locus. // // ```text - // 5 │ │ 0 0 => "FizzBuzz" - // 6 │ │ 0 _ => "Fizz" - // 7 │ │ _ 0 => "Buzz" + // ┌─ test:2:9 // ``` - for line_index in (start_line_index + 1)..end_line_index { - let line_range = files.line_range(label.file_id, line_index)?; - let line_number = files.line_number(label.file_id, line_index)?; + if !labeled_file.lines.is_empty() { + renderer.render_snippet_start( + outer_padding, + &Locus { + name: labeled_file.name, + location: labeled_file.location, + }, + )?; + renderer.render_snippet_empty( + outer_padding, + severity, + labeled_file.num_multi_labels, + &[], + )?; + } - outer_padding = std::cmp::max(outer_padding, count_digits(line_number)); + let mut lines = labeled_file + .lines + .iter() + .filter(|(_, line)| line.must_render) + .peekable(); + + while let Some((line_index, line)) = lines.next() { + renderer.render_snippet_source( + outer_padding, + line.number, + &source[line.range.clone()], + severity, + &line.single_labels, + labeled_file.num_multi_labels, + &line.multi_labels, + )?; + + // Check to see if we need to render any intermediate stuff + // before rendering the next line. + if let Some((next_line_index, next_line)) = lines.peek() { + match next_line_index.checked_sub(*line_index) { + // Consecutive lines + Some(1) => {} + // One line between the current line and the next line + Some(2) => { + // Write a source line + let file_id = labeled_file.file_id; + + // This line was not intended to be rendered initially. + // To render the line right, we have to get back the original labels. + let labels = labeled_file + .lines + .get(&(line_index + 1)) + .map_or(&[][..], |line| &line.multi_labels[..]); + + renderer.render_snippet_source( + outer_padding, + files.line_number(file_id, line_index + 1)?, + &source[files.line_range(file_id, line_index + 1)?], + severity, + &[], + labeled_file.num_multi_labels, + labels, + )?; + } + // More than one line between the current line and the next line. + Some(_) | None => { + // Source break + // + // ```text + // · + // ``` + renderer.render_snippet_break( + outer_padding, + severity, + labeled_file.num_multi_labels, + &next_line.multi_labels, + )?; + } + } + } + } - let line = labeled_file.get_or_insert_line(line_index, line_range, line_number); + // Check to see if we should render a trailing border after the + // final line of the snippet. + if labeled_files.peek().is_none() && final_snippet { + // We don't render a border if we are at the final newline + // without trailing notes, because it would end up looking too + // spaced-out in combination with the final new line. + } else { + // Render the trailing snippet border. + renderer.render_snippet_empty( + outer_padding, + severity, + labeled_file.num_multi_labels, + &[], + )?; + } + } + Ok(()) + } - line.multi_labels - .push((label_index, label.style, MultiLabel::Left)); + // Keep track of the outer padding to use when rendering the + // snippets of source code. + let mut outer_padding = 0; - // 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 - || - // Is this line part of the context before the end of the label? - end_line_index - line_index <= self.config.end_context_lines; - } + // TODO: Make this data structure external, to allow for allocation reuse + let mut labeled_files = Vec::>::new(); + group_labels_by_file( + self.diagnostic.labels.iter(), + &mut labeled_files, + &mut outer_padding, + self.config, + files, + )?; - // Last labeled line - // - // ```text - // 8 │ │ _ _ => num - // │ ╰──────────────^ `case` clauses have incompatible types - // ``` - let label_end = label.range.end - end_line_range.start; - - let end_line = labeled_file.get_or_insert_line( - end_line_index, - end_line_range, - end_line_number, - ); - - end_line.multi_labels.push(( - label_index, - label.style, - MultiLabel::Bottom(label_end, &label.message), - )); - - // The last line has to be rendered so the end of the label is visible. - end_line.must_render = true; - } + // For each note: its labeles grouped by the files they occur in + let mut labeled_files_notes = + Vec::>>::with_capacity(self.diagnostic.notes.len()); + for note in self.diagnostic.notes.iter() { + let mut labeled_files = Vec::new(); + group_labels_by_file( + note.labels.iter(), + &mut labeled_files, + &mut outer_padding, + self.config, + files, + )?; + labeled_files_notes.push(labeled_files); } // Header and message @@ -305,120 +474,14 @@ where self.diagnostic.message.as_str(), )?; - // Source snippets - // - // ```text - // ┌─ test:2:9 - // │ - // 2 │ (+ test "") - // │ ^^ expected `Int` but found `String` - // │ - // ``` - let mut labeled_files = labeled_files.into_iter().peekable(); - while let Some(labeled_file) = labeled_files.next() { - let source = files.source(labeled_file.file_id)?; - let source = source.as_ref(); - - // Top left border and locus. - // - // ```text - // ┌─ test:2:9 - // ``` - if !labeled_file.lines.is_empty() { - renderer.render_snippet_start( - outer_padding, - &Locus { - name: labeled_file.name, - location: labeled_file.location, - }, - )?; - renderer.render_snippet_empty( - outer_padding, - self.diagnostic.severity, - labeled_file.num_multi_labels, - &[], - )?; - } - - let mut lines = labeled_file - .lines - .iter() - .filter(|(_, line)| line.must_render) - .peekable(); - - while let Some((line_index, line)) = lines.next() { - renderer.render_snippet_source( - outer_padding, - line.number, - &source[line.range.clone()], - self.diagnostic.severity, - &line.single_labels, - labeled_file.num_multi_labels, - &line.multi_labels, - )?; - - // Check to see if we need to render any intermediate stuff - // before rendering the next line. - if let Some((next_line_index, next_line)) = lines.peek() { - match next_line_index.checked_sub(*line_index) { - // Consecutive lines - Some(1) => {} - // One line between the current line and the next line - Some(2) => { - // Write a source line - let file_id = labeled_file.file_id; - - // This line was not intended to be rendered initially. - // To render the line right, we have to get back the original labels. - let labels = labeled_file - .lines - .get(&(line_index + 1)) - .map_or(&[][..], |line| &line.multi_labels[..]); - - renderer.render_snippet_source( - outer_padding, - files.line_number(file_id, line_index + 1)?, - &source[files.line_range(file_id, line_index + 1)?], - self.diagnostic.severity, - &[], - labeled_file.num_multi_labels, - labels, - )?; - } - // More than one line between the current line and the next line. - Some(_) | None => { - // Source break - // - // ```text - // · - // ``` - renderer.render_snippet_break( - outer_padding, - self.diagnostic.severity, - labeled_file.num_multi_labels, - &next_line.multi_labels, - )?; - } - } - } - } - - // Check to see if we should render a trailing border after the - // final line of the snippet. - if labeled_files.peek().is_none() && self.diagnostic.notes.is_empty() { - // We don't render a border if we are at the final newline - // without trailing notes, because it would end up looking too - // spaced-out in combination with the final new line. - } else { - // Render the trailing snippet border. - renderer.render_snippet_empty( - outer_padding, - self.diagnostic.severity, - labeled_file.num_multi_labels, - &[], - )?; - } - } + render_labels( + renderer, + labeled_files, + outer_padding, + self.diagnostic.severity, + self.diagnostic.notes.is_empty(), + files, + )?; // Additional notes // @@ -426,8 +489,18 @@ where // = expected type `Int` // found type `String` // ``` - for note in &self.diagnostic.notes { - renderer.render_snippet_note(outer_padding, note)?; + let notes = self.diagnostic.notes.iter().zip(labeled_files_notes); + let mut notes = notes.peekable(); + while let Some((note, labeled_files)) = notes.next() { + renderer.render_snippet_note(outer_padding, ¬e.message)?; + render_labels( + renderer, + labeled_files, + outer_padding, + Severity::Note, + notes.peek().is_none(), + files, + )?; } renderer.render_empty() } @@ -504,7 +577,7 @@ where // found type `String` // ``` for note in &self.diagnostic.notes { - renderer.render_snippet_note(0, note)?; + renderer.render_snippet_note(0, ¬e.message)?; } } diff --git a/codespan-reporting/tests/snapshots/term__note_with_labels__medium_color.snap b/codespan-reporting/tests/snapshots/term__note_with_labels__medium_color.snap new file mode 100644 index 00000000..bb96c88b --- /dev/null +++ b/codespan-reporting/tests/snapshots/term__note_with_labels__medium_color.snap @@ -0,0 +1,7 @@ +--- +source: codespan-reporting/tests/term.rs +expression: TEST_DATA.emit_color(&config) +--- +one_line.rs:1:1: {fg:Red bold bright}error{bold bright}: cycle detected when evaluating constant `A`{/} + {fg:Blue}={/} ...which requires evaluating constant `B`... + diff --git a/codespan-reporting/tests/snapshots/term__note_with_labels__medium_no_color.snap b/codespan-reporting/tests/snapshots/term__note_with_labels__medium_no_color.snap new file mode 100644 index 00000000..8d2ded01 --- /dev/null +++ b/codespan-reporting/tests/snapshots/term__note_with_labels__medium_no_color.snap @@ -0,0 +1,7 @@ +--- +source: codespan-reporting/tests/term.rs +expression: TEST_DATA.emit_no_color(&config) +--- +one_line.rs:1:1: error: cycle detected when evaluating constant `A` + = ...which requires evaluating constant `B`... + diff --git a/codespan-reporting/tests/snapshots/term__note_with_labels__rich_ascii_no_color.snap b/codespan-reporting/tests/snapshots/term__note_with_labels__rich_ascii_no_color.snap new file mode 100644 index 00000000..399b22cd --- /dev/null +++ b/codespan-reporting/tests/snapshots/term__note_with_labels__rich_ascii_no_color.snap @@ -0,0 +1,17 @@ +--- +source: codespan-reporting/tests/term.rs +expression: TEST_DATA.emit_no_color(&config) +--- +error: cycle detected when evaluating constant `A` + --> one_line.rs:1:1 + | +1 | const A: u32 = B; + | ^^^^^^^^^^^^^^^^^ + | + = ...which requires evaluating constant `B`... + --> one_line.rs:2:1 + | +2 | const B: u32 = A; + | ----------------- + + diff --git a/codespan-reporting/tests/snapshots/term__note_with_labels__rich_color.snap b/codespan-reporting/tests/snapshots/term__note_with_labels__rich_color.snap new file mode 100644 index 00000000..ff0b5b4c --- /dev/null +++ b/codespan-reporting/tests/snapshots/term__note_with_labels__rich_color.snap @@ -0,0 +1,17 @@ +--- +source: codespan-reporting/tests/term.rs +expression: TEST_DATA.emit_color(&config) +--- +{fg:Red bold bright}error{bold bright}: cycle detected when evaluating constant `A`{/} + {fg:Blue}┌─{/} one_line.rs:1:1 + {fg:Blue}│{/} +{fg:Blue}1{/} {fg:Blue}│{/} {fg:Red}const A: u32 = B;{/} + {fg:Blue}│{/} {fg:Red}^^^^^^^^^^^^^^^^^{/} + {fg:Blue}│{/} + {fg:Blue}={/} ...which requires evaluating constant `B`... + {fg:Blue}┌─{/} one_line.rs:2:1 + {fg:Blue}│{/} +{fg:Blue}2{/} {fg:Blue}│{/} const B: u32 = A; + {fg:Blue}│{/} {fg:Blue}-----------------{/} + + diff --git a/codespan-reporting/tests/snapshots/term__note_with_labels__rich_no_color.snap b/codespan-reporting/tests/snapshots/term__note_with_labels__rich_no_color.snap new file mode 100644 index 00000000..e97cf086 --- /dev/null +++ b/codespan-reporting/tests/snapshots/term__note_with_labels__rich_no_color.snap @@ -0,0 +1,17 @@ +--- +source: codespan-reporting/tests/term.rs +expression: TEST_DATA.emit_no_color(&config) +--- +error: cycle detected when evaluating constant `A` + ┌─ one_line.rs:1:1 + │ +1 │ const A: u32 = B; + │ ^^^^^^^^^^^^^^^^^ + │ + = ...which requires evaluating constant `B`... + ┌─ one_line.rs:2:1 + │ +2 │ const B: u32 = A; + │ ----------------- + + diff --git a/codespan-reporting/tests/snapshots/term__note_with_labels__short_color.snap b/codespan-reporting/tests/snapshots/term__note_with_labels__short_color.snap new file mode 100644 index 00000000..341e4562 --- /dev/null +++ b/codespan-reporting/tests/snapshots/term__note_with_labels__short_color.snap @@ -0,0 +1,6 @@ +--- +source: codespan-reporting/tests/term.rs +expression: TEST_DATA.emit_color(&config) +--- +one_line.rs:1:1: {fg:Red bold bright}error{bold bright}: cycle detected when evaluating constant `A`{/} + diff --git a/codespan-reporting/tests/snapshots/term__note_with_labels__short_no_color.snap b/codespan-reporting/tests/snapshots/term__note_with_labels__short_no_color.snap new file mode 100644 index 00000000..695fb6ef --- /dev/null +++ b/codespan-reporting/tests/snapshots/term__note_with_labels__short_no_color.snap @@ -0,0 +1,6 @@ +--- +source: codespan-reporting/tests/term.rs +expression: TEST_DATA.emit_no_color(&config) +--- +one_line.rs:1:1: error: cycle detected when evaluating constant `A` + diff --git a/codespan-reporting/tests/term.rs b/codespan-reporting/tests/term.rs index eb5f7ad1..60a1d5cd 100644 --- a/codespan-reporting/tests/term.rs +++ b/codespan-reporting/tests/term.rs @@ -1,4 +1,4 @@ -use codespan_reporting::diagnostic::{Diagnostic, Label}; +use codespan_reporting::diagnostic::{Diagnostic, Label, Note}; use codespan_reporting::files::{SimpleFile, SimpleFiles}; use codespan_reporting::term::{termcolor::Color, Chars, Config, DisplayStyle, Styles}; @@ -158,7 +158,7 @@ mod same_line { Diagnostic::error() .with_message("aborting due to previous error") .with_notes(vec![ - "For more information about this error, try `rustc --explain E0499`.".to_owned(), + Note::new("For more information about this error, try `rustc --explain E0499`.".to_owned()), ]), ]; @@ -294,16 +294,16 @@ mod overlapping { .with_message("required by this bound in `std::thread::spawn`"), ]) .with_notes(vec![ - "help: within `[closure@no_send_res_ports.rs:29:19: 33:6 x:main::Foo]`, the trait `std::marker::Send` is not implemented for `std::rc::Rc<()>`".to_owned(), - "note: required because it appears within the type `Port<()>`".to_owned(), - "note: required because it appears within the type `main::Foo`".to_owned(), - "note: required because it appears within the type `[closure@no_send_res_ports.rs:29:19: 33:6 x:main::Foo]`".to_owned(), + Note::new("help: within `[closure@no_send_res_ports.rs:29:19: 33:6 x:main::Foo]`, the trait `std::marker::Send` is not implemented for `std::rc::Rc<()>`".to_owned()), + Note::new("note: required because it appears within the type `Port<()>`".to_owned()), + Note::new("note: required because it appears within the type `main::Foo`".to_owned()), + Note::new("note: required because it appears within the type `[closure@no_send_res_ports.rs:29:19: 33:6 x:main::Foo]`".to_owned()), ]), Diagnostic::error() .with_message("aborting due 5 previous errors") .with_notes(vec![ - "Some errors have detailed explanations: E0121, E0277, E0666.".to_owned(), - "For more information about an error, try `rustc --explain E0121`.".to_owned(), + Note::new("Some errors have detailed explanations: E0121, E0277, E0666.".to_owned()), + Note::new("For more information about an error, try `rustc --explain E0121`.".to_owned()), ]), ]; @@ -355,10 +355,10 @@ mod message_and_notes { let files = SimpleFiles::new(); let diagnostics = vec![ - Diagnostic::error().with_message("a message").with_notes(vec!["a note".to_owned()]), - Diagnostic::warning().with_message("a message").with_notes(vec!["a note".to_owned()]), - Diagnostic::note().with_message("a message").with_notes(vec!["a note".to_owned()]), - Diagnostic::help().with_message("a message").with_notes(vec!["a note".to_owned()]), + Diagnostic::error().with_message("a message").with_notes(vec![Note::new("a note".to_owned())]), + Diagnostic::warning().with_message("a message").with_notes(vec![Note::new("a note".to_owned())]), + Diagnostic::note().with_message("a message").with_notes(vec![Note::new("a note".to_owned())]), + Diagnostic::help().with_message("a message").with_notes(vec![Note::new("a note".to_owned())]), ]; TestData { files, diagnostics } @@ -517,13 +517,13 @@ mod multifile { .with_message("unknown builtin: `NATRAL`") .with_labels(vec![Label::primary(file_id1, 96..102).with_message("unknown builtin")]) .with_notes(vec![ - "there is a builtin with a similar name: `NATURAL`".to_owned(), + Note::new("there is a builtin with a similar name: `NATURAL`".to_owned()), ]), // Unused parameter warning Diagnostic::warning() .with_message("unused parameter pattern: `n₂`") .with_labels(vec![Label::primary(file_id1, 285..289).with_message("unused parameter")]) - .with_notes(vec!["consider using a wildcard pattern: `_`".to_owned()]), + .with_notes(vec![Note::new("consider using a wildcard pattern: `_`".to_owned())]), // Unexpected type error Diagnostic::error() .with_message("unexpected type in application of `_+_`") @@ -532,12 +532,12 @@ mod multifile { Label::primary(file_id2, 37..44).with_message("expected `Nat`, found `String`"), Label::secondary(file_id1, 130..155).with_message("based on the definition of `_+_`"), ]) - .with_notes(vec![unindent::unindent( + .with_notes(vec![Note::new(unindent::unindent( " expected type `Nat` found type `String` ", - )]), + ))]), ]; TestData { files, diagnostics } @@ -594,12 +594,12 @@ mod fizz_buzz { Label::secondary(file_id, 62..166).with_message("`case` clauses have incompatible types"), Label::secondary(file_id, 41..47).with_message("expected type `String` found here"), ]) - .with_notes(vec![unindent::unindent( + .with_notes(vec![Note::new(unindent::unindent( " expected type `String` found type `Nat` ", - )]), + ))]), // Incompatible match clause error Diagnostic::error() .with_message("`case` clauses have incompatible types") @@ -612,12 +612,12 @@ mod fizz_buzz { Label::secondary(file_id, 306..312).with_message("this is found to be of type `String`"), Label::secondary(file_id, 186..192).with_message("expected type `String` found here"), ]) - .with_notes(vec![unindent::unindent( + .with_notes(vec![Note::new(unindent::unindent( " expected type `String` found type `Nat` ", - )]), + ))]), ]; TestData { files, diagnostics } @@ -663,12 +663,12 @@ mod multiline_overlapping { Label::secondary((), 8..362).with_message("`match` arms have incompatible types"), Label::secondary((), 167..195).with_message("this is found to be of type `Result`"), ]) - .with_notes(vec![unindent::unindent( + .with_notes(vec![Note::new(unindent::unindent( " expected type `Result` found type `LineIndexOutOfBoundsError` ", - )]), + ))]), ]; TestData { files: file, diagnostics } @@ -847,7 +847,7 @@ mod unicode { Label::primary((), prefix.len()..(prefix.len() + abi.len())) .with_message("invalid ABI"), ]) - .with_notes(vec![unindent::unindent( + .with_notes(vec![Note::new(unindent::unindent( " valid ABIs: - aapcs @@ -871,11 +871,11 @@ mod unicode { - win64 - x86-interrupt ", - )]), + ))]), Diagnostic::error() .with_message("aborting due to previous error") .with_notes(vec![ - "For more information about this error, try `rustc --explain E0703`.".to_owned(), + Note::new("For more information about this error, try `rustc --explain E0703`.".to_owned()), ]), ]; @@ -1041,7 +1041,7 @@ mod multiline_omit { Label::secondary(file_id2, 55..55).with_message("missing whitespace"), ]) .with_notes(vec![ - "note:\texpected type `()`\n\tfound type `{integer}`".to_owned() + Note::new("note:\texpected type `()`\n\tfound type `{integer}`".to_owned()) ]), ]; @@ -1106,3 +1106,45 @@ mod surrounding_lines { test_emit!(rich_no_color); } + +mod note_with_labels { + use super::*; + + lazy_static::lazy_static! { + static ref TEST_DATA: TestData<'static, SimpleFiles<&'static str, String>> = { + let mut files = SimpleFiles::new(); + + let file_id = files.add( + "one_line.rs", + unindent::unindent(r#" + const A: u32 = B; + const B: u32 = A; + "#), + ); + + let diagnostics = vec![ + Diagnostic::error() + .with_message("cycle detected when evaluating constant `A`") + .with_labels(vec![ + Label::primary(file_id, 0..17), + ]) + .with_notes(vec![ + Note::new("...which requires evaluating constant `B`...".to_owned()) + .with_labels(vec![ + Label::secondary(file_id, 18..35) + ]), + ]), + ]; + + TestData { files, diagnostics } + }; + } + + test_emit!(rich_color); + test_emit!(medium_color); + test_emit!(short_color); + test_emit!(rich_no_color); + test_emit!(medium_no_color); + test_emit!(short_no_color); + test_emit!(rich_ascii_no_color); +} From 3c2054621bbcb8a361d92f3ef0f8ab94df5c2bb3 Mon Sep 17 00:00:00 2001 From: Christofer Nolander Date: Wed, 27 Jul 2022 23:02:31 +0200 Subject: [PATCH 2/2] Add additional note to note_with_labels test --- .../snapshots/term__note_with_labels__medium_color.snap | 1 + .../snapshots/term__note_with_labels__medium_no_color.snap | 1 + .../term__note_with_labels__rich_ascii_no_color.snap | 6 ++++++ .../tests/snapshots/term__note_with_labels__rich_color.snap | 6 ++++++ .../snapshots/term__note_with_labels__rich_no_color.snap | 6 ++++++ codespan-reporting/tests/term.rs | 4 ++++ 6 files changed, 24 insertions(+) diff --git a/codespan-reporting/tests/snapshots/term__note_with_labels__medium_color.snap b/codespan-reporting/tests/snapshots/term__note_with_labels__medium_color.snap index bb96c88b..de6bfcb5 100644 --- a/codespan-reporting/tests/snapshots/term__note_with_labels__medium_color.snap +++ b/codespan-reporting/tests/snapshots/term__note_with_labels__medium_color.snap @@ -4,4 +4,5 @@ expression: TEST_DATA.emit_color(&config) --- one_line.rs:1:1: {fg:Red bold bright}error{bold bright}: cycle detected when evaluating constant `A`{/} {fg:Blue}={/} ...which requires evaluating constant `B`... + {fg:Blue}={/} ...which requires evaluating constant `A` diff --git a/codespan-reporting/tests/snapshots/term__note_with_labels__medium_no_color.snap b/codespan-reporting/tests/snapshots/term__note_with_labels__medium_no_color.snap index 8d2ded01..2eee651a 100644 --- a/codespan-reporting/tests/snapshots/term__note_with_labels__medium_no_color.snap +++ b/codespan-reporting/tests/snapshots/term__note_with_labels__medium_no_color.snap @@ -4,4 +4,5 @@ expression: TEST_DATA.emit_no_color(&config) --- one_line.rs:1:1: error: cycle detected when evaluating constant `A` = ...which requires evaluating constant `B`... + = ...which requires evaluating constant `A` diff --git a/codespan-reporting/tests/snapshots/term__note_with_labels__rich_ascii_no_color.snap b/codespan-reporting/tests/snapshots/term__note_with_labels__rich_ascii_no_color.snap index 399b22cd..4c3fa450 100644 --- a/codespan-reporting/tests/snapshots/term__note_with_labels__rich_ascii_no_color.snap +++ b/codespan-reporting/tests/snapshots/term__note_with_labels__rich_ascii_no_color.snap @@ -13,5 +13,11 @@ error: cycle detected when evaluating constant `A` | 2 | const B: u32 = A; | ----------------- + | + = ...which requires evaluating constant `A` + --> one_line.rs:1:1 + | +1 | const A: u32 = B; + | ----------------- diff --git a/codespan-reporting/tests/snapshots/term__note_with_labels__rich_color.snap b/codespan-reporting/tests/snapshots/term__note_with_labels__rich_color.snap index ff0b5b4c..8695a0d1 100644 --- a/codespan-reporting/tests/snapshots/term__note_with_labels__rich_color.snap +++ b/codespan-reporting/tests/snapshots/term__note_with_labels__rich_color.snap @@ -13,5 +13,11 @@ expression: TEST_DATA.emit_color(&config) {fg:Blue}│{/} {fg:Blue}2{/} {fg:Blue}│{/} const B: u32 = A; {fg:Blue}│{/} {fg:Blue}-----------------{/} + {fg:Blue}│{/} + {fg:Blue}={/} ...which requires evaluating constant `A` + {fg:Blue}┌─{/} one_line.rs:1:1 + {fg:Blue}│{/} +{fg:Blue}1{/} {fg:Blue}│{/} const A: u32 = B; + {fg:Blue}│{/} {fg:Blue}-----------------{/} diff --git a/codespan-reporting/tests/snapshots/term__note_with_labels__rich_no_color.snap b/codespan-reporting/tests/snapshots/term__note_with_labels__rich_no_color.snap index e97cf086..bb326fc0 100644 --- a/codespan-reporting/tests/snapshots/term__note_with_labels__rich_no_color.snap +++ b/codespan-reporting/tests/snapshots/term__note_with_labels__rich_no_color.snap @@ -13,5 +13,11 @@ error: cycle detected when evaluating constant `A` │ 2 │ const B: u32 = A; │ ----------------- + │ + = ...which requires evaluating constant `A` + ┌─ one_line.rs:1:1 + │ +1 │ const A: u32 = B; + │ ----------------- diff --git a/codespan-reporting/tests/term.rs b/codespan-reporting/tests/term.rs index 60a1d5cd..ac9bdb86 100644 --- a/codespan-reporting/tests/term.rs +++ b/codespan-reporting/tests/term.rs @@ -1133,6 +1133,10 @@ mod note_with_labels { .with_labels(vec![ Label::secondary(file_id, 18..35) ]), + Note::new("...which requires evaluating constant `A`".to_owned()) + .with_labels(vec![ + Label::secondary(file_id, 0..17) + ]), ]), ];