obi1kenobi / cargo-semver-checks

Scan your Rust crate for semver violations.
Apache License 2.0
1.18k stars 75 forks source link

New lints: an enum variant changes from unit/tuple/struct to another kind. #884

Open suaviloquence opened 2 months ago

suaviloquence commented 2 months ago

Describe your use case

We have lints for when a struct changes "kind":

However, currently cargo-semver-checks does not check when an enum variant changes kind like this.

from:

pub enum Enum { 
    WasTuple(),
    WasStruct {},
    WasUnit,
    TupleItem (()),
    StructItem { a: (), },
    Unit,
}

to:

pub enum Enum {
    WasTuple {},
    WasStruct,
    WasUnit(),
    TupleItem { a: () },
    StructItem,
    Unit(()),
}

the first three variants would break with:

let _ = Enum::WasTuple;
// since it's now a tuple variant, this still gives us a value,
// but it's a `fn(()) -> Enum` tuple constructor, not an `Enum` instance.
let _: Enum = Enum::WasStruct;
let _ = Enum::WasUnit;

the second three also have breakage in a similar fashion, and cargo-semver-checks currently does not catch the fields_missing/removed lints because these variants changed kind

Describe the solution you'd like

Thus, it would be helpful to have lint(s) that detect this change for constructible enum variants - if a tuple/struct variant is marked #[non_exhaustive] in the baseline version, this is not a breaking change because it was not possible to construct these variants in the baseline version.

Alternatives, if applicable

No response

Additional Context

No response

obi1kenobi commented 2 months ago

Nice catch!

Similarly to the #[non_exhaustive] case, if the variant was #[doc(hidden)] we probably don't want to lint on it since the variant isn't meant to be constructed by downstream code.

For a piece of code like:

pub enum Example {
    Plain,
    Tuple(),
    Struct {},
}

Note that #[non_exhaustive] tuple variants can't be constructed by an external crate, so witness (proof-of-breakage) code can't rely on constructing the variant.

We want the following lints:

Remaining questions: