rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
11.01k stars 1.47k forks source link

`lint_groups_priority` clippy lint seems to misattribute lints to groups #12920

Open palant opened 2 weeks ago

palant commented 2 weeks ago

I’m using the following Cargo.toml:

[package]
name = "testing"
version = "0.1.0"
edition = "2021"

[lints.clippy]
lint_groups_priority = "warn"

[lints.rust]
keyword_idents = "deny"
macro_use_extern_crate = "deny"
rust_2018_idioms = "deny"
single_use_lifetimes = "deny"

Running cargo clippy produces two warnings here:

warning: lint group `keyword_idents` has the same priority (0) as a lint
  --> Cargo.toml:10:1
   |
10 | keyword_idents = "deny"
   | ^^^^^^^^^^^^^^   ------ has an implicit priority of 0
11 | macro_use_extern_crate = "deny"
   | ---------------------- has the same priority as this lint
   |
   = note: the order of the lints in the table is ignored by Cargo
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#lint_groups_priority
   = note: requested on the command line with `-W clippy::lint-groups-priority`
help: to have lints override the group set `keyword_idents` to a lower priority
   |
10 | keyword_idents = { level = "deny", priority = -1 }
   |                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

warning: lint group `rust_2018_idioms` has the same priority (0) as a lint
  --> Cargo.toml:12:1
   |
12 | rust_2018_idioms = "deny"
   | ^^^^^^^^^^^^^^^^   ------ has an implicit priority of 0
13 | single_use_lifetimes = "deny"
   | -------------------- has the same priority as this lint
   |
   = note: the order of the lints in the table is ignored by Cargo
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#lint_groups_priority
help: to have lints override the group set `rust_2018_idioms` to a lower priority
   |
12 | rust_2018_idioms = { level = "deny", priority = -1 }
   |                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

While making the proposed changes silences the warnings, these look like false positives. According to documentation, single_use_lifetimes isn’t part of rust_2018_idioms, and keyword_idents isn’t a group at all.

Meta

rustc --version --verbose:

rustc 1.81.0-nightly (d0227c6a1 2024-06-11)
binary: rustc
commit-hash: d0227c6a19c2d6e8dceb87c7a2776dc2b10d2a04
commit-date: 2024-06-11
host: x86_64-unknown-linux-gnu
release: 1.81.0-nightly
LLVM version: 18.1.7
palant commented 2 weeks ago

I can now see why this only started happening with 1.80.0-beta.1 for me – I have the lints defined in the workspace, so before https://github.com/rust-lang/rust/commit/fa8f4b88cbf3cfb6bad3076c24d0d11625003295 they weren’t being checked at all. I guess this isn’t a regression after all.

Also, it seems that lint_groups_priority doesn’t check for conflicting lints at all – it will just flag any lint and any group existing within the same priority. This is unexpected, not sure whether this is intended to work like this. At least the tests use lints that are actually part of the group being flagged.

And technically speaking keyword_idents is a group, consisting of the lints keyword_idents_2018 and keyword_idents_2024.

So all of this has an explanation it seems. But the current behavior still doesn’t make much sense to me.

veera-sivarajan commented 2 weeks ago

@rustbot label -needs-triage +A-clippy +D-confusing

fmease commented 2 weeks ago

@rustbot transfer rust-clippy

y21 commented 2 weeks ago

I believe this was fixed by #12827 (which is not on nightly yet).

palant commented 2 weeks ago

I believe this was fixed by #12827 (which is not on nightly yet).

Yes, judging by the code it fixes my specific use case. It will still flag unrelated lints and groups if the level is different however, e.g. here:

[package]
name = "testing"
version = "0.1.0"
edition = "2021"

[lints.clippy]
lint_groups_priority = "warn"

[lints.rust]
keyword_idents = "deny"
macro_use_extern_crate = "warn"
rust_2018_idioms = "deny"
single_use_lifetimes = "allow"

The order shouldn’t matter here because these lints aren’t part of the flagged groups. The code still won’t check that however.