Point at enum definition when match patterns are not exhaustive

[estebank]

error[E0004]: non-exhaustive patterns: type `X` is non-empty
 --> file.rs:9:11
  |
1 | / enum X {
2 | |     A,
  | |     - variant not covered
3 | |     B,
  | |     - variant not covered
4 | |     C,
  | |     - variant not covered
5 | | }
  | |_- `X` defined here
...
9 |       match x {
  |             ^
  |
  = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms

error[E0004]: non-exhaustive patterns: `B` and `C` not covered
  --> file.rs:11:11
   |
1  | / enum X {
2  | |     A,
3  | |     B,
4  | |     C,
   | |     - not covered
5  | | }
   | |_- `X` defined here
...
11 |       match x {
   |             ^ patterns `C` not covered

When a match expression doesn't have patterns covering every variant,
point at the enum's definition span. On a best effort basis, point at the
variant(s) that are missing. This does not handle the case when the missing
pattern is due to a field's enum variants:

enum E1 {
    A,
    B,
    C,
}
enum E2 {
    A(E1),
    B,
}
fn foo() {
    match E2::A(E1::A) {
        E2::A(E1::B) => {}
        E2::B => {}
    }
    //~^ ERROR `E2::A(E1::A)` and `E2::A(E1::C)` not handled
}

Unify look between match with no arms and match with some missing patterns.

Fix #37518.

[rust-highfive]

r? @eddyb

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

[estebank]

r? @zackmdavis

[zackmdavis]

naming/commentary quibbles, otherwise r=me

[zackmdavis]

I usually write empty vectors as Vec::new() (thinking, "no need to the macro sugar when there aren't any elements to initialize"), but rustfmt doesn't seem to have an opinion. (I guess you could argue that it's technically a semantic change.)

[zackmdavis]

(I realized that this was already here and not a part of the present changeset, but) do we even want to bother saying "type is non-empty" here? If we don't expect users to think or care about the empty enum very often, this might not be the most helpful time to educate them.

[estebank]

changed to "non-exhaustive patterns: variant Foo of type Bar is not handled"

[zackmdavis]

Can we call the variable missing_variant_spans or similar? (I balked at err.span_label(*variant on line 241.)

[estebank]

collected the variant idents instead, keeping the vector name the same

[zackmdavis]

(possibly a bad idea, probably out of scope for this PR) I find it counterintuitive for the primary span of a "non-exaustive patterns" error to point to the x in match x, rather than the body of match arms. (That could get very visually cluttered, though, especially with the secondary definition-spans introduced here. Also, pointing at the discriminant does seem intuitive when it's a pattern rather than an opaque variable, as in the examples in src/test/ui/check_match/issue-35609.stderr.)

[estebank]

I have the same thoughts you do, but I lay on the side of using the smallest possible primary span, for VSCode's sake.

[estebank]

@bors r=zackmdavis

[bors]

📌 Commit d651281 has been approved by zackmdavis

[bors]

⌛ Testing commit d651281 with merge a9da8fc...

[bors]

☀️ Test successful - checks-travis, status-appveyor
Approved by: zackmdavis
Pushing a9da8fc to master...