rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
11.44k stars 1.54k forks source link

Lint to ensure all variants of an enum are used #9255

Open Nemo157 opened 2 years ago

Nemo157 commented 2 years ago

What it does

Check that all variants of an enum are used inside a function.

This is similar to standard exhaustiveness checking of a match, and the non-exhaustive-omitted-patterns lint for non-exhaustive enums; but generalized to usecases that are not using match.

Lint Name

No response

Category

restriction

Advantage

Ensures that when new variants are added to an enum (whether because it's an internal enum, public enum with a major version bump, or an external non-exhaustive enum) those variants are handled.

A major usecase I see for this is things that produce values of the enum and are expected to be able to produce every variant, such as parsers.

Drawbacks

How do you determine which enum to lint on?

Example

enum Intensity { Normal, Bold, Faint }
struct Unknown;
impl FromStr for Intensity {
    type Err = Unknown;
    fn from_str(s: &str) -> Result<Self, Self::Err> {
        match s {
            "normal" => Ok(Intensity::Normal),
            "bold" => Ok(Intensity::Bold),
            _ => Err(Unknown),
        }
    }
}

Could be written as:

enum Intensity { Normal, Bold, Faint }
struct Unknown;
impl FromStr for Intensity {
    type Err = Unknown;
    fn from_str(s: &str) -> Result<Self, Self::Err> {
        match s {
            "normal" => Ok(Intensity::Normal),
            "bold" => Ok(Intensity::Bold),
            "faint" => Ok(Intensity::Faint),
            _ => Err(Unknown),
        }
    }
}
5225225 commented 2 years ago

I wonder if you could have an attribute like #[all_constructed(Intensity)] that goes on some block, and then all variants of that enum must be constructed inside that scope, or a warning is fired.

That would solve the "how do you know what to lint on" issue nicely. But that wouldn't quite be a lint, because it needs an argument.

And, as I understand it, all lints are always emitted, but allowed ones are just ignored. Which leads to performance issues when you have lints that would be triggered a lot, and you only care about it in a few places. I believe lints against implicit drops run into that issue, and I'm not sure if that's been fixed yet.

kpreid commented 9 months ago

The discussion above mentions producing all variants within some function, but some cases use cases might want to lint for producing all variants within the crate — for example, an error enum might have its different variants constructed by many different functions. In this case, there's no need to define the scope; it could just be a lint you enable or disable as usual — on the enum, or even a single variant of the enum (if a “deprecated” variant is no longer ever produced but needs to still exist for compatibility).