redundant_closure suggests wrong fix when the function being called is not Copy

[utkarshkukreti]

Code reduced from my actual project:

fn main() {
    let f = |_| true;
    while Some(1).map_or(false, |x| f(x)) {}
}

Clippy:

warning: redundant closure found
 --> src/main.rs:3:33
  |
3 |     while Some(1).map_or(false, |x| f(x)) {}
  |                                 ^^^^^^^^
  |
  = note: #[warn(redundant_closure)] on by default
help: remove closure as shown:
  |     while Some(1).map_or(false, f) {}
  = help: for further information visit https://github.com/Manishearth/rust-clippy/wiki#redundant_closure

Clippy's suggestion doesn't compile:

error[E0382]: use of moved value: `f`
 --> src/main.rs:3:33
  |
3 |     while Some(1).map_or(false, f) {}
  |                                 ^ value moved here in previous iteration of loop
  |
  = note: move occurs because `f` has type `[closure@src/main.rs:2:13: 2:21]`, which does not implement the `Copy` trait

error: aborting due to previous error

$ rustc -V
rustc 1.17.0-nightly (b1e31766d 2017-03-03)
$ cargo clippy -V
0.0.118

[oli-obk]

This should be fixed by suggesting an & before the closure name in case it is not Copy

[utkarshkukreti]

That works for the code I posted above but doesn't work in my actual code. Here's a slightly bigger test case which doesn't work with &.

This compiles but Clippy gives the same warning:

fn take_while<F>(str: &str, mut f: F)
    where F: FnMut(char) -> bool
{
    let mut chars = str.chars();
    while chars.clone().next().map_or(false, |ch| f(ch)) {
        chars.next();
    }
}
fn main() {
    take_while("...!", |ch| ch == '.');
}

With &f:

error[E0277]: the trait bound `F: std::ops::Fn<(char,)>` is not satisfied
 --> src/main.rs:5:32
  |
5 |     while chars.clone().next().map_or(false, &f) {
  |                                ^^^^^^ the trait `std::ops::Fn<(char,)>` is not implemented for `F`
  |
  = help: consider adding a `where F: std::ops::Fn<(char,)>` bound
  = note: required because of the requirements on the impl of `std::ops::FnOnce<(char,)>` for `&F`

With f:

error[E0382]: use of moved value: `f`
 --> src/main.rs:5:46
  |
5 |     while chars.clone().next().map_or(false, f) {
  |                                              ^ value moved here in previous iteration of loop
  |
  = note: move occurs because `f` has type `F`, which does not implement the `Copy` trait

[oli-obk]

Right, in that case we need &mut

[utkarshkukreti]

Ahh, thanks! I tried a lot of things to make &f compile but I forgot that FnMut would be implemented for &mut f and not &f.

[cipriancraciun]

I also got the same issue with an non-Copy Fn used in a loop. (However in my case an & before the function did solve the issue...) (clippy version is 0.0.211)

(If needed I can provide the source code.)