Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix question_mark FP on custom error type #7860

Merged
merged 5 commits into from
Oct 26, 2021

Conversation

dswij
Copy link
Member

@dswij dswij commented Oct 22, 2021

Closes #7859

#7840 aims to ignore question_mark when the return type is custom, which is covered here. But this fails when there is a call in conditional predicate

changelog: [question_mark] Fix false positive when there is call in conditional predicate

@rust-highfive
Copy link

r? @giraffate

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Oct 22, 2021
clippy_lints/src/question_mark.rs Outdated Show resolved Hide resolved
Comment on lines 160 to 166
let _ = if let Ok(x) = func_returning_result() {
x
} else {
return Err("some error".to_string());
};

if func_returning_result().is_err() {
return func_returning_result();
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should cover for #7859.

@giraffate
Copy link
Contributor

Sorry for the late reviewing. I can go back to reviewing in a few days.

Copy link
Contributor

@giraffate giraffate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good, thanks!

I made small comments. I prefer to use expr instead of expression for naming, if there is no specific reason.

clippy_lints/src/question_mark.rs Outdated Show resolved Hide resolved
clippy_lints/src/question_mark.rs Outdated Show resolved Hide resolved
@dswij
Copy link
Member Author

dswij commented Oct 26, 2021

Thanks for the review!

Addressed the nits 🙂

@giraffate
Copy link
Contributor

@bors r+

Thanks!

@bors
Copy link
Contributor

bors commented Oct 26, 2021

📌 Commit fb0fbad has been approved by giraffate

@bors
Copy link
Contributor

bors commented Oct 26, 2021

⌛ Testing commit fb0fbad with merge ba2ac3e...

@bors
Copy link
Contributor

bors commented Oct 26, 2021

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: giraffate
Pushing ba2ac3e to master...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

false positive: question_mark when returning custom Error type
4 participants