Detect enums with cfg controlled variants that aren't non_exhaustive

[joshtriplett]

What it does

If an enum has variants controlled by cfg (e.g. feature flags), it should almost certainly be non_exhaustive. This lint would flag exhaustive enums controlled by cfg.

This should not flag enums with cfg variants that nonetheless have the same variants in all cfgs.

Lint Name

cfg_enum_exhaustive

Category

suspicious

Advantage

• Avoid requiring cfg in every match
• Avoid having unexpected errors that only arise in some configurations

Drawbacks

Possible false positives.

Example

enum E {
    V1,
    #[cfg(feature = "xyz")]
    V2,
}

Could be written as:

#[non_exhaustive]
enum E {
    V1,
    #[cfg(feature = "xyz")]
    V2,
}

[pali6]

Should an analogous lint exist for structs and enum variants too?

[joshtriplett]

@pali6 Yes, I think so, if they have public fields that are cfg-gated, and don't have any private fields.

[pali6]

I've tried experimenting with implementing the lint but sadly reached a dead end. Per Zulip discussion I'll post some of my observations here:

Variants with the same name

In order to make sure that there is a variant not present in all configuration combinations we need to also detect cases where two or more variants with the same name have cfg conditions that cover the whole configuration space. Example:

pub enum E {
    V1,
    #[cfg(feature = "foo")]
    V2(u32),
    #[not(feature = "foo")]
    V2(u64),
}

Due to existence of and, or and not this translates to checking whether a Boolean formula is a tautology so unless we only want to lint for trivial cases this might need to use a SAT-solver even if an inefficient one. In my experiments I (ab)used the already included dependency quine-mc_cluskey for this purpose but I am not sure if it handles the most common cases well enough. (The most common cases being that there are possibly multiple logic variables but most are only used once in a single-variable clause.)

It might or might not be a good idea to also suppress the lint when the whole enum has #[cfg_attr(condition, non_exhaustive)] in a way that toggles non_exhaustive whenever some variant is missing. If this is desired, it is fairly simple to add to the formula thrown at the SAT-solver.

Exported items

This was not mentioned in the issue but presumably this lint should only trigger when the enum is exported similarly to how other non_exhaustive lints do it.

Where to find #[cfg] attributes?

And this is where I got stuck. Originally I implemented this in a late pass due to checking that the enum is exported. After debugging for a while, I ended up realizing that at this point items with inactive cfg attributes are gone which makes sense. Early pass exhibits the same behaviour. After advice from zulip I looked at some match lints which check for cfg but those only check for their existence and essentially they re-lex the relevant span. Per the first section of this post here we would need some actual parsing and that feels way too complex for me to do correctly.

[joshtriplett]

@pali6

This was not mentioned in the issue but presumably this lint should only trigger when the enum is exported similarly to how other non_exhaustive lints do it.

You're right, this should only apply to pub items.

In order to make sure that there is a variant not present in all configuration combinations we need to also detect cases where two or more variants with the same name have cfg conditions that cover the whole configuration space.

I think it'd be completely reasonable to just handle the simple cases: if there are multiple variants with the same name, either just don't emit the warning (and people are no worse off than they are now), or throw it at a SAT solver but give up if it's too complex.