Clippy seems to want me to clone things, doesn't like this: fn(a: &String)

Re: this clippy lint: ALL the Clippy Lints

I have an example I'm confused by:
https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=d443ff37091d1eca577f6c51c6115953
run Clippy to get

warning: writing `&String` instead of `&str` involves a new object where a slice will do.
 --> src/main.rs:4:12
  |
4 | fn myfn(s: &Account) -> bool{
  |            ^^^^^^^^ help: change this to: `&str`
  |
  = note: `#[warn(clippy::ptr_arg)]` on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_arg

warning: 1 warning emitted

Where the options are:

  1. Have two type aliases for every String aliased type that I would like to borrow (in this case, type Account=String, perhaps I should also have type Account_s=&str?)
  2. Make Clippy mad and pass function arguments of the form fn(a: &Account)
  3. Clone errystring

Feels like 3 is never the move (pun intended).
2 could be the move, in which case I might want to submit an issue to the Clippy maintainers.
1 could also be the move, but I'm initially grossed out by it. If someferris has a way to make it not feel gross, I'm all crab claws.

1 Like

Clippy isn't wanting you to Clone everything. It's talking about a different issue.

If you take an immutable String (or Vector, or similar owning data structure) by reference in a function signature, then you will only be able to call that function if you have a reference to that exact owning type.

If all you have is some other type that refers to a string, e.g. a &'static str literal, then you will only be able to call your function if you allocate a new String and pass it by reference, since there's no other way to convert a &str to a &String. This is true for accepting &Vec in a function parameter and trying to call it with e.g. an array [T; N].

This means that you should generally prefer string slices and regular slices in function signatures, because the conversion exists the other way around (&String -> &str is trivial and in fact implicit, so is &Vec<T> -> &[T]). Therefore, if you accept a slice or a sling slice, you will be able to pass Vec and String by reference, but you will also be able to pass additional types that deref to slices.

Since in your code, Account is String, hence the complaint from Clippy. Which in turn means that you should probably not use type aliases like type Account = String. You should rather wrap the String in a newtype (struct Account(String);), and if you want to accept arbitrary strings in your functions, still take &str, and impl Deref<Target=str> for Account.

12 Likes

Cool, this is a step beyond my current understanding. I'm gonna try to guess something you're implicitly saying, and you're invited to tell me what I guessed wrong.

I rarely use newtype tuple structs. struct Account(String), strikes me as unergonomic. Eg. If I wanted a trait that String posseses, I often have to manually implement it for Account. Sometimes I can get away with a derive macro. You may know something I don't.

For some context, I've inherited a piece of code from another dev and need to figure out whether to maintain the type String=Account bit. They didn't use Clippy, and passed argument types as &String in many places.

It sounds like @H2CO3 prefers a rewrite as struct Account(String), or type Account=&'static str. Is that correct?

I'm leaning toward the latter, for the reasons I've described above.

The latter of these is almost certainly incorrect. &'static str can only be created by writing a string literal into the source code or leaking memory. Your two choices are:

  • Define an Account structure. It will probably start as a newtype, but might accumulate other fields over time. The primary advantage here is that you can validate the account string on creation, and other code can safely assume that any Account object refers to a valid account. It also provides a place to collect account-specific methods that don’t make sense for general Strings.
  • Change the existing functions to accept &str instead of &Account. This is a minimally-invasive change, but every function needs to handle the possibility that it’s been passed a copy of Anna Karenina instead of an account name.
5 Likes

On the other hand, if you use type Account = String; and then find out you don't want a trait that String has, you're out of luck -- you can't selectively opt out of String's features because Account is String. And if the functionality you want for Account is exactly String's functionality, then why have Account at all when you could just call it String?

If you're writing a library that exposes an Account type, it's often because Account has some invariants or qualities that String lacks, and you don't want to accidentally mix them up. With a type alias, it's hard to guarantee that invariants are preserved because the true type is exposed, and similarly, the compiler won't notice if you pass a String where an Account was required. It's probably better to start with a newtype and add just the functionality that makes sense, rather than start with String and then have to do a major breaking rewrite to restore type safety later on.

Type aliases are mostly used to shorten long (possibly generic) type names, like io::Result<T> is an alias for Result<T, io::Error>.

12 Likes

You don't, if you impl Deref<Target=str> for Account, as I suggested. (The newtype pattern is used pervasively in Rust, so you will have to familiarize yourself with it eventually.)

Not type Account = &'static str;, because that won't work for anything dynamic, and you also loose the distinction between Accounts and strings, which is the very purpose of the newtype pattern.

Amazing answers. Thanks for the knowledge bombs. So a follow up question related to the thread:

Supposing that I opt not to use the newtype pattern, so as to obey minimally invasive change:

  1. I need to be prepared for the "Anna Karenina input moment"
  2. My argument types go from &String to &str.

A natural followup arises: When would I use an argument of type String rather than &str?

The main examples that occur to me:

  • I want to pass around a mutable String. This would be reasonably likely to occur in an internal helper function, and want to do some transformation on that String.
  • I want to pass a string to a helper function that checks the String for some invariant before passing back the string or an Error.

It seems like these claims might also be true:

  • type Blah = String doesn't accomplish the main benefits of the newtype pattern, namely input validation and selective trait implementation.
  • &str is a preferred type for public API arguments, from which a newtype might be created, where the newtype validates the input
  • Strings, being slices of a &str, might most commonly arise as argument types in internal helper functions

When you semantically need to take ownership, i.e. when the function (or the type returned by it) is responsible for dropping the string, and not the caller.

In this case, you might want to have a &mut String argument - note that this cannot be replaced by &mut str, since the latter can't change the string length.

Yes, this is a valid case. However, I'd advise to return not a String itself, but a newtype wrapper around it - so that you can encode the fact that this string passed validation at the type-level.

Not sure why do you think so. In fact, the reverse is true: String is self-contained, but &str is a slice from either String or binary data.

2 Likes

Right. Take ownership. I suppose I'm mentally jumping straight to mutability, when there are other valid reasons to take ownership as well.

Good tip.

For that, you'd use &mut String. You can't use either &String, &str, or a plain by-value String to mutate the argument (you can mutate a String of which you have ownership, but that won't be visible outside the caller.) – so, there's still no good reason to ever accept a &String, unlike &mut String, String, or &str.

String is an owned type, it is not a slice of &str. If you sub-slice a String, you get back an &str, and if you sub-slice an &str, you also get a &str.

Maybe just

type Account = str;
type AccountBuf = String;

?

1 Like