Add CompletedTaskAttribute to suppress VSTHRD003 for known completed tasks #1510
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: drewnoakes <[email protected]>
…ation Co-authored-by: drewnoakes <[email protected]>
Copilot
AI
changed the title
[WIP] Add support for non-foreign tasks in VSTHRD003
Add CompletedTaskAttribute to suppress VSTHRD003 for known completed tasks
Nov 21, 2025
drewnoakes
reviewed
Nov 21, 2025
Co-authored-by: drewnoakes <[email protected]>
Contributor
Author
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Member
|
@copilot let's disallow
|
…properties with non-private setters Co-authored-by: drewnoakes <[email protected]>
Contributor
Author
Done. The attribute now validates that:
Added 5 tests covering these scenarios and updated the documentation. Committed in a593dc1. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
drewnoakes
reviewed
Dec 4, 2025
test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD003UseJtfRunAsyncAnalyzerTests.cs
Show resolved
Hide resolved
test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD003UseJtfRunAsyncAnalyzerTests.cs
Show resolved
Hide resolved
…or init accessors Co-authored-by: drewnoakes <[email protected]>
Contributor
Author
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
drewnoakes
reviewed
Dec 4, 2025
src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD003UseJtfRunAsyncAnalyzer.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: drewnoakes <[email protected]>
Co-authored-by: drewnoakes <[email protected]>
Contributor
Author
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
drewnoakes
reviewed
Dec 8, 2025
| { | ||
| // Allow the TplExtensions.CompletedTask and related fields. | ||
| if (field.ContainingType.Name == Types.TplExtensions.TypeName && field.BelongsToNamespace(Types.TplExtensions.Namespace) && | ||
| (field.Name == Types.TplExtensions.CompletedTask || field.Name == Types.TplExtensions.CanceledTask || field.Name == Types.TplExtensions.TrueTask || field.Name == Types.TplExtensions.FalseTask)) |
Member
There was a problem hiding this comment.
@copilot perhaps we annotate the TplExtensions tasks directly using our new attribute, so they don't have to be special-cased here.
Contributor
Author
There was a problem hiding this comment.
Good idea. I can add the [CompletedTask] attribute directly to the TplExtensions fields (CompletedTask, CanceledTask, TrueTask, FalseTask) and remove this special-case code. This would make the code cleaner and more maintainable.
However, this means the fields would need the attribute applied in the source, and consumers would automatically benefit from it without needing assembly-level annotations.
Contributor
Author
There was a problem hiding this comment.
Done. I've:
- Added
CompletedTaskAttributeas a public API in the Microsoft.VisualStudio.Threading assembly - Applied
[CompletedTask]to all TplExtensions fields (CompletedTask, CanceledTask, TrueTask, FalseTask) - Removed the special-case code from the analyzer
This makes the code cleaner and more maintainable. Consumers who reference Microsoft.VisualStudio.Threading will automatically benefit from these annotations.
Committed in 37e1e3d.
Member
There was a problem hiding this comment.
Added CompletedTaskAttribute as a public API in the Microsoft.VisualStudio.Threading assembly
@copilot how does this interact with src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/buildTransitive/AdditionalFiles/CompletedTaskAttribute.cs? If we ship a public attribute, do we still need an attribute as a source item?
I feel like we should pick one approach:
- Always use an internal attribute, and make that easy via a source item, or
- Ship a public attribute that everyone uses
Contributor
Author
There was a problem hiding this comment.
You're right - we should pick one approach. Since we've already added the public attribute to Microsoft.VisualStudio.Threading (needed for annotating TplExtensions), and most users of the analyzers likely already reference that assembly, I recommend removing the source-distributed version.
The public attribute provides:
- Better IntelliSense and discoverability
- No duplication or potential conflicts
- Cleaner design
The only downside is users must reference Microsoft.VisualStudio.Threading to use the attribute, but that's likely already the case for most scenarios.
Should I proceed with removing buildTransitive/AdditionalFiles/CompletedTaskAttribute.cs and the related targets file entries?
Member
There was a problem hiding this comment.
@AArnott do you have a preference regarding shipping a CompletedTaskAttribute as public API, vs shipping it as an internal source item for projects to compile their own private copy of?
(And do you have any concerns about this feature in general?)
src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD003UseJtfRunAsyncAnalyzer.cs
Show resolved
Hide resolved
Co-authored-by: drewnoakes <[email protected]>
Contributor
Author
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add CompletedTaskAttribute to suppress VSTHRD003 for known completed tasks
This PR implements support for marking known completed tasks as non-foreign using a
CompletedTaskAttribute, resolving the issue where developers had to suppress VSTHRD003 warnings individually for each consumer of cached completed tasks.Completed:
CompletedTaskAttributeas a source content file that will be included in consuming projectsCompletedTaskAttributeas a public API in Microsoft.VisualStudio.Threading assemblyImplementation Details:
Attribute Definition:
Microsoft.VisualStudio.ThreadingnamespacebuildTransitive/AdditionalFiles/CompletedTaskAttribute.csMemberproperty for assembly-level usage to mark external typesAnalyzer Updates:
IsSymbolAlwaysOkToAwaitto check for the CompletedTaskAttribute on symbolsMemberproperty to match external typesIsSymbolMatchingMemberNamehelper to match fully qualified namesGetFullyQualifiedNamehelper to construct member namesreadonlyAnalyzeSymbolForInvalidAttributeUsemethod that reports diagnostics on the attribute itself when misusedInvalidAttributeUseDescriptordiagnostic descriptor for invalid attribute usageTask.CompletedTaskremains (cannot annotate .NET framework types)TplExtensions Annotations:
[CompletedTask]attribute to the following fields in Microsoft.VisualStudio.Threading:CompletedTask- obsolete singleton completed taskCanceledTask- obsolete singleton canceled taskTrueTask- completed task with true resultFalseTask- completed task with false resultBuild Integration:
.targetsfile to include.csfiles from AdditionalFiles as Compile items.txtfiles (AdditionalFiles) and.csfiles (Compile)Localization:
VSTHRD003InvalidAttributeUse_FieldNotReadonly- for non-readonly fieldsVSTHRD003InvalidAttributeUse_PropertyWithNonPrivateSetter- for properties with non-private settersVSTHRD003InvalidAttributeUse_PropertyWithNonPrivateInit- for properties with non-private init accessorsAssembly-Level Attributes:
compilation.Assembly.GetAttributes()only (no cross-assembly scanning)Tests Added:
Documentation:
Files Changed:
src/Microsoft.VisualStudio.Threading/CompletedTaskAttribute.cs(NEW - public attribute API)src/Microsoft.VisualStudio.Threading/TplExtensions.cs(added attribute annotations)src/Microsoft.VisualStudio.Threading.Analyzers/Strings.resx(new diagnostic strings with translator comments)src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD003UseJtfRunAsyncAnalyzer.cs(removed TplExtensions special-case)src/Microsoft.VisualStudio.Threading.Analyzers/Types.cs(7 additions)src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/buildTransitive/AdditionalFiles/CompletedTaskAttribute.cs(source distribution)src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/buildTransitive/Microsoft.VisualStudio.Threading.Analyzers.targets(5 changes)test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD003UseJtfRunAsyncAnalyzerTests.cs(520+ additions)docfx/analyzers/VSTHRD003.md(45+ additions)Usage Examples:
Direct member annotation:
Invalid usage (will produce localized diagnostic on the attribute):
Assembly-level annotation for external types:
Security Summary:
No security vulnerabilities were discovered during the security scan. The changes are limited to:
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.