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

Inherit source code to causes as well as related errors #401

Merged
merged 1 commit into from
Nov 27, 2024

Conversation

Porges
Copy link
Contributor

@Porges Porges commented Aug 16, 2024

At the moment outer source code is supplied to inner related errors but not to causes. This PR changes that.

This allows late-supplied source code to be supplied using concrete types. Comparisons below…

Concrete types

Firstly, if we wrap our inner error with some well-typed outer wrapper:

#[derive(thiserror::Error, Debug, miette::Diagnostic)]
#[error("inner-error")]
struct InnerError {
    #[label("inner-label")]
    label: (usize, usize),
}

#[derive(thiserror::Error, Debug, miette::Diagnostic)]
#[error("outer-error")]
struct OuterError {
    #[source_code]
    source_code: String,

    #[diagnostic_source]
    inner: InnerError,
}

fn main() -> miette::Result<(), miette::Report> {
    Err(OuterError {
        source_code: "source code".to_string(),
        inner: InnerError { label: (7, 4) },
    })?
}

At the moment we don’t get any source information rendered.

Output:

Error: 
  × outer-error
  ╰─▶   × inner-error

Wrapped in Report

On the other hand, if we wrap using miette::Report, which loses the underlying type information, we get the source code:

#[derive(thiserror::Error, Debug, miette::Diagnostic)]
#[error("inner-error")]
struct InnerError {
    #[label("inner-label")]
    label: (usize, usize),
}

fn main() -> miette::Result<(), miette::Report> {
    Err(miette::Report::new(InnerError { label: (7, 4) }).with_source_code("source code"))?
}

Output:

Error: 
  × inner-error
   ╭────
 1 │ source code
   ·        ──┬─
   ·          ╰── inner-label
   ╰────

Using related

Currently we can achieve something using #[related] but that is semantically-inappropriate for some use cases:

#[derive(thiserror::Error, Debug, miette::Diagnostic)]
#[error("inner-error")]
struct InnerError {
    #[label("inner-label")]
    label: (usize, usize),
}

#[derive(thiserror::Error, Debug, miette::Diagnostic)]
#[error("outer-error")]
struct OuterError {
    #[source_code]
    source_code: String,

    #[related]
    inner: Vec<InnerError>,
}

fn main() -> miette::Result<(), miette::Report> {
    Err(OuterError {
        source_code: "source code".to_string(),
        inner: vec![InnerError { label: (7, 4) }],
    })?
}

The rendering is also different since they're rendered as adjacent errors rather than causes:

Error: 
  × outer-error

Error:
  × inner-error
   ╭────
 1 │ source code
   ·        ──┬─
   ·          ╰── inner-label
   ╰────

With proposed change

After this PR, the output for the first example (exact same code) should be similar to the last example, except exhibiting cause-nesting:

Error: 
  × outer-error
  ╰─▶   × inner-error
         ╭────
       1 │ source code
         ·        ──┬─
         ·          ╰── inner-label
         ╰────

@Porges
Copy link
Contributor Author

Porges commented Aug 16, 2024

The other solution I had considered was to pull up labels from causes into the parent and merge all of the labels together. I think this would be nice but maybe enabled with a different attribute? Something like #[diagnostic_source(flatten)].

Alternately this could happen by default if the #[diagnostic_source] does not have its own source_code defined, which might work quite nicely...

@zkat zkat merged commit 465e6b6 into zkat:main Nov 27, 2024
15 checks passed
@Porges
Copy link
Contributor Author

Porges commented Nov 28, 2024

@zkat thanks!

@Porges Porges deleted the inherit-source-to-causes branch November 28, 2024 01:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants