Matching field-less enum variants that aren't imported should deny-lint, not just give lots of warnings

[igor37]

When the enum variants listed in a match block omit the preceding EnumName:: the compiler may still accept the code, even if some cases were not handled or nonexistent variants are listed.

I tried this code:

enum Enum {
    A,
    B,
    C,
}

fn main() {
    let b = Enum::B;

    match b {
        A => println!("A"),
        B => println!("B"),
        Foo => {},
    }
}

I expected to see this happen: Compiler errors regarding missing Enum::, missing variant Enum::C, and invalid variant.

Instead, this happened: the code compiled and printed "A"

Meta

rustc --version --verbose:

rustc 1.64.0 (a55dd71d5 2022-09-19)
binary: rustc
commit-hash: a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52
commit-date: 2022-09-19
host: aarch64-apple-darwin
release: 1.64.0
LLVM version: 14.0.6

Also tested with rustc 1.66.0-nightly (f83e0266c 2022-10-03)

[aDotInTheVoid]

If you look at the warnings, they explain what is happening

warning[[E0170]](https://doc.rust-lang.org/stable/error-index.html#E0170): pattern binding `A` is named the same as one of the variants of the type `Enum`
  --> src/main.rs:11:9
   |
11 |         A => println!("A"),
   |         ^ help: to match on the variant, qualify the path: `Enum::A`
   |
   = note: `#[warn(bindings_with_variant_name)]` on by default

warning[[E0170]](https://doc.rust-lang.org/stable/error-index.html#E0170): pattern binding `B` is named the same as one of the variants of the type `Enum`
  --> src/main.rs:12:9
   |
12 |         B => println!("B"),
   |         ^ help: to match on the variant, qualify the path: `Enum::B`

warning: unreachable pattern
  --> src/main.rs:12:9
   |
11 |         A => println!("A"),
   |         - matches any value
12 |         B => println!("B"),
   |         ^ unreachable pattern
   |
   = note: `#[warn(unreachable_patterns)]` on by default

warning: unreachable pattern
  --> src/main.rs:13:9
   |
11 |         A => println!("A"),
   |         - matches any value
12 |         B => println!("B"),
13 |         Foo => {},
   |         ^^^ unreachable pattern

warning: unused variable: `A`
  --> src/main.rs:11:9
   |
11 |         A => println!("A"),
   |         ^ help: if this is intentional, prefix it with an underscore: `_A`
   |
   = note: `#[warn(unused_variables)]` on by default

warning: unused variable: `B`
  --> src/main.rs:12:9
   |
12 |         B => println!("B"),
   |         ^ help: if this is intentional, prefix it with an underscore: `_B`

warning: unused variable: `Foo`
  --> src/main.rs:13:9
   |
13 |         Foo => {},
   |         ^^^ help: if this is intentional, prefix it with an underscore: `_Foo`

warning: variants `A` and `C` are never constructed
 --> src/main.rs:2:5
  |
1 | enum Enum {
  |      ---- variants in this enum
2 |     A,
  |     ^
3 |     B,
4 |     C,
  |     ^
  |
  = note: `#[warn(dead_code)]` on by default

warning: variable `A` should have a snake case name
  --> src/main.rs:11:9
   |
11 |         A => println!("A"),
   |         ^ help: convert the identifier to snake case: `a`
   |
   = note: `#[warn(non_snake_case)]` on by default

warning: variable `B` should have a snake case name
  --> src/main.rs:12:9
   |
12 |         B => println!("B"),
   |         ^ help: convert the identifier to snake case: `b`

warning: variable `Foo` should have a snake case name
  --> src/main.rs:13:9
   |
13 |         Foo => {},
   |         ^^^ help: convert the identifier to snake case (notice the capitalization): `foo`

For more information about this error, try `rustc --explain E0170`.
warning: `playground` (bin "playground") generated 11 warnings

A is a pattern that matches any value, because is isn't an enum variant. This is confusing, but the compiller has good warnings.

This issue can be closed as Not A Bug

[igor37]

Ah yes, didn't consider that, sorry.

I'm used to getting errors in such cases because matching more complex variants like this will usually fail.

[leonardo-m]

A case of "Warning Blindness"?

[scottmcm]

I think this is a case where we should have a specific lint that we can make deny-by-default. The generic ones are good, but it would be nice to have one for "I see what happened here, and you definitely didn't want this" that can block compilation (by default -- it could be allowed if you really wanted).

Specifically, I'm thinking a lint that looks for a pattern with a match-all binding whose name is the same as one of the variants of the enum. It can suggest changing the pattern from A to TheEnum::A.

See https://rustc-dev-guide.rust-lang.org/diagnostics.html for how to make a lint, if anyone is interested in picking this up!

(Perhaps it should also support A @ _ as a way to get such a binding and suppress this new lint, because at that point they said really clearly that they want a binding that matches anything. But that's low pri because I that's almost certainly not what someone wanted.)

[amustaque97]

@rustbot claim

[timrobertsdev]

@rustbot claim

[est31]

@scottmcm how would that lint be different from bindings_with_variant_name, and can't one just change that one to be deny by default?

[est31]

Regarding some history of that lint, it was added in 5804a30 (#19115) as an unconditional warning, and later turned into a lint in 2c3e5d3 (#67770).

[scottmcm]

Oh, thanks @est31 ! If it already exists that's even better -- just a matter of making it deny-by-default.

(Want to send a PR for that, timrobertsdev ?)

[zhang-ray]

I got the same problem recently (#104966) ... :) I totally agree with this proposal!

[est31]

@scottmcm you're welcome :). It's interesting that I was the first person to point that out after your comment: many people have probably seen it, and seen the warning output in the comment posted by @aDotInTheVoid where bindings_with_variant_name was included, but didn't make the "wait a moment, isn't the first warning in this comment precisely what @scottmcm wants" thought. It seems that the "warning blindness" is a real effect, where too many warnings make you not read them... Speaks in favour of #104154 I guess, because, unless you use a script for your identifiers that doesn't have a upper/lower case distinction, you will get at least two warnings, one for non_snake_case, and one for bindings_with_variant_name. You will most likely get another unused_variables warning for not using the unintentional binding, so that makes three or two depending on the script you use.