Refine error span for trait error into borrowed expression

[Nathan-Fenner]

Extends the error span refinement in #106477 to drill into borrowed expressions just like tuples/struct/enum literals. For example,

trait Fancy {}
trait Good {}
impl <'a, T> Fancy for &'a T where T: Good {}
impl <S> Good for Option<S> where S: Iterator {}

fn want_fancy<F>(f: F) where F: Fancy {}

fn example() {
    want_fancy(&Some(5));
//  (BEFORE)   ^^^^^^^^ `{integer}` is not an iterator 
//  (AFTER)          ^  `{integer}` is not an iterator
}

Existing heuristics try to find the right part of the expression to "point at"; current heuristics look at e.g. struct constructors and tuples. This PR adds a new check for borrowed expressions when looking into a borrowed type.

[rustbot]

r? @WaffleLapkin

(rustbot has picked a reviewer for you, use r? to override)

[WaffleLapkin]

Looks reasonable. Do you think there are any other expression kinds that we might want to "drill" into?

[WaffleLapkin]

@bors r+

[bors]

📌 Commit fbcca2a has been approved by WaffleLapkin

It is now in the queue for this repository.

[Nathan-Fenner]

There's a few more expressions that could refine further, but they're all a bit more complicated or less common

• array literals; one issue is choosing the "best" span, since any/every element may be responsible. In an array like [None, None, Some(4), Some(5)] if the problem is that i32: Iterator fails, should we point at the 4 or the 5?
• function calls; propagating the same constraints downward is possible in a similar way, though there's similar ambiguities in terms of which thing to highlight if multiple match
• if/match; could drill into either case, again needing to decide which is "best"

Beyond that I don't think there's as many cases where it's useful, since often you can't point more specifically to some constant/variable that's "at fault" for a missing constraint impl.

[WaffleLapkin]

one issue is choosing the "best" span [...]

@Nathan-Fenner one option is to return MultiSpan so that both 4 and 5 are highlighted. Although I see how this can be tricky to implement. Another is to choose the only option if there is only one or fallback to old behaviour if there are multiple.

[WaffleLapkin]

@Nathan-Fenner I've noticed that with current playground's nightly rustc (1.69.0-nightly (2023-02-26 d962ea5)) your example in the PR description doesn't produce the expected error:

trait Fancy {}
trait Good {}
impl <'a, T> Fancy for &'a T where T: Good {}
impl <S> Good for Option<S> where S: Iterator {}

fn want_fancy<F>(f: F) where F: Fancy {}

fn example() {
    want_fancy(&Some(5));
//  (BEFORE)   ^^^^^^^^ `{integer}` is not an iterator 
//  (AFTER)          ^  `{integer}` is not an iterator
}

error[[E0277]](https://doc.rust-lang.org/nightly/error_codes/E0277.html): `{integer}` is not an iterator
 --> src/lib.rs:9:17
  |
9 |     want_fancy(&Some(5));
  |     ----------  ^^^^^^^ `{integer}` is not an iterator
  |     |
  |     required by a bound introduced by this call
  |
  = help: the trait `Iterator` is not implemented for `{integer}`
  = note: if you want to iterate between `start` until a value `end`, use the exclusive range syntax `start..end` or the inclusive range syntax `start..=end`
  = help: the trait `Fancy` is implemented for `&'a T`
note: required for `Option<{integer}>` to implement `Good`
 --> src/lib.rs:4:10
  |
4 | impl <S> Good for Option<S> where S: Iterator {}
  |          ^^^^     ^^^^^^^^^          -------- unsatisfied trait bound introduced here
note: required for `&Option<{integer}>` to implement `Fancy`
 --> src/lib.rs:3:14
  |
3 | impl <'a, T> Fancy for &'a T where T: Good {}
  |              ^^^^^     ^^^^^          ---- unsatisfied trait bound introduced here
note: required by a bound in `want_fancy`
 --> src/lib.rs:6:33
  |
6 | fn want_fancy<F>(f: F) where F: Fancy {}
  |                                 ^^^^^ required by this bound in `want_fancy`

[play]

I'm fairly certain this version should include this PR already. Do you have an idea why the error's span is bigger than it should be? (note though that it's still smaller than stable's — stable includes & in the error span, while nightly doesn't)

[Nathan-Fenner]

@WaffleLapkin That's a good catch. I think this is actually a regression/miss from #106477 which should have handled this case. I'll take a look at it later - I probably missed some case for the Some constructor, either because it's in the prelude or it's unqualified.