Skip to content

needless_match falsely suggesting replacement of if let with side effects #8595

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

Closed
netthier opened this issue Mar 27, 2022 · 0 comments · Fixed by #8549
Closed

needless_match falsely suggesting replacement of if let with side effects #8595

netthier opened this issue Mar 27, 2022 · 0 comments · Fixed by #8549
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@netthier
Copy link

Summary

// pieces is &mut Vec<Piece>
if let Some(piece) = pieces.pop() {
  Some(piece)
} else {
  *pieces = DEFAULT_BAG.to_vec();
  self.rng.shuffle(pieces);
  pieces.pop()
}

Clippy is suggesting to replace this complete part of code with just

pieces.pop()

As the else arm has side effects, namely re-filling the Vec should it be empty, the suggestion is simply incorrect in this case.
This is especially an issue as this lint is marked as MachineApplicable.

Lint Name

needless_match

Reproducer

I tried this code:

fn main() { }

fn clippy_test(vec: &mut Vec<i32>) -> Option<i32> {
    if let Some(num) = vec.pop() {
        Some(num)
    } else {
        *vec = vec![1, 2, 3];
        vec.pop()
    }
}

I saw this happen:

warning: this if-let expression is unnecessary
 --> src/main.rs:4:5
  |
4 | /     if let Some(num) = vec.pop() {
5 | |         Some(num)
6 | |     } else {
7 | |         *vec = vec![1, 2, 3];
8 | |         vec.pop()
9 | |     }
  | |_____^ help: replace it with: `vec.pop()`
  |
  = note: `#[warn(clippy::needless_match)]` on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_match

I expected to see this happen:
A correct and working suggestion, or none at all.

Version

rustc 1.61.0-nightly (1d9c262ee 2022-03-26)
binary: rustc
commit-hash: 1d9c262eea411ec5230f8a4c9ba50b3647064da4
commit-date: 2022-03-26
host: x86_64-unknown-linux-gnu
release: 1.61.0-nightly
LLVM version: 14.0.0

Additional Labels

No response

@netthier netthier added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Mar 27, 2022
@bors bors closed this as completed in 81e004a Apr 6, 2022
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 I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant