tamer: diagnose::report::SpanLabel: Remove in favor of separate Level and Label

`SpanLabel` was created during a very early refactoring of this system, and
I've just been fighting with it sense.  This removes it, and simplifies
some things in the process.

It also makes clear that `Level` is never optional and removes the awkward
`Level::default` that was there previously; the default is now the lowest
level, which will always be able to be escalated.

DEV-12151
main
Mike Gerwitz 2022-04-29 12:10:32 -04:00
parent 9a5a2c4f3f
commit 2ce0dbdd84
4 changed files with 43 additions and 76 deletions

View File

@ -62,7 +62,6 @@ pub enum Level {
/// A user-resolvable error.
///
/// These represent errors resulting from the user's input.
#[default]
Error,
/// Useful information that supplements other messages.
@ -79,6 +78,7 @@ pub enum Level {
/// Unlike other severity levels which provide concrete factual
/// information,
/// help messages may be more speculative.
#[default]
Help,
}

View File

@ -124,21 +124,19 @@ where
diagnostic.describe().into_iter().scan(
UNKNOWN_SPAN,
move |prev_span, AnnotatedSpan(span, level, olabel)| {
let slabel = olabel.map(|label| SpanLabel(level, label));
// Avoid re-resolving
// (and allocating memory for the source lines of)
// a span that was just resolved,
// which will just be squashed with the previous anyway.
if *prev_span == span {
return Some(MaybeResolvedSpan::Elided(span, slabel));
return Some(MaybeResolvedSpan::Elided(span, level, olabel));
}
*prev_span = span;
Some(match resolve(span) {
Ok(rspan) => MaybeResolvedSpan::Resolved(rspan, slabel),
Err(e) => MaybeResolvedSpan::Unresolved(span, slabel, e),
Ok(rspan) => MaybeResolvedSpan::Resolved(rspan, level, olabel),
Err(e) => MaybeResolvedSpan::Unresolved(span, level, olabel, e),
})
},
)
@ -158,9 +156,9 @@ where
/// never be masked by an error of our own.
#[derive(Debug, PartialEq, Eq)]
enum MaybeResolvedSpan<'d, S: ResolvedSpanData> {
Resolved(S, Option<SpanLabel<'d>>),
Elided(Span, Option<SpanLabel<'d>>),
Unresolved(Span, Option<SpanLabel<'d>>, SpanResolverError),
Resolved(S, Level, Option<Label<'d>>),
Elided(Span, Level, Option<Label<'d>>),
Unresolved(Span, Level, Option<Label<'d>>, SpanResolverError),
}
impl<'d, S: ResolvedSpanData> MaybeResolvedSpan<'d, S> {
@ -170,30 +168,30 @@ impl<'d, S: ResolvedSpanData> MaybeResolvedSpan<'d, S> {
/// not hinder them by masking it.
fn system_lines(&self) -> Vec<SectionLine<'static>> {
match self {
Self::Resolved(rspan, _) if rspan.col_num().is_none() => vec![
SectionLine::Footnote(SpanLabel(
Self::Resolved(rspan, ..) if rspan.col_num().is_none() => vec![
SectionLine::Footnote(
Level::Help,
"unable to calculate columns because the line is \
not a valid UTF-8 string"
.into(),
)),
SectionLine::Footnote(SpanLabel(
),
SectionLine::Footnote(
Level::Help,
"you have been provided with 0-indexed \
line-relative inclusive byte offsets"
.into(),
)),
),
],
Self::Unresolved(_, _, e) => {
vec![SectionLine::Footnote(SpanLabel(
Self::Unresolved(.., e) => {
vec![SectionLine::Footnote(
Level::Help,
format!(
"an error occurred while trying to look up \
information about this span: {e}"
)
.into(),
))]
)]
}
_ => vec![],
@ -363,15 +361,10 @@ where
let mut line_max = NonZeroU32::MIN;
let (span, level) = match mspan {
MaybeResolvedSpan::Resolved(rspan, oslabel) => {
MaybeResolvedSpan::Resolved(rspan, level, mut olabel) => {
let span = rspan.unresolved_span();
let src = rspan.into_lines();
let (level, mut olabel) = match oslabel {
Some(SpanLabel(level, label)) => (level, Some(label)),
None => (Default::default(), None),
};
let nlines = src.len();
src.into_iter().enumerate().for_each(|(i, srcline)| {
@ -396,20 +389,19 @@ where
}),
]);
} else {
body.extend(label.map(|l| {
SectionLine::Footnote(SpanLabel(level, l))
}));
body.extend(
label.map(|l| SectionLine::Footnote(level, l)),
);
}
});
(span, level)
}
MaybeResolvedSpan::Elided(span, olabel)
| MaybeResolvedSpan::Unresolved(span, olabel, _) => {
let level =
olabel.as_ref().map(SpanLabel::level).unwrap_or_default();
body.extend(olabel.map(SectionLine::Footnote));
MaybeResolvedSpan::Elided(span, level, olabel)
| MaybeResolvedSpan::Unresolved(span, level, olabel, _) => {
body.extend(
olabel.map(|label| SectionLine::Footnote(level, label)),
);
(span, level)
}
};
@ -466,7 +458,7 @@ where
/// Span header containing the (hopefully resolved) context.
fn from(mspan: &'s MaybeResolvedSpan<'d, S>) -> Self {
match mspan {
MaybeResolvedSpan::Resolved(rspan, _) => SpanHeading(
MaybeResolvedSpan::Resolved(rspan, _, _) => SpanHeading(
rspan.context(),
HeadingLineNum::Resolved(
rspan.line_num(),
@ -483,8 +475,8 @@ where
// Note that elided will be squashed anyway,
// so the fact that this results in an unresolved heading is
// okay.
MaybeResolvedSpan::Elided(span, _)
| MaybeResolvedSpan::Unresolved(span, _, _) => {
MaybeResolvedSpan::Elided(span, _, _)
| MaybeResolvedSpan::Unresolved(span, _, _, _) => {
SpanHeading(span.context(), HeadingLineNum::Unresolved(*span))
}
}
@ -570,30 +562,13 @@ impl Display for HeadingColNum {
}
}
/// A label describing a span.
#[derive(Debug, PartialEq, Eq)]
struct SpanLabel<'d>(Level, Label<'d>);
impl<'d> SpanLabel<'d> {
fn level(&self) -> Level {
self.0
}
}
impl<'d> Display for SpanLabel<'d> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let Self(level, label) = self;
write!(f, " {level}: {label}")
}
}
/// Line of output in a [`Section`] body.
#[derive(Debug, PartialEq, Eq)]
enum SectionLine<'d> {
SourceLinePadding,
SourceLine(SectionSourceLine),
SourceLineMark(LineMark<'d>),
Footnote(SpanLabel<'d>),
Footnote(Level, Label<'d>),
}
impl<'d> SectionLine<'d> {
@ -634,7 +609,7 @@ impl<'d> SectionLine<'d> {
Self::SourceLinePadding => None,
Self::SourceLine(..) => None,
Self::SourceLineMark(LineMark { level, label, .. }) => {
label.map(|l| Self::Footnote(SpanLabel(level, l)))
label.map(|l| Self::Footnote(level, l))
}
Self::Footnote(..) => Some(self),
@ -648,7 +623,7 @@ impl<'d> Display for SectionLine<'d> {
Self::SourceLinePadding => Ok(()),
Self::SourceLine(line) => line.fmt(f),
Self::SourceLineMark(mark) => mark.fmt(f),
Self::Footnote(label) => label.fmt(f),
Self::Footnote(level, label) => write!(f, " {level}: {label}"),
}
}
}

View File

@ -172,7 +172,8 @@ fn section_from_mspan_resolved() {
span: Some(span),
src_lines: Some(src_lines.clone()),
},
Some(SpanLabel(Level::Note, "test label".into())),
Level::Note,
Some("test label".into()),
)),
Section {
heading: SpanHeading(
@ -229,6 +230,7 @@ fn section_from_mspan_resolved_no_label() {
span: Some(span),
src_lines: None,
},
Level::Note,
None,
)),
Section {
@ -243,9 +245,7 @@ fn section_from_mspan_resolved_no_label() {
)
),
span,
// Level is normally derived from the label,
// so in this case it gets defaulted.
level: Level::default(),
level: Level::Note,
body: vec![],
// Line comes from `src_lines`.
line_max: 1.unwrap_into(),
@ -260,7 +260,8 @@ fn section_from_mspan_unresolved() {
let mspan = MaybeResolvedSpan::Unresolved::<StubResolvedSpan>(
span,
Some(SpanLabel(Level::Note, "test label".into())),
Level::Note,
Some("test label".into()),
SpanResolverError::Io(io::ErrorKind::NotFound),
);
@ -271,11 +272,8 @@ fn section_from_mspan_unresolved() {
span,
level: Level::Note,
body: vec![
SectionLine::Footnote(SpanLabel(
Level::Note,
"test label".into()
)),
SectionLine::Footnote(SpanLabel(
SectionLine::Footnote(Level::Note, "test label".into()),
SectionLine::Footnote(
Level::Help,
// This hard-coding is not ideal,
// as it makes the test fragile.
@ -285,7 +283,7 @@ fn section_from_mspan_unresolved() {
io::ErrorKind::NotFound
)
.into()
)),
),
],
line_max: 1.unwrap_into(),
}
@ -295,12 +293,9 @@ fn section_from_mspan_unresolved() {
#[test]
fn section_footnote_into_footnote() {
assert_eq!(
SectionLine::Footnote(SpanLabel(Level::Note, "test footnote".into()))
SectionLine::Footnote(Level::Note, "test footnote".into())
.into_footnote(),
Some(SectionLine::Footnote(SpanLabel(
Level::Note,
"test footnote".into()
))),
Some(SectionLine::Footnote(Level::Note, "test footnote".into())),
);
}
@ -330,10 +325,7 @@ fn section_mark_with_label_into_footnote() {
label: Some("kept label".into())
})
.into_footnote(),
Some(SectionLine::Footnote(SpanLabel(
Level::Help,
"kept label".into()
))),
Some(SectionLine::Footnote(Level::Help, "kept label".into())),
);
}

View File

@ -153,7 +153,7 @@ fn no_spans() {
"test with no spans",
vec![],
// No spans will result in the `Display` of the error only.
"error: test with no spans\n"
"help: test with no spans\n"
);
}