Skip to content

Commit

Permalink
Remove the exhaustive_enums lint. (#83)
Browse files Browse the repository at this point in the history
## Summary

The
[`exhaustive_enums`](https://rust-lang.github.io/rust-clippy/master/index.html#exhaustive_enums)
Clippy lint is annoying at best, but even harmful in certain cases. We
don't actually use it in practice. It's best to just remove it and make
the de facto ignoring official.

## We already ignore it everywhere

For a non-exhaustive list of examples, see any of the following crates:


[`xilem`](https://github.com/linebender/xilem/blob/ebc25ca449df9f5ad4ad385f1de17071a6d1ad16/xilem/src/lib.rs#L27),
[`xilem_core`](https://github.com/linebender/xilem/blob/ebc25ca449df9f5ad4ad385f1de17071a6d1ad16/xilem_core/src/lib.rs#L27),
[`xilem_web`](https://github.com/linebender/xilem/blob/ebc25ca449df9f5ad4ad385f1de17071a6d1ad16/xilem_web/src/lib.rs#L27),
[`masonry`](https://github.com/linebender/xilem/blob/ebc25ca449df9f5ad4ad385f1de17071a6d1ad16/masonry/src/lib.rs#L121),
[`vello`](https://github.com/linebender/vello/blob/3275ec85d831180be81820de06cca29a97a757f5/vello/src/lib.rs#L110),
[`vello_encoding`](https://github.com/linebender/vello/blob/3275ec85d831180be81820de06cca29a97a757f5/vello_encoding/src/lib.rs#L37),
[`vello_shaders`](https://github.com/linebender/vello/blob/3275ec85d831180be81820de06cca29a97a757f5/vello_shaders/src/lib.rs#L37),
[`vello_tests`](https://github.com/linebender/vello/blob/3275ec85d831180be81820de06cca29a97a757f5/vello_tests/src/lib.rs#L28),
[`kurbo`](https://github.com/linebender/kurbo/blob/ebb855376937616086c75b47c163c3b3f0c81229/src/lib.rs#L101),
[`parley`](https://github.com/linebender/parley/blob/050fd296b57ff0a8b8378ddb90062c1e7465916c/parley/src/lib.rs#L91),
[`fontique`](https://github.com/linebender/parley/blob/050fd296b57ff0a8b8378ddb90062c1e7465916c/fontique/src/lib.rs#L23),
[`peniko`](https://github.com/linebender/peniko/blob/bb85156814ef9624bff0ad226e51cc0d9f6bf1e9/src/lib.rs#L24-L27),
[`interpoli`](https://github.com/linebender/interpoli/blob/8fee94198d39f506e6ba6f34709ed48914e62a11/src/lib.rs#L24),
[`velato`](https://github.com/linebender/velato/blob/2d6cd9516f93d662c6ea4096bbf837b8151dfc76/src/lib.rs#L71).

## Exhaustive enums are great

First, we have the classics from the Rust standard library:

```rust
pub enum Option<T> {
    Some(T),
    None,
}

pub enum Result<T, E> {
    Ok(T),
    Err(E),
}
```

These should already be a dead giveaway that having exhaustive enums is
not some kind of edge case. However, let's also review a few examples
from our own code:

```rust
pub enum PointerButton {
    None,
    Primary,
    Secondary,
    Auxiliary,
    X1,
    X2,
    Other,
}

pub enum WindowTheme {
    Light,
    Dark,
}

pub enum Axis {
    Horizontal,
    Vertical,
}

pub enum Origin {
    TopLeft,
    BottomLeft,
}

pub enum Fill {
    NonZero,
    EvenOdd,
}
```

These examples are merely scratching the surface. There are a lot more
of these. Requiring all of these to have a Clippy exception is
unreasonable and requiring them to be non-exhaustive is even more
unreasonable. What are the odds that we're going to add a third `Axis`
variant to that enum? What's more, even if we do, what are the odds that
we don't want that to be clearly signaled to the enum consumer via a
compiler error that they're not handling the new third axis?

## A few reasons why non-exhaustive enums are worse

In the case of new variants it moves the moment of failure from compile
time to runtime. An antithesis of Rust programming.

Code that can't accept silent corruption of state can't just ignore
unknown variants. It has to return some sort of `UnknownVariant` error,
panic, or something else of that nature. That means that a function that
otherwise could be really simple now either introduces panics or
requires the caller to start propagating an error up the stack. While
with exhaustive enums it is known at compile time that this function
does indeed handle all cases, with no additional error handling
required.

It will be exceedingly common for consumers of our crates to just do a
[`NOP`](https://en.wikipedia.org/wiki/NOP_(code)) in the unknown variant
case, even if it isn't a good idea. That's because doing a `NOP` is
easy, it's the path of least resistance. However, this will start
introducing bugs into their code. We will be tasked with loud marketing
whenever a new variant is added that shouldn't just be ignored. However,
with exhaustive enums, the Rust compiler will handle the notifications
for us, without the consumer even needing to have read our changelog.

## The official Clippy rationale

[The
docs](https://rust-lang.github.io/rust-clippy/master/index.html#exhaustive_enums)
state that *exhaustive is a stability commitment: adding a variant is a
breaking change*.

Adding new variants to a non-exhaustive enum can be just as much of a
breaking change. It's not breaking only in the case where the new
variant doesn't have any overlap with any of the previous variants and
can also be safely ignored.

What's more, most of our releases are breaking anyway. We're not at the
stage of hardcore backwards compatibility with any of our projects.

## We're smart enough to use `#[non_exhaustive]` where appropriate

For the cases where non-exhaustive enums do make sense we're perfectly
capable of realizing that and adding the attribute without Clippy
screaming at us.

## The solution

Just remove the `exhaustive_enums` lint from our set. That is exactly
what this PR does.
  • Loading branch information
xStrom authored Dec 8, 2024
1 parent 721dc41 commit 27951dc
Showing 1 changed file with 1 addition and 2 deletions.
3 changes: 1 addition & 2 deletions content/wiki/canonical_lints.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ All Linebender projects should include the following set of lints:
# This one may vary depending on the project.
rust.unsafe_code = "forbid"

# LINEBENDER LINT SET - Cargo.toml - v3
# LINEBENDER LINT SET - Cargo.toml - v4
# See https://linebender.org/wiki/canonical-lints/
rust.keyword_idents_2024 = "forbid"
rust.non_ascii_idents = "forbid"
Expand Down Expand Up @@ -43,7 +43,6 @@ clippy.collection_is_never_read = "warn"
clippy.dbg_macro = "warn"
clippy.debug_assert_with_mut_call = "warn"
clippy.doc_markdown = "warn"
clippy.exhaustive_enums = "warn"
clippy.fn_to_numeric_cast_any = "warn"
clippy.infinite_loop = "warn"
clippy.large_include_file = "warn"
Expand Down

0 comments on commit 27951dc

Please sign in to comment.