New lint: if let Some iter element

[camsteffen]

What it does

Checks for iteration of Options with if let Some inside. Can use flatten() instead. The lint should also work in iter.for_each().

Categories (optional)

• Kind: complexity

What is the advantage of the recommended code over the original code

It is simpler.

Drawbacks

None.

Example

for x in y {
    if let Some(z) = x {
        println!("{}", z);
    }
}

Could be written as:

for z in y.flatten() {
    println!("{}", z);
}

[nahuakang]

Nice suggestion Cam 😄 Can I take this issue?

[camsteffen]

@nahuakang yes!

[camsteffen]

It can also cover option.map(), result.map() and if let Ok.

[nahuakang]

@flip1995 @camsteffen I've started looking into the code base. Seems like this lint should be included in loops.rs and be run inside fn check_for_loop?

Cam, can I cover the original case first before looking into option.map(), result.map() and if let Ok? (I also don't understand them unless you give some snippets 🙈)

[camsteffen]

Seems like this lint should be included in loops.rs and be run inside fn check_for_loop?

Sounds like a good start!

Cam, can I cover the original case first before looking into option.map(), result.map() and if let Ok?

Absolutely. You can always leave some cases to be added later as an enhancement. You also can open a PR to get feedback before you are done.

Here are some examples (each can be changed to use flatten()):

for n in vec![Some(1)] {
    // if let Some
    if let Some(n) = n {
        println!("sup");
    }
}
let vec: Vec<Result<i32, i32>> = vec![];
for n in vec.clone() {
    // if let Ok
    if let Ok(n) = n {
        println!("sup");
    }
}
for n in vec.clone() {
    // Option::map or Result::map
    n.map(|n| println!("sup"));
}
// for_each and match
Some(Some(1)).into_iter().for_each(|n| match n {
    Some(_) => println!("sup"),
    None => {}
});

One note that may help is that if let .. is "desugared" into an equivalent match .. expression, so your code will just be looking for match instead of if let.

Good luck!

[flip1995]

You have to be careful that the if let ... expression is the only expression inside the for loop. Otherwise applying this lint would change the semantics. (Best way to address such cases is to add tests for it right away, before you even defined the lint. This also makes sure, that you don't address the happy path with your tests)

[flip1995]

Implemented in #6646