Improve suggestions for option_map_unit_fn

[pdolezal]

The suggestion for option_map_unit_fn could include more details and options for solving the lint.

Let's consider an invocation chain on Option which consumes the value, if any, at the end. Clippy issues the warning for such a piece of code.

give_me_some_iterator()
    .next()
    .filter(|&n| n % 2 == 0)
    .map(|n| n * 2)
    .filter(|&n| n > 100)
    .map(|n| println!("And the winning number is… {}", n));

The lint's suggestion in such situations does not look very appealing. Using if let on the whole statement does not improve readability, IMO. I guess that the next thing to try would be using a helper variable:

let n = give_me_some_iterator()
    .next()
    .filter(|&n| n % 2 == 0)
    .map(|n| n * 2)
    .filter(|&n| n > 100);

if let Some(found) = n {
    println!("And the winning number is… {}", n);
}

Obviously, it breaks the natural chained flow, which does not look very good either. The next step could be:

let _ = give_me_some_iterator()
    .next()
    .filter(|&n| n % 2 == 0)
    .map(|n| n * 2)
    .filter(|&n| n > 100)
    .map(|n| println!("And the winning number is… {}", n));

Although it is close to the original, the assignment feels like a workaround just to mute Clippy. I was pointed out that there is yet another way, which I – as a newbie – would hardly figure out:

give_me_some_iterator()
    .next()
    .filter(|&n| n % 2 == 0)
    .map(|n| n * 2)
    .filter(|&n| n > 100)
    .map_or((), |n| println!("And the winning number is… {}", n));

This is quite nice, but I'm not sure how much idiomatic it is considered or how much known and used it is. There is a topic on Rust Internals with a few more details and a proposal of an extension of Option to express better the intent of consuming the value. Anyway, I think that an improved lint would resolve some concerns shown in the discussion and offer an idiomatic alternative that keeps the original readability and compactness.

[giraffate]

@rustbot modify labels: +A-suggestion +L-enhancement

[kaimast]

I do not like this lint either.

Maybe there should be another function (e.g., "apply") that behaves like "map" but does not return a value.

[ada4a]

I think this should be treated the same way as Iterator::for_each -- usually to be avoided in favor of a for-loop, but fine to use if the method chain is a longer one. So maybe this lint should ignore method chains longer than, say, 3 calls (this could be configurable)