New Lint: inspect_then_for_each

[nahuakang]

Work In Progress

This PR addresses Issue 5209 and adds a new lint called inspect_then_for_each.

Current seek some guidance.

If you added a new lint, here's a checklist for things that will be
checked during review or continuous integration.

• Followed lint naming conventions
• Added passing UI tests (including committed .stderr file)
• cargo test passes locally
• Executed cargo dev update_lints
• Added lint documentation
• Run cargo dev fmt

---

changelog: Add [inspect_for_each] lint for the use of inspect().for_each() on Iterators.

[rust-highfive]

r? @llogiq

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

[nahuakang]

r? @flip1995

[nahuakang]

Andre gave some good feedback to the lint, most specifically:

Note however that we could cover the simple case where the patterns are exactly equal (with a spanless_eq, search clippy's internal utils).

I skimmed utils/hir_utils but, unfortunately, I still don't know how I could get Pat in this situation to compare the closure parameter patterns. Would appreciate some further hints if anyone sees this comment 🙏

[llogiq]

That would be an ExprKind::Closure from which you can get the BodyId. Then use the ctx to look up the Body, which has a slice of Params, each of which has a Pat.

[flip1995]

This PR LGTM. The suggestion generation is a whole other topic, which I would address in a separate PR.

The span and the help message has to be adapted and this should be good to go.

[flip1995]

Suggested change

	let hint = "this is more succinctly expressed by calling ` for_each(..)` instead";
	let hint = "move the code from `inspect(..)` to `for_each(..)` and remove the `inspect(..)`";

I think a span_lint_and_help is enough here for now. Building a good looking suggestion by combining the closures into one is way more involved (getting the indentation right and whatnot).

---

Some guidance how this can be done:

1. extract the closure bodies from _inspect_args[1] and _for_each_args[1]
2. check if the closure body of for_each is already a block
   a. If it is, get the span of everything inside the block.
   b. If not, just get the span of the expression
3. build the suggestion with a block around it, where the body of inspect is placed at the top and is indented the same as everything inside the for_each function.

This will involve much fiddling. I done this before and utils has many methods to achieve this (search for usages of indent_of in the code base). It'll be mostly trial and error though.

Anyway, this should probably be done in a separate PR.

[nahuakang]

You're right that it could be done in a separate PR. In any case, I've taken notes about your suggestions :)

[nahuakang]

@flip1995 Many thanks for your suggestions!

commit 32405b8 reflects all the suggestions you made.

commit ff1229a incorporates feedback from @llogiq
and includes some additional checks for the two closure's parameter patterns before running the lint.

Let me know which you prefer and I can adjust again!

[flip1995]

Why is it important that the patterns of the closures are the same? I can see that this may cause problems when producing suggestions, but for now this shouldn't matter. Am I missing something?

Please also add a test for this.

[nahuakang]

It does make only sense to check for the closure patterns if we aim to produce suggestions. In this scope, you're right we should keep the lint as simple as possible. The change is reverted now.

[flip1995]

Just the lint name

[flip1995]

Thanks! Can you squash some of your commits, please? After that, this is ready to merge.

[nahuakang]

Just did 😃 Manual squashing gets easier each time one does it haha. Thank you!

[flip1995]

@bors r+

Thanks!

[bors]

📌 Commit 3269070 has been approved by flip1995

[bors]

⌛ Testing commit 3269070 with merge d71dea4...

[bors]

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