Fix if_then_some_else_none FP when encountering await codes

[relaxcn]

changelog: Fix FP of [if_then_some_else_none] when the then block contains await codes.

That is because bool:then doesn't work with await code.

Closes: #16176

[rustbot]

r? @Jarcho

rustbot has assigned @Jarcho.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[y21]

Could we just use can_move_expr_to_closure on the then expression here? I'm not sure if it would introduce FNs but it seems like it answers exactly the question of "can we move that code into a closure", taking into account control flow constructs and the like (yield, return, continue, break, ...).

[y21]

There's also a bug in the current logic here because it only runs the visitor on the tail expression and not the statements, so it would miss await in statements of the block.

If can_move_expr_to_closure doesn't work out it might still be better to remove the contains_return and instead just have a single for_each_expr_without_closures on the block that checks for everything we want to ignore. That saves us from visiting the whole block twice.

Rather than using desugar_await(e).is_some() it might also be better to just check for ExprKind::Yield since that's ultimately the root cause. It also can't really be used in bool::then and await uses yield.

[relaxcn]

Thank you. I think we need to use can_move_expr_to_closure consistently.
And I also modified can_partially_move_ty to make Ref return false because Ref can't be moved partially.

[github-actions]

Lintcheck changes for 87e8ad1

Lint	Added	Removed	Changed
clippy::if_then_some_else_none	0	7	2
clippy::manual_map	1	0	0
clippy::option_if_let_else	75	0	8

This comment will be updated if you push new changes

[rustbot]

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[ada4a]

The new FP in manual_map is only partially related to these changes:

• It looks like can_move_expr_to_closure fails to notice the use of input inside trace!, most likely because the latter expands to nothing, and thus wrongly returns true.
• But in theory, manual_map should've avoided linting by itself, as it shouldn't lint then-block which contains statements (in this case, the trace!), but doesn't, as it doesn't notice trace!, as it expands to nothing.

Opened a separate issue for the latter: #16193

[relaxcn]

The new FP in manual_map is only partially related to these changes:

• It looks like can_move_expr_to_closure fails to notice the use of input inside trace!, most likely because the latter expands to nothing, and thus wrongly returns true.
• But in theory, manual_map should've avoided linting by itself, as it shouldn't lint then-block which contains statements (in this case, the trace!), but doesn't, as it doesn't notice trace!, as it expands to nothing.

Opened a separate issue for the latter: #16193

Yes. The new issue #16193 seems to be caused by can_move_expr_to_closure returns true wrongly.

Even though the manual_map could prevent it in the early age, I prefer to improve can_move_expr_to_closure to keep the lint code simple.