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

Warn when a lint table has a group at the same priority as lints #12918

Open
Alexendoo opened this issue Nov 5, 2023 · 7 comments
Open

Warn when a lint table has a group at the same priority as lints #12918

Alexendoo opened this issue Nov 5, 2023 · 7 comments
Labels
A-lints-table Area: [lints] table C-bug Category: bug S-needs-rfc Status: Needs an RFC to make progress.

Comments

@Alexendoo
Copy link
Member

Problem

The sorting behaviour is causing confusion, me included as I thought this was a bug until I read the RFC

rust-lang/rust-clippy#11751 (comment)
rust-lang/rust-clippy#11237

Steps

No response

Possible Solution(s)

No response

Notes

No response

Version

No response

@Alexendoo Alexendoo added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels Nov 5, 2023
@epage epage added S-needs-rfc Status: Needs an RFC to make progress. A-lints-table Area: [lints] table and removed S-triage Status: This issue is waiting on initial triage. labels Nov 6, 2023
@epage
Copy link
Contributor

epage commented Nov 6, 2023

This was left as a future possibility in the RFC.

To implement this, either cargo needs to pass the lints down to the tool in a way to communicate the priority batches, allow cargo to query the group memberships from the linter, or we hard code this at compile-time like rust-analyzer (lints, generate). One thing to keep in mind is the potential for custom tools in the future.

@kristof-mattei
Copy link

I'm confused by this.

[lints.rust]
dead-code = { level = "allow", priority = 1 }
warnings = { level = "deny", priority = 0 }

Does what I expect it to do

[lints.rust]
dead-code = { level = "allow", priority = 0 }
warnings = { level = "deny", priority = 1 }

Works just fine too.

I would expect that if I override a specific lint from a group that was included, that the override wins. Which is what Rust itself seems to do, regardless of sort order.

Only in the case of 2 groups disagreeing on a setting is when I would think to start using priorities.

@epage
Copy link
Contributor

epage commented Nov 20, 2023

warnings is a dynamic group of everything that is currently set to warn.

@clayrab
Copy link

clayrab commented Jan 11, 2024

Is there a workaround for this in the meantime? I keep coming back around to this issue. I'm forced to just use -A flag on cargo directly and still can't seem to make anything work in config.toml or Cargo.toml.

@epage
Copy link
Contributor

epage commented Jan 11, 2024

There is a PR for a clippy lint for when groups are the same priority as lints.

For why you are having difficulty, its hard to say without a reproduction case.

bors added a commit to rust-lang/rust-clippy that referenced this issue Jan 31, 2024
Add `lint_groups_priority` lint

Warns when a lint group in Cargo.toml's `[lints]` section shares the same priority as a lint. This is in the cargo section but is categorised as `correctness` so it's on by default, it doesn't call `cargo metadata` though and parses the `Cargo.toml` directly

The lint should be temporary until rust-lang/cargo#12918 is resolved, but in the meanwhile this is an common issue to run into

- #11237
- #11751
- #11830

changelog: Add [`lint_groups_priority`] lint

r? `@flip1995`
jwodder added a commit to jwodder/rsrepo that referenced this issue Feb 10, 2024
@andrewbanchich
Copy link

i'm not sure if this is a bug in this new behavior, but after updating to 1.78 i'm getting

error: lint group `pedantic` has the same priority (0) as a lint
   |
16 | pedantic = "warn"
   | ^^^^^^^^   ------ has an implicit priority of 0
17 | wildcard_enum_match_arm = "warn"
   | ----------------------- 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 `pedantic` to a lower priority
   |
16 | pedantic = { level = "warn", priority = -1 }

these both have the same level, so i'm not sure why it would need a priority. pedantic and wildcard_enum_match_arm are both allow by default, so i need this setting to enable them. wildcard_enum_match_arm is not in the pedantic group, so i'm not sure why i'd need to set a priority for one over the other.

@epage
Copy link
Contributor

epage commented May 20, 2024

FYI that is a clippy lint and there is at least rust-lang/rust-clippy#12270 opened against it which is similar to what you are seeing.

SifraiTeam pushed a commit to grandinetech/grandine that referenced this issue May 25, 2024
Cargo.lock was updated to version 4 manually.
All Git refs in our Cargo.lock appear to be encoded correctly.
See:
- rust-lang/cargo#12280
- rust-lang/cargo#12852

The clippy::incompatible_msrv lint may be useful in the future.
We currently do not specify rust-version in Cargo.toml.
Our libraries are not designed to be used outside Grandine.
rust-toolchain.toml makes rust-version redundant for builds inside this repository.
See:
- https://doc.rust-lang.org/1.78.0/cargo/reference/manifest.html#the-rust-version-field
- https://rust-lang.github.io/rust-clippy/rust-1.78.0/#/incompatible_msrv

The clippy::lint_groups_priority lint produces false positives:
- rust-lang/cargo#12918 (comment)
- rust-lang/rust-clippy#12270
Luckily, lints inherited from a workspace do not trigger it.
The issue appears to be fixed in rust-lang/rust-clippy#12827.

The reason public functions trigger clippy::single_call_fn is actually our setting of avoid-breaking-exported-api.
We assumed it was a bug.
SifraiTeam pushed a commit to grandinetech/grandine that referenced this issue Jul 12, 2024
Cargo.lock was updated to version 4 manually.
All Git refs in our Cargo.lock appear to be encoded correctly.
See:
- rust-lang/cargo#12280
- rust-lang/cargo#12852

The clippy::incompatible_msrv lint may be useful in the future.
We currently do not specify rust-version in Cargo.toml.
Our libraries are not designed to be used outside Grandine.
rust-toolchain.toml makes rust-version redundant for builds inside this repository.
See:
- https://doc.rust-lang.org/1.78.0/cargo/reference/manifest.html#the-rust-version-field
- https://rust-lang.github.io/rust-clippy/rust-1.78.0/#/incompatible_msrv

The clippy::lint_groups_priority lint produces false positives:
- rust-lang/cargo#12918 (comment)
- rust-lang/rust-clippy#12270
Luckily, lints inherited from a workspace do not trigger it.
The issue appears to be fixed in rust-lang/rust-clippy#12827.

The reason public functions trigger clippy::single_call_fn is actually our setting of avoid-breaking-exported-api.
We assumed it was a bug.
girlbossceo added a commit to girlbossceo/conduwuit that referenced this issue Jul 25, 2024
Signed-off-by: strawberry <strawberry@puppygock.gay>
girlbossceo added a commit to girlbossceo/conduwuit that referenced this issue Jul 26, 2024
Signed-off-by: strawberry <strawberry@puppygock.gay>
girlbossceo added a commit to girlbossceo/conduwuit that referenced this issue Jul 26, 2024
Signed-off-by: strawberry <strawberry@puppygock.gay>
Aranjedeath pushed a commit to Aranjedeath/conduwuit that referenced this issue Jul 26, 2024
Signed-off-by: strawberry <strawberry@puppygock.gay>
0xdea added a commit to 0xdea/backdoo-rs that referenced this issue Sep 14, 2024
0xdea added a commit to 0xdea/raptor-rust-template that referenced this issue Sep 14, 2024
0xdea added a commit to 0xdea/zero2prod that referenced this issue Sep 14, 2024
0xdea added a commit to 0xdea/raptor-rust-template that referenced this issue Sep 16, 2024
0xdea added a commit to 0xdea/backdoo-rs that referenced this issue Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lints-table Area: [lints] table C-bug Category: bug S-needs-rfc Status: Needs an RFC to make progress.
Projects
None yet
Development

No branches or pull requests

5 participants