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 Rules for Lock class usage and to avoid public usage of lock (this) #106907

Closed
elachlan opened this issue Jul 25, 2024 · 7 comments
Closed

New Rules for Lock class usage and to avoid public usage of lock (this) #106907

elachlan opened this issue Jul 25, 2024 · 7 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-Meta code-analyzer Marks an issue that suggests a Roslyn analyzer needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration
Milestone

Comments

@elachlan
Copy link
Contributor

Describe the problem you are trying to solve

In C#13 there is a new system.threading.lock class (dotnet/csharplang#7104). It may be more performant to use over a standard object lock. An analyzer that identifies object properties which are exclusively used for locking and suggests using the new Lock class instead, with a code fix.

Additionally, it would be nice to have a rule that identifies usages of lock (this) inside public classes to discourage the practice.

Additional context

References:

@buyaa-n
Copy link
Member

buyaa-n commented Aug 23, 2024

Moving to runtime as it is a new analyzer proposal

@buyaa-n buyaa-n transferred this issue from dotnet/roslyn-analyzers Aug 23, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Aug 23, 2024
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-meta
See info in area-owners.md if you want to be subscribed.

@buyaa-n buyaa-n added api-suggestion Early API idea and discussion, it is NOT ready for implementation code-analyzer Marks an issue that suggests a Roslyn analyzer labels Aug 23, 2024
@buyaa-n buyaa-n added this to the 10.0.0 milestone Aug 23, 2024
@buyaa-n buyaa-n added needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration and removed untriaged New issue has not been triaged by the area owner labels Aug 23, 2024
@CyrusNajmabadi
Copy link
Member

Roslyn already added an analyzer for this: dotnet/roslyn#74170

Source: me :-)

@elachlan
Copy link
Contributor Author

elachlan commented Sep 1, 2024

@CyrusNajmabadi that just leaves the usage of lock(this) on public apis.

@CyrusNajmabadi
Copy link
Member

That seems entirely different and unrelated to c#13 or the new Lock type.

@buyaa-n buyaa-n closed this as completed Sep 2, 2024
@buyaa-n
Copy link
Member

buyaa-n commented Sep 3, 2024

that just leaves the usage of lock(this) on public apis.

@elachlan closed the issue as it was resolved, feel free to open new issue for this or update the description and reopen this

@elachlan
Copy link
Contributor Author

elachlan commented Sep 3, 2024

@buyaa-n thanks, I have created a new issue at #107269

@github-actions github-actions bot locked and limited conversation to collaborators Oct 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-Meta code-analyzer Marks an issue that suggests a Roslyn analyzer needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration
Projects
None yet
Development

No branches or pull requests

3 participants