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.

Sign up for GitHub

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

Jump to bottom

Correctness: no to_string in impl Display #3876

Closed
Nokel81 opened this issue Mar 12, 2019 · 8 comments · Fixed by #5831
Closed

Correctness: no to_string in impl Display #3876

Nokel81 opened this issue Mar 12, 2019 · 8 comments · Fixed by #5831
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy L-correctness Lint: Belongs in the correctness lint group

Comments

@Nokel81
Copy link

Nokel81 commented Mar 12, 2019

I have recently been implementing some Display traits for my types and accidentally used to_string() of the type being implemented for. This of course resulted in a stack overflow. I think that this is always wrong and should be linted
@flip1995 flip1995 added good-first-issue These issues are a good way to get started with Clippy A-lint Area: New lints L-correctness Lint: Belongs in the correctness lint group labels Mar 13, 2019
@quells
Copy link

quells commented Apr 27, 2019

I would like to try adding this.

@flip1995
Copy link
Member

Great! If you have any questions ask here or open a WIP PR.

@quells
Copy link

quells commented Sep 5, 2019

Sorry, I don’t think I’ll be able to do this.

@flip1995
Copy link
Member

flip1995 commented Sep 5, 2019

No, problem. And thanks for letting us know!

Also a counter example can be found here: #4396. If you violate the inherent_to_string_shadow_display lint, you can use the to_string method in the Display implementation.

@marcin-serwin
Copy link
Contributor

Hi, Could I take a look at this issue?

@flip1995
Copy link
Member

Sure, go ahead!

@marcin-serwin marcin-serwin mentioned this issue May 6, 2020
Closed
@marcin-serwin
Copy link
Contributor

Hey @flip1995,
I'm afraid I won't be able to do it in the near future due to a lack of time. Sorry I didn't respond earlier.

@chansuke
Copy link
Contributor

I'd like to try this issue.

@chansuke chansuke mentioned this issue Jul 22, 2020
Merged
bors added a commit that referenced this issue Aug 16, 2020
@bors
Auto merge of #5831 - chansuke:to_string_in_display, r=flip1995
a2d8331 
Don't use `to_string` in impl Display

fixes #3876

this PR is derived from [Toxyxer's implementation](#5574).
changelog: none
@bors bors closed this as completed in e522ca3 Aug 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy L-correctness Lint: Belongs in the correctness lint group
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Don't use to_string in impl Display chansuke/rust-clippy
No to_string in display
5 participants
@chansuke @quells @Nokel81 @flip1995 @marcin-serwin