Fix return_and_then FP when return type is not Option or Result

[profetia]

Closes #14927

changelog: [return_and_then] fix FP when return type is not Option or Result

[rustbot]

r? @y21

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

So this fixes the issue by checking if the enclosing function's return type is Option or Result so that the ? operator would be valid, right?

Thinking about this, it seems surprising to me that we lint and_then() calls in an if let scrutinee position like the test case to begin with. The lint description sounds like it should specifically lint when and_then() is actually being returned, which should also make such a return type check redundant (because then the function must return an Option or Result in order to type check).

So even with this fix, we would presumably still emit a warning on the test case if the closure had Option as its return type, which still seems odd.

Makes me wonder if this is a false positive with the tcx.hir_get_fn_id_for_return_block(..) function returning Some(_) - based on its doc I'd have expected it to return None here, because the if let scrutinee should have no effect on the return type.

Though I also haven't looked at its usages in rustc, this might just be intended and we're using this in ways it's not meant to be used. Perhaps we could just replace that call with some manual checks that it really is the tail return expression of the function.

What do you think about instead of checking the return type, fixing the logic in how it checks if the expression is actually returned?

[profetia]

Not sure if it is a FP of hir_get_fn_id_for_return_block, but the if .. else if .. expr is indeed in the return expr.