-
Notifications
You must be signed in to change notification settings - Fork 1.9k
C#: Exclude Task from cs/local-not-disposed. #18950
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
C#: Exclude Task from cs/local-not-disposed. #18950
Conversation
16391a9 to
13226ed
Compare
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.
PR Overview
This PR updates the local disposal query for C# to no longer flag un-disposed tasks, in line with best practices noted in the referenced blog post.
- Adds System.Threading.Tasks namespace and an example using Task to verify that un-disposed tasks are acceptable in this context.
- Updates change notes to document that un-disposed tasks are now allowed.
Reviewed Changes
| File | Description |
|---|---|
| csharp/ql/test/query-tests/API Abuse/NoDisposeCallOnLocalIDisposable/NoDisposeCallOnLocalIDisposable.cs | Adds a test case demonstrating that Tasks are not flagged for disposal. |
| csharp/ql/src/change-notes/2025-03-10-task-not-disposed.md | Adds change note documentation for the new behavior. |
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused. Learn more
...est/query-tests/API Abuse/NoDisposeCallOnLocalIDisposable/NoDisposeCallOnLocalIDisposable.cs
Outdated
Show resolved
Hide resolved
…osable/NoDisposeCallOnLocalIDisposable.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
LGTM. I think we might want to do the same exclusion for System.Net.Http.HttpClient.
Could you elaborate a bit on why? |
We can discuss this on Slack, but I'm under the impression that Ref: https://learn.microsoft.com/en-us/aspnet/core/fundamentals/http-requests?view=aspnetcore-9.0 |
In most cases, there is no need to dispose tasks (it is not an anti pattern, not to do so).
In this PR we no longer flag undisposed tasks in the
cs/local-not-disposedquery.DCA Looks good.
(cc @rvermeulen)