Type mismatch error should have spans if it's too long

[Manishearth]

error: mismatched types: expected `session_types::Session<session_types::Cap<(), session_types::Rec<session_types::Offer<_, session_types::Rcv<collections::string::String, session_types::Choose<session_types::Snd<_, _>, session_types::Snd<_, session_types::Offer<_, session_types::Rcv<collections::string::String, session_types::Choose<session_types::Snd<(), _>, session_types::Snd<_, session_types::Rec<session_types::Offer<_, session_types::Offer<_, session_types::Snd<collections::string::String, session_types::Eps>>>>>>>>>>>>>>, (), _>`, found `session_types::Session<session_types::Cap<(), session_types::Rec<session_types::Offer<session_types::Snd<collections::string::String, session_types::Eps>, session_types::Rcv<collections::string::String, session_types::Choose<session_types::Snd<collections::string::String, session_types::Var<session_types::Z>>, session_types::Snd<collections::string::String, session_types::Offer<session_types::Snd<collections::string::String, session_types::Eps>, session_types::Rcv<collections::string::String, session_types::Choose<session_types::Snd<collections::string::String, session_types::Var<session_types::Z>>, session_types::Snd<collections::string::String, session_types::Rec<session_types::Offer<session_types::Snd<isize, session_types::Snd<isize, session_types::Var<session_types::Z>>>, session_types::Offer<session_types::Rcv<isize, session_types::Choose<session_types::Snd<collections::string::String, session_types::Snd<collections::string::String, session_types::Var<session_types::Z>>>, session_types::Snd<collections::string::String, session_types::Var<session_types::Z>>>>, session_types::Snd<collections::string::String, session_types::Eps>>>>>>>>>>>>>>, (), ()>` (expected (), found struct collections::string::String) 

It would be useful if there was a note as to where the () and String from (expected (), found struct collections::string::String) were for large types.

An additional note that's shown when the error report is above x (300?) characters which highlights the mismatched types (using something like our ^~~~~~ for code spans) would be useful.

[larsbergstrom]

cc me

[nikomatsakis]

I think there may already be an issue on this (@wycats, do you remember?), but I think another approach here is to change the formatting so that the expected/found types are on separate lines, aligned horizontally, with diffs (identified textually) highlighted in red. This would be fantastic to implement but probably requires a richer alternative to span_err (I'm all for it though)

[nikomatsakis]

Something like span_expected_found_err

[larsbergstrom]

cc @Munksgaard @laumann

[kmcallister]

I think another approach here is to change the formatting so that the expected/found types are on separate lines, aligned horizontally, with diffs (identified textually) highlighted in red.

Big +1. This would be a killer feature and one that every beginner would soon be aware of :)

[Munksgaard]

We frequently run into problems like these. Thankfully we have already seen some significant improvements, primarily that errors like this are now split up into several lines, and that Rust actually tries (with varying success) to point out where it happened. For example, the error mentioned by @Manishearth now looks like this:

foo.rs:172:19: 172:24 error: mismatched types:
 expected `session_types::Session<session_types::Cap<(), session_types::Rec<session_types::Offer<_, session_types::Rcv<collections::string::String, session_types::Choose<session_types::Snd<_, _>, session_types::Snd<_, session_types::Offer<_, session_types::Rcv<collections::string::String, session_types::Choose<session_types::Snd<(), _>, session_types::Snd<_, session_types::Rec<session_types::Offer<_, session_types::Offer<_, session_types::Snd<collections::string::String, session_types::Eps>>>>>>>>>>>>>>, (), _>`,
    found `session_types::Session<session_types::Cap<(), session_types::Rec<session_types::Offer<session_types::Snd<collections::string::String, session_types::Eps>, session_types::Rcv<collections::string::String, session_types::Choose<session_types::Snd<collections::string::String, session_types::Var<session_types::Z>>, session_types::Snd<collections::string::String, session_types::Offer<session_types::Snd<collections::string::String, session_types::Eps>, session_types::Rcv<collections::string::String, session_types::Choose<session_types::Snd<collections::string::String, session_types::Var<session_types::Z>>, session_types::Snd<collections::string::String, session_types::Rec<session_types::Offer<session_types::Snd<isize, session_types::Snd<isize, session_types::Var<session_types::Z>>>, session_types::Offer<session_types::Rcv<isize, session_types::Choose<session_types::Snd<collections::string::String, session_types::Snd<collections::string::String, session_types::Var<session_types::Z>>>, session_types::Snd<collections::string::String, session_types::Var<session_types::Z>>>>, session_types::Snd<collections::string::String, session_types::Eps>>>>>>>>>>>>>>, (), ()>`
(expected (),
    found struct `collections::string::String`)
foo.rs:172     connect(cli2, srv());
                              ^~~~~

Which is already much better.

