needless_return_with_question_mark ignore let-else

[dswij]

Fixes #11765

This PR makes needless_return_with_question_mark to ignore expr inside let-else.

changelog: [needless_return_with_question_mark] ignore let-else

[rustbot]

r? @xFrednet

(rustbot has picked a reviewer for you, use r? to override)

[xFrednet]

Pretty nit picky:

I feel like the if let Node::Local(Local { els: Some(_), .. }) check could be a bit too general, as it doesn't check, that the expression comes from the else branch:

fn x() -> Result<(), ()> {
    let Some(_): Option<()> = ({
        return Err(())?;
    }) else {
        panic!();
    };
}

Even if that's pretty weird code...

Could you try if this works:

fn is_let_else_block(tcx: TyCtxt<'_>, e: &Expr<'_>) -> bool {
    let mut child_id = e.hir_id;
    for (parent_id, node) in tcx.hir().parent_iter(e.hir_id) {
        if let Node::Local(Local { els: Some(els), .. }) = node
            && els.hir_id == child_id
        {
            return true;
        }
        child_id = parent_id;
    }

    false
}

Then we could probably move this into clippy_utils in case other lints need something like this :). If not, it's totally fine. I think this code is already good enough of a heuristic, to avoid FPs and r+ the PR :)

[dswij]

🤔 yeah fair enough, if we're going to move this to util, then this needs to be as precise as possible.

fn x() -> Result<(), ()> {
    let Some(_): Option<()> = ({
        return Err(())?;
    }) else {
        panic!();
    };
}

This is kind of outrageous for an FN of this lint, there's more to worry about than a needless return 😅

[dswij]

Oh, just remembered that we'd need to ignore this as well

fn x() -> Result<(), ()> {
    let Some(_): Option<()> = ({
        return Err(())?;
    }) else {
        panic!();
    };
}

removing the return here leads to more complications than benefits; e.g. here it leads to compilation error, and if we have the same type, it might change the behavior.

Could you try if this works:

But this does work

[xFrednet]

LGTM, thank you for the update :)

@bors r+

[bors]

📌 Commit 48f38eb has been approved by xFrednet

It is now in the queue for this repository.

[bors]

⌛ Testing commit 48f38eb with merge 7ad3373...

[bors]

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