zkat/miette

Is there a reason that `Report` doesn't implement `Error` and `Diagnostic`?

TheLostLambda opened this issue · 2 comments

I've maybe not done enough thinking on this yet, but it would seem that https://docs.rs/miette/latest/miette/struct.Error.html can be converted to all sorts of Error or Diagnostic trait objects, but doesn't implement either of those traits itself?

I was wondering if that was done intentionally, or if that might be another helpful PR?

Perhaps in addition to that, is there a reason there isn't a blanket impl for Box<T> where T: Diagnostic?

I've got this kinda hacky code, and I'll be adding an impl to manually deal with Report, but ideally I wouldn't need this intermediate trait?

pub trait UnwrapDiagnostic<E> {
    fn unwrap_diagnostic(self) -> E;
}

impl<T: Debug, E: Diagnostic> UnwrapDiagnostic<E> for Result<T, E> {
    fn unwrap_diagnostic(self) -> E {
        self.unwrap_err()
    }
}

impl<T: Debug, E: Diagnostic> UnwrapDiagnostic<E> for Result<T, Box<E>> {
    fn unwrap_diagnostic(self) -> E {
        *self.unwrap_err()
    }
}

macro_rules! assert_miette_snapshot {
    ($diag:expr) => {{
        use insta::{with_settings, assert_snapshot};
        use miette::{GraphicalReportHandler, GraphicalTheme};
        use crate::testing_tools::UnwrapDiagnostic;

        let mut out = String::new();
        GraphicalReportHandler::new_themed(GraphicalTheme::unicode_nocolor())
            .with_width(80)
            .render_report(&mut out, &$diag.unwrap_diagnostic())
            .unwrap();
        with_settings!({
            description => stringify!($diag)
        }, {
            assert_snapshot!(out);
        });
    }};
}

My manual impl for Report also isn't working, I think because that Box<T> blanket impl is missing... This code:

impl<T: Debug> UnwrapDiagnostic<Box<dyn Diagnostic>> for Result<T, Report> {
    fn unwrap_diagnostic(self) -> Box<dyn Diagnostic> {
        self.unwrap_err().into()
    }
}

returns this error, once I try to pass it to .report_error():

image

Let me know if I'm crazy, or if adding those impls would help ergonomics!

zkat commented

Same reason anyhow::Error doesn't implement it: the blanket impl in std for impl From<T> for T

Haha, a good answer, and a very very painfully correct one: #368

Even if we give up on having a From impl, there appear to be a lot more dragons lurking...