Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

New lint: option_needless_deref #7596

Merged
merged 2 commits into from
Sep 5, 2021

Conversation

lengyijun
Copy link
Contributor

changelog: [option_needless_deref]
fix #7571

@rust-highfive
Copy link

r? @llogiq

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Aug 24, 2021
@lengyijun lengyijun force-pushed the option_needless_deref branch 2 times, most recently from d835769 to 86e3aa0 Compare August 24, 2021 23:04
@bors
Copy link
Contributor

bors commented Sep 2, 2021

☔ The latest upstream changes (presumably #7604) made this pull request unmergeable. Please resolve the merge conflicts.

@lengyijun lengyijun force-pushed the option_needless_deref branch from 86e3aa0 to 38b8697 Compare September 3, 2021 01:13
@camsteffen
Copy link
Contributor

I think the lint name should include as_deref. needless_as_deref would be good IMO.

@lengyijun
Copy link
Contributor Author

option_needless_as_deref or needless_as_deref ?

@camsteffen
Copy link
Contributor

needless_option_as_deref if anything. But I'd pick needless_as_deref.

@lengyijun lengyijun force-pushed the option_needless_deref branch 3 times, most recently from e1fdc67 to 4184cc3 Compare September 4, 2021 06:18
@lengyijun
Copy link
Contributor Author

I did some fix.

Result also has a as_deref method, but this lint doesn't work on it.
So I temporally rename to needless_option_as_deref.

Copy link
Contributor

@llogiq llogiq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only have a small nit, otherwise this looks good.

Co-authored-by: llogiq <bogusandre@gmail.com>
@llogiq
Copy link
Contributor

llogiq commented Sep 5, 2021

Thank you! @bors r+

@bors
Copy link
Contributor

bors commented Sep 5, 2021

📌 Commit 37af742 has been approved by llogiq

@bors
Copy link
Contributor

bors commented Sep 5, 2021

⌛ Testing commit 37af742 with merge 7a0d7d8...

@bors
Copy link
Contributor

bors commented Sep 5, 2021

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: llogiq
Pushing 7a0d7d8 to master...

@bors bors merged commit 7a0d7d8 into rust-lang:master Sep 5, 2021
@lengyijun lengyijun deleted the option_needless_deref branch October 27, 2023 10:41
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add lint checking for no-op uses of Option::{as_deref,as_deref_mut}
5 participants