ptr_arg: clarify about original type when newtype is just a Vec<_>

[matthiaskrgr]

clippy 0.0.212 (b1d0343 2018-10-19)

fn main() {}

type SomeType = Vec<String>;

pub fn do_not_eat(a: &SomeType) -> SomeType {
	for i in a {
		println!("{}", i);
	}
	a.to_vec()
}

clippy warns

warning: writing `&Vec<_>` instead of `&[_]` involves one more reference and cannot be used with non-Vec-based slices.
 --> src/main.rs:5:22
  |
5 | pub fn do_not_eat(a: &SomeType) -> SomeType {
  |                      ^^^^^^^^^
  |
  = note: #[warn(clippy::ptr_arg)] on by default

but I think the message looks like a false positive if we don't see (as in this example) that the type SomeType is actually just a Vec<_> in disguise.

Can we perhaps print some detail on what the actual type is in case of newtypes to make this slightly more clear?

I found similar code in rls where syntax::ast::GenericBounds is actually just type GenericBounds = Vec<GenericBound>;

[androm3da]

but I think the message looks like a false positive if we don't see (as in this example) that the type SomeType is actually just a Vec<_> in disguise.

I thought this might be a false positive but what the lint is trying to tell you here is that for the sake of your interface you don't need to be as specific as SomeType. It's to indicate that while you might use SomeType to instantiate your actual content, the interface overspecifies a Vec. The body of do_not_eat would work well with a slice instead.

e.g. this code avoids the lint.

fn main() {
}

type SomeType = Vec<String>;
type SomeTypeArg = [String];

pub fn do_not_eat(a: &SomeTypeArg) -> SomeType {
    for i in a {
        println!("{}", i);
    }
    a.to_vec()
}

AFAICT this issue should be closed. The type alias is not important here.

I will concede that when I saw this lint message, I was a little confused. I also had a type alias like in this example, but that didn't confuse me quite as much as "...involves one more reference...". It wasn't immediately obvious that the fix was to specify a slice here.

[flip1995]

Defining another type just for function arguments seems a little bit off IMO.

For type definitions like Vec<_> I would add a hint: "SomeType is defined as Vec<_>"

AFAICT this issue should be closed. The type alias is not important here.

A hint how to fix this warning is always nice. Clippy only wants to annoy users, but doesn't want to confuse them!

---

As a site note:
If you have a type definition and a function taking this type as an argument it might make sense to refactor your code like this:

struct SomeType(Vec<String>);

impl Deref for SomeType {
    type Target = Vec<String>;

    fn deref(&self) -> &Self::Target {
        &self.0
    }
}

impl SomeType {
    pub fn do_not_eat(&self) -> Self { 
        // ...
    }
}

[flip1995]

Maybe we could also move the user-defined type part of this lint into a pedantic lint?

[guswynn]

This also applies to type Something = String:
https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=4b8ccb2abaee4ba4deba3a29f9ecdd08

This pattern seems valuable to me when things are represented as Strings, but are not equivalent at all (like different types of id's)