miette fails silently when span is wrong

[mimoo]

OK I now understand why I just get an error message without the full beautiful report:

it's because my span is wrong. It would have been nice if an error would have been thrown so that it's easier to understand why no report is displayed : o

[Benjamin-L]

Could you send an example of the input that you're providing, so I can try to reproduce this behavior? There are several different ways that a span can be wrong, and I'm not sure exactly what's going on here.

[mimoo]

I don't have one anymore, but basically if the span in your error points to an offset that's too far off the size of your source, then this will happen. For example using "" as source, and (4, 5) as span

[reissbaker]

This bit me too and it was fairly confusing.

[passcod]

Got hit by that too because I emitted spans which were (start, end) and converting them for the error with .into(), without realising they should have been (start, length).

[zkat]

Yeah this doesn't look great. :\

I don't have time to get to it myself right now, but if anyone wants to talk about potential solutions, I'm all for it. It's kinda tricky to figure out what a failure coming from a failure library should even look like (should it panic?)

[passcod]

Just to have a minimal example, this:

use miette::{Diagnostic, SourceSpan, Result};
use thiserror::Error;

#[derive(Debug, Diagnostic, Error)]
#[error("parse error")]
struct ParseError {
    #[source_code]
    src: &'static str,

    #[label("uh oh")]
    wrong: SourceSpan,
}

fn main() -> Result<()> {
    Err(ParseError {
        src: "did a fucky",
        wrong: (6, 11).into(),
    })?;
    Ok(())
}

outputs:

Error:
  × parse error

while changing the span to (6, 5) outputs:

Error:                
  × parse error
   ╭────
 1 │ did a fucky
   ·       ──┬──
   ·         ╰── uh oh
   ╰────

while using the JSON handler with the first works fine (it happily outputs 'wrong' spans).

[passcod]

I think there's a number of points to improve on:

1. Documentation.
   The (1, 2).into() syntax is introduced in the crate doc. The text does mention: "This is a basic byte-offset and length" but this is easy to miss; the code sample only says:

   snip2: (usize, usize), // `(usize, usize)` is `Into<SourceSpan>`!

   I propose changing that to say:

   snip2: (usize, usize), // (offset, length) -- `(usize, usize)` is `Into<SourceSpan>`!

2. Constructor.
   While using (usize, usize) directly won't benefit, SourceSpan::new() could look like:

   pub fn new(start: impl Into<SourceOffset>, length: impl Into<SourceOffset>) -> Self;

   such that this works:

   SourceSpan::new(6, 5)

   and in your editor, would show up as:

   SourceSpan::new(start: 6, length: 5)
   

   alternatively it could be a new method if that's API-breaking. A new method with an evocative name could also help when there's no IDE/RA.
3. Error reporting.
   That's the tough one, because as the example above shows, the bug simply appears to not show the snippet if the sourcespan is wrong. As a dev, if you're expecting one, you might try to look a bit further by doing this:

   println!("{:?}", Report::from(ParseError {
       src: "did a fucky",
       wrong: (6, 11).into(),
   }));

   which outputs:

   thread 'main' panicked at 'failed printing to stdout: formatter error', library/std/src/io/stdio.rs:1009:9
   note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
   

   and if you're really determined, you might try:

   let mut out = String::new();
   miette::GraphicalReportHandler::new().render_report(&mut out, &ParseError {
       src: "did a fucky",
       wrong: (6, 11).into(),
   }).into_diagnostic()?;
   dbg!(out);

   which outputs:

   Error:
     × an error occurred when formatting an argument
   

   As a first step, improving whichever of these errors is possible to make better could help.
4. Being more forgiving with spans.
   One fairly easy way to avoid a silent error would be to be more forgiving with "invalid" spans: if a span appears to point to outside the source, clamp it to the actual bounds of the source. So for spans which are (start, end) instead of (start, length), the span shown would appear to wrongly extend much further than it should, but it would show. For spans which are entirely outside of the source, the display could perhaps be pointing at the last byte.
5. Fail inline.
   When the handler detects an invalid span, instead of crashing, it could show as much of the error as it can, but for each invalid span replace the span+label with "BUG: this source span is invalid".

[mimoo]

an easy way to fix this is to panic only in debug mode, another idea is to change the error message to say something like "the given span does not correctly fit in the source file"