Rust-clippy: Suggest changing `&mut Vec<_>` parameters to `&mut [_]` when only slice methods are used.

[scottmcm]

This mistake comes up fairly frequently on URLO, such as
https://users.rust-lang.org/t/iterators-vs-index-loops-performance/52131?u=scottmcm
https://users.rust-lang.org/t/we-all-know-iter-is-faster-than-loop-but-why/51486/3?u=scottmcm

What it does

• For a free function or inherent method that takes &mut Vec<_> or &Vec<_> [ed: the &Vec<_> one already exists]
• But only uses it via its deref to slice (len, iter_mut, etc)
• Suggest changing the parameter to a slice instead

Categories (optional)

• Kind: perf & style both seem appropriate

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

• More general in what it accepts, as one can pass sub-slices of Vecs instead of only being able to pass entire ones
• Faster, because LLVM optimizes it better when it doesn't have to do the extra hop)
• Clearer, because it makes it obvious that you're not changing the length of the passed Vec

For example:

• Remove bounce checking inserted by ...
• Remove the need to duplicating/storing/typo ...

Drawbacks

None.

Example

fn use_for(vec: &mut Vec<usize>) {
    for i in 0..vec.len() {
        vec[i] = i;
    }
}

Could be written as:

fn use_for(vec: &mut [usize]) {
    for i in 0..vec.len() {
        vec[i] = i;
    }
}

[Volker-Weissmann]

I think its better if clipyy suggests using an iterator over the Vector. Changing a vec to an array might involve more code change than chaning a loop to an iterator.

[scottmcm]

Note that I'm not suggesting an array here, but a slice -- which is what code like .len() on a Vec is actually already using.

clippy will already suggest replacing the indexing with .iter_mut().enumerate(), but even with that change the parameter type should still change.

[Volker-Weissmann]

Ok. But I think that proposing .iter_mut().enumerate() is better than proposing taking a slice, because the latter is probably the bigger change.

[scottmcm]

I see this as a _both_, not an either-or.

I totally agree that

fn use_for(vec: &mut Vec<usize>) {
    for i in 0..vec.len() {
        vec[i] = i;
    }
}

should change to

fn use_for(slice: &mut [usize]) {
    for (i, x) in slice.iter_mut().enumerate() {
        *x = i;
    }
}

But this issue is just about one part. They're separable.

[ebroto]

@Volker-Weissmann it does not require a change other than the type of the function parameter:

fn use_for(vec: &mut [usize]) {
    for i in 0..vec.len() {
        vec[i] = i;
    }
}

fn main() {
    let mut v = vec![1, 2, 3, 4, 5];
    use_for(&mut v);
}

(playground)

You can see in the playground too that Clippy will already suggest .iter_mut().enumerate().

EDIT: @scottmcm beat me to it :)

[flip1995]

Clippy already suggests changing &Vec<..> to &[..]: ptr_arg

But it intentionally doesn't lint &mut args. We could start linting those to with the check for only-slice-methods.

[scottmcm]

Thanks, @flip1995 -- I hadn't checked that one. I've edited the OP.