Skip to content

Can't disable new struct_field_names lint #12093

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

Closed
ziimakc opened this issue Jan 4, 2024 · 3 comments
Closed

Can't disable new struct_field_names lint #12093

ziimakc opened this issue Jan 4, 2024 · 3 comments

Comments

@ziimakc
Copy link

ziimakc commented Jan 4, 2024

Description

New lint struct_field_names can't be disabled, while other rules are disabled fine when running cargo clippy --fix --allow-dirty.

Workspace root Cargo.toml:

[workspace.lints.clippy]
all = "deny"
pedantic = "deny"
nursery = "deny"
module_name_repetitions = "allow" # working
single_match_else = "allow" # working
future_not_send = "allow" # working
needless_pass_by_value = "allow" # working
struct_field_names = "allow" # not working

Actual behavior:

Warning:

6 |     pub user_id: Uuid,
  |     ^^^^^^^^^^^^^^^^^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#struct_field_names
  = note: `-D clippy::struct-field-names` implied by `-D clippy::pedantic`
  = help: to override `-D clippy::pedantic` add `#[allow(clippy::struct_field_names)]`

Expected

No warnings when struct_field_names = "allow"

Version

rustc 1.75.0 (82e1608df 2023-12-21)
binary: rustc
commit-hash: 82e1608dfa6e0b5569232559e3d385fea5a93112
commit-date: 2023-12-21
host: x86_64-unknown-linux-gnu
release: 1.75.0
LLVM version: 17.0.6

Additional Labels

No response

@y21
Copy link
Member

y21 commented Jan 4, 2024

I'm pretty sure that this is "expected" behavior with how lint tables are implemented. The order in which cargo passes lint level arguments for the same priority is arbitrary if I remember the RFC right, so struct_field_names might happen to be placed before a deny of a lint group and get overridden that way. Does it work if you give pedantic a lower priority, like -1?

pedantic = { priority = -1, level = "deny" }

@ziimakc
Copy link
Author

ziimakc commented Jan 4, 2024

@y21 yes, it works, but why it works for other rules without priority = -1, for example needless_pass_by_value?

@y21
Copy link
Member

y21 commented Jan 4, 2024

Quoting the reference level explanation:

Sort them by priority and then an unspecified order within priority that we may change in the future.

On initial release, the sort will likely be reverse alphabetical which would cause all to be last, making it unlikely to do what the user wants which would raise awareness of the need for setting priority for all groups.


If I sort your lint table in reverse alphabetical order I get: struct_field_names, single_match_else, pedantic, nursery, needless_pass_by_value. ...

So, due to this unstable ordering that cargo currently uses, struct_field_names and single_match_else are placed behind pedantic = "deny" and thus get overridden by the later rules.

We can verify that this is true by running cargo clippy --verbose, which shows the rustc invocation, which includes the lint level arguments in the order that cargo sorted it:

rustc ... '--allow=clippy::struct_field_names' '--allow=clippy::single_match_else' '--deny=clippy::pedantic' '--deny=clippy::nursery' '--allow=clippy::needless_pass_by_value' '--allow=clippy::module_name_repetitions' '--allow=clippy::future_not_send' '--deny=clippy::all' ...

(as you can see this is not just struct_field_names: single_match_else = "allow" is also ignored).

The default priority is 0, which is why using a priority of -1 for the pedantic group forces it to be in front of all the default lints.

FWIW, there's an open PR that implements a lint that should hopefully help with this confusion to some degree: #11832

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