From 5c0e224d3c6f15330e6aec4794a12c3c0637d990 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 28 Apr 2022 23:37:07 -0400 Subject: [PATCH] tamer: diagnose::report: Line numbers in gutter Alright, starting to settle on an abstraction now, and things are coming together. This gives us line numbers in the previously-empty gutter, and widens the gutter to accommodate. Gutters are normalized across sections. Sections are not yet collapsed for sequential line numbers in the same context. Exciting! Here's an example, on an xmlo file: error: expected closing tag for `preproc:symtable` --> /home/.../foo.xmlo:16:4 | 16 | | ----------------- note: element `preproc:symtable` is opened here --> /home/.../foo.xmlo:11326:4 | 11326 | | ^^^^^^^^^^^^^^^^ error: expected `` DEV-12151 --- tamer/src/diagnose/report.rs | 96 ++++++++++++++++-- tamer/src/diagnose/report/test/integration.rs | 98 ++++++++++++++----- tamer/src/diagnose/resolver.rs | 4 + tamer/src/lib.rs | 1 + 4 files changed, 169 insertions(+), 30 deletions(-) diff --git a/tamer/src/diagnose/report.rs b/tamer/src/diagnose/report.rs index 144e8428..89c05ce3 100644 --- a/tamer/src/diagnose/report.rs +++ b/tamer/src/diagnose/report.rs @@ -34,6 +34,7 @@ use crate::span::{Context, Span, UNKNOWN_SPAN}; use std::{ fmt::{self, Display, Write}, num::NonZeroU32, + ops::Add, }; pub trait Reporter { @@ -105,6 +106,11 @@ impl Reporter for VisualReporter { let mut report = Report::empty(Message(diagnostic)); report.extend(mspans.into_iter().map(Into::into)); + + // Make each section's gutters the same size, + // which is more aesthetically pleasing. + report.normalize_gutters(); + report } } @@ -170,6 +176,7 @@ pub struct Report<'d, D: Diagnostic> { msg: Message<'d, D>, secs: Vec>, level: Level, + line_max: NonZeroU32, } impl<'d, D: Diagnostic> Report<'d, D> { @@ -178,6 +185,17 @@ impl<'d, D: Diagnostic> Report<'d, D> { msg, secs: Vec::new(), level: Level::default(), + line_max: NonZeroU32::MIN, + } + } + + /// Make all sections' gutters the same width. + /// + /// This is only necessary because [`Section`] is expected to be wholly + /// self-contained when rendering. + fn normalize_gutters(&mut self) { + for sec in self.secs.iter_mut() { + sec.line_max = self.line_max; } } } @@ -186,6 +204,7 @@ impl<'d, D: Diagnostic> Extend> for Report<'d, D> { fn extend>>(&mut self, secs: T) { for sec in secs { self.level = self.level.min(sec.level()); + self.line_max = self.line_max.max(sec.line_max); // Add the section if it cannot be squashed into the previous. let remain = sec.maybe_squash_into(self.secs.last_mut()); @@ -238,6 +257,7 @@ struct Section<'d> { level: Level, span: Span, body: Vec>, + line_max: NonZeroU32, } impl<'s, 'd> Section<'d> { @@ -280,6 +300,25 @@ impl<'s, 'd> Section<'d> { _ => Some(self), } } + + /// Maximum width of the text in the gutter. + /// + /// Note that the gutter contains a single character of padding before + /// its delimiter, + /// which this width _does not_ account for. + /// + /// The minimum width is 2. + /// + /// ```text + /// --> heading + /// | + /// 1 | source line + /// ^^ + /// gutter width is 2 + /// ``` + fn gutter_text_width(&self) -> usize { + self.line_max.log10().add(1).max(2) as usize + } } impl<'d, 'a, S> From> for Section<'d> @@ -291,6 +330,7 @@ where let syslines = mspan.system_lines(); let mut body = Vec::new(); + let mut line_max = NonZeroU32::MIN; let (span, level) = match mspan { MaybeResolvedSpan::Resolved(rspan, oslabel) => { @@ -305,6 +345,13 @@ where let nlines = src.len(); src.into_iter().enumerate().for_each(|(i, srcline)| { + let line_num = srcline.num(); + + // Note that lines are intentionally _not_ ordered, + // since reports may jump around a file to produce a + // narrative. + line_max = line_max.max(line_num); + let label = if i == nlines - 1 { olabel.take() } else { None }; @@ -343,16 +390,24 @@ where span, level, body, + line_max, } } } impl<'d> Display for Section<'d> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, " {heading}\n", heading = self.heading)?; + let gutterw = self.gutter_text_width(); + + // The heading has a hanging indentation, + // which is accomplished by simply omitting spaces above the + // gutter's `" |"`, + // which amounts to two characters wide. + write!(f, "{:gutterw$}{heading}\n", "", heading = self.heading)?; self.body.iter().try_for_each(|line| { - write!(f, " {delim}{line}\n", delim = line.gutter_delim()) + line.fmt_gutter(gutterw, f)?; + write!(f, "{line}\n") }) } } @@ -507,20 +562,35 @@ enum SectionLine<'d> { } impl<'d> SectionLine<'d> { - /// Delimiter used to separate the gutter from the body. + /// Format the gutter to the left of the section body for this line. + /// + /// Note that the provided `text_width` _does not include_ a single + /// character of padding and a single-character delimiter that are + /// expected to follow. + /// The width is guaranteed to be at least as wide as the number of + /// characters needed to represent the line number in base-10. /// /// For example: /// /// ```text /// | - /// 12 | source line - /// | ^^^^^^ + /// 1 | source line + /// | ------ /// = note: notice the delim change for this footnote + /// ^^ + /// gutter `text_width` of 2 /// ``` - fn gutter_delim(&self) -> char { + fn fmt_gutter( + &self, + text_width: usize, + f: &mut fmt::Formatter, + ) -> fmt::Result { match self { - Self::Footnote(..) => '=', - _ => '|', + Self::SourceLinePadding | Self::SourceLineMark(..) => { + write!(f, "{:text_width$} |", "") + } + Self::SourceLine(src) => write!(f, "{:>text_width$} |", src.num()), + Self::Footnote(..) => write!(f, "{:text_width$} =", ""), } } @@ -551,6 +621,12 @@ impl<'d> Display for SectionLine<'d> { #[derive(Debug, PartialEq, Eq)] struct SectionSourceLine(SourceLine); +impl SectionSourceLine { + fn num(&self) -> NonZeroU32 { + self.0.num() + } +} + impl From for SectionSourceLine { fn from(line: SourceLine) -> Self { Self(line) @@ -799,6 +875,7 @@ mod test { label: Some("test label".into()), }), ], + line_max: 2.unwrap_into(), } ); } @@ -839,6 +916,8 @@ mod test { // so in this case it gets defaulted. level: Level::default(), body: vec![], + // Line comes from `src_lines`. + line_max: 1.unwrap_into(), } ); } @@ -877,6 +956,7 @@ mod test { .into() )), ], + line_max: 1.unwrap_into(), } ); } diff --git a/tamer/src/diagnose/report/test/integration.rs b/tamer/src/diagnose/report/test/integration.rs index 92bb46d1..d62567dc 100644 --- a/tamer/src/diagnose/report/test/integration.rs +++ b/tamer/src/diagnose/report/test/integration.rs @@ -99,6 +99,19 @@ const FILE_INVALID_UTF8: &[u8] = b"bad \xC0!"; // |---- | // 0 5 +const FILE_MANY_LINES: &[u8] = b"\ + 01\n02\n03\n04\n05\n06\n07\n08\n09\ +\n10\n11\n12\n13\n14\n15\n16\n17\n18\n19\ +\n20\n21\n22\n23\n24\n25\n26\n27\n28\n29\ +\n30\n31\n32\n33\n34\n35\n36\n37\n38\n39\ +\n40\n41\n42\n43\n44\n45\n46\n47\n48\n49\ +\n50\n51\n52\n53\n54\n55\n56\n57\n58\n59\ +\n60\n61\n62\n63\n64\n65\n66\n67\n68\n69\ +\n70\n71\n72\n73\n74\n75\n76\n77\n78\n79\ +\n80\n81\n82\n83\n84\n85\n86\n87\n88\n89\ +\n90\n91\n92\n93\n94\n95\n96\n97\n98\n99\ +\n100"; + macro_rules! assert_report { ($msg:expr, $aspans:expr, $expected:expr) => { let mut resolver = HashMap::>::new(); @@ -106,6 +119,7 @@ macro_rules! assert_report { let ctx_foo_bar = Context::from("foo/bar"); let ctx_bar_baz = Context::from("bar/baz"); let ctx_inv_utf = Context::from("invalid/utf8"); + let ctx_mny_lns = Context::from("many/lines"); resolver.insert( ctx_foo_bar, @@ -119,6 +133,10 @@ macro_rules! assert_report { ctx_inv_utf, BufSpanResolver::new(Cursor::new(FILE_INVALID_UTF8), ctx_inv_utf), ); + resolver.insert( + ctx_mny_lns, + BufSpanResolver::new(Cursor::new(FILE_MANY_LINES), ctx_mny_lns), + ); let mut sut = VisualReporter::new(resolver); @@ -151,7 +169,7 @@ fn span_error_no_label() { error: single span no label --> foo/bar:4:9 | - | foo/bar line 4 + 4 | foo/bar line 4 | ^^^^ " ); @@ -169,7 +187,7 @@ fn span_error_with_label() { error: single span with label --> bar/baz:3:1 | - | bar/baz line 3 + 3 | bar/baz line 3 | ^^^ error: span label here " ); @@ -191,7 +209,7 @@ fn adjacent_eq_span_no_labels_collapsed() { error: multiple adjacent same span no label --> foo/bar:4:9 | - | foo/bar line 4 + 4 | foo/bar line 4 | ^ " ); @@ -215,7 +233,7 @@ fn adjacent_eq_span_labels_collapsed() { error: multiple adjacent same span with labels --> bar/baz:1:9 | - | bar/baz line 1 + 1 | bar/baz line 1 | ^^^^^^ error: A label = error: C label " @@ -249,24 +267,24 @@ fn adjacent_eq_context_neq_offset_len_spans_not_collapsed() { error: eq context neq offset/len --> bar/baz:1:1 | - | bar/baz line 1 + 1 | bar/baz line 1 | ^^^ = error: A, first label, after mark --> bar/baz:1:1 | - | bar/baz line 1 + 1 | bar/baz line 1 | ^^^^^^^ error: B, different length = error: B, collapse --> bar/baz:2:1 | - | bar/baz line 2 + 2 | bar/baz line 2 | ^^^^ error: C, different offset --> bar/baz:1:1 | - | bar/baz line 1 + 1 | bar/baz line 1 | ^^^^^^^ error: B', not adjacent " ); @@ -309,30 +327,30 @@ fn adjacent_neq_context_spans_not_collapsed() { error: multiple adjacent different context --> foo/bar:1:1 | - | foo/bar line 1 + 1 | foo/bar line 1 | ^^^^^^^ = error: A, first, after marked = error: A, collapsed --> bar/baz:1:1 | - | bar/baz line 1 + 1 | bar/baz line 1 | ^^^^^^^ error: B, first, no marked = error: B, collapsed --> foo/bar:1:1 | - | foo/bar line 1 + 1 | foo/bar line 1 | ^^^^^^^ error: A, not collapsed --> bar/baz:1:1 | - | bar/baz line 1 + 1 | bar/baz line 1 | ^^^^^^^ --> foo/bar:1:1 | - | foo/bar line 1 + 1 | foo/bar line 1 | ^^^^^^^ " ); @@ -355,7 +373,7 @@ fn severity_levels_reflected() { internal error: multiple spans with labels of different severity level --> foo/bar:4:9 | - | foo/bar line 4 + 4 | foo/bar line 4 | !!!!!! internal error: an internal error = error: an error = note: a note @@ -380,10 +398,10 @@ fn multi_line_span() { error: multi-line span --> foo/bar:1:9 | - | foo/bar line 1 + 1 | foo/bar line 1 | ^^^^^^ | - | foo/bar line 2 + 2 | foo/bar line 2 | ^^^^^^^^^^^^ error: label to be on last line " ); @@ -466,7 +484,7 @@ fn offset_at_newline_marked() { error: offset at newline --> foo/bar:1:15 | - | foo/bar line 1 + 1 | foo/bar line 1 | ^ " ); @@ -487,7 +505,7 @@ fn zero_length_span_column() { error: offset at newline --> foo/bar:1:8 | - | foo/bar line 1 + 1 | foo/bar line 1 | ^ " ); @@ -509,23 +527,59 @@ fn error_level_mark_styling() { internal error: multiple level mark styles --> foo/bar:1:1 | - | foo/bar line 1 + 1 | foo/bar line 1 | !!!!!!! internal error: A --> foo/bar:2:1 | - | foo/bar line 2 + 2 | foo/bar line 2 | ^^^^^^^ error: B --> foo/bar:3:1 | - | foo/bar line 3 + 3 | foo/bar line 3 | ------- note: C --> foo/bar:4:1 | - | foo/bar line 4 + 4 | foo/bar line 4 | ------- help: D " ); } + +// This tests _both_ that wide gutters can exist and that the width of a +// later section is applied across other sections. +#[test] +fn wide_gutter_normalized_across_sections() { + let ctx = Context::from("many/lines"); + + let span_1 = ctx.span(0, 2); // 01 + let span_100 = ctx.span(3 * 99, 3); // 100 + + assert_report!( + "wide gutter", + vec![ + span_1.mark_error(), + span_100.mark_error(), + span_1.mark_error(), + ], + "\ +error: wide gutter + --> many/lines:1:1 + | + 1 | 01 + | ^^ + + --> many/lines:100:1 + | +100 | 100 + | ^^^ + + --> many/lines:1:1 + | + 1 | 01 + | ^^ +" + ); +} diff --git a/tamer/src/diagnose/resolver.rs b/tamer/src/diagnose/resolver.rs index 0807fff0..5ac698d9 100644 --- a/tamer/src/diagnose/resolver.rs +++ b/tamer/src/diagnose/resolver.rs @@ -262,6 +262,10 @@ pub struct SourceLine { } impl SourceLine { + pub fn num(&self) -> NonZeroU32 { + self.num + } + pub fn column(&self) -> Option { self.column } diff --git a/tamer/src/lib.rs b/tamer/src/lib.rs index 90d47271..32bb7d55 100644 --- a/tamer/src/lib.rs +++ b/tamer/src/lib.rs @@ -68,6 +68,7 @@ // Convenience features that are easily replaced if not stabalized. #![feature(nonzero_min_max)] #![feature(nonzero_ops)] +#![feature(int_log)] // We build docs for private items. #![allow(rustdoc::private_intra_doc_links)]