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

disallowed_names for identifiers and not just variables #11306

Open
ojeda opened this issue Aug 8, 2023 · 1 comment
Open

disallowed_names for identifiers and not just variables #11306

ojeda opened this issue Aug 8, 2023 · 1 comment

Comments

@ojeda
Copy link
Contributor

ojeda commented Aug 8, 2023

disallowed_names complains about variables, like the lint description explains:

echo 'fn main() { let foo = 0; }' | rustup run 1.71.1 clippy-driver -Aunused_variables -

However, it does not warn on identifiers in general:

echo 'struct foo; fn main() {}' | rustup run 1.71.1 clippy-driver -Adead_code -Anon_camel_case_types -
echo 'fn foo() {} fn main() {}' | rustup run 1.71.1 clippy-driver -Adead_code -

This is particularly noticeable in a sample that may use a struct and then instantiate it. I have hit this in a Linux kernel example:

#![expect(clippy::disallowed_names)]

#[pin_data]
struct Foo {
    #[pin]
    a: Mutex<usize>,
    b: u32,
}

let foo = pin_init!(Foo {
    a <- new_mutex!(42, "Foo::a"),
    b: 24,
});

Is it intended that the lint only covers variables? If yes, should there be a disallowed_idents even if it overlaps with this one? (disallowed_types and others seem to be intended to cover particular paths that should not be used for a given reason, rather than just a name on any path; and even if those lints supported it, then it would still require repeating the name for the different entities, which would not be ideal).

If it is not intended, and disallowed_names is expanded to cover more entities:

  • Which entities should be covered? All identifiers? Something else?
  • Would it be worth renaming it to disallowed_idents or similar to be consistent?
  • Should it be case-insensitive (so that the default list Foo triggers the lint for e.g. a struct)? If not, should the default list be expanded to include Foo and so on?
@CobaltCause
Copy link

FWIW I just noticed that disallowed_names also doesn't cover struct field names, which seems odd.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants