Better error message when missing prefix in multiple patterns

[fbenkstein]

Given the example code:

enum E { A, B, C }
fn f() -> E { unimplemented!(); }

fn main() {
    match f() {
        A => print!("It was A!"),
        _ => print!("Something else"),
    }
}

I get a really nice warning message about what I did wrong:

warning[E0170]: pattern binding `A` is named the same as one of the variants of the type `E`
 --> src/main.rs:6:9
  |
6 |         A => print!("It was A!"),
  |         ^
  |
  = help: if you meant to match on a variant, consider making the path in the pattern qualified: `E::A`

However, if I change the main function to match multiple patterns:

fn main() {
    match f() {
        A => print!("It was A!"),
        B | C => print!("It was B or C!"),
    }
}

The error message is really confusing with no hint how to fix it:

error[E0408]: variable `B` is not bound in all patterns
 --> src/main.rs:8:13
  |
8 |         B | C => print!("It was B or C!"),
  |         -   ^ pattern doesn't bind `B`
  |         |
  |         variable not in all patterns

Please expand the error message for the second case to look a bit more like the first case.

[estebank]

Triage: no change.

We need to supply the line

 = help: if you meant to match on a variant, consider making the path in the pattern qualified: `E::A`

in E0408.

[jakubadamw]

@estebank, so I thought I would implement the change you've proposed but have been thinking how best to restrict the kinds of cases where this help note would be appended to the error as I figured it may be rather noisy and/or confusing within the context of some false negatives. So during resolution we of course cannot look at the types to see if it's possible we're matching against an enum. But what do you think about having the help note kick in only if the binding starts with an upper case letter? I know this is a convention that should not really be a concern of the compiler but seeing that using it would only make the diagnostic note a bit more fine-grained, perhaps it's not a terrible idea?

[estebank]

@jakubadamw we already do that in some places, like the parser to infer intent, but in this case how would you figure out that you want to suggest E::B and E::C without resolving E?

I believe a more holistic solution would be to detect "variable not bound in all patterns" errors where all bindings are single idents, record that fact while throwing the current error and recovering as if there had been a single variable, and later when we have access to the machinery E0170 uses emit a warning suggesting E::B and E::C. It is a bit more involved but if we recover as if only B had been present the current diagnostic should kick in suggesting E::B (leaving E::C without a suggestion).

---

Edit: that won't work as things stand, as the warning won't be emitted today if there are errors, I think. That would also need to be changed to make it always be emitted :-/

[jakubadamw]

@estebank, no, I didn't mean to say we need to suggest possible variants. I too concluded that it's non-trivial with the current ordering of passes and the early bailout (indeed, type checking is impossible without resolution succeeding).

I was going to add the help note you proposed back in May, just that I would make it dependent on bindings having a Variant-alike naming. From what you're saying there's a precedent in libsyntax, so I guess this is an acceptable solution? I'll make a PR then.

[estebank]

@jakubadamw sorry, I misunderstood. Go ahead, it sounds good.

[jakubadamw]

@estebank, no, it's likely me who wasn't clear enough. :) I submitted a PR.