Improve suggestions for needless_range_loop

[vbrandl]

Hey there. First of all: Great work to all the devs. Clippy is a real handy tool when developing in Rust.

Now about my proposal: the needless_range_loop gives pretty good suggestions even for non default ranges. But if a range loop like the following is linted, one could suggest to use a mutable iterator using iter_mut():

for i in 1..arr.len()-1 {
    arr[i] = <some_value>;
}

Clippy would suggest something like for <item> in arr.iter().take(arr.len()-1).skip(1) { but in this case iter_mut() must be used to assign new values to <item>. I don't know if there is an easy way to detect cases like this one but I think it would be a great improvement.

[vbrandl]

Also another case I stumbled across:

for i in 1..field.len() {
    let to_insert = field[i];
    let mut j: usize = i;
    while j >= 1 && field[j - 1] > to_insert {
        field[j] = field[j - 1];
        j -= 1;
    }
    field[j] = to_insert;
}

This is an implementation of insertionsort.

I'm new to Rust so there might be a way that I don't know of yet but in this case, clippy suggests the following: for (i, <item>) in field.iter().enumerate().skip(1) {

The problem is that in the while loop, I assign to another field in my array, that is not field[i] or <item> when using the iterator. But since iter() borrows my array, I cannot take a mutable borrow of the same array inside the loop. So I don't know if this is due to me not knowing better or if it is an actual bug in clippy. But maybe you can have a look at it.

[oli-obk]

Thanks! You're right, we should totally fix those suggestions

[mockersf]

for the first case, clippy now suggest something that doesn't compile:

fn main() {
    let mut arr = vec![1, 2, 3];
    for i in 1..arr.len()-1 {
        arr[i] = 6;
    }
}

warning: the loop variable `i` is only used to index `arr`.
 --> src/main.rs:4:14
  |
4 |     for i in 1..arr.len()-1 {
  |              ^^^^^^^^^^^^^^
  |
  = note: #[warn(needless_range_loop)] on by default
  = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.205/index.html#needless_range_loop
help: consider using an iterator
  |
4 |     for <item> in arr.iter_mut().take(arr.len()-1).skip(1) {
  |

applying the suggestion:

fn main() {
    let mut arr = vec![1, 2, 3];
    for item in arr.iter_mut().take(arr.len()-1).skip(1) {
        *item = 6
    }
}

error[E0502]: cannot borrow `arr` as immutable because it is also borrowed as mutable
 --> src/main.rs:4:37
  |
4 |     for item in arr.iter_mut().take(arr.len()-1).skip(1) {
  |                 ---                 ^^^                - mutable borrow ends here
  |                 |                   |
  |                 |                   immutable borrow occurs here
  |                 mutable borrow occurs here

error: aborting due to previous error