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

Recommending .to_owned() instead of .to_string() #5600

Closed
ClementNerma opened this issue May 14, 2020 · 2 comments
Closed

Recommending .to_owned() instead of .to_string() #5600

ClementNerma opened this issue May 14, 2020 · 2 comments

Comments

@ClementNerma
Copy link

ClementNerma commented May 14, 2020

I currently have a place in my code when I accidently converted an &str to a String using the format! macro, which is not the best thing to do. So when I run Clippy it reported this:

warning: useless use of `format!`
   --> mrvm_tools/src/exceptions/native.rs:137:42
    |
137 |             Self::DivisionOrModByZero => format!("<long message>"),
    |                                          ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `"<long message>".to_string()`

The problem here is the recommandation of .to_string(). As you can find in many explanations on vairous forums, .to_owned() is preferred to .to_string() as the latter may result in additional allocations to perform the conversion, while .to_owned() directly converts the value to a String.

So I think the lint should display this instead:

warning: useless use of `format!`
   --> mrvm_tools/src/exceptions/native.rs:137:42
    |
137 |             Self::DivisionOrModByZero => format!("<long message>"),
    |                                          ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.to_owned()`: `"<long message>".to_owned()`

If I miss a reason that makes Clippy recommend .to_string() over .to_owned() please let me know, I might be mistaken here but I think it would be better to recommend .to_owned().

@ClementNerma ClementNerma changed the title Recommanding .to_owned() instead of .to_string() Recommending .to_owned() instead of .to_string() May 14, 2020
@ebroto
Copy link
Member

ebroto commented May 14, 2020

If I'm not mistaken that used to be the case, but now it's equally performant and the "officially recommended" way (in the book) to convert to String.

See this thread comment, PR and the book

@ClementNerma
Copy link
Author

Alright it seems I indeed overlooked that.
Thanks for your comment :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants