Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New lint: warn about missing variants/fields for non-exhaustive enums/structs #5557

Closed
ebroto opened this issue May 1, 2020 · 7 comments
Closed
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy T-AST Type: Requires working with the AST

Comments

@ebroto
Copy link
Member

ebroto commented May 1, 2020

As suggested in the non_exhaustive RFC, when using non-exhaustive enums and structs in patterns, the lint would warn the user for missing variants or fields despite having a wildcard arm or a rest pattern.

This can be useful when new fields/variants are added by the upstream crate so that they don't go unnoticed.

Enums

Given the following definition

#[non_exhaustive]
pub enum E {
    First,
    Second,
    Third,
}

The following use should be linted:

    match e {
        E::First => {}
        E::Second => {}
        _ => {}
    }

Structs

Given the following definition

#[derive(Default)]
#[non_exhaustive]
pub struct S {
    pub a: i32,
    pub b: i32,
    pub c: i32,
}

The following cases should be linted

    let S { a: _, b: _, .. } = S::default();
    match S::default() {
        S { a: 42, b: 21, .. } => {}
        S { a: _, b: _, .. } => {}
    }
    if let S { a: 42, b: _, .. } = S::default() {}
    while let S { a: 42, b: _, .. } = S::default() {
        break;
    }
    let v = vec![S::default()];
    for S { a: _, b: _, .. } in v {}

In addition to let destructuring in function parameters

pub fn take_s(S { a, b, .. }: S) -> (i32, i32) {
    (a, b)
}

A struct variant of an enum should be linted in the same cases if it's the only variant of the enum. If it's not the only member, it can be linted only in the match, if let and while let cases.

Tuple structs

Tuple structs can't be used in patterns if there are private fields using the T() syntax, but the T{0: _, 1: _} syntax can be used an so it should be linted like with structs.

Given the following definition

#[derive(Default)]
#[non_exhaustive]
pub struct T(pub i32, pub i32, pub i32);

The following cases should be linted

    let T { 0: _, 1: _, .. } = T::default();
    match T::default() {
        T { 0: 42, 1: 21, .. } => {}
        T { 0: _, 1: _, .. } => {}
    }
    if let T { 0: 42, 1: _, .. } = T::default() {}
    while let T { 0: 42, 1: _, .. } = T::default() {
        break;
    }
    let v = vec![T::default()];
    for T { 0: _, 1: _, .. } in v {}

In addition to let destructuring in function parameters

pub fn take_t(T { 0: a, 1: b, .. }: T) -> (i32, i32) {
    (a, b)
}

Notes

  • The non-exhaustive data structure must come from another crate (otherwise the restrictions associated to non-exhaustive data structures do not apply).
  • The lint should not trigger if only the rest pattern is used, so at least one field should be used in the pattern.
  • The lint should be allow-by-default.
  • Although described in the RFC, FRU is not supported currently so constructing a non-exhaustive struct is not possible.
  • Unit structs are not candidates for this lint because new fields can't be added.
@ebroto
Copy link
Member Author

ebroto commented May 2, 2020

I've updated the description with some details to make it clearer.

@flip1995 flip1995 added good-first-issue These issues are a good way to get started with Clippy A-lint Area: New lints T-AST Type: Requires working with the AST labels May 2, 2020
@giraffate
Copy link
Contributor

Hi, is this issue already resolved? If so, it would be better to close this.

@flip1995
Copy link
Member

flip1995 commented Jul 3, 2020

Nope, this issue is still relevant

@jrqc
Copy link
Contributor

jrqc commented Aug 18, 2020

I'd like to give this issue a shot

@jrqc
Copy link
Contributor

jrqc commented Nov 12, 2020

Added a new lint called not_exhaustive_enough.
Opening a WIP PR now

@DevinR528
Copy link
Contributor

DevinR528 commented Jun 11, 2021

This issue should be closed and move to rustic rust-lang/rust#84332?

@flip1995
Copy link
Member

Yes this should be implemented in rustc and is tracked in the issue you linked. Thanks for noticing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy T-AST Type: Requires working with the AST
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants