Deleting enum variant unexpectedly changes behavior in match statement

[jmikkola]

This came up when I was deleting some variants of a rather large enum. The variants were never used (indeed, the compiler warned me that they were unused), so deleting them should be safe. Much to my surprise, this caused test failures.

Upon closer inspection, it turned out that although my code never created those enum values, it did match them in a match statement (which I had not cleaned up). This caused the reference to that variant in the pattern to turn into a variable, which matched everything.

Here's a simplified example where B represents the enum variant[s] deleted.

I tried this code:

playground

enum MyEnum {
    A,
    // B, // the assertion below works if B is uncommented
    C,
}

fn to_int(my_enum: MyEnum) -> i32 {
    use MyEnum::*;
    match my_enum {
        A => 1,
        B => 2,
        C => 3,
    }
}

fn main() {
    assert_eq!(3, to_int(MyEnum::C));
}

I expected to see this happen:

When removing the variant B of the enum, I expected the match statement to fail to compile because it still referenced B.

Instead, this happened:

It compiled with a warning that the C variant is unreachable because B matches any pattern.

---

This feels like Rust behaving correctly according to the indented behavior, but the intended behavior is confusing.

I'm not sure if there is anything useful that could be done about this (perhaps disallow capitalized variables in patterns? or replace the warning with an error?).

[workingjubilee]

The following version of this example does not compile:

enum MyEnum {
    A,
    // B,
    C,
}

fn to_int(my_enum: MyEnum) -> i32 {
    use MyEnum::{A, B, C};  //~ error[E0432]: unresolved import `MyEnum::B`
    match my_enum {
        A => 1,
        B => 2,
        C => 3,
    }
}

Probably something like "globbing enum variants, specifically" should be linted against.

[leonardo-m]

It compiled with a warning that the C variant is unreachable because B matches any pattern.

In Rust (and other languages) warnings should be taken seriously.

Perhaps the solution is to turn the "unreachable pattern" warning into an error?

[workingjubilee]

Hm. What standard must be met to make a lint deny-by-default? I am hesitant to say "hard error", because it may be that some people #![allow(unreachable_patterns)] in otherwise sound code. Unused variables, for instance, are very rarely desired, and it would be reasonable to say you should never have them in "production" code, but people are only warned, not denied or errored, on them. And I think that disallowing unreachable patterns by default risks making it too hard to refactor match code.