false positive on ptr_arg against &Vec<_>

[llogiq]

It's not possible to call any of Vec's methods on a slice, whether mutable or not. So if any method of Vec is called, the lint message has to be considered a false positive.

Example: fn foo(v: &Vec<Name>, n: Name) { v.push(n); }

[birkenfeld]

I don't think you can call push on an immutable reference :) There's currently only one method on Vec<T> that isn't on [T] that doesn't require a mutable reference: capacity. So I think we can with good conscience keep that.

But if the lint currently also warns on &mut Vec<T> or &mut String, it should definitely not do that.

[birkenfeld]

I'll do a PR for that.

[vnermolaev]

The issue still persists when a reference on a vector is passed for code that uses immutable vector methods. Please see below or in (playground)

fn test(v: &Vec<u8>) -> Result<(), &'static str> {
    // Complicated logic to retrieve values from db.
    let from_db = vec![vec![1, 2], vec![3, 4]];
    
    if !from_db.contains(v) {
        return Err("Not present");
    }
    
    Ok(())
}

fn main() -> Result<(), &'static str> {
    let v: Vec<u8> = vec![5, 6];
    
    test(&v)
}

I does compile and run, but if I lint it with Clippy, it will complain

warning: writing `&Vec<_>` instead of `&[_]` involves one more reference and cannot be used with non-Vec-based slices.
 --> src/main.rs:1:12
  |
1 | fn test(v: &Vec<u8>) -> Result<(), &'static str> {
  |            ^^^^^^^^ help: change this to: `&[u8]`
  |
  = note: `#[warn(clippy::ptr_arg)]` on by default

Changing it to &[u8] expectedly breaks the compilation due to Vec::contains.

cargo clippy -V
clippy 0.0.212 (4ee12063 2020-02-01)

[birkenfeld]

IMO this is a rare case, and we can't do the sort of checks that would catch this false positive. At the same time, this lint is a very useful one since it teaches newbies about slices and coercion.

So in this case, the best solution is to put #[allow(clippy::ptr_arg)] on the function.

[vnermolaev]

Indeed, I addressed the issue how you indicated, but thought it would be valuable to showcase this scenario. Mainly because that following Clippy's advice breaks the compilation.
It is perhaps worth to add a note in the docs.

[birkenfeld]

Agreed!

[Ryan1729]

When I first saw the example above, I wondered why Vec::contains couldn't be written in such a way that the given case works. After a bit of searching I found rust-lang/rust#62367 which contains a suggestion that the restriction was a mistake in the initial implementation of Vec::contains, which cannot be corrected since it would cause type inference regressions. Note that Vec::contains currently just directly calls slice::contains.

[flip1995]

Keeping this open, because only the documentation was added.