Skip to content

Unnecessary call to to_string when the underlying type implements AsRef<str> #7933

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
smoelius opened this issue Nov 4, 2021 · 2 comments · Fixed by #7978
Closed

Unnecessary call to to_string when the underlying type implements AsRef<str> #7933

smoelius opened this issue Nov 4, 2021 · 2 comments · Fixed by #7978
Labels
A-lint Area: New lints

Comments

@smoelius
Copy link
Contributor

smoelius commented Nov 4, 2021

What it does

Checks for expressions of the form &X.to_string(), used in a position requiring a &str, where X implements AsRef<str>.

In such a case, the call to to_string is unnecessary.

Similar to to_string/AsRef<str>, there's also to_path_buf/AsRef<Path> and to_vec/AsRef<[T]>.

There are probably others.

I am happy to tackle this if we can agree on the check, and whether it is useful.

Categories (optional)

  • Kind: perf

Drawbacks

Potential "false positives" if X's AsRef<str> and to_string implementations produce different strings. But I would expect such cases to be rare.

Example

use std::{fmt::Write, io::Write as OtherWrite};

fn main() {
    let mut s = String::new();
    let dir = std::env::current_dir().unwrap();
    s.write_str(&dir.to_string_lossy().to_string()).unwrap();
    //                                ^^^^^^^^^^^^ unnecessary
    s.write_str(" is the current directory.\n").unwrap();
    std::io::stdout().write(s.as_bytes()).unwrap();
}
@smoelius smoelius added the A-lint Area: New lints label Nov 4, 2021
@skius
Copy link

skius commented Nov 7, 2021

Hi, I believe your example works because .to_string_lossy() returns a Deref<Target = str>, not because it returns an AsRef<str>. If that's not a lint already, I agree that it would be a good addition!

@smoelius
Copy link
Contributor Author

smoelius commented Nov 7, 2021

Thanks, @skius. I think you're right.

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

Successfully merging a pull request may close this issue.

2 participants