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 againt matching on all-private struct #8333

Open
Kixunil opened this issue Jan 22, 2022 · 3 comments
Open

New lint: warn againt matching on all-private struct #8333

Kixunil opened this issue Jan 22, 2022 · 3 comments
Labels
A-lint Area: New lints

Comments

@Kixunil
Copy link

Kixunil commented Jan 22, 2022

What it does

Firstly a disclaimer: I think this belongs to the compiler rather than clippy but putting it into clippy first is probably a good first step as it could allow experimentation with minimal annoyance. Feel free to close if you disagree.

When the code matches on a struct with all fields private or an empty struct with #[non_exhaustive] attribute using Foo { .. } syntax clippy warns against this because it breaks future-proofing of the type which library authors may not be aware of.
Disussion: https://internals.rust-lang.org/t/disallow-matching-on-all-private-structs/15993

Lint Name

match_fully_private_struct

Category

suspicious

Advantage

The code will not break if the library changes the type from struct to enum.

Drawbacks

  • Can FP on v @ Foo { .. }, but perhaps there's a way to improve the code here too? It could be also whitelisted.
  • A crate author might have meant to commit to the type staying a struct. Maybe this could be mitigated by having an attribute on the struct definition to (dis)allow it. (Is such thing possible in clippy?)

Example

mod foo {
    pub struct Foo {
        value: u8,
    }

    pub fn create() -> Foo {
        Foo { value: 42 }
    }
}

fn main() {
    let foo::Foo { .. } = foo::create();
}

Could be written as:

// mod foo omitted for brevity since it's the same as above

fn main() {
    let _: foo::Foo = foo::create();
}
@Kixunil Kixunil added the A-lint Area: New lints label Jan 22, 2022
@camsteffen
Copy link
Contributor

I think it is equally lintable if the fields are public. Could call it struct_patterns_without_bindings.

@Kixunil
Copy link
Author

Kixunil commented Jan 25, 2022

Yeah, maybe. The case with all-private fields is more serious and I think the message should explain incompatibility in that case.

@camsteffen
Copy link
Contributor

I see. Yeah we should pursue a rustc change first. I'm wondering if we could "Allow matching with Name { .. } on enums".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints
Projects
None yet
Development

No branches or pull requests

2 participants