Consider reinstating str_to_string and string_to_string

[ndmitchell]

The rules str_to_string and string_to_string have both been removed, so things like "foo".to_string() can no longer be warned for. I find those warnings very valuable, and have been "manually" linting them in code reviews. In particular, it's useful to distinguish between converting something to a string (relatively rare. deserves special attention in code review) and managing memory so it has the right form (very common, can mostly be glossed over). I think it's reasonable that these hints are off-by-default, and in the pedantic category, but they are certainly something I'd turn on.

FWIW, I went through clippy to try and implement the above hints, while is when I found they had previously been implemented and removed.

[flip1995]

The intentions of those lints were, that to_owned/clone should be used, since they are more efficient. This isn't the case anymore and which function to use doesn't matter.

redundant_clone exists to detect unnecessary clones of strings besides others. Do you have an example in mind, where to_string is not necessary?

[ndmitchell]

There's no performance benefit. But to_string does a multitude of things - I don't know if I'm converting an isize or cloning a string. In contrast, to_owned/clone do relatively specific things. Therefore, for human readability, I often ask in code reviews for foo.to_string() to be replaced with foo.clone() if that's the underlying effect of the operation. It's never not necessary, just easier to follow if you use a function that only does a small subset of what to_string does.

[flip1995]

Fair point. I think we can re-add these as restriction lints, we didn't had this group back in the day.

[PunitLodha]

I would like to work on this. From what I can gather from the conversation, I have to add the two lints to the restriction lints. I found the PR #795 which deprecated the lints, but i can't find the lint files itself for both the lints. I also tried looking through git history for any file named str_to_string, but no success. Any way i can find those files, so that i can add them to the restriction lints?

[flip1995]

You won't have much luck finding those files, since the repo was at least restructured 2 times since then. You will have to reimplement those lints.

[PunitLodha]

I think i am missing something but I can't find any variant for String in ty::TyKind. There's a Str variant for string slice, but no variant for the String type. So is there any way to know if method is being called on a String?

[flip1995]

See

rust-clippy/clippy_lints/src/methods/mod.rs

Line 1903 in 6b01c39

if is_type_diagnostic_item(cx, arg_ty, sym!(string_type)) {

[julianskartor]

There's no performance benefit. But to_string does a multitude of things - I don't know if I'm converting an isize or cloning a string. In contrast, to_owned/clone do relatively specific things. Therefore, for human readability, I often ask in code reviews for foo.to_string() to be replaced with foo.clone() if that's the underlying effect of the operation. It's never not necessary, just easier to follow if you use a function that only does a small subset of what to_string does.

Interesting. I actually thought about this the other way around. i.e. to_string tells me clearly I receive a String but I have no such information around clone.

[ndmitchell]

to_string tells me clearly I receive a String but I have no such information around clone.

Generally I use the inlay type hints in Rust Analyzer to figure that out. You can think of clone as being a set of very narrow pipes (one for each type), whereas to_string is a big funnel (all types in, always String out). Anything that acts like a funnel tends to allow more programs to type check than you might like, whereas separate pipes keep things separate and don't have that issue.