For us, it would be immensely helpful, if a lot of the superfluous stuff was removed from the "found" part of the error message. The "expected" part is nicely trimmed, with underscores replacing irrelevant type parameters. Ideally, the error message would look something like:

foo.rs:172:19: 172:24 error: mismatched types:
 expected `Session<Cap<(), Rec<Offer<_, Rcv<String, Choose<Snd<_, _>, Snd<_, Offer<_, Rcv<String, Choose<Snd<(), _>, _>>>>>>>>>, (), _>`,
    found `Session<Cap<(), Rec<Offer<_, Rcv<String, Choose<Snd<_, _>, Snd<_, Offer<_, Rcv<String, Choose<Snd<String, _>, _>>>>>>>>>, (), ()>`
(expected (),
    found struct `String`)
foo.rs:172     connect(cli2, srv());
                              ^~~~~

(With or without the collections::string:: and session_types:: that I stripped for my sanity)

[Munksgaard]

Or, to put it more simply, consider the following code

struct Foo<T, U> ( u8 );

fn bar(x: Foo<u8, i8>) {

}

fn main() {
  let x: Foo<u8, u8> = Foo(42);
  bar(x);
}

We get the following error message:

foo.rs:9:7: 9:8 error: mismatched types:
 expected `Foo<u8, i8>`,
    found `Foo<u8, i64>`
(expected i8,
    found i64)
foo.rs:9   bar(x);
               ^
error: aborting due to previous error

Ideally, Rust could replace Foo<u8, i64> in the expected part with Foo<_, i64>, since the first type parameter to Foo is not relevant to this error message.

[dhardy]

Another case:

mismatched types: expected `core::result::Result<lossfunction::scenario::PredictionRecord, Box<misc::OMErrorT + 'static>>`, found `lossfunction::scenario::PredictionRecord` (expected enum core::result::Result, found struct lossfunction::scenario::PredictionRecord)

would be much easier to read if (a) it factored out the common type and (b) suggested a fix:

mismatched types: did you forget to wrap with Ok( ... ) ?
expected: `core::result::Result<P, Box<misc::OMErrorT + 'static>>`
found: `P`
where: `P` = `lossfunction::scenario::PredictionRecord`

[steveklabnik]

Triage; still lots of room for improvement here.

[estebank]

I can see five possible cases:

1:
expected: `A<B<C, D>, E>`, found: `B<C, D>`  // You likely forgot to use `Ok` or `Some`,
                                             // discussed in previous comments
2:
expected: `A<B<C, D>, E>`, found: `B<X, Y>`  // You likely forgot to use `Ok` and above
                                             // that you're also providing a wrong type
                                             // regardless
3:
expected: `A<B<C, D>, E>`, found: `A<F, E>`  // You provided the wrong type, already
                                             // handled somewhat by the current
                                             // implementation
4:
expected: `A<B<C, D>, E>`, found: `A<F, J>`  // You provided the wrong type for both
                                             // the success and failure types
5:
expected: `B<C, D>`, found: `A<B<C, D>, E>`  // You probably used `Ok` or `Some` when
                                             // you didn't need to

For the 1st case, I like @dhardy's proposal:

expected: `core::result::Result<P, Box<misc::OMErrorT + 'static>>`
   found: `P`
   where: `P` = `lossfunction::scenario::PredictionRecord`

For the 2nd case, I can't think of a nice way to present it over what we already have: the types are simply wrong.

For the 3rd case it currently would look a bit like this, which I don't think is too bad:

 expected `core::result::Result<lossfunction::scenario::PredictionRecord, Box<misc::OMErrorT + 'static>>`,
    found `core::result::Result<&str, Box<misc::OMErrorT + 'static>>`
(expected lossfunction::scenario::PredictionRecord,
    found &str)

but could be changed to look this way to look consistent with the 1st case's (this example is for the 4th case, but it'd look practically the same for the 3rd case):

mismatched types in `core::result::Result<P, Q>`, where `P`, `Q`:
    expected: `P` = `lossfunction::scenario::PredictionRecord`
       found: `P` = `&str`
    expected: `Q` = `Box<misc::OMErrorT + 'static>`
       found: `Q` = `misc::OMErrorT + 'static`

Supporting this case could get hairy quickly. For example, how many type parameters before you are noisier than the current message?

The 5th case is the same as the 1st one, only reversed:

expected: `P`
   found: `core::result::Result<P, Box<misc::OMErrorT + 'static>>`
   where: `P` = `lossfunction::scenario::PredictionRecord`

The way I see it, there's nothing that could be done for the 2nd case, supporting the 4th could be tricky to get right, and the 3rd case is already somewhat clean.

That leaves case 1 and 5 as the most highly actionable messages to work on (finding if there're sub matches on type inequality), with the possibility to clean up the 3rd case slightly as asked by @Munksgaard (replace types parameters that are are equal with underscore on message). Does that look right?