Skip to content

fix manual_filter false positive#10091

Merged
bors merged 2 commits intorust-lang:masterfrom
ericwu17:manual-filter-FP
Dec 21, 2022
Merged

fix manual_filter false positive#10091
bors merged 2 commits intorust-lang:masterfrom
ericwu17:manual-filter-FP

Conversation

@ericwu17
Copy link
Contributor

@ericwu17 ericwu17 commented Dec 16, 2022

fixes #10088
fixes #9766

changelog: FP: [manual_filter]: Now ignores if expressions where the else branch has side effects or doesn't return None
#10091

do explicit checks for the other branch being None
@rustbot
Copy link
Collaborator

rustbot commented Dec 16, 2022

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

Please see the contribution instructions for more information.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Dec 16, 2022
@kraktus
Copy link
Contributor

kraktus commented Dec 16, 2022

Hey, thanks for the fix! Could you add a test for #9766 as I believe your fix will fix this issue too

The related issue is rust-lang#9766 where the `manual_filter`
lint would remove side effects
@ericwu17
Copy link
Contributor Author

Thanks @kraktus, I wasn't aware that there was a related issue. That's great that this fix also resolves the other issue!

@ericwu17
Copy link
Contributor Author

Sorry about the force push from 62cd8d9 to 3c14075. I was working on a separate bug and accidentally pushed changes up to the wrong branch. Commit 3c14075 contains my intended changes.

@llogiq
Copy link
Contributor

llogiq commented Dec 21, 2022

I hope that won't disturb the history, but I have faith in our bot. Thank you for working on this.

@bors r+

@bors
Copy link
Contributor

bors commented Dec 21, 2022

📌 Commit 3c14075 has been approved by llogiq

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Dec 21, 2022

⌛ Testing commit 3c14075 with merge 065c6f7...

@bors
Copy link
Contributor

bors commented Dec 21, 2022

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: llogiq
Pushing 065c6f7 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.

manual_filter suggestion broken on nested if let manual_filter wrongly suggesting filter when match has side-effects

5 participants

Comments