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

while_let false positive with break checks before match statement #771

Closed
ipetkov opened this issue Mar 16, 2016 · 1 comment · Fixed by #4064
Closed

while_let false positive with break checks before match statement #771

ipetkov opened this issue Mar 16, 2016 · 1 comment · Fixed by #4064
Labels
C-bug Category: Clippy is not doing the correct thing

Comments

@ipetkov
Copy link

ipetkov commented Mar 16, 2016

Sample program:

#![feature(plugin)]
#![plugin(clippy)]

fn main() {
    let mut count = 100;
    loop {
        // Some check to avoid calling match statement
        if count > 10 {
            break;
        }

        match delete_the_database() {
            Some(_) => count = 0,
            None => break,
        }
    }
}

fn delete_the_database() -> Option<()> {
    Some(())
}

Suggestion given completely ignores the possibility of breaking before the match is even reached. This is a trivial example, but it is possible the earlier checks are imperative for breaking the loop before something irreversible is called further in the body.

src/main.rs:6:5: 16:6 warning: this loop could be written as a `while let` loop, #[warn(while_let_loop)] on by default
src/main.rs: 6     loop {
src/main.rs: 7         // Some check to avoid calling match statement
src/main.rs: 8         if count > 10 {
src/main.rs: 9             break;
src/main.rs:10         }
src/main.rs:11 
               ...
src/main.rs:6:5: 16:6 help: try
src/main.rs:       while let Some(_) = delete_the_database() { .. }
@Manishearth Manishearth added the C-bug Category: Clippy is not doing the correct thing label Mar 16, 2016
@msiemens
Copy link

A variation of that seems to occur in this case:

loop {
    if let Some(_) = Some(1) {
        // ...
    }

    if let Some(_) = Some(2) {
        // ...
    } else {
        break
    }
}

@ghost ghost mentioned this issue May 7, 2019
bors added a commit that referenced this issue May 7, 2019
Add test for #771.

Closes #771

<!--
Thank you for making Clippy better!

We're collecting our changelog from pull request descriptions.
If your PR only updates to the latest nightly, you can leave the
`changelog` entry as `none`. Otherwise, please write a short comment
explaining your change.

If your PR fixes an issue, you can add "fixes #issue_number" into this
PR description. This way the issue will be automatically closed when
your PR is merged.

If you added a new lint, here's a checklist for things that will be
checked during review or continuous integration.

- [ ] Followed [lint naming conventions][lint_naming]
- [ ] Added passing UI tests (including committed `.stderr` file)
- [ ] `cargo test` passes locally
- [ ] Executed `util/dev update_lints`
- [ ] Added lint documentation
- [ ] Run `cargo fmt`

Note that you can skip the above if you are just opening a WIP PR in
order to get feedback.

Delete this line and everything above before opening your PR -->

changelog: none
@bors bors closed this as completed in #4064 May 7, 2019
flip1995 pushed a commit to flip1995/rust-clippy that referenced this issue May 5, 2020
Changes:
````
rustup rust-lang/rust#60586
Add test for rust-lang#771.
Clean up while_loop tests
````
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants