Skip to content

fix(21943): remove duplicate lints#22054

Merged
ShoyuVanilla merged 1 commit intorust-lang:masterfrom
jdrouet:jdrouet/remove-duplicate-lints
Apr 20, 2026
Merged

fix(21943): remove duplicate lints#22054
ShoyuVanilla merged 1 commit intorust-lang:masterfrom
jdrouet:jdrouet/remove-duplicate-lints

Conversation

@jdrouet
Copy link
Copy Markdown
Contributor

@jdrouet jdrouet commented Apr 15, 2026

In this PR, I filter the lints that have already been added by name.
There are 2 main groups, the default lints and the rustdoc lints.
This should avoid having duplicates.

Fixes #21943

PS: I've tried regenerating the lints, it introduced many changes which are not really related, so I manually removed them instead.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 15, 2026
@jdrouet jdrouet force-pushed the jdrouet/remove-duplicate-lints branch from ee7dc02 to 96cfb49 Compare April 15, 2026 12:42
Copy link
Copy Markdown
Contributor

@ChayimFriedman2 ChayimFriedman2 left a comment

Choose a reason for hiding this comment

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

Using a hashmap for deduplication, especially with the default hasher but not only, risks ordering changes across runs. Instead, sort and dedup().

View changes since this review

Comment thread crates/ide-completion/src/tests/attribute.rs Outdated
@jdrouet
Copy link
Copy Markdown
Contributor Author

jdrouet commented Apr 17, 2026

Using a hashmap for deduplication, especially with the default hasher but not only, risks ordering changes across runs. Instead, sort and dedup().

View changes since this review

Thanks for your answer, but I'm not sure I understand properly.
As you said, the HashSet is only used for deduplication, so, as long as we don't change the order of the values in the Vec, the result will be the same considering we only call .contains 🤔 The hasher is not involved in this, is it?

Thanks in advance 😉

Copy link
Copy Markdown
Contributor

@ChayimFriedman2 ChayimFriedman2 left a comment

Choose a reason for hiding this comment

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

Right, my mistake (I thought the deduplication is also inside lints and groups, not just between lints and groups). But please remove the test.

View changes since this review

Copy link
Copy Markdown
Contributor

@ChayimFriedman2 ChayimFriedman2 left a comment

Choose a reason for hiding this comment

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

Please squash, then LGTM.

View changes since this review

Signed-off-by: Jeremie Drouet <jeremie.drouet@gmail.com>
@jdrouet jdrouet force-pushed the jdrouet/remove-duplicate-lints branch from 2c933e4 to 07cf5e4 Compare April 20, 2026 06:48
@jdrouet
Copy link
Copy Markdown
Contributor Author

jdrouet commented Apr 20, 2026

One commit is probably enough for this 😉 done!

Copy link
Copy Markdown
Member

@ShoyuVanilla ShoyuVanilla left a comment

Choose a reason for hiding this comment

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

@ShoyuVanilla ShoyuVanilla enabled auto-merge April 20, 2026 06:50
@ShoyuVanilla ShoyuVanilla added this pull request to the merge queue Apr 20, 2026
Merged via the queue into rust-lang:master with commit 0b696c0 Apr 20, 2026
18 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 20, 2026
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

Successfully merging this pull request may close these issues.

Duplicate warnings lint complete

4 participants