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

Fix sorting in cargo dev update_lints script #9509

Merged
merged 1 commit into from
Oct 2, 2022

Conversation

schubart
Copy link
Contributor

@schubart schubart commented Sep 21, 2022

changelog: none

The old code cloned and sorted usable_lints into sorted_usable_lints, but then failed to do anything with sorted_usable_lints.

This was discovered by my new collection_is_never_read lint (#9267) that I'm working on!

Fix: I renamed the sorted vector to usable_lints. Therefore it now gets used where the unsorted one was used previously.

@rust-highfive
Copy link

r? @Manishearth

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Sep 21, 2022
@schubart
Copy link
Contributor Author

schubart commented Oct 2, 2022

@Manishearth, do you have any thoughts on this?

An alternative way of fixing this would be to simply remove sorted_usable_lints. It hasn't been used since at least 03f0431 by @flip1995 in November 2020, so maybe it's fine not to have any sorting.

@llogiq
Copy link
Contributor

llogiq commented Oct 2, 2022

Manish seems to be pretty busy lately.

So I've just had a look and I think it makes sense to sort; makes it easier to find things. Thank you for making clippy better and sorry you had to wait for so long.

@bors r+

@bors
Copy link
Contributor

bors commented Oct 2, 2022

📌 Commit 033dae9 has been approved by llogiq

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Oct 2, 2022

⌛ Testing commit 033dae9 with merge cb8da67...

@bors
Copy link
Contributor

bors commented Oct 2, 2022

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: llogiq
Pushing cb8da67 to master...

@bors bors merged commit cb8da67 into rust-lang:master Oct 2, 2022
@schubart schubart deleted the fix_sorting branch October 2, 2022 09:10
@Manishearth
Copy link
Member

Oh, sorry, I must have missed the original ping

Yeah, this seems fine, thanks for stepping in @llogiq !

bors added a commit that referenced this pull request Mar 7, 2023
Add `collection_is_never_read`

Fixes #9267

`@flip1995` and `@llogiq,` I talked with you about this one at Rust Nation in London last week. :-)

This is my first contribution to Clippy, so lots of feedback would be greatly appreciated.

- \[ ] Followed [lint naming conventions][lint_naming]
- \[x] Added passing UI tests (including committed `.stderr` file)
- \[x] `cargo test` passes locally
- \[x] Executed `cargo dev update_lints`
- \[x] Added lint documentation
- \[x] Run `cargo dev fmt`

`dogfood` found one true positive (see #9509) and no false positives.

`lintcheck` found no (true or false) positives, even when running on an extended set of crates.

---

changelog: new lint [`collection_is_never_read`]
[#10415](#10415)
<!-- changelog_checked -->
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.

5 participants