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

lint_groups_priority: ignore lints & groups at the same level #12827

Merged
merged 1 commit into from
Jun 6, 2024

Conversation

Alexendoo
Copy link
Member

Fixes #12270

changelog: none

@rustbot
Copy link
Collaborator

rustbot commented May 20, 2024

r? @Jarcho

rustbot has assigned @Jarcho.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label May 20, 2024
SifraiTeam pushed a commit to grandinetech/grandine that referenced this pull request 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.
Comment on lines +101 to +103
if let Some((conflict, _)) = lints
.iter()
.rfind(|(_, lint_config)| lint_config.priority() == priority && lint_config.level() != level)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason to only lint one conflict instead of linting on all conflicts here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to keep the number of warnings reasonable, e.g. with 2 groups and 5 lints you get 2 warnings, one for each group, rather than 10

@Jarcho
Copy link
Contributor

Jarcho commented Jun 5, 2024

Just the one comment, otherwise looks fine otherwise. r=me if you don't feel like making the change.

@Alexendoo
Copy link
Member Author

Thanks, @bors r=Jarcho

@bors
Copy link
Collaborator

bors commented Jun 6, 2024

📌 Commit 9e5523e has been approved by Jarcho

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jun 6, 2024

⌛ Testing commit 9e5523e with merge 4e7f974...

@bors
Copy link
Collaborator

bors commented Jun 6, 2024

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Jarcho
Pushing 4e7f974 to master...

@bors bors merged commit 4e7f974 into rust-lang:master Jun 6, 2024
5 checks passed
@Alexendoo Alexendoo deleted the lint-groups-priority branch June 6, 2024 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lint_groups_priority is raised for rust_2018_idioms = "deny", warnings = "deny"
4 participants