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
}
}
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'tnon_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.
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.
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 }.
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...
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!