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.

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

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.

clippy_lints/src/needless_option_as_deref.rs Outdated Show resolved Hide resolved
@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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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