-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Start removing snippet_opt in favor of get_source_text
#13244
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
Conversation
|
r? @Alexendoo rustbot has assigned @Alexendoo. Use |
Remove more `snippet_opt` calls First commit is the same as #13244 changelog: none
clippy_lints/src/from_over_into.rs
Outdated
| let from = from.as_str(); | ||
| let into = into.as_str(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These look like they'd be unneeded
| e.span, | ||
| "if you would like to reborrow, try removing `&*`", | ||
| snippet_opt(cx, deref_target.span).unwrap(), | ||
| deref_text.as_str(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we impl the Into<DiagnosticThingy> to pass this as is? I think this will come up a bunch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
span_suggestion only requires ToString. deref_text is used right afterwards so it doesn't help here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh yeah, was thinking of the wrong arg 🤦♂️
9f020d8 to
ac25730
Compare
ac25730 to
8a4c34a
Compare
|
Thanks! @bors r+ |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
Continuing the job of removing unnecessary allocations.
changelog: none