Add new lint, checking for .to_string().parse().unwrap()

[LeoniePhiline]

Inspired by rust-lang/rust#120048 (comment)

• Followed lint naming conventions
• Added passing UI tests (including committed .stderr file)
• cargo test passes locally
• Executed cargo dev update_lints
• Added lint documentation
• Run cargo dev fmt

r? @llogiq

---

changelog: add [parse_to_string] lint

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @llogiq (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

[samueltardieu]

The commit message mentions .unwrap(), but the presence of unwrap() is never checked.

I am a bit torn about this lint, as many uses found on GitHub look legitimate.

[samueltardieu]

You should at least check that the intermediate type is String.

[LeoniePhiline]

Wouldn't it make more sense to check that

• the receiver of to_string isn't &str, since impl ToString for &str + str::parse is not actually suspicious
• the receiver of parse is &str (delegating to FromStr)

[LeoniePhiline]

On second thought, impl ToString for &str + str::parse is indeed suspicious, as the allocation as owned string is unnecessary when parsing only requires a string slice.

Am I missing something in the search result I had deemed not suspicious?

https://github.com/clearloop/leetcode-cli/blob/04a611b7996d8a6d8b622feecb844f248c465bd1/src/helper.rs#L147

It should be using <Captures as Index>::index, which yields a string slice already, making the to_string call suspicious.

[samueltardieu]

Suggested change

	"parsing a the output of `.to_string()`",
	"parsing the output of `.to_string()`",

[LeoniePhiline]

Thanks for your review!

The unwrap is mentioned in the enclosing match arm, which is outside the diff's scope.

I agree about some of the search results being non-suspicious:

E.g. https://github.com/clearloop/leetcode-cli/blob/04a611b7996d8a6d8b622feecb844f248c465bd1/src/helper.rs#L147 uses ToString::to_string on &str, which is specialized and lossless.

I do not think that the output type of the matched to_string call must be checked to be String, as any non-idiomatic to_string implementation returning a different type would be just as suspicious as the entire construct. (This is a topic for a separate lint.)

However, this lint should indeed skip &str receivers.

I'm new to clippy development and it might take a while for me to make time to learn the API well enough to make all required changes. However, I am nevertheless interested in massaging this PR to completion (or rejection).

Edit: reviewing a few more search results, it's tricky. Some seem indeed suspicious, others are a legitimate, e.g. formatting a UUID as string, then parsing it as HTTP header value.

One way I see to make this not have far too many false positives is to check if a fallible or infallible conversion between the input and output types exists - which still doesn't take lossy conversions into account, so it fails at achieving the lint's original goal.

Is this only going to work by explicitly checking for input and output type pairs which are known to convert lossily via string?

[llogiq]

So you would avoid linting cases where the input type implements neither Into<_> nor TryInto<_> to the output type? That might work in reducing false positives a bit. The lint could also return if the enclosing item belongs to any implementation of those traits (or the respective (Try)From ones). However that would likely still leave some cases we may or may not want to lint, especially by default.