trigger obfuscated_if_else for .then(..).unwrap_or(..)

[lapla-cogito]

part of #9100

The obfuscated_if_else lint currently only triggers for the pattern .then_some(..).unwrap_or(..), but there're other cases where this lint should be triggered, one of which is .then(..).unwrap_or(..).

changelog: [obfuscated_if_else]: trigger lint for the .then(..).unwrap_or(..) pattern as well

[rustbot]

r? @llogiq

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

[lapla-cogito]

Also I'm willing to implement for other patterns that should be covered by obfuscated_if_else. (But in other PRs)

[samueltardieu]

Also I'm willing to implement for other patterns that should be covered by obfuscated_if_else. (But in other PRs)

May I ask why you don't want to do it in this PR? As far as I can tell, those patterns are all pretty similar and can be split into three categories:

• either a subpattern takes a value (.then_some(), .unwrap_or()), and they must be checked for side effects before the expression can be transformed into the if
• or they take a lambda expression (.then(), .unwrap_or_else()), which is not evaluated if the condition doesn't match so this is already compatible with the if syntax you are proposing
• or it is .unwrap_or_default() which doesn't take anything and may be replaced with a else { Default::default() }

Or are there some subtleties I am missing there?

[lapla-cogito]

May I ask why you don't want to do it in this PR?

Aren't there quite a lot of patterns that should really be handled by obfuscated_if_else? That is, I think this lint should finally support the following method combinations:
[then, then_some] x [unwrap_or, map_or, unwrap_or_default, map_or_else, is_some_and]
Since it's already implemented for the then_some and unwrap_or combinations, we need to address the rest of the patterns, but I thought it would be excessive to make changes to all of them in this single PR.

[samueltardieu]

Aren't there quite a lot of patterns that should really be handled by obfuscated_if_else?

In my opinion, this lint is for something which doesn't return an Option, as in this case the if form might not be the best one (or it would have to move to restriction).

For example, some of the combinations listed, such as some_bool.then_some(v).map_or(|x| f(x, 3)), would probably be best handled by a lint which transforms it into some_bool.then(|| f(v, 3)) (which might in turn trigger the present lint after the fix if it is followed by an .unwrap_or() for example). (you are right about .map_or())

Since it's already implemented for the then_some and unwrap_or combinations

I'll open an issue btw since this gave us a chance to notice that it doesn't detect side effects (not caused by your PR of course).

[lapla-cogito]

The problems with the previous implementation of obfuscated_if_else should be fixed here, so I think this patch is ready for review on its own. @llogiq

[llogiq]

Only a few small nits, otherwise this looks good to me.

[llogiq]

Thank you!