diff --git a/tamer/src/diagnose/report.rs b/tamer/src/diagnose/report.rs index 7492b798..9f19cc9b 100644 --- a/tamer/src/diagnose/report.rs +++ b/tamer/src/diagnose/report.rs @@ -32,7 +32,7 @@ use super::{ }; use crate::span::{Context, Span, UNKNOWN_SPAN}; use std::{ - fmt::{self, Display}, + fmt::{self, Display, Write}, num::NonZeroU32, }; @@ -197,7 +197,17 @@ impl<'d, D: Diagnostic> Extend> for Report<'d, D> { impl<'d, D: Diagnostic> Display for Report<'d, D> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{level}: {msg}\n", level = self.level, msg = self.msg)?; - self.secs.iter().try_for_each(|sec| sec.fmt(f)) + + self.secs.iter().try_fold(true, |first, sec| { + if !first { + f.write_char('\n')?; + } + + sec.fmt(f)?; + Ok(false) + })?; + + Ok(()) } } @@ -255,7 +265,14 @@ impl<'s, 'd> Section<'d> { // TODO: At the time of writing this will cause duplication of // system labels, // which is not desirable. - extend_sec.body.extend(self.body); + extend_sec.body.extend( + // TODO: The system wastefully allocates duplicate source + // lines when resolving spans only to discard them here. + self.body + .into_iter() + .filter_map(SectionLine::into_footnote), + ); + None } @@ -270,7 +287,9 @@ where { fn from(mspan: MaybeResolvedSpan<'d, S>) -> Self { let heading = SpanHeading::from(&mspan); - let mut body = mspan.system_lines(); + let syslines = mspan.system_lines(); + + let mut body = Vec::new(); let (span, level) = match mspan { MaybeResolvedSpan::Resolved(rspan, oslabel) => { @@ -312,6 +331,8 @@ where } }; + body.extend(syslines); + Section { heading, span, @@ -471,7 +492,7 @@ impl<'d> SpanLabel<'d> { impl<'d> Display for SpanLabel<'d> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let Self(level, label) = self; - write!(f, " {level}: {label}") + write!(f, " = {level}: {label}") } } @@ -486,6 +507,19 @@ enum SectionLine<'d> { Footnote(SpanLabel<'d>), } +impl<'d> SectionLine<'d> { + fn into_footnote(self) -> Option { + match self { + Self::SourceLine(SectionSourceLine { + mark: LineMark { level, label, .. }, + .. + }) => label.map(|l| Self::Footnote(SpanLabel(level, l))), + + Self::Footnote(..) => Some(self), + } + } +} + impl<'d> Display for SectionLine<'d> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { @@ -504,7 +538,9 @@ struct SectionSourceLine<'d> { impl<'d> Display for SectionSourceLine<'d> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - // TODO + write!(f, " |\n")?; + write!(f, " | {src}\n", src = self.src)?; + write!(f, " |\n")?; write!(f, "{}", self.mark) } } @@ -521,7 +557,7 @@ struct LineMark<'d> { impl<'d> Display for LineMark<'d> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { if let Some(label) = self.label.as_ref() { - write!(f, " {level}: {label}\n", level = self.level)?; + write!(f, " = {level}: {label}\n", level = self.level)?; } Ok(()) @@ -779,6 +815,10 @@ mod test { span, level: Level::Note, body: vec![ + SectionLine::Footnote(SpanLabel( + Level::Note, + "test label".into() + )), SectionLine::Footnote(SpanLabel( Level::Help, // This hard-coding is not ideal, @@ -790,12 +830,71 @@ mod test { ) .into() )), - SectionLine::Footnote(SpanLabel( - Level::Note, - "test label".into() - )), ], } ); } + + #[test] + fn section_footnote_into_footnote() { + assert_eq!( + SectionLine::Footnote(SpanLabel( + Level::Note, + "test footnote".into() + )) + .into_footnote(), + Some(SectionLine::Footnote(SpanLabel( + Level::Note, + "test footnote".into() + ))), + ); + } + + #[test] + fn section_src_line_with_label_into_footnote() { + assert_eq!( + SectionLine::SourceLine(SectionSourceLine { + src: SourceLine::new_stub( + 1.unwrap_into(), + None, + DUMMY_SPAN, + "discarded".into() + ), + mark: LineMark { + level: Level::Help, + col: None, + label: Some("kept label".into()) + } + }) + .into_footnote(), + Some(SectionLine::Footnote(SpanLabel( + Level::Help, + "kept label".into() + ))), + ); + } + + #[test] + fn section_src_line_without_label_into_footnote() { + assert_eq!( + SectionLine::SourceLine(SectionSourceLine { + src: SourceLine::new_stub( + 1.unwrap_into(), + None, + DUMMY_SPAN, + "discarded".into() + ), + mark: LineMark { + level: Level::Help, + col: None, + label: None, + } + }) + .into_footnote(), + None + ); + } + + // TODO: Section squashing is currently only covered by integration + // tests! } diff --git a/tamer/src/diagnose/report/test/integration.rs b/tamer/src/diagnose/report/test/integration.rs index 0e559249..91321f9b 100644 --- a/tamer/src/diagnose/report/test/integration.rs +++ b/tamer/src/diagnose/report/test/integration.rs @@ -149,6 +149,9 @@ fn span_error_no_label() { "\ error: single span no label --> foo/bar:4:6 + | + | foo/bar line 4 + | " ); } @@ -164,7 +167,10 @@ fn span_error_with_label() { "\ error: single span with label --> bar/baz:3:1 - error: span label here + | + | bar/baz line 3 + | + = error: span label here " ); } @@ -184,6 +190,9 @@ fn adjacent_eq_span_no_labels_collapsed() { "\ error: multiple adjacent same span no label --> foo/bar:4:6 + | + | foo/bar line 4 + | " ); } @@ -205,8 +214,11 @@ fn adjacent_eq_span_labels_collapsed() { "\ error: multiple adjacent same span with labels --> bar/baz:1:11 - error: A label - error: C label + | + | bar/baz line 1 + | + = error: A label + = error: C label " ); } @@ -219,29 +231,44 @@ fn adjacent_eq_context_neq_offset_len_spans_not_collapsed() { "eq context neq offset/len", vec![ // --> - ctx.span(10, 5).mark_error(), - ctx.span(10, 5).error("A, first label"), // collapse + ctx.span(0, 5).mark_error(), + ctx.span(0, 5).error("A, first label"), // collapse // --> - ctx.span(10, 6).error("B, different length"), - ctx.span(10, 6).mark_error(), // collapse - ctx.span(10, 6).error("B, collapse"), + ctx.span(0, 6).error("B, different length"), + ctx.span(0, 6).mark_error(), // collapse + ctx.span(0, 6).error("B, collapse"), // --> ctx.span(15, 6).error("C, different offset"), // --> // Back to (10, 6), but not adjacent to previous - ctx.span(10, 6).error("B', not adjacent"), + ctx.span(0, 6).error("B', not adjacent"), ], "\ error: eq context neq offset/len - --> bar/baz:1:11 - error: A, first label - --> bar/baz:1:11 - error: B, different length - error: B, collapse + --> bar/baz:1:1 + | + | bar/baz line 1 + | + = error: A, first label + + --> bar/baz:1:1 + | + | bar/baz line 1 + | + = error: B, different length + = error: B, collapse + --> bar/baz:2:1 - error: C, different offset - --> bar/baz:1:11 - error: B', not adjacent + | + | bar/baz line 2 + | + = error: C, different offset + + --> bar/baz:1:1 + | + | bar/baz line 1 + | + = error: B', not adjacent " ); } @@ -279,15 +306,34 @@ fn adjacent_neq_context_spans_not_collapsed() { "\ error: multiple adjacent different context --> foo/bar:1:11 - error: A, first - error: A, collapsed + | + | foo/bar line 1 + | + = error: A, first + = error: A, collapsed + --> bar/baz:1:11 - error: B, first - error: B, collapsed + | + | bar/baz line 1 + | + = error: B, first + = error: B, collapsed + --> foo/bar:1:11 - error: A, not collapsed + | + | foo/bar line 1 + | + = error: A, not collapsed + --> bar/baz:1:11 + | + | bar/baz line 1 + | + --> foo/bar:1:11 + | + | foo/bar line 1 + | " ); } @@ -308,10 +354,13 @@ fn severity_levels_reflected() { "\ internal error: multiple spans with labels of different severity level --> foo/bar:4:6 - internal error: an internal error - error: an error - note: a note - help: a help message + | + | foo/bar line 4 + | + = internal error: an internal error + = error: an error + = note: a note + = help: a help message " ); } @@ -323,13 +372,21 @@ fn multi_line_span() { // First two lines. let span = ctx.span(0, 29); + // This is obviously terrible-looking; + // it'll be condensed as this evolves further. assert_report!( "multi-line span", vec![span.error("label to be on last line")], "\ error: multi-line span --> foo/bar:1:1 - error: label to be on last line + | + | foo/bar line 1 + | + | + | foo/bar line 2 + | + = error: label to be on last line " ); } @@ -365,8 +422,8 @@ fn fallback_when_span_fails_to_resolve() { format!("\ error: unresolvable context fallback --> unknown/context offset 50--55 - help: an error occurred while trying to look up information about this span: {ioerr} - error: an error we do not want to suppress + = error: an error we do not want to suppress + = help: an error occurred while trying to look up information about this span: {ioerr} ") ); } @@ -381,6 +438,8 @@ fn fallback_when_column_fails_to_resolve() { let span = ctx.span(4, 2); + let lossy = String::from_utf8_lossy(FILE_INVALID_UTF8); + // It's not ideal that the help appears first, // but this should only happen under very exceptional // circumstances so it's not worth trying to resolve. @@ -390,12 +449,15 @@ fn fallback_when_column_fails_to_resolve() { assert_report!( "column resolution failure", vec![span.error("an error we do not want to suppress"),], - "\ + format!("\ error: column resolution failure --> invalid/utf8:1 bytes 4--6 - help: unable to calculate columns because the line is not a valid UTF-8 string - help: you have been provided with 0-indexed line-relative inclusive byte offsets - error: an error we do not want to suppress -" + | + | {lossy} + | + = error: an error we do not want to suppress + = help: unable to calculate columns because the line is not a valid UTF-8 string + = help: you have been provided with 0-indexed line-relative inclusive byte offsets +") ); } diff --git a/tamer/src/diagnose/resolver.rs b/tamer/src/diagnose/resolver.rs index 3c8feb52..ddf2efa3 100644 --- a/tamer/src/diagnose/resolver.rs +++ b/tamer/src/diagnose/resolver.rs @@ -266,6 +266,14 @@ impl SourceLine { } } +impl Display for SourceLine { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + // TODO: Just store String instead of a byte vector so we're not + // validating UTF-8 twice. + write!(f, "{}", String::from_utf8_lossy(&self.text)) + } +} + /// Resolve a [`Span`] using any generic [`BufRead`]. pub struct BufSpanResolver { reader: R,