rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
11.21k stars 1.5k forks source link

manual_flatten ignores nested refutable pattern #12645

Open crumblingstatue opened 4 months ago

crumblingstatue commented 4 months ago

Summary

The manual_flatten lint doesn't take into account nested refutable patterns.

Lint Name

manual_flatten

Reproducer

I tried this code:

enum Data {
    Wanted(i32),
    Unwanted(String),
}

fn iter() -> impl IntoIterator<Item = Result<Data, ()>> {
    vec![Ok(Data::Wanted(42))]
}

fn main() {
    for item in iter() {
        if let Ok(Data::Wanted(n)) = item {
            println!("{n}");
        }
    }
}

Clippy output:

warning: variant `Unwanted` is never constructed
 --> src/main.rs:3:5
  |
1 | enum Data {
  |      ---- variant in this enum
2 |     Wanted(i32),
3 |     Unwanted(String),
  |     ^^^^^^^^
  |
  = note: `#[warn(dead_code)]` on by default

warning: unnecessary `if let` since only the `Ok` variant of the iterator element is used
  --> src/main.rs:11:5
   |
11 |       for item in iter() {
   |       ^           ------ help: try: `iter().into_iter().flatten()`
   |  _____|
   | |
12 | |         if let Ok(Data::Wanted(n)) = item {
13 | |             println!("{n}");
14 | |         }
15 | |     }
   | |_____^
   |
help: ...and remove the `if let` statement in the for loop
  --> src/main.rs:12:9
   |
12 | /         if let Ok(Data::Wanted(n)) = item {
13 | |             println!("{n}");
14 | |         }
   | |_________^
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_flatten
   = note: `#[warn(clippy::manual_flatten)]` on by default

It shouldn't suggest to remove the if let statement, because it's still needed to refutably match Data::Wanted.

Version

rustc 1.79.0-nightly (aa1c45908 2024-04-06)
binary: rustc
commit-hash: aa1c45908df252a5b0c14e1bcb38c6c55ae02efe
commit-date: 2024-04-06
host: x86_64-unknown-linux-gnu
release: 1.79.0-nightly
LLVM version: 18.1.2

Additional Labels

No response

crumblingstatue commented 4 months ago

I guess technically the suggestion can be applied (without removing the if let though), so I'm having second thoughts about whether this really is a false positive, but the suggestion is definitely misleading.