zkat / miette

Fancy extension for std::error::Error with pretty, detailed diagnostic printing.
https://docs.rs/miette
Apache License 2.0
1.85k stars 106 forks source link

Add `UnwrapPretty` trait #345

Open esoterra opened 4 months ago

esoterra commented 4 months ago

Add a trait like the following that people can choose to import that lets them call unwrap_pretty() on results where E is a Diagnostic which is quite useful in things like unit tests.

use miette::{Diagnostic, Report};

pub trait UnwrapPretty {
    type Output;

    fn unwrap_pretty(self) -> Self::Output;
}

impl<T, E> UnwrapPretty for Result<T, E>
where
    E: Diagnostic + Sync + Send + 'static
{
    type Output = T;

    fn unwrap_pretty(self) -> Self::Output {
        match self {
            Ok(output) => output,
            Err(diagnostic) => {
                panic!("{:?}", Report::new(diagnostic));
            },
        }
    }
}
esoterra commented 4 months ago

Another possible idea that I'm less sure about is OkPretty as a replacement for calling ok() printing the diagnostic when it fails.

pub trait OkPretty {
    type Output;

    fn ok_pretty(self) -> Option<Self::Output>;
}

impl<T, E> OkPretty for Result<T, E>
where
    E: Diagnostic + Sync + Send + 'static
{
    type Output = T;

    fn ok_pretty(self) -> Option<Self::Output> {
        match self {
            Ok(output) => Some(output),
            Err(diagnostic) => {
                println!("{:?}", Report::new(diagnostic));
                None
            },
        }
    }
}

If I could pick one, it'd definitely be UnwrapPretty but I think both would be nice.

zkat commented 4 months ago

I kinda like this idea, but I'm also debating whether it's good to include in miette.

Philosophically speaking, I think it's important to avoid unwraps as much as possible, and miette is designed with the intention of making it easier to avoid them.

But I can see how sometimes you just want to unwrap, such as tests! and how UnwrapPretty might be useful there.

That said: tests are not typically a place where I think fancy-looking diagnostics are really necessary. Fancy diagnostics are more intended for end-users of a tool.

I also definitely wouldn't want OkPretty because we should absolutely not be logging arbitrarily like that. I think doing that is a very application-specific decision to make, not something to encourage.

esoterra commented 4 months ago

Fair, OkPretty is pretty out there.

As someone writing a compiler right now, being able to unwrap in my tests and actually see where the error happened is incredibly helpful.

zkat commented 4 months ago

Yes, but the cost of just adding this as your own utility, since you find it useful to you, is very very low. It might be best to keep it that way.

esoterra commented 4 months ago

Fair I did in fact implement this for my project.

I just meant that there are real use cases for this. Whether there are enough to justify inclusion 🤷‍♀️

https://github.com/esoterra/claw-lang/blob/main/crates/common/src/diagnostic.rs