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

add new lint [once_cell_lazy] to detect usage of static Lazy type #12968

Closed
wants to merge 1 commit into from

Conversation

J-ZhengLi
Copy link
Member

@J-ZhengLi J-ZhengLi commented Jun 20, 2024

partially fix: #12895

cc: @pitaj

This only checks for once_cell::sync::Lazy, I want to split the implementations to at least 2 prs, as checking lazy_static! might needs EarlyPass to be done properly. Also preventing this PR got too big (it already is...)


changelog: add new lint [once_cell_lazy] to detect usage of static Lazy type.

@rustbot
Copy link
Collaborator

rustbot commented Jun 20, 2024

r? @blyxyas

rustbot has assigned @blyxyas.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jun 20, 2024
@J-ZhengLi J-ZhengLi added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Jun 20, 2024
Copy link
Contributor

@pitaj pitaj left a comment

Choose a reason for hiding this comment

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

Thanks for taking this on!

On my phone, so haven't looked at the implementation much yet. Just some minor changes so far.

@rustbot author

clippy_lints/src/once_cell_lazy.rs Outdated Show resolved Hide resolved
clippy_lints/src/once_cell_lazy.rs Outdated Show resolved Hide resolved
clippy_lints/src/once_cell_lazy.rs Outdated Show resolved Hide resolved
clippy_lints/src/once_cell_lazy.rs Outdated Show resolved Hide resolved
clippy_lints/src/once_cell_lazy.rs Outdated Show resolved Hide resolved
tests/ui/lazy_lock_like/lazy_lock_like_fixable.rs Outdated Show resolved Hide resolved
tests/ui/lazy_lock_like/auxiliary/once_cell.rs Outdated Show resolved Hide resolved
clippy_lints/src/once_cell_lazy.rs Outdated Show resolved Hide resolved
@J-ZhengLi
Copy link
Member Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Jun 21, 2024
@bors
Copy link
Contributor

bors commented Jun 27, 2024

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

@xFrednet
Copy link
Member

Hey @J-ZhengLi, could you rebase on master?

@pitaj
Copy link
Contributor

pitaj commented Jul 27, 2024

Hello @J-ZhengLi,

Thanks again for working on this!

I ran a poll of clippy contributors and the conclusion is that we'd like both lazy_static and once_cell::sync::Lazy handled by a single lint.

I think it's okay if you want to do the implementation in two separate PRs, but please rename the lint you add here to match both cases.

@J-ZhengLi
Copy link
Member Author

I ran a poll of clippy contributors and the conclusion is that we'd like both lazy_static and once_cell::sync::Lazy handled by a single lint.

Awesome~ Thank you for keep tracking this (although I almost forgot about it lol). I'll get back to this in a few days~

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Jul 30, 2024
@J-ZhengLi
Copy link
Member Author

closing this for now, if someone want to continue working on it, that will be wonderful~

(I personally don't have enough time and energy to finish this, but if someone want to pick it up, all that's left to do is adding a check for lazy_static!, which should be easier because it appears that we don't need to offer suggestion to replace it base on this discussion. And ofc, rename the lint~)

@J-ZhengLi J-ZhengLi closed this Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Recommend std::sync::LazyLock over other lazy static options
6 participants