[manual_unwrap_or_default]: Check for Default trait implementation in initial condition when linting and use IfLetOrMatch

[ARandomDev99]

Fixes #12564

changelog: Fix [manual_unwrap_or_default] false positive when initial match/if let condition doesn't implement Default but the return type does.

[rustbot]

r? @dswij

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

[J-ZhengLi]

The implementation looks fine to me but I just have one nit suggestion.
(also sorry if my last PR #12579 causes conflict that you have to abandon your last PR 😄.)

It's not in your modifications but these two lines:

if let ExprKind::If(cond, if_block, Some(else_expr)) = expr.kind
    && let ExprKind::Let(let_) = cond.kind

could be replaced with clippy_utils::higher::IfLet, since the cond is no longer needed after the second line. Doing so also prevent this lint from triggering in a while let loop (I think).

Oh, btw, if conflicts ever come up, the easiest way would be doing a git pull master --rebase or git rebase master (assuming master branch is up-to-date), then solve the conflicts and do a force push.

And even if you messed up, you can always do a hard reset then do a force push. I learn that from the hard way here, lol 🤣

[ARandomDev99]

I'll try and make the change you suggested.

Doing so also prevent this lint from triggering in a while let loop (I think).

About this, even the original code won't trigger in a while let loop as the first condition is:

if let ExprKind::If(cond, if_block, Some(else_expr)) = expr.kind

so we're essentially just checking if let.

[J-ZhengLi]

About this, even the original code won't trigger in a while let loop as the first condition is:

if let ExprKind::If(cond, if_block, Some(else_expr)) = expr.kind

so we're essentially just checking if let.

Yes, but afaik it could be a de-sugared while loop, the IfLet::hir(..) is basically doing the same thing as the original code except it went back a level to make sure of it.

Oh wait, there's a IfLetOrMatch, I have no idea when was it added (how come something been there for 3 years and I only discover it until now 🤦‍♂️). Anyway, with that we might finally be able to merge the handle_match and handle_if_let functions together, thus avoiding adding same code twice everytime! But doing so might making things more complicated, so if you don't what to make the change its totally fine~

[ARandomDev99]

Oh wait, there's a IfLetOrMatch, I have no idea when was it added (how come something been there for 3 years and I only discover it until now 🤦‍♂️).

I'll see what I can do with it. That would greatly simplify the code.

[ARandomDev99]

I tried to make the code as readable as possible and added comments. Apologies if it looks like spaghetti code.

[dswij]

Nice work!

[dswij]

Thank you! @bors r+

[bors]

📌 Commit ac18c24 has been approved by dswij

It is now in the queue for this repository.

[bors]

⌛ Testing commit ac18c24 with merge 1c9e965...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: dswij
Pushing 1c9e965 to master...