Skip to content

New lint: to_string() on a temporary Cow<'_, str> #5987

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

Closed
Veetaha opened this issue Aug 30, 2020 · 3 comments
Closed

New lint: to_string() on a temporary Cow<'_, str> #5987

Veetaha opened this issue Aug 30, 2020 · 3 comments
Labels
A-lint Area: New lints

Comments

@Veetaha
Copy link

Veetaha commented Aug 30, 2020

What it does

The lint is meant to detect calling .to_string() on a temporary Cow<'_, str>.
This is not a rare case when a beginner makes such a mistake.

Categories (optional)

  • Kind: clippy::perf

What is the advantage of the recommended code over the original code:

  • Avoid wasting a potentially owned string inside of the temporary Cow<'_, str>.

Drawbacks

None.

Example

let path_str: String = path.to_string_lossy().to_string();

Could be written as:

let path_str: String = path.to_string_lossy().into_owned();
@Veetaha Veetaha added the A-lint Area: New lints label Aug 30, 2020
@camsteffen
Copy link
Contributor

Why only apply it to a temporary Cow?

Scanning the implementors of ToOwned, you could similarly lint on OsStr::to_os_string and Path::to_path_buf.

Another case is (*cow).to_owned(). But maybe that's too contrived?

@Kixunil
Copy link

Kixunil commented Nov 25, 2020

How about into() or String::from()? One advantage of such conversion is that it keeps working when the type changes a bit. Also useful for Box<str>.

Also why only temporary? I think doing the same when the cow is no longer used even when it's bound should be doable. (I realize this is kinda reimplementing part of borrow checking, but I hope it's simple enough.)

In general any conversion of Cow<'_, Foo> into Foo::Owned when the original value is not used later should be linted.

@Veetaha
Copy link
Author

Veetaha commented Dec 7, 2020

This is a duplicate of #2387
I suggest proceeding with discussions there

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

No branches or pull requests

3 participants