Fix redundant closure call async false positive

[lkirkwood]

Previously, async closures containing early returns which were immediately called triggered the redundant_closure_call lint. Regular non-async closures with these early return statements did not trigger these lints.

Async closures of both kinds (async || {} and || async {}) trigger some desugaring which needs to be appropriately handled when looking for early return statements. This commit implements that logic.

fixes: #16232

changelog: [redundant_closure_call]: Fix false positive on async closures with early returns

[rustbot]

r? @samueltardieu

rustbot has assigned @samueltardieu.
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

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: 8 candidates
• 8 candidates expanded to 8 candidates
• Random selection from Jarcho, ada4a, dswij, llogiq, samueltardieu

[samueltardieu]

Could you add exemple where "use" appears in the suggestion? I think that only "move" does.

View changes since this review

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[lkirkwood]

Thanks for the review!

Could you add exemple where "use" appears in the suggestion? I think that only "move" does.

Yeah happy to remove it. I added it because its one of the possible "capture clauses" in the ast. I don't know what the policy is on supporting unstable features. The following code compiles on nightly:

#![allow(unused)]
#![feature(ergonomic_clones)]

fn maybe() -> Option<()> {
    Some(())
}

async fn test() {
    let async_result = (async use || {
        if maybe().is_none() {
            Some(0)
        } else {
            Some(42)
        }
    })()
    .await;
}

fn main() {}

But obviously the feature may never be stabilised so you could argue its dead code. Up to you.

[lkirkwood]

@rustbot ready

[samueltardieu]

But obviously the feature may never be stabilised so you could argue its dead code. Up to you.

It can be used right now even when unstable, so I'd prefer we keep it and add tests. If this is removed from the compiler, Clippy won't compile anymore unless the relevant code is removed as well, so we won't accumulate technical debt there.

[lkirkwood]

Ok, done! I elected for #![allow(incomplete_features)] instead of expect so it wont need changing if the feature becomes complete, but happy to change it if that's the wrong approach. Thanks for your patience on this.

@rustbot ready

[samueltardieu]

Ok, done! I elected for #![allow(incomplete_features)] instead of expect so it wont need changing if the feature becomes complete, but happy to change it if that's the wrong approach. Thanks for your patience on this.

On the contrary, I would prefer that we are forced to change it when the feature becomes complete to not keep a useless #![allow] in our source code.