[Pre-RFC] Add an `unmatched_variant` lint

Summary

Add a opt-in lint which warns when the user writes a match expression on an enum type without matching all of the enum's variants.

Motivation

The recently stabilized #[non_exhaustive] attribute gives library authors the ability to add new variants to enums without causing compilation errors in downstream code. However, there is no mechanism to inform downstream users that a new enum variant has been added. This lint would allow downstream users to opt into warnings or errors that new variants have been added. This complements the #[non_exhaustive] attribute which gives Semantic Versioning stability to library authors by also allowing down stream users to receive feedback when this occurs.

Additionally, some users prefer to always match every variant in an enum rather than using the _ wildcard match arm. This lint would flag instances where an enum variant is only covered by the wildcard match arm.

Explanation

The unmatched_variant lint can be set to warn or err at the function, module, or crate level. When enabled, a lint at the appropriate warning or error level will be emitted when a match expression is found which does not test all variants of the scrutinee expression type (regardless of whether that enum is #[non_exhaustive] or not).

Example

enum Foo {
  A(u8),
  B,
  C
}

#[err(unmatched_variant)]
fn test(f: Foo) {
  match f {
    Foo::A(n) => {},
    _ => {}, //~ ERROR variants `Foo::B` and `Foo::C` are not matched
}

#[non_exhaustive]
enum Bar {
  Baz,
  Bat,
}

#[warn(unmatched_variant)]
fn test(b: Bar) {
  match b {
    Bar::Baz => {},
    _ => {}, //~ WARN variant `Bar::Bat` is not matched
  }
}
7 Likes

For use with non_exhaustive, this seems reasonable as a diagnostic tool, as long as it's allow-by-default.

Note, though, that leaving a variant unmatched with an enum that isn't non_exhaustive is already an error, and shouldn't become a warning or less. So this lint needs a name that reflects its interaction with non_exhaustive.

Yes, I'm very much in favor of this.

4 Likes

I'm not sure what you mean here. I think the intent of such a lint is to make it so you must explicitly name each variant in some pattern.

1 Like

This lint could uniformly work just as fine for non-non_exhaustive pattern matching, by requiring the wildcard _ pattern to not be used (or be unreachable) in all cases.

Now, the question is whether we want that. I'd say yes, if you're asking to be warned about it for non_exhaustive enums, you should be fine with being warned about use of the wildcard, and explicitly matching the remaining cases (or locally allowing the lint).

But here's an actual implementation question: how deep is the lint. Consider a struct S { a: u32, b: u32 }. Does a match arm against S { a: _, b: _ } trigger the lint? What if instead of u32, it were an enum, another struct, or a nonexhaustive enum?

The simplest form of the lint would be #![warn(wildcard_match)], which would warn on any use of the wildcard _ in a match pattern unless the pattern is (currently) unreachable. That'd make for a great clippy::pedantic lint, imo.

Oh, I see! I misread the original post, but this makes much more sense.

That sounds like exactly the right name and semantic.

1 Like

I don't know, I'd also think about disallowing the x => ... pattern (and, in general, patterns which do not name a variant). Though perhaps you might want to allow a x if ... => .... Hmm.

You could theoretically define a lint by if the pattern "covers" the entire space by itself (i.e. it would be valid to omit every other arm). That'd "miss" wildcards like S { a: 5, b: _ }, but also allow x if condition(&x) and catch S { a, b }.

Perhaps it would be a good idea to split this proposal into two different lints:

  1. The lint for "you forgot to match a variant on an #[non_exhaustive] enum"
  2. The lint for "disallow wildcards"

Thanks to @CAD97 for raising excellent points around the interactions with structs and more complex matches.

1 Like

The

Agreed. This is basically the same thing as the existing concept of an irrefutable pattern, except that irrefutable patterns combined with match guards wouldn't count.

Would it make sense to root for a, say, macro that expresses "if this code can't be proven to be unreachable, warn", maybe with an error variant? That way, someone who matches on a non-exhaustive enum could use _ => warn_if_reachable!() to get a warning if a variant gets added, but it would also allow for much finer grained control. Not sure if feasible, though...

1 Like

There's already a clippy lint for disallow wildcards for enums: https://rust-lang.github.io/rust-clippy/master/index.html#wildcard_enum_match_arm which I think handles the edge-cases discussed above (ignoring #[non_exhaustive]), except possibly for exhaustive guard clauses.

I've been meaning to update it to not trigger in the #[non_exhaustive] case - I can happily make it more specifically not trigger in the #[non_exhaustive] case only if the existing exhaustive cases are already checked... Hopefully I can get a PR out in a few days, but I wouldn't complain if anyone beat me to it!

I don't think I saw the motivation for splitting it into two lints; can you compare the trade-offs with what I describe in this post? Thanks!

ETA: https://github.com/rust-lang/rust-clippy/pull/4934