add [assertions_on_result_states] lint

[tabokie]

Close #9162

changelog: add [assertions_on_result_states] lint

Signed-off-by: tabokie [email protected]

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @xFrednet (or someone else) soon.

Please see the contribution instructions for more information.

[Jarcho]

Two small things, and then one suggested extension which should be easy enough to add.

Can you also add a check for is_err with the suggestion to use unwrap_err to the same lint. Could be named something like assert_result_state.

[tabokie]

@Jarcho Thanks for the review.

Can you also add a check for is_err with the suggestion to use unwrap_err to the same lint.

I have doubts about this. unwrap is more favorable in the error message part, but unwrap_err is pretty much equivalent to is_err. So I think we shouldn't enforce it, at least not in the same level as is_ok.

Forget it, I was being silly.

[Jarcho]

There's one more issue to handle after the previous smaller things. This change does result in a behaviour change. Namely unwrap consumes it's argument whereas is_ok does not. If you restrict this to Result types which implement Copy (use `clippy_utils::ty::is_copy) that would be fine.

If you want to go further you can check if the value is both movable, and not used afterwards. I wouldn't recommend this for the current PR as it's a fair amount of extra work.

[Jarcho]

Everything looks good. You can just squash it down and we're good to merge.

[tabokie]

@Jarcho It's done. Thanks.

[Jarcho]

Thank you.

@bors r+

[bors]

📌 Commit 8454602 has been approved by Jarcho

It is now in the queue for this repository.

[bors]

⌛ Testing commit 8454602 with merge 1cce047...

[bors]

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