Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

new lint for &str to String conversion #2824

Open
mockersf opened this issue May 30, 2018 · 7 comments
Open

new lint for &str to String conversion #2824

mockersf opened this issue May 30, 2018 · 7 comments
Labels
A-lint Area: New lints L-style Lint: Belongs in the style lint group

Comments

@mockersf
Copy link
Contributor

There are a few different ways to convert from a &str to a String:

fn main() {
    let _: String = "hello".to_owned();
    let _: String = "hello".to_string();
    let _: String = "hello".into();
    let _: String = String::from("hello");
    let _: String = format!("hello");
}

Without going into a discussion about which is the best (this lint has been deprecated), I think a lint to ensure the same one is used throughout a crate would be interesting.

This could be done either through configuration, or by keeping stats on which is used and then emit warning for the non majority one.

I can work on it, but wanted to have a general point of view before. What do you think ?

@oli-obk
Copy link
Contributor

oli-obk commented May 31, 2018

I think we can rule out the format! variant, it's very expensive. And we can rule out into as that only works if the target type is specified (you can't call "hello".into().len()).

So we're left with

  • "hello".to_owned()
  • "hello".to_string()
  • String::from("hello")

Statistics on the crate seem like a good idea, but might end up being expensive (you need to first go through the entire crate looking at all method/function calls, collect the stats, and then run the lint normally). So we'll need to benchmark some large crates and see how expensive it is in reality.

@clarfonthey
Copy link
Contributor

Honestly, I'd rule out String::from for literals, as the method style is more idiomatic.

@oli-obk
Copy link
Contributor

oli-obk commented May 31, 2018

I remember @steveklabnik having opinions on this

@steveklabnik
Copy link
Member

The official style is to use .to_string() everywhere. There's also a pretty significant minority style of "String::from for literals, .to_string() for variables." IMHO, to_owned is for generic code only; you should say directly what you mean: I want a String.

(format! is, IIRC, significantly slower; that's why people started using .to_owned, because it skipped the formatting machinery, whereas .to_string did not. Now they're equivalent.)

@clarfonthey
Copy link
Contributor

to_string is also consistent with to_vec

@phansch phansch added A-lint Area: New lints L-style Lint: Belongs in the style lint group labels Jun 11, 2018
@spunit262
Copy link

I have to disagree. "abc".to_string() fails to describe what it's doing, it only says it's making a string form a string. "abc".to_owned() however highlight the important part of what it's doing, creating an owned string.

It's not much of a difference, but to_owned is slightly more newcomer friendly than to_string and as such should generally be preferred.

@Kixunil
Copy link

Kixunil commented Oct 31, 2021

@spunit262 exactly, to_string() is meant to display stuff. When newbie sees:

lex foo = "foo".to_string();

"WTF, isn't it a string already?"

It's actually the other way around: use to_string() in generic code if you need to display stuff.

@clarfonthey I think to_owned() should be used for slices instead of to_vec as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints L-style Lint: Belongs in the style lint group
Projects
None yet
Development

No branches or pull requests

7 participants