Lint against Iterator::map receiving a callable that returns ()

[estebank]

Code

fn foo(items: &mut Vec<u8>){
    items.sort();
}

fn main() {
    let mut x: Vec<Vec<u8>> = vec![
        vec![0, 2, 1],
        vec![5, 4, 3],
    ];
    x.iter_mut().map(foo);
    println!("{x:?}");
}

Current output

Passes

Desired output

warning: `Iterator::map` call that discard the iterator's values
   |
LL | fn foo(items: &mut Vec<u8>) {
   |    --- this function returns `()`, which is likely not what you wanted
...
LL |     x.iter_mut().map(foo)
   |                  --- ^^^ called `Iterator::map` with callable that returns `()`
   |                  |
   |                  after this call to map, the resulting iterator is `impl Iterator<Item = ()>`, which means the only information carried by the iterator is the number of items
   |
   = note: `Iterator::map`, like many of the methods on `Iterator`, gets executed lazily, meaning that its effects won't be visible until it is iterated
help: you might have meant to use `Iterator::for_each`
   |
LL -     x.iter_mut().map(foo)
LL +     x.iter_mut().for_each(foo)
   |

Rationale and extra context

Mapping to () is almost always a mistake. The for_each suggestion should only be emitted if it would make sense with foo, like if it modifies the argument or has side-effects like printing or logging (the later would be hard to check for).

Other cases

No response

Anything else?

No response

[matthiaskrgr]

Like this? 😄 https://rust-lang.github.io/rust-clippy/master/index.html#option_map_unit_fn

edit: ah this might only lint options but maybe not too hard to expand?

[estebank]

@matthiaskrgr yes 😅

I'd like to uplift it so that it still triggers even in the face of other errors. If you run clippy on https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=51211f1c12a2b1995b24bab269a02ce0, it will only complain about the return type.

[obeis]

@rustbot claim

[estebank]

I'm reopening the ticket to track the comment in this one case:

https://github.com/rust-lang/rust/pull/107890/files#diff-0e3b31971aaa3e0b1e633c38a8c5ea34fcc99b7e96b12f61bf3b53820213f86e

fn foo(items: &mut Vec<u8>) {
    items.sort();
}

fn bar() -> impl Iterator<Item = i32> {
    //~^ ERROR expected `foo` to be a fn item that returns `i32`, but it returns `()` [E0271]
    let mut x: Vec<Vec<u8>> = vec![vec![0, 2, 1], vec![5, 4, 3]];
    x.iter_mut().map(foo)
}

fn main() {
    bar();
}

should provide more context than

error[E0271]: expected `foo` to be a fn item that returns `i32`, but it returns `()`
  --> $DIR/issue-106991.rs:5:13
   |
LL | fn bar() -> impl Iterator<Item = i32> {
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^ expected `()`, found `i32`
   |
   = note: required for `Map<std::slice::IterMut<'_, Vec<u8>>, for<'a> fn(&'a mut Vec<u8>) {foo}>` to implement `Iterator`