Skip to content

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