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

Extend needless_collect #8744

Merged
merged 2 commits into from
Nov 7, 2022
Merged

Extend needless_collect #8744

merged 2 commits into from
Nov 7, 2022

Conversation

Jarcho
Copy link
Contributor

@Jarcho Jarcho commented Apr 24, 2022

Extends when is_empty and contains are linted.

is_empty will be linted when <IterTy as Iterator>::Item is the same as <CollectTy as IntoIterator>::Item. This can be a false positive if the FromIterator implementation filters out items, but I don't know of any which do that also implement IntoIterator with a matching Item type.

contains will be linted when the argument type is &<IterTy as Iterator>::Item. It has the same false positives as is_empty with the same note that I know of nothing that actually causes that in practice.

changelog: Lint needless_collect when is_empty or contains is called on some non-std types

@rust-highfive
Copy link

r? @camsteffen

(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 Apr 24, 2022
@bors
Copy link
Collaborator

bors commented May 8, 2022

☔ The latest upstream changes (presumably #8802) made this pull request unmergeable. Please resolve the merge conflicts.

@Jarcho
Copy link
Contributor Author

Jarcho commented Oct 2, 2022

r? @llogiq
Assign this to somebody still doing reviews.

@rust-highfive rust-highfive assigned llogiq and unassigned camsteffen Oct 2, 2022
Copy link
Contributor

@llogiq llogiq left a comment

Choose a reason for hiding this comment

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

This looks good in general, although some of the stuff goes over my head.

Unfortunately I got some pink eye today, so my first review is rather cursory, as I try to limit my screen time. Will come back to this once I got some antibiotics, but today is a bank holiday here in 🇩🇪.

clippy_lints/src/methods/needless_collect.rs Outdated Show resolved Hide resolved
clippy_lints/src/methods/needless_collect.rs Outdated Show resolved Hide resolved
clippy_lints/src/methods/needless_collect.rs Outdated Show resolved Hide resolved
clippy_utils/src/ty.rs Show resolved Hide resolved
@llogiq
Copy link
Contributor

llogiq commented Oct 9, 2022

Ok, I'm back. My eyes have fully recovered. Also, this looks good in general, and if you don't want to refactor it, I'm fine with merging it after a rebase and doing the refactor myself later (if nobody beats me to it).

@Jarcho
Copy link
Contributor Author

Jarcho commented Oct 9, 2022

I'll get to it this sometime this week.

@llogiq
Copy link
Contributor

llogiq commented Oct 23, 2022

Ping @Jarcho anything I can do to help?

@Jarcho
Copy link
Contributor Author

Jarcho commented Oct 24, 2022

This is still on my todo list. Ended up getting sick instead of working on this. Better now though.

@llogiq
Copy link
Contributor

llogiq commented Nov 6, 2022

Ping! So I guess this fell off the top of your Todo list. No hard feelings here, but it would be great to see this through.

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Nov 6, 2022
@Jarcho Jarcho force-pushed the needless_collect_fp branch 2 times, most recently from 0e9183d to 9878f57 Compare November 7, 2022 18:13
@Jarcho
Copy link
Contributor Author

Jarcho commented Nov 7, 2022

Should be good now.

@llogiq
Copy link
Contributor

llogiq commented Nov 7, 2022

The current commit fails dogfood. Apparently because of the recently introduced format args lint.

@llogiq
Copy link
Contributor

llogiq commented Nov 7, 2022

Thanks!

@bors r+

@bors
Copy link
Collaborator

bors commented Nov 7, 2022

📌 Commit 8bfc8bc has been approved by llogiq

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Nov 7, 2022

⌛ Testing commit 8bfc8bc with merge 5857a01...

@bors
Copy link
Collaborator

bors commented Nov 7, 2022

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

@bors bors merged commit 5857a01 into rust-lang:master Nov 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants