Add iter_kv_map lint

[kartva]

fixes #9376

before	after
hmap.iter().map(|(key, _)| key)	hmap.keys()
hmap.iter().map(|(_, v)| v + 2)	hmap.values().map(|v| v + 2)
hmap.into_iter().map(|(key, _)| key)	hmap.into_keys()

Is MachineApplicable

changelog: [iter_kv_map]: added lint

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @xFrednet (or someone else) soon.

Please see the contribution instructions for more information.

[daira]

Untested ACK.

[xFrednet]

Welcome, Clippy 👋

There has been a change to cargo dev update_lints on master which causes the CI fail. The fix should be as simple as rebasing and running that comment. 🙃

[xFrednet]

Overall looks good to me and I really like this lint! I made some style related comments, which should be simple to fix. You're welcome to reach out if you need any help with them :)

[xFrednet]

You seem to have resolved some comments, even if they haven't been addressed or a comment why you think that they don't apply. I've now unresolved them. If you have questions or are unsure what I mean by them, please say so. Please tell me, if I misunderstood something.

This review sadly took some time, I'll try to reply faster next time.

Another note, Clippy uses rusts no merge commit policy. Merge commits are fine for now, but will need to be dropped or squashed before the merge. That's a bridge we can cross when we come to it 🙃

[xFrednet]

@rustbot author

[kartva]

You seem to have resolved some comments, even if they haven't been addressed

Really sorry - I don't know what was going through my mind when I did that! Rest assured, I'll try to be more attentive moving forwards.

This review sadly took some time, I'll try to reply faster next time.

Please don't say that - I'm used to reviews taking a week, so this was super quick for me.

Clippy uses rusts no merge commit policy.

Ah, acknowledged. Should I squash/rebase right now?

[xFrednet]

Really sorry - I don't know what was going through my mind when I did that! Rest assured, I'll try to be more attentive moving forwards.

No worries, I was just confused. This happens every once in a while :)

This review sadly took some time, I'll try to reply faster next time.

Please don't say that - I'm used to reviews taking a week, so this was super quick for me.

Thank you for the encouragement! I try to review every PR within 5 days. But life got the better of me, so now I'm just catching up on everything :). I'll give this a full review over the week :)

Ah, acknowledged. Should I squash/rebase right now?

You can do that, like you did, if I see it correctly. For larger PR it's better to wait until the review is done since GitHub is a bit iffy when it comes to linking old review comments to rebased code. But I remember them in this case, and they should all be addressed.

[xFrednet]

Alright, this version already looks good to me. I had a few more suggestions for test, and then we can get this merged.

Could you also squash your commits? It's fine to have more than one if they're named properly, or just to have one review commit. 🙃

Small side note: I ran this on 26 Crates with our lintcheck, and it identified 5 cases, which were all correct. So it looks like time implementation is good to go 🎉

[kartva]

Could you also squash your commits?

Squashed.

I ran this on 26 Crates with our lintcheck, and it identified 5 cases

Huh, and here I thought this was too niche a lint. Happy to hear that!

[kartva]

Oof, a pre-commit hook is correcting |(_, val)| {val} to |(_, val)| val. Even worse, adding #[rustfmt::skip] doesn't help because then it disables the rustfix tests. Trying to add the attribute to the statement shows me that that feature is still in nightly.

Would it be fine to just skip this test? @xFrednet

[xFrednet]

Oof, a pre-commit hook is correcting |(_, val)| {val} to |(_, val)| val. Even worse, adding #[rustfmt::skip] doesn't help because then it disables the rustfix tests. Trying to add the attribute to the statement shows me that that feature is still in nightly.

Would it be fine to just skip this test? @xFrednet

Alright, then it's fine if we skip that test. Could you fix the CI? Then we can merge it :)

[xFrednet]

Looks good to me, thank you for this implementation. The lint will be a great addition to Clippy. I hope you had fun while working on Clippy 🙃

@bors r+

[bors]

📌 Commit c6219b2 has been approved by xFrednet

It is now in the queue for this repository.

[bors]

⌛ Testing commit c6219b2 with merge 481dc2e...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: xFrednet
Pushing 481dc2e to master...