Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Group related diagnostics visually #171

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ indenter = "0.3.0"
rustversion = "1.0"
trybuild = { version = "1.0.19", features = ["diff"] }
syn = { version = "1.0", features = ["full"] }
pretty_assertions = "1.2.1"

[features]
default = []
Expand Down
6 changes: 4 additions & 2 deletions miette-derive/src/diagnostic_source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ impl DiagnosticSource {
};
quote! {
Self::#ident #display_pat => {
std::option::Option::Some(#rel.as_ref())
use std::borrow::Borrow;
std::option::Option::Some(#rel.borrow())
}
}
})
Expand All @@ -71,7 +72,8 @@ impl DiagnosticSource {
let rel = &self.0;
Some(quote! {
fn diagnostic_source<'a>(&'a self) -> std::option::Option<&'a dyn miette::Diagnostic> {
std::option::Option::Some(&self.#rel)
use std::borrow::Borrow;
std::option::Option::Some(self.#rel.borrow())
}
})
}
Expand Down
70 changes: 56 additions & 14 deletions src/handlers/graphical.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::fmt::{self, Write};

use owo_colors::{OwoColorize, Style};

use crate::diagnostic_chain::DiagnosticChain;
use crate::diagnostic_chain::{DiagnosticChain, ErrorKind};
use crate::handlers::theme::*;
use crate::protocol::{Diagnostic, Severity};
use crate::{LabeledSpan, MietteError, ReportHandler, SourceCode, SourceSpan, SpanContents};
Expand Down Expand Up @@ -133,7 +133,6 @@ impl GraphicalReportHandler {
diagnostic: &(dyn Diagnostic),
) -> fmt::Result {
self.render_header(f, diagnostic)?;
writeln!(f)?;
self.render_causes(f, diagnostic)?;
let src = diagnostic.source_code();
self.render_snippets(f, diagnostic, src)?;
Expand Down Expand Up @@ -172,13 +171,15 @@ impl GraphicalReportHandler {
);
write!(header, "{}", link)?;
writeln!(f, "{}", header)?;
writeln!(f)?;
} else if let Some(code) = diagnostic.code() {
write!(header, "{}", code.style(severity_style),)?;
if self.links == LinkStyle::Text && diagnostic.url().is_some() {
let url = diagnostic.url().unwrap(); // safe
write!(header, " ({})", url.style(self.theme.styles.link))?;
}
writeln!(f, "{}", header)?;
writeln!(f)?;
}
Ok(())
}
Expand Down Expand Up @@ -231,7 +232,21 @@ impl GraphicalReportHandler {
let opts = textwrap::Options::new(width)
.initial_indent(&initial_indent)
.subsequent_indent(&rest_indent);
writeln!(f, "{}", textwrap::fill(&error.to_string(), opts))?;
match error {
ErrorKind::Diagnostic(diag) => {
let mut inner = String::new();

// Don't print footer for inner errors
let mut inner_renderer = self.clone();
inner_renderer.footer = None;
inner_renderer.render_report(&mut inner, diag)?;

writeln!(f, "{}", textwrap::fill(&inner, opts))?;
}
ErrorKind::StdError(err) => {
writeln!(f, "{}", textwrap::fill(&err.to_string(), opts))?;
}
}
}
}

Expand All @@ -257,20 +272,47 @@ impl GraphicalReportHandler {
parent_src: Option<&dyn SourceCode>,
) -> fmt::Result {
if let Some(related) = diagnostic.related() {
writeln!(f)?;
for rel in related {
let related: Vec<_> = related.collect();
writeln!(
f,
"{}{}There were {} related diagnostics:",
if related.is_empty() {
self.theme.characters.lcross
} else {
self.theme.characters.ltop
},
self.theme.characters.hbar,
related.len()
)?;
let width = self.termwidth.saturating_sub(2);
let mut inner = String::new();
for (idx, rel) in related.into_iter().enumerate() {
let init_ident = format!(
"{}{} {}.",
self.theme.characters.lcross,
self.theme.characters.hbar,
idx + 1
);
let subseq_ident = format!("{} ", self.theme.characters.vbar);
let opts = textwrap::Options::new(width)
.initial_indent(&init_ident)
.subsequent_indent(&subseq_ident);
match diagnostic.severity() {
Some(Severity::Error) | None => write!(f, "Error: ")?,
Some(Severity::Warning) => write!(f, "Warning: ")?,
Some(Severity::Advice) => write!(f, "Advice: ")?,
Some(Severity::Error) | None => write!(&mut inner, "Error: ")?,
Some(Severity::Warning) => write!(&mut inner, "Warning: ")?,
Some(Severity::Advice) => write!(&mut inner, "Advice: ")?,
};
self.render_header(f, rel)?;
writeln!(f)?;
self.render_causes(f, rel)?;
self.render_header(&mut inner, rel)?;
writeln!(&mut inner)?;
self.render_causes(&mut inner, rel)?;
let src = rel.source_code().or(parent_src);
self.render_snippets(f, rel, src)?;
self.render_footer(f, rel)?;
self.render_related(f, rel, src)?;
self.render_snippets(&mut inner, rel, src)?;
self.render_footer(&mut inner, rel)?;
self.render_related(&mut inner, rel, src)?;

writeln!(f, "{}", textwrap::fill(&inner, opts))?;

inner.clear();
}
}
Ok(())
Expand Down
99 changes: 70 additions & 29 deletions tests/graphical.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use miette::{
NarratableReportHandler, Report, SourceSpan,
};
use thiserror::Error;
use pretty_assertions::assert_eq;

fn fmt_report(diag: Report) -> String {
let mut out = String::new();
Expand Down Expand Up @@ -784,11 +785,22 @@ fn related() -> Result<(), MietteError> {
let err = MyBad {
src: NamedSource::new("bad_file.rs", src.clone()),
highlight: (9, 4).into(),
related: vec![MyBad {
src: NamedSource::new("bad_file.rs", src),
highlight: (0, 6).into(),
related: vec![],
}],
related: vec![
MyBad {
src: NamedSource::new("bad_file.rs", src.clone()),
highlight: (0, 6).into(),
related: vec![MyBad {
src: NamedSource::new("bad_file.rs", src.clone()),
highlight: (0, 6).into(),
related: vec![],
}],
},
MyBad {
src: NamedSource::new("bad_file.rs", src.clone()),
highlight: (0, 6).into(),
related: vec![],
},
],
};
let out = fmt_report(err.into());
println!("Error: {}", out);
Expand All @@ -803,18 +815,46 @@ fn related() -> Result<(), MietteError> {
3 │ here
╰────
help: try doing it better next time?

Error: oops::my::bad

× oops!
╭─[bad_file.rs:1:1]
1 │ source
· ───┬──
· ╰── this bit here
2 │ text
╰────
help: try doing it better next time?

╭─There were 2 related diagnostics:
├─ 1.Error: oops::my::bad
Comment on lines +820 to +821
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is doubling the number of empty new lines deliberate or an oversight?
It does hurt the sense of visual grouping for me.

│ × oops!
│ ╭─[bad_file.rs:1:1]
│ 1 │ source
│ · ───┬──
│ · ╰── this bit here
│ 2 │ text
│ ╰────
│ help: try doing it better next time?
│ ╭─There were 1 related diagnostics:
│ ├─ 1.Error: oops::my::bad
│ │
│ │
│ │ × oops!
│ │ ╭─[bad_file.rs:1:1]
│ │ 1 │ source
│ │ · ───┬──
│ │ · ╰── this bit here
│ │ 2 │ text
│ │ ╰────
│ │ help: try doing it better next time?
│ │ ├─There were 0 related diagnostics:
│ │
├─ 2.Error: oops::my::bad
│ × oops!
│ ╭─[bad_file.rs:1:1]
│ 1 │ source
│ · ───┬──
│ · ╰── this bit here
│ 2 │ text
│ ╰────
│ help: try doing it better next time?
│ ├─There were 0 related diagnostics:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd expect this line to simply be omitted when there are no related diagnostics (what I expect to be the most common case to be fair).

"#
.trim_start()
.to_string();
Expand Down Expand Up @@ -865,16 +905,18 @@ fn related_source_code_propagation() -> Result<(), MietteError> {
3 │ here
╰────
help: try doing it better next time?

Error: oops::my::bad

× oops!
╭─[bad_file.rs:1:1]
1 │ source
· ───┬──
· ╰── this bit here
2 │ text
╰────
╭─There were 1 related diagnostics:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The vertical | works quite nicely to "mark" that the entire block is composed of related diagnostics, but it does feel like it crowds the left area: I would find it more difficult to quickly scan the output and determine how many errors I encountered.

I wonder if this could be helped by nesting the vertical line itself a bit more - e.g.

  There were 1 related diagnostics:
  │ 
  ├─ 1.Error: oops::my::bad
  │ 
  │   × oops!
  │    ╭─[bad_file.rs:1:1]
  │  1 │ source
  │    · ───┬──
  │    ·    ╰── this bit here
  │  2 │   text
  │    ╰────

├─ 1.Error: oops::my::bad
│ × oops!
│ ╭─[bad_file.rs:1:1]
│ 1 │ source
│ · ───┬──
│ · ╰── this bit here
│ 2 │ text
│ ╰────
"#
.trim_start()
.to_string();
Expand All @@ -900,8 +942,7 @@ fn zero_length_eol_span() {
let out = fmt_report(err.into());
println!("Error: {}", out);

let expected = r#"
× oops!
let expected = r#" × oops!
╭─[issue:1:1]
1 │ this is the first line
2 │ this is the second line
Expand Down
87 changes: 85 additions & 2 deletions tests/test_diagnostic_source_macro.rs
Original file line number Diff line number Diff line change
@@ -1,20 +1,103 @@
use miette::Diagnostic;

#[derive(Debug, miette::Diagnostic, thiserror::Error)]
#[error("A complex error happened")]
struct SourceError {
#[source_code]
code: String,
#[help]
help: String,
#[label("here")]
label: (usize, usize),
}

#[derive(Debug, miette::Diagnostic, thiserror::Error)]
#[error("AnErr")]
struct AnErr;

#[derive(Debug, miette::Diagnostic, thiserror::Error)]
#[error("TestError")]
struct TestError {
struct TestStructError {
#[diagnostic_source]
asdf_inner_foo: AnErr,
}

#[derive(Debug, miette::Diagnostic, thiserror::Error)]
#[error("TestError")]
enum TestEnumError {
Without,
WithTuple(#[diagnostic_source] AnErr),
WithStruct {
#[diagnostic_source]
inner: AnErr,
},
}

#[derive(Debug, miette::Diagnostic, thiserror::Error)]
#[error("TestError")]
struct TestTupleError(#[diagnostic_source] AnErr);

#[derive(Debug, miette::Diagnostic, thiserror::Error)]
#[error("TestError")]
struct TestBoxedError(#[diagnostic_source] Box<dyn Diagnostic>);

#[derive(Debug, miette::Diagnostic, thiserror::Error)]
#[error("TestError")]
struct TestArcedError(#[diagnostic_source] std::sync::Arc<dyn Diagnostic>);

#[test]
fn test_diagnostic_source() {
let error = TestError {
let error = TestStructError {
asdf_inner_foo: AnErr,
};
assert!(error.diagnostic_source().is_some());

let error = TestEnumError::Without;
assert!(error.diagnostic_source().is_none());

let error = TestEnumError::WithTuple(AnErr);
assert!(error.diagnostic_source().is_some());

let error = TestEnumError::WithStruct { inner: AnErr };
assert!(error.diagnostic_source().is_some());

let error = TestTupleError(AnErr);
assert!(error.diagnostic_source().is_some());

let error = TestBoxedError(Box::new(AnErr));
assert!(error.diagnostic_source().is_some());

let error = TestArcedError(std::sync::Arc::new(AnErr));
assert!(error.diagnostic_source().is_some());
}

#[test]
fn test_diagnostic_source_pass_extra_info() {
let diag = TestBoxedError(Box::new(SourceError {
code: String::from("Hello\nWorld!"),
help: format!("Have you tried turning it on and off again?"),
label: (1, 4),
}));
let mut out = String::new();
miette::GraphicalReportHandler::new_themed(miette::GraphicalTheme::unicode_nocolor())
.with_width(80)
.with_footer("this is a footer".into())
.render_report(&mut out, &diag)
.unwrap();
println!("Error: {}", out);
let expected = r#" × TestError
╰─▶ × A complex error happened
╭─[1:1]
1 │ Hello
· ──┬─
· ╰── here
2 │ World!
╰────
help: Have you tried turning it on and off again?


this is a footer
"#
.to_string();
assert_eq!(expected, out);
}