From 868498b3d5c30a731d71421167edd5bbe09ecdfb Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Fri, 21 Aug 2026 10:17:41 -0600 Subject: [PATCH 1/7] Fix VSTHRD003 task origin edge cases Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docfx/analyzers/VSTHRD003.md | 30 +++ docfx/docs/threading_rules.md | 13 +- .../VSTHRD003UseJtfRunAsyncAnalyzer.cs | 49 +++- .../Types.cs | 16 ++ .../VSTHRD003UseJtfRunAsyncAnalyzerTests.cs | 213 ++++++++++++++++++ 5 files changed, 315 insertions(+), 6 deletions(-) diff --git a/docfx/analyzers/VSTHRD003.md b/docfx/analyzers/VSTHRD003.md index 49d8ad87b..c60528c64 100644 --- a/docfx/analyzers/VSTHRD003.md +++ b/docfx/analyzers/VSTHRD003.md @@ -10,6 +10,36 @@ When required to await a task that was started earlier, start it within a delega `JoinableTaskFactory.RunAsync`, storing the resulting `JoinableTask` in a field or variable. You can safely await the `JoinableTask` later. +## Marking known-completed tasks + +Awaiting a task that is known to have already completed cannot deadlock. To identify cached +completed tasks that the analyzer cannot recognize automatically, define an attribute with the +fully qualified name `Microsoft.VisualStudio.Threading.CompletedTaskAttribute` in your project: + +```csharp +namespace Microsoft.VisualStudio.Threading +{ + [AttributeUsage(AttributeTargets.Field | AttributeTargets.Property | AttributeTargets.Method)] + internal sealed class CompletedTaskAttribute : Attribute + { + } +} +``` + +Apply `[CompletedTask]` to methods that always return completed tasks, `readonly` fields, or +get-only properties: + +```csharp +[CompletedTask] +private static readonly Task TrueTask = Task.FromResult(true); + +[CompletedTask] +private static Task FalseTask { get; } = Task.FromResult(false); +``` + +The analyzer intentionally does not recognize the attribute on mutable fields or settable +properties, because their values can later be replaced with incomplete tasks. + ## Simple examples of patterns that are flagged by this analyzer The following example would likely deadlock if `MyMethod` were called on the main thread, diff --git a/docfx/docs/threading_rules.md b/docfx/docs/threading_rules.md index cf6115457..8eaca81d3 100644 --- a/docfx/docs/threading_rules.md +++ b/docfx/docs/threading_rules.md @@ -59,18 +59,25 @@ JoinableTask longRunningAsyncWork = joinableTaskFactoryInstance.RunAsync( }); ``` -then later that async work becomes blocking: +then later asynchronous code can join that work while waiting for it: ```csharp -longRunningAsyncWork.Join(); +await longRunningAsyncWork.JoinAsync(cancellationToken); ``` -or perhaps +When cancellation is not required, directly awaiting the `JoinableTask` is equivalent to +calling `JoinAsync(CancellationToken.None)`: ```csharp await longRunningAsyncWork; ``` +Synchronous code can join and block on the work: + +```csharp +longRunningAsyncWork.Join(); +``` + Note however that this extra step is not necessary when awaiting is done immediately after kicking off an asynchronous operation. diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD003UseJtfRunAsyncAnalyzer.cs b/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD003UseJtfRunAsyncAnalyzer.cs index 19864c651..7c5ce8c68 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD003UseJtfRunAsyncAnalyzer.cs +++ b/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD003UseJtfRunAsyncAnalyzer.cs @@ -73,6 +73,19 @@ public override void Initialize(AnalysisContext context) private static bool IsSymbolAlwaysOkToAwait(ISymbol? symbol) { + if (symbol?.GetAttributes().Any(attribute => + attribute.AttributeClass?.Name == Types.CompletedTaskAttribute.TypeName && + attribute.AttributeClass.ContainingType is null && + attribute.AttributeClass.BelongsToNamespace(Types.CompletedTaskAttribute.Namespace)) is true) + { + // Mutable members may only happen to contain a completed task at the time the attribute is applied. + // Restrict the convention to members whose value cannot be replaced later. + if (symbol is IMethodSymbol or IFieldSymbol { IsReadOnly: true } or IPropertySymbol { SetMethod: null, ReturnsByRef: false }) + { + return true; + } + } + if (symbol is IFieldSymbol field) { // Allow the TplExtensions.CompletedTask and related fields. @@ -98,8 +111,13 @@ private static bool IsSymbolAlwaysOkToAwait(ISymbol? symbol) private void AnalyzeArrowExpressionClause(SyntaxNodeAnalysisContext context) { var arrowExpressionClause = (ArrowExpressionClauseSyntax)context.Node; - if (arrowExpressionClause.Parent is MethodDeclarationSyntax) + if (arrowExpressionClause.Parent is MethodDeclarationSyntax methodDeclaration) { + if (IsSymbolAlwaysOkToAwait(context.SemanticModel.GetDeclaredSymbol(methodDeclaration, context.CancellationToken))) + { + return; + } + Diagnostic? diagnostic = this.AnalyzeAwaitedOrReturnedExpression(arrowExpressionClause.Expression, context, context.CancellationToken); if (diagnostic is object) { @@ -124,6 +142,20 @@ private void AnalyzeLambdaExpression(SyntaxNodeAnalysisContext context) private void AnalyzeReturnStatement(SyntaxNodeAnalysisContext context) { var returnStatement = (ReturnStatementSyntax)context.Node; + CSharpUtils.ContainingFunctionData containingFunction = CSharpUtils.GetContainingFunction(returnStatement); + ISymbol? containingSymbol = containingFunction.Function switch + { + MethodDeclarationSyntax methodDeclaration => context.SemanticModel.GetDeclaredSymbol(methodDeclaration, context.CancellationToken), + LocalFunctionStatementSyntax localFunction => context.SemanticModel.GetDeclaredSymbol(localFunction, context.CancellationToken), + AccessorDeclarationSyntax { Parent.Parent: PropertyDeclarationSyntax propertyDeclaration } => context.SemanticModel.GetDeclaredSymbol(propertyDeclaration, context.CancellationToken), + AccessorDeclarationSyntax { Parent.Parent: IndexerDeclarationSyntax indexerDeclaration } => context.SemanticModel.GetDeclaredSymbol(indexerDeclaration, context.CancellationToken), + _ => null, + }; + if (IsSymbolAlwaysOkToAwait(containingSymbol)) + { + return; + } + Diagnostic? diagnostic = this.AnalyzeAwaitedOrReturnedExpression(returnStatement.Expression, context, context.CancellationToken); if (diagnostic is object) { @@ -228,7 +260,7 @@ private void AnalyzeAwaitExpression(SyntaxNodeAnalysisContext context) symbolType = parameterSymbol.Type; dataflowAnalysisCompatibleVariable = true; break; - case IFieldSymbol fieldSymbol: + case IFieldSymbol fieldSymbol when !IsSymbolAlwaysOkToAwait(fieldSymbol): symbolType = fieldSymbol.Type; // If the field is readonly and initialized with Task.FromResult, it's OK. @@ -287,7 +319,7 @@ private void AnalyzeAwaitExpression(SyntaxNodeAnalysisContext context) } break; - case IMethodSymbol methodSymbol: + case IMethodSymbol methodSymbol when !IsSymbolAlwaysOkToAwait(methodSymbol): if (Utils.IsTask(methodSymbol.ReturnType) && focusedExpression is InvocationExpressionSyntax invocationExpressionSyntax) { // Consider all arguments @@ -305,6 +337,8 @@ private void AnalyzeAwaitExpression(SyntaxNodeAnalysisContext context) return expressionsToConsider.Select(e => this.AnalyzeAwaitedOrReturnedExpression(e, context, cancellationToken)).FirstOrDefault(r => r is object); } + return null; + case IFieldSymbol or IMethodSymbol: return null; default: return null; @@ -317,6 +351,15 @@ private void AnalyzeAwaitExpression(SyntaxNodeAnalysisContext context) // Report warning if the task was not initialized within the current delegate or lambda expression containingFunc ??= CSharpUtils.GetContainingFunction(focusedExpression); + if (containingFunc.Value.BlockOrExpression is null && + symbolToConsider.Symbol is ILocalSymbol && + focusedExpression.Ancestors().Any(ancestor => ancestor is GlobalStatementSyntax)) + { + // Top-level locals belong to the compiler-generated Main method, even though there is no + // method declaration in syntax for GetContainingFunction to discover. + return null; + } + if (containingFunc.Value.BlockOrExpression is BlockSyntax delegateBlock) { if (dataflowAnalysisCompatibleVariable) diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers/Types.cs b/src/Microsoft.VisualStudio.Threading.Analyzers/Types.cs index f6d24b8db..44c87baaf 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers/Types.cs +++ b/src/Microsoft.VisualStudio.Threading.Analyzers/Types.cs @@ -96,6 +96,22 @@ public static class AsyncMethodBuilderAttribute public static readonly ImmutableArray Namespace = Namespaces.SystemRuntimeCompilerServices; } + /// + /// Contains descriptors for the convention-based CompletedTaskAttribute type. + /// + public static class CompletedTaskAttribute + { + /// + /// The name of the attribute type. + /// + public const string TypeName = nameof(CompletedTaskAttribute); + + /// + /// The namespace containing the attribute type. + /// + public static readonly ImmutableArray Namespace = Namespaces.MicrosoftVisualStudioThreading; + } + /// /// Contains descriptors for the JoinableTaskFactory type. /// diff --git a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD003UseJtfRunAsyncAnalyzerTests.cs b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD003UseJtfRunAsyncAnalyzerTests.cs index ce64483d5..16798878e 100644 --- a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD003UseJtfRunAsyncAnalyzerTests.cs +++ b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD003UseJtfRunAsyncAnalyzerTests.cs @@ -115,6 +115,29 @@ public static T WaitAndGetResult(Task task) await CSVerify.VerifyAnalyzerAsync(test, expected); } + [Fact] + public async Task ReportWarningWhenTaskFieldAssignedOutsideExpressionLambdaIsReturnedFromJtfRun() + { + string test = """ + using System.Threading.Tasks; + using Microsoft.VisualStudio.Threading; + + class Tests + { + private Task task; + private JoinableTaskFactory jtf; + + public void Test() + { + this.task = this.jtf.RunAsync(async () => await Task.Yield()).Task; + this.jtf.Run(() => [|this.task|]); + } + } + """; + + await CSVerify.VerifyAnalyzerAsync(test); + } + [Fact] public async Task ReportWarningWhenTaskIsReturnedDirectlyFromMethod() { @@ -1058,6 +1081,146 @@ public Task GetTask(int i) await CSVerify.VerifyAnalyzerAsync(test); } + [Fact] + public async Task DoNotReportWarningForMembersMarkedAsCompletedTasks() + { + string test = """ + using System; + using System.Threading.Tasks; + using Microsoft.VisualStudio.Threading; + + namespace Microsoft.VisualStudio.Threading + { + [AttributeUsage(AttributeTargets.Field | AttributeTargets.Property | AttributeTargets.Method)] + internal sealed class CompletedTaskAttribute : Attribute + { + } + } + + class Tests + { + private static Task task; + + [CompletedTask] + private static readonly Task CompletedField = Task.Delay(1); + + [CompletedTask] + private static Task CompletedProperty { get; } = Task.Delay(1); + + [CompletedTask] + private static Task CompletedBlockProperty + { + get + { + return task; + } + } + + [CompletedTask] + private static Task ReturnCompletedTask(Task task) + { + return task; + } + + [CompletedTask] + private static Task ReturnCompletedTaskExpression(Task task) => task; + + public Task GetField() => CompletedField; + + public Task GetProperty() => CompletedProperty; + + public Task GetBlockProperty() => CompletedBlockProperty; + + public Task GetMethodResult(Task task) => ReturnCompletedTask(task); + + public Task GetExpressionMethodResult(Task task) => ReturnCompletedTaskExpression(task); + + public Task GetLocalFunctionResult(Task task) + { + [CompletedTask] + static Task ReturnCompletedTaskLocal(Task task) + { + return task; + } + + return ReturnCompletedTaskLocal(task); + } + } + """; + + await CSVerify.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task ReportWarningForMutableMembersMarkedAsCompletedTasks() + { + string test = """ + using System; + using System.Threading.Tasks; + using Microsoft.VisualStudio.Threading; + + namespace Microsoft.VisualStudio.Threading + { + [AttributeUsage(AttributeTargets.Field | AttributeTargets.Property | AttributeTargets.Method)] + internal sealed class CompletedTaskAttribute : Attribute + { + } + } + + class Tests + { + [CompletedTask] + private static Task CompletedField = Task.Delay(1); + + [CompletedTask] + private static Task CompletedProperty { get; set; } = Task.Delay(1); + + private static Task task = Task.Delay(1); + + [CompletedTask] + private static ref Task CompletedRefProperty => ref task; + + public Task GetField() => [|CompletedField|]; + + public Task GetProperty() => [|CompletedProperty|]; + + public Task GetRefProperty() => [|CompletedRefProperty|]; + } + """; + + await CSVerify.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task ReportWarningForNestedCompletedTaskAttributeLookalike() + { + string test = """ + using System; + using System.Threading.Tasks; + + namespace Microsoft.VisualStudio.Threading + { + internal static class Container + { + [AttributeUsage(AttributeTargets.Field)] + internal sealed class CompletedTaskAttribute : Attribute + { + } + } + } + + class Tests + { + [Microsoft.VisualStudio.Threading.Container.CompletedTask] + private static readonly Task CompletedField = Task.Delay(1); + + public Task GetField() => [|CompletedField|]; + } + """; + + await CSVerify.VerifyAnalyzerAsync(test); + } + [Fact] public async Task DoNotReportWarningWhenTaskFromResultIsReturnedDirectlyFromMethod() { @@ -1453,6 +1616,56 @@ static async Task ListenAndWait() await CSVerify.VerifyAnalyzerAsync(test); } + [Fact] + public async Task DoNotReportWarningForTopLevelLocals() + { + string test = """ + using System; + using System.Threading; + using System.Threading.Tasks; + + using var cts = new CancellationTokenSource(); + Task loopTask = Task.Run(() => Console.WriteLine("loop"), cts.Token); + Task serverTask = Task.Run(() => Console.WriteLine("server"), cts.Token); + Task exitTask = Task.Run(() => cts.Cancel(), cts.Token); + + await Task.WhenAny(loopTask, serverTask, exitTask); + """; + + await new CSVerify.Test + { + TestState = + { + Sources = { test }, + OutputKind = OutputKind.ConsoleApplication, + }, + }.RunAsync(); + } + + [Fact] + public async Task ReportWarningForForeignFieldInTopLevelStatements() + { + string test = """ + using System.Threading.Tasks; + + await [|State.Task|]; + + static class State + { + internal static Task Task = System.Threading.Tasks.Task.Delay(1); + } + """; + + await new CSVerify.Test + { + TestState = + { + Sources = { test }, + OutputKind = OutputKind.ConsoleApplication, + }, + }.RunAsync(); + } + private DiagnosticResult CreateDiagnostic(int line, int column, int length) => CSVerify.Diagnostic().WithSpan(line, column, line, column + length); } From 65602eab9246e06892d0231d666b5c109e65994b Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Fri, 21 Aug 2026 10:40:13 -0600 Subject: [PATCH 2/7] Diagnose invalid completed task annotations Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docfx/analyzers/VSTHRD003.md | 6 +- .../VSTHRD003UseJtfRunAsyncAnalyzer.cs | 57 +++++++++++--- .../Strings.resx | 8 ++ .../Helpers/CSharpCodeFixVerifier`2+Test.cs | 2 + .../VSTHRD003UseJtfRunAsyncAnalyzerTests.cs | 78 ++++++++++--------- 5 files changed, 103 insertions(+), 48 deletions(-) diff --git a/docfx/analyzers/VSTHRD003.md b/docfx/analyzers/VSTHRD003.md index c60528c64..3a056fc77 100644 --- a/docfx/analyzers/VSTHRD003.md +++ b/docfx/analyzers/VSTHRD003.md @@ -37,8 +37,10 @@ private static readonly Task TrueTask = Task.FromResult(true); private static Task FalseTask { get; } = Task.FromResult(false); ``` -The analyzer intentionally does not recognize the attribute on mutable fields or settable -properties, because their values can later be replaced with incomplete tasks. +The analyzer reports VSTHRD003 on `[CompletedTask]` when it is applied to a mutable field, +settable property, or ref-returning property, because its value can later be replaced with an +incomplete task. Consumers still take the attribute at face value, so the diagnostic is reported +at the invalid attribute application rather than at each use of the member. ## Simple examples of patterns that are flagged by this analyzer diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD003UseJtfRunAsyncAnalyzer.cs b/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD003UseJtfRunAsyncAnalyzer.cs index 7c5ce8c68..357c2861f 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD003UseJtfRunAsyncAnalyzer.cs +++ b/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD003UseJtfRunAsyncAnalyzer.cs @@ -49,12 +49,21 @@ public class VSTHRD003UseJtfRunAsyncAnalyzer : DiagnosticAnalyzer defaultSeverity: DiagnosticSeverity.Warning, isEnabledByDefault: true); + internal static readonly DiagnosticDescriptor InvalidCompletedTaskAttributeDescriptor = new DiagnosticDescriptor( + id: Id, + title: new LocalizableResourceString(nameof(Strings.VSTHRD003_InvalidCompletedTaskAttribute_Title), Strings.ResourceManager, typeof(Strings)), + messageFormat: new LocalizableResourceString(nameof(Strings.VSTHRD003_InvalidCompletedTaskAttribute_MessageFormat), Strings.ResourceManager, typeof(Strings)), + helpLinkUri: Utils.GetHelpLink(Id), + category: "Usage", + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true); + /// public override ImmutableArray SupportedDiagnostics { get { - return ImmutableArray.Create(Descriptor); + return ImmutableArray.Create(Descriptor, InvalidCompletedTaskAttributeDescriptor); } } @@ -69,21 +78,16 @@ public override void Initialize(AnalysisContext context) context.RegisterSyntaxNodeAction(Utils.DebuggableWrapper(this.AnalyzeArrowExpressionClause), SyntaxKind.ArrowExpressionClause); context.RegisterSyntaxNodeAction(Utils.DebuggableWrapper(this.AnalyzeLambdaExpression), SyntaxKind.SimpleLambdaExpression); context.RegisterSyntaxNodeAction(Utils.DebuggableWrapper(this.AnalyzeLambdaExpression), SyntaxKind.ParenthesizedLambdaExpression); + context.RegisterSyntaxNodeAction(Utils.DebuggableWrapper(this.AnalyzeCompletedTaskAttribute), SyntaxKind.Attribute); } private static bool IsSymbolAlwaysOkToAwait(ISymbol? symbol) { - if (symbol?.GetAttributes().Any(attribute => - attribute.AttributeClass?.Name == Types.CompletedTaskAttribute.TypeName && - attribute.AttributeClass.ContainingType is null && - attribute.AttributeClass.BelongsToNamespace(Types.CompletedTaskAttribute.Namespace)) is true) + if (symbol is IMethodSymbol or IFieldSymbol or IPropertySymbol && + symbol.GetAttributes().Any(attribute => IsCompletedTaskAttribute(attribute.AttributeClass))) { - // Mutable members may only happen to contain a completed task at the time the attribute is applied. - // Restrict the convention to members whose value cannot be replaced later. - if (symbol is IMethodSymbol or IFieldSymbol { IsReadOnly: true } or IPropertySymbol { SetMethod: null, ReturnsByRef: false }) - { - return true; - } + // Consumers take this assertion at face value. Invalid applications are diagnosed at the declaration. + return true; } if (symbol is IFieldSymbol field) @@ -108,6 +112,37 @@ attribute.AttributeClass.ContainingType is null && return false; } + private static bool IsCompletedTaskAttribute(INamedTypeSymbol? attributeType) => + attributeType?.Name == Types.CompletedTaskAttribute.TypeName && + attributeType.ContainingType is null && + attributeType.BelongsToNamespace(Types.CompletedTaskAttribute.Namespace); + + private void AnalyzeCompletedTaskAttribute(SyntaxNodeAnalysisContext context) + { + var attribute = (AttributeSyntax)context.Node; + if (context.SemanticModel.GetSymbolInfo(attribute, context.CancellationToken).Symbol is not IMethodSymbol attributeConstructor || + !IsCompletedTaskAttribute(attributeConstructor.ContainingType)) + { + return; + } + + ISymbol? mutableMember = attribute.Parent?.Parent switch + { + FieldDeclarationSyntax field when !field.Modifiers.Any(SyntaxKind.ReadOnlyKeyword) => + context.SemanticModel.GetDeclaredSymbol(field.Declaration.Variables[0], context.CancellationToken), + PropertyDeclarationSyntax property when context.SemanticModel.GetDeclaredSymbol(property, context.CancellationToken) is IPropertySymbol { SetMethod: not null } or { ReturnsByRef: true } => + context.SemanticModel.GetDeclaredSymbol(property, context.CancellationToken), + IndexerDeclarationSyntax indexer when context.SemanticModel.GetDeclaredSymbol(indexer, context.CancellationToken) is IPropertySymbol { SetMethod: not null } or { ReturnsByRef: true } => + context.SemanticModel.GetDeclaredSymbol(indexer, context.CancellationToken), + _ => null, + }; + + if (mutableMember is not null) + { + context.ReportDiagnostic(Diagnostic.Create(InvalidCompletedTaskAttributeDescriptor, attribute.GetLocation(), mutableMember.Name)); + } + } + private void AnalyzeArrowExpressionClause(SyntaxNodeAnalysisContext context) { var arrowExpressionClause = (ArrowExpressionClauseSyntax)context.Node; diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers/Strings.resx b/src/Microsoft.VisualStudio.Threading.Analyzers/Strings.resx index 97e248e4d..5ba34be0c 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers/Strings.resx +++ b/src/Microsoft.VisualStudio.Threading.Analyzers/Strings.resx @@ -172,6 +172,14 @@ Start the work within this context, or use JoinableTaskFactory.RunAsync to start Avoid awaiting foreign Tasks + + CompletedTaskAttribute cannot be applied to mutable member "{0}". Apply it only to methods, readonly fields, or non-ref get-only properties. + CompletedTaskAttribute is a type name and should not be translated. {0} is the name of a field, property, or indexer. + + + Apply CompletedTaskAttribute only to immutable members + CompletedTaskAttribute is a type name and should not be translated. + Invoking or blocking on async code in a Lazy<T> value factory can deadlock. Use AsyncLazy<T> instead. diff --git a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/Helpers/CSharpCodeFixVerifier`2+Test.cs b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/Helpers/CSharpCodeFixVerifier`2+Test.cs index b767fa30e..32212077e 100644 --- a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/Helpers/CSharpCodeFixVerifier`2+Test.cs +++ b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/Helpers/CSharpCodeFixVerifier`2+Test.cs @@ -10,6 +10,7 @@ #endif using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Testing; +using Microsoft.CodeAnalysis.Testing; using Microsoft.CodeAnalysis.Text; using IOleServiceProvider = Microsoft.VisualStudio.OLE.Interop.IServiceProvider; @@ -22,6 +23,7 @@ public class Test : CSharpCodeFixTest public Test() { this.ReferenceAssemblies = ReferencesHelper.DefaultReferences; + this.MarkupOptions = MarkupOptions.UseFirstDescriptor; this.SolutionTransforms.Add((solution, projectId) => { diff --git a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD003UseJtfRunAsyncAnalyzerTests.cs b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD003UseJtfRunAsyncAnalyzerTests.cs index 16798878e..2b0ab0393 100644 --- a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD003UseJtfRunAsyncAnalyzerTests.cs +++ b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD003UseJtfRunAsyncAnalyzerTests.cs @@ -37,8 +37,8 @@ public async Task SomeOperationAsync() "; DiagnosticResult[] expected = { - CSVerify.Diagnostic().WithLocation(15, 19), - CSVerify.Diagnostic().WithLocation(16, 19), + Diagnostic().WithLocation(15, 19), + Diagnostic().WithLocation(16, 19), }; await CSVerify.VerifyAnalyzerAsync(test, expected); } @@ -71,7 +71,7 @@ public async Task SomeOperationAsync() } } "; - DiagnosticResult expected = CSVerify.Diagnostic().WithLocation(14, 19); + DiagnosticResult expected = Diagnostic().WithLocation(14, 19); await CSVerify.VerifyAnalyzerAsync(test, expected); } @@ -91,7 +91,7 @@ public static T WaitAndGetResult(Task task) } } "; - DiagnosticResult expected = CSVerify.Diagnostic().WithLocation(10, 59); + DiagnosticResult expected = Diagnostic().WithLocation(10, 59); await CSVerify.VerifyAnalyzerAsync(test, expected); } @@ -111,7 +111,7 @@ public static T WaitAndGetResult(Task task) } } "; - DiagnosticResult expected = CSVerify.Diagnostic().WithLocation(10, 68); + DiagnosticResult expected = Diagnostic().WithLocation(10, 68); await CSVerify.VerifyAnalyzerAsync(test, expected); } @@ -130,7 +130,7 @@ class Tests public void Test() { this.task = this.jtf.RunAsync(async () => await Task.Yield()).Task; - this.jtf.Run(() => [|this.task|]); + this.jtf.Run(() => {|VSTHRD003:this.task|}); } } """; @@ -224,7 +224,7 @@ class Tests public async Task AwaitAndGetResult() {{ - await [|task|].ConfigureAwait({(continueOnCapturedContext ? "true" : "false")}); + await {{|VSTHRD003:task|}}.ConfigureAwait({(continueOnCapturedContext ? "true" : "false")}); }} }} "; @@ -245,7 +245,7 @@ class Tests public async Task AwaitAndGetResult() {{ - return await [|task|].ConfigureAwait({(continueOnCapturedContext ? "true" : "false")}); + return await {{|VSTHRD003:task|}}.ConfigureAwait({(continueOnCapturedContext ? "true" : "false")}); }} }} "; @@ -265,7 +265,7 @@ class Tests public async Task AwaitAndGetResult() { - await [|task|].ConfigureAwaitRunInline(); + await {|VSTHRD003:task|}.ConfigureAwaitRunInline(); } } "; @@ -285,7 +285,7 @@ class Tests public async Task AwaitAndGetResult() { - return await [|task|].ConfigureAwaitRunInline(); + return await {|VSTHRD003:task|}.ConfigureAwaitRunInline(); } } "; @@ -334,7 +334,7 @@ public static T WaitAndGetResult(Task task, CancellationToken cancellation } } "; - DiagnosticResult expected = CSVerify.Diagnostic().WithLocation(11, 59); + DiagnosticResult expected = Diagnostic().WithLocation(11, 59); await CSVerify.VerifyAnalyzerAsync(test, expected); } @@ -413,7 +413,7 @@ public async Task SomeOperationAsync() } } "; - DiagnosticResult expected = CSVerify.Diagnostic().WithLocation(14, 19); + DiagnosticResult expected = Diagnostic().WithLocation(14, 19); await CSVerify.VerifyAnalyzerAsync(test, expected); } @@ -446,7 +446,7 @@ public async Task SomeOperationAsync() } } "; - DiagnosticResult expected = CSVerify.Diagnostic().WithLocation(14, 19); + DiagnosticResult expected = Diagnostic().WithLocation(14, 19); await CSVerify.VerifyAnalyzerAsync(test, expected); } @@ -686,7 +686,7 @@ public async Task SomeOperationAsync() } } "; - DiagnosticResult expected = CSVerify.Diagnostic().WithLocation(14, 19); + DiagnosticResult expected = Diagnostic().WithLocation(14, 19); await CSVerify.VerifyAnalyzerAsync(test, expected); } @@ -720,7 +720,7 @@ public async Task SomeOperationAsync() } } "; - DiagnosticResult expected = CSVerify.Diagnostic().WithLocation(16, 19); + DiagnosticResult expected = Diagnostic().WithLocation(16, 19); await CSVerify.VerifyAnalyzerAsync(test, expected); } @@ -755,7 +755,7 @@ public async Task SomeOperationAsync() } } "; - DiagnosticResult expected = CSVerify.Diagnostic().WithLocation(16, 23); + DiagnosticResult expected = Diagnostic().WithLocation(16, 23); await CSVerify.VerifyAnalyzerAsync(test, expected); } @@ -791,9 +791,9 @@ public async Task SomeOperationAsync() "; DiagnosticResult[] expected = { - CSVerify.Diagnostic().WithLocation(14, 19), - CSVerify.Diagnostic().WithLocation(15, 19), - CSVerify.Diagnostic().WithLocation(16, 19), + Diagnostic().WithLocation(14, 19), + Diagnostic().WithLocation(15, 19), + Diagnostic().WithLocation(16, 19), }; await CSVerify.VerifyAnalyzerAsync(test, expected); @@ -919,7 +919,7 @@ public async Task SomeOperationAsync() } } "; - DiagnosticResult expected = CSVerify.Diagnostic().WithLocation(17, 23); + DiagnosticResult expected = Diagnostic().WithLocation(17, 23); await CSVerify.VerifyAnalyzerAsync(test, expected); } @@ -961,7 +961,7 @@ public async Task SomeOperationAsync() } } "; - DiagnosticResult expected = CSVerify.Diagnostic().WithLocation(24, 19); + DiagnosticResult expected = Diagnostic().WithLocation(24, 19); await CSVerify.VerifyAnalyzerAsync(test, expected); } @@ -1169,26 +1169,32 @@ internal sealed class CompletedTaskAttribute : Attribute class Tests { - [CompletedTask] + [{|#0:CompletedTask|}] private static Task CompletedField = Task.Delay(1); - [CompletedTask] + [{|#1:CompletedTask|}] private static Task CompletedProperty { get; set; } = Task.Delay(1); private static Task task = Task.Delay(1); - [CompletedTask] + [{|#2:CompletedTask|}] private static ref Task CompletedRefProperty => ref task; - public Task GetField() => [|CompletedField|]; + public Task GetField() => CompletedField; - public Task GetProperty() => [|CompletedProperty|]; + public Task GetProperty() => CompletedProperty; - public Task GetRefProperty() => [|CompletedRefProperty|]; + public Task GetRefProperty() => CompletedRefProperty; } """; - await CSVerify.VerifyAnalyzerAsync(test); + DiagnosticResult[] expected = + { + Diagnostic().WithLocation(0).WithArguments("CompletedField"), + Diagnostic().WithLocation(1).WithArguments("CompletedProperty"), + Diagnostic().WithLocation(2).WithArguments("CompletedRefProperty"), + }; + await CSVerify.VerifyAnalyzerAsync(test, expected); } [Fact] @@ -1214,7 +1220,7 @@ class Tests [Microsoft.VisualStudio.Threading.Container.CompletedTask] private static readonly Task CompletedField = Task.Delay(1); - public Task GetField() => [|CompletedField|]; + public Task GetField() => {|VSTHRD003:CompletedField|}; } """; @@ -1412,7 +1418,7 @@ class Tests { async Task GetTask(TaskCompletionSource tcs) { - await [|tcs.Task|]; + await {|VSTHRD003:tcs.Task|}; } } "; @@ -1466,7 +1472,7 @@ static async Task GetTask() // Assigned, but not to a newly created object. TaskCompletionSource tcs3 = tcs2; - await [|tcs3.Task|]; + await {|VSTHRD003:tcs3.Task|}; } } "; @@ -1582,8 +1588,8 @@ class Tests async Task GetTask() { - await [|this.MyTaskProperty|]; - await [|MyTaskProperty|]; + await {|VSTHRD003:this.MyTaskProperty|}; + await {|VSTHRD003:MyTaskProperty|}; } } "; @@ -1648,7 +1654,7 @@ public async Task ReportWarningForForeignFieldInTopLevelStatements() string test = """ using System.Threading.Tasks; - await [|State.Task|]; + await {|VSTHRD003:State.Task|}; static class State { @@ -1666,6 +1672,8 @@ static class State }.RunAsync(); } + private static DiagnosticResult Diagnostic() => new("VSTHRD003", DiagnosticSeverity.Warning); + private DiagnosticResult CreateDiagnostic(int line, int column, int length) => - CSVerify.Diagnostic().WithSpan(line, column, line, column + length); + Diagnostic().WithSpan(line, column, line, column + length); } From 48b54bee9e7aae69af159615264d084a72524799 Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Fri, 21 Aug 2026 10:49:12 -0600 Subject: [PATCH 3/7] Assign VSTHRD116 to invalid completed task annotations Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- doc/analyzers/VSTHRD116.md | 1 + docfx/analyzers/VSTHRD003.md | 2 +- docfx/analyzers/VSTHRD116.md | 28 +++++++++++++++++++ docfx/analyzers/index.md | 1 + docfx/analyzers/toc.yml | 1 + .../VSTHRD003UseJtfRunAsyncAnalyzer.cs | 10 ++++--- .../Strings.resx | 4 +-- .../Helpers/CSharpCodeFixVerifier`2+Test.cs | 2 -- .../VSTHRD003UseJtfRunAsyncAnalyzerTests.cs | 8 ++++-- 9 files changed, 45 insertions(+), 12 deletions(-) create mode 100644 doc/analyzers/VSTHRD116.md create mode 100644 docfx/analyzers/VSTHRD116.md diff --git a/doc/analyzers/VSTHRD116.md b/doc/analyzers/VSTHRD116.md new file mode 100644 index 000000000..b64a0942a --- /dev/null +++ b/doc/analyzers/VSTHRD116.md @@ -0,0 +1 @@ +This content has been moved to [GitHub Pages](https://microsoft.github.io/vs-threading/analyzers/VSTHRD116.html). diff --git a/docfx/analyzers/VSTHRD003.md b/docfx/analyzers/VSTHRD003.md index 3a056fc77..42b7f0ede 100644 --- a/docfx/analyzers/VSTHRD003.md +++ b/docfx/analyzers/VSTHRD003.md @@ -37,7 +37,7 @@ private static readonly Task TrueTask = Task.FromResult(true); private static Task FalseTask { get; } = Task.FromResult(false); ``` -The analyzer reports VSTHRD003 on `[CompletedTask]` when it is applied to a mutable field, +The analyzer reports [VSTHRD116](VSTHRD116.md) on `[CompletedTask]` when it is applied to a mutable field, settable property, or ref-returning property, because its value can later be replaced with an incomplete task. Consumers still take the attribute at face value, so the diagnostic is reported at the invalid attribute application rather than at each use of the member. diff --git a/docfx/analyzers/VSTHRD116.md b/docfx/analyzers/VSTHRD116.md new file mode 100644 index 000000000..522de8588 --- /dev/null +++ b/docfx/analyzers/VSTHRD116.md @@ -0,0 +1,28 @@ +# VSTHRD116 Apply `CompletedTaskAttribute` only to immutable members + +`Microsoft.VisualStudio.Threading.CompletedTaskAttribute` tells VSTHRD003 that a member always +produces a completed task. Applying it to a mutable member is unsafe because the member can later +be assigned an incomplete task. + +## Examples of patterns that are flagged by this analyzer + +```csharp +[CompletedTask] +private static Task CachedTask { get; set; } = Task.CompletedTask; +``` + +The diagnostic is reported on `[CompletedTask]`, not at each use of `CachedTask`. Consumers take +the attribute at face value and do not report VSTHRD003. + +## Solution + +Apply `[CompletedTask]` only to methods that always return completed tasks, `readonly` fields, or +non-ref get-only properties: + +```csharp +[CompletedTask] +private static readonly Task CachedTask = Task.CompletedTask; +``` + +If the member must remain mutable, remove `[CompletedTask]`. VSTHRD003 will then analyze each use +normally. diff --git a/docfx/analyzers/index.md b/docfx/analyzers/index.md index 4d2077c25..cb08ad655 100644 --- a/docfx/analyzers/index.md +++ b/docfx/analyzers/index.md @@ -29,6 +29,7 @@ ID | Title | Severity | Supports | Default diagnostic severity [VSTHRD113](VSTHRD113.md) | Check for `System.IAsyncDisposable` | Advisory | | Info [VSTHRD114](VSTHRD114.md) | Avoid returning null from a `Task`-returning method. | Advisory | | Warning [VSTHRD115](VSTHRD115.md) | Avoid creating a JoinableTaskContext with an explicit `null` `SynchronizationContext` | Advisory | | Warning +[VSTHRD116](VSTHRD116.md) | Apply `CompletedTaskAttribute` only to immutable members | Advisory | [VSTHRD003](VSTHRD003.md) | Warning [VSTHRD200](VSTHRD200.md) | Use `Async` naming convention | Guideline | [VSTHRD103](VSTHRD103.md) | Warning ## Severity descriptions diff --git a/docfx/analyzers/toc.yml b/docfx/analyzers/toc.yml index cc7bee4ff..bf9c598e3 100644 --- a/docfx/analyzers/toc.yml +++ b/docfx/analyzers/toc.yml @@ -26,4 +26,5 @@ items: - href: VSTHRD113.md - href: VSTHRD114.md - href: VSTHRD115.md +- href: VSTHRD116.md - href: VSTHRD200.md diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD003UseJtfRunAsyncAnalyzer.cs b/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD003UseJtfRunAsyncAnalyzer.cs index 357c2861f..50fd94d62 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD003UseJtfRunAsyncAnalyzer.cs +++ b/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD003UseJtfRunAsyncAnalyzer.cs @@ -40,6 +40,8 @@ public class VSTHRD003UseJtfRunAsyncAnalyzer : DiagnosticAnalyzer { public const string Id = "VSTHRD003"; + public const string InvalidCompletedTaskAttributeId = "VSTHRD116"; + internal static readonly DiagnosticDescriptor Descriptor = new DiagnosticDescriptor( id: Id, title: new LocalizableResourceString(nameof(Strings.VSTHRD003_Title), Strings.ResourceManager, typeof(Strings)), @@ -50,10 +52,10 @@ public class VSTHRD003UseJtfRunAsyncAnalyzer : DiagnosticAnalyzer isEnabledByDefault: true); internal static readonly DiagnosticDescriptor InvalidCompletedTaskAttributeDescriptor = new DiagnosticDescriptor( - id: Id, - title: new LocalizableResourceString(nameof(Strings.VSTHRD003_InvalidCompletedTaskAttribute_Title), Strings.ResourceManager, typeof(Strings)), - messageFormat: new LocalizableResourceString(nameof(Strings.VSTHRD003_InvalidCompletedTaskAttribute_MessageFormat), Strings.ResourceManager, typeof(Strings)), - helpLinkUri: Utils.GetHelpLink(Id), + id: InvalidCompletedTaskAttributeId, + title: new LocalizableResourceString(nameof(Strings.VSTHRD116_Title), Strings.ResourceManager, typeof(Strings)), + messageFormat: new LocalizableResourceString(nameof(Strings.VSTHRD116_MessageFormat), Strings.ResourceManager, typeof(Strings)), + helpLinkUri: Utils.GetHelpLink(InvalidCompletedTaskAttributeId), category: "Usage", defaultSeverity: DiagnosticSeverity.Warning, isEnabledByDefault: true); diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers/Strings.resx b/src/Microsoft.VisualStudio.Threading.Analyzers/Strings.resx index 5ba34be0c..1a25d1bd3 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers/Strings.resx +++ b/src/Microsoft.VisualStudio.Threading.Analyzers/Strings.resx @@ -172,11 +172,11 @@ Start the work within this context, or use JoinableTaskFactory.RunAsync to start Avoid awaiting foreign Tasks - + CompletedTaskAttribute cannot be applied to mutable member "{0}". Apply it only to methods, readonly fields, or non-ref get-only properties. CompletedTaskAttribute is a type name and should not be translated. {0} is the name of a field, property, or indexer. - + Apply CompletedTaskAttribute only to immutable members CompletedTaskAttribute is a type name and should not be translated. diff --git a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/Helpers/CSharpCodeFixVerifier`2+Test.cs b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/Helpers/CSharpCodeFixVerifier`2+Test.cs index 32212077e..b767fa30e 100644 --- a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/Helpers/CSharpCodeFixVerifier`2+Test.cs +++ b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/Helpers/CSharpCodeFixVerifier`2+Test.cs @@ -10,7 +10,6 @@ #endif using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Testing; -using Microsoft.CodeAnalysis.Testing; using Microsoft.CodeAnalysis.Text; using IOleServiceProvider = Microsoft.VisualStudio.OLE.Interop.IServiceProvider; @@ -23,7 +22,6 @@ public class Test : CSharpCodeFixTest public Test() { this.ReferenceAssemblies = ReferencesHelper.DefaultReferences; - this.MarkupOptions = MarkupOptions.UseFirstDescriptor; this.SolutionTransforms.Add((solution, projectId) => { diff --git a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD003UseJtfRunAsyncAnalyzerTests.cs b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD003UseJtfRunAsyncAnalyzerTests.cs index 2b0ab0393..f39072c56 100644 --- a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD003UseJtfRunAsyncAnalyzerTests.cs +++ b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD003UseJtfRunAsyncAnalyzerTests.cs @@ -1190,9 +1190,9 @@ class Tests DiagnosticResult[] expected = { - Diagnostic().WithLocation(0).WithArguments("CompletedField"), - Diagnostic().WithLocation(1).WithArguments("CompletedProperty"), - Diagnostic().WithLocation(2).WithArguments("CompletedRefProperty"), + InvalidCompletedTaskAttributeDiagnostic().WithLocation(0).WithArguments("CompletedField"), + InvalidCompletedTaskAttributeDiagnostic().WithLocation(1).WithArguments("CompletedProperty"), + InvalidCompletedTaskAttributeDiagnostic().WithLocation(2).WithArguments("CompletedRefProperty"), }; await CSVerify.VerifyAnalyzerAsync(test, expected); } @@ -1674,6 +1674,8 @@ static class State private static DiagnosticResult Diagnostic() => new("VSTHRD003", DiagnosticSeverity.Warning); + private static DiagnosticResult InvalidCompletedTaskAttributeDiagnostic() => new("VSTHRD116", DiagnosticSeverity.Warning); + private DiagnosticResult CreateDiagnostic(int line, int column, int length) => Diagnostic().WithSpan(line, column, line, column + length); } From 7d3a716ab978d7d3c15781560c6a57e2aca34303 Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Fri, 21 Aug 2026 11:47:43 -0600 Subject: [PATCH 4/7] Renumber completed task annotation diagnostic Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- doc/analyzers/{VSTHRD116.md => VSTHRD013.md} | 2 +- docfx/analyzers/VSTHRD003.md | 2 +- docfx/analyzers/{VSTHRD116.md => VSTHRD013.md} | 2 +- docfx/analyzers/index.md | 2 +- docfx/analyzers/toc.yml | 2 +- .../VSTHRD003UseJtfRunAsyncAnalyzer.cs | 6 +++--- src/Microsoft.VisualStudio.Threading.Analyzers/Strings.resx | 4 ++-- .../VSTHRD003UseJtfRunAsyncAnalyzerTests.cs | 2 +- 8 files changed, 11 insertions(+), 11 deletions(-) rename doc/analyzers/{VSTHRD116.md => VSTHRD013.md} (56%) rename docfx/analyzers/{VSTHRD116.md => VSTHRD013.md} (93%) diff --git a/doc/analyzers/VSTHRD116.md b/doc/analyzers/VSTHRD013.md similarity index 56% rename from doc/analyzers/VSTHRD116.md rename to doc/analyzers/VSTHRD013.md index b64a0942a..5bba1e001 100644 --- a/doc/analyzers/VSTHRD116.md +++ b/doc/analyzers/VSTHRD013.md @@ -1 +1 @@ -This content has been moved to [GitHub Pages](https://microsoft.github.io/vs-threading/analyzers/VSTHRD116.html). +This content has been moved to [GitHub Pages](https://microsoft.github.io/vs-threading/analyzers/VSTHRD013.html). diff --git a/docfx/analyzers/VSTHRD003.md b/docfx/analyzers/VSTHRD003.md index 42b7f0ede..20a253017 100644 --- a/docfx/analyzers/VSTHRD003.md +++ b/docfx/analyzers/VSTHRD003.md @@ -37,7 +37,7 @@ private static readonly Task TrueTask = Task.FromResult(true); private static Task FalseTask { get; } = Task.FromResult(false); ``` -The analyzer reports [VSTHRD116](VSTHRD116.md) on `[CompletedTask]` when it is applied to a mutable field, +The analyzer reports [VSTHRD013](VSTHRD013.md) on `[CompletedTask]` when it is applied to a mutable field, settable property, or ref-returning property, because its value can later be replaced with an incomplete task. Consumers still take the attribute at face value, so the diagnostic is reported at the invalid attribute application rather than at each use of the member. diff --git a/docfx/analyzers/VSTHRD116.md b/docfx/analyzers/VSTHRD013.md similarity index 93% rename from docfx/analyzers/VSTHRD116.md rename to docfx/analyzers/VSTHRD013.md index 522de8588..5762777f2 100644 --- a/docfx/analyzers/VSTHRD116.md +++ b/docfx/analyzers/VSTHRD013.md @@ -1,4 +1,4 @@ -# VSTHRD116 Apply `CompletedTaskAttribute` only to immutable members +# VSTHRD013 Apply `CompletedTaskAttribute` only to immutable members `Microsoft.VisualStudio.Threading.CompletedTaskAttribute` tells VSTHRD003 that a member always produces a completed task. Applying it to a mutable member is unsafe because the member can later diff --git a/docfx/analyzers/index.md b/docfx/analyzers/index.md index cb08ad655..bf0c27b74 100644 --- a/docfx/analyzers/index.md +++ b/docfx/analyzers/index.md @@ -13,6 +13,7 @@ ID | Title | Severity | Supports | Default diagnostic severity [VSTHRD010](VSTHRD010.md) | Invoke single-threaded types on Main thread | Critical | [1st rule](../docs/threading_rules.md#Rule1) | Warning [VSTHRD011](VSTHRD011.md) | Use `AsyncLazy` | Critical | [3rd rule](../docs/threading_rules.md#Rule3) | Error [VSTHRD012](VSTHRD012.md) | Provide JoinableTaskFactory where allowed | Critical | [All rules](../docs/threading_rules.md) | Warning +[VSTHRD013](VSTHRD013.md) | Apply `CompletedTaskAttribute` only to immutable members | Critical | [VSTHRD003](VSTHRD003.md) | Warning [VSTHRD100](VSTHRD100.md) | Avoid `async void` methods | Advisory | | Warning [VSTHRD101](VSTHRD101.md) | Avoid unsupported async delegates | Advisory | [VSTHRD100](VSTHRD100.md) | Warning [VSTHRD102](VSTHRD102.md) | Implement internal logic asynchronously | Advisory | [2nd rule](../docs/threading_rules.md#Rule2) | Info @@ -29,7 +30,6 @@ ID | Title | Severity | Supports | Default diagnostic severity [VSTHRD113](VSTHRD113.md) | Check for `System.IAsyncDisposable` | Advisory | | Info [VSTHRD114](VSTHRD114.md) | Avoid returning null from a `Task`-returning method. | Advisory | | Warning [VSTHRD115](VSTHRD115.md) | Avoid creating a JoinableTaskContext with an explicit `null` `SynchronizationContext` | Advisory | | Warning -[VSTHRD116](VSTHRD116.md) | Apply `CompletedTaskAttribute` only to immutable members | Advisory | [VSTHRD003](VSTHRD003.md) | Warning [VSTHRD200](VSTHRD200.md) | Use `Async` naming convention | Guideline | [VSTHRD103](VSTHRD103.md) | Warning ## Severity descriptions diff --git a/docfx/analyzers/toc.yml b/docfx/analyzers/toc.yml index bf9c598e3..c4efe0be8 100644 --- a/docfx/analyzers/toc.yml +++ b/docfx/analyzers/toc.yml @@ -10,6 +10,7 @@ items: - href: VSTHRD010.md - href: VSTHRD011.md - href: VSTHRD012.md +- href: VSTHRD013.md - href: VSTHRD100.md - href: VSTHRD101.md - href: VSTHRD102.md @@ -26,5 +27,4 @@ items: - href: VSTHRD113.md - href: VSTHRD114.md - href: VSTHRD115.md -- href: VSTHRD116.md - href: VSTHRD200.md diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD003UseJtfRunAsyncAnalyzer.cs b/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD003UseJtfRunAsyncAnalyzer.cs index 50fd94d62..72dab962d 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD003UseJtfRunAsyncAnalyzer.cs +++ b/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD003UseJtfRunAsyncAnalyzer.cs @@ -40,7 +40,7 @@ public class VSTHRD003UseJtfRunAsyncAnalyzer : DiagnosticAnalyzer { public const string Id = "VSTHRD003"; - public const string InvalidCompletedTaskAttributeId = "VSTHRD116"; + public const string InvalidCompletedTaskAttributeId = "VSTHRD013"; internal static readonly DiagnosticDescriptor Descriptor = new DiagnosticDescriptor( id: Id, @@ -53,8 +53,8 @@ public class VSTHRD003UseJtfRunAsyncAnalyzer : DiagnosticAnalyzer internal static readonly DiagnosticDescriptor InvalidCompletedTaskAttributeDescriptor = new DiagnosticDescriptor( id: InvalidCompletedTaskAttributeId, - title: new LocalizableResourceString(nameof(Strings.VSTHRD116_Title), Strings.ResourceManager, typeof(Strings)), - messageFormat: new LocalizableResourceString(nameof(Strings.VSTHRD116_MessageFormat), Strings.ResourceManager, typeof(Strings)), + title: new LocalizableResourceString(nameof(Strings.VSTHRD013_Title), Strings.ResourceManager, typeof(Strings)), + messageFormat: new LocalizableResourceString(nameof(Strings.VSTHRD013_MessageFormat), Strings.ResourceManager, typeof(Strings)), helpLinkUri: Utils.GetHelpLink(InvalidCompletedTaskAttributeId), category: "Usage", defaultSeverity: DiagnosticSeverity.Warning, diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers/Strings.resx b/src/Microsoft.VisualStudio.Threading.Analyzers/Strings.resx index 1a25d1bd3..c53cc7113 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers/Strings.resx +++ b/src/Microsoft.VisualStudio.Threading.Analyzers/Strings.resx @@ -172,11 +172,11 @@ Start the work within this context, or use JoinableTaskFactory.RunAsync to start Avoid awaiting foreign Tasks - + CompletedTaskAttribute cannot be applied to mutable member "{0}". Apply it only to methods, readonly fields, or non-ref get-only properties. CompletedTaskAttribute is a type name and should not be translated. {0} is the name of a field, property, or indexer. - + Apply CompletedTaskAttribute only to immutable members CompletedTaskAttribute is a type name and should not be translated. diff --git a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD003UseJtfRunAsyncAnalyzerTests.cs b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD003UseJtfRunAsyncAnalyzerTests.cs index f39072c56..d4b9b5d8e 100644 --- a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD003UseJtfRunAsyncAnalyzerTests.cs +++ b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD003UseJtfRunAsyncAnalyzerTests.cs @@ -1674,7 +1674,7 @@ static class State private static DiagnosticResult Diagnostic() => new("VSTHRD003", DiagnosticSeverity.Warning); - private static DiagnosticResult InvalidCompletedTaskAttributeDiagnostic() => new("VSTHRD116", DiagnosticSeverity.Warning); + private static DiagnosticResult InvalidCompletedTaskAttributeDiagnostic() => new("VSTHRD013", DiagnosticSeverity.Warning); private DiagnosticResult CreateDiagnostic(int line, int column, int length) => Diagnostic().WithSpan(line, column, line, column + length); From 5e977de3226f6d01384256e04565018e9ecc311d Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Fri, 21 Aug 2026 11:52:51 -0600 Subject: [PATCH 5/7] Address VSTHRD003 review feedback Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docfx/analyzers/VSTHRD003.md | 4 +- docfx/docs/threading_rules.md | 7 +- .../VSTHRD003UseJtfRunAsyncAnalyzer.cs | 58 +++++++++++------ .../VSTHRD003UseJtfRunAsyncAnalyzerTests.cs | 65 +++++++++++++++---- 4 files changed, 98 insertions(+), 36 deletions(-) diff --git a/docfx/analyzers/VSTHRD003.md b/docfx/analyzers/VSTHRD003.md index 20a253017..2027bba2b 100644 --- a/docfx/analyzers/VSTHRD003.md +++ b/docfx/analyzers/VSTHRD003.md @@ -19,8 +19,8 @@ fully qualified name `Microsoft.VisualStudio.Threading.CompletedTaskAttribute` i ```csharp namespace Microsoft.VisualStudio.Threading { - [AttributeUsage(AttributeTargets.Field | AttributeTargets.Property | AttributeTargets.Method)] - internal sealed class CompletedTaskAttribute : Attribute + [System.AttributeUsage(System.AttributeTargets.Field | System.AttributeTargets.Property | System.AttributeTargets.Method)] + internal sealed class CompletedTaskAttribute : System.Attribute { } } diff --git a/docfx/docs/threading_rules.md b/docfx/docs/threading_rules.md index 8eaca81d3..15dea4e30 100644 --- a/docfx/docs/threading_rules.md +++ b/docfx/docs/threading_rules.md @@ -59,10 +59,13 @@ JoinableTask longRunningAsyncWork = joinableTaskFactoryInstance.RunAsync( }); ``` -then later asynchronous code can join that work while waiting for it: +Then later asynchronous code can join that work while waiting for it: ```csharp -await longRunningAsyncWork.JoinAsync(cancellationToken); +async Task WaitForLongRunningWorkAsync(CancellationToken cancellationToken) +{ + await longRunningAsyncWork.JoinAsync(cancellationToken); +} ``` When cancellation is not required, directly awaiting the `JoinableTask` is equivalent to diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD003UseJtfRunAsyncAnalyzer.cs b/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD003UseJtfRunAsyncAnalyzer.cs index 72dab962d..62a83284b 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD003UseJtfRunAsyncAnalyzer.cs +++ b/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD003UseJtfRunAsyncAnalyzer.cs @@ -128,38 +128,54 @@ private void AnalyzeCompletedTaskAttribute(SyntaxNodeAnalysisContext context) return; } - ISymbol? mutableMember = attribute.Parent?.Parent switch + string? mutableMemberName; + switch (attribute.Parent?.Parent) { - FieldDeclarationSyntax field when !field.Modifiers.Any(SyntaxKind.ReadOnlyKeyword) => - context.SemanticModel.GetDeclaredSymbol(field.Declaration.Variables[0], context.CancellationToken), - PropertyDeclarationSyntax property when context.SemanticModel.GetDeclaredSymbol(property, context.CancellationToken) is IPropertySymbol { SetMethod: not null } or { ReturnsByRef: true } => - context.SemanticModel.GetDeclaredSymbol(property, context.CancellationToken), - IndexerDeclarationSyntax indexer when context.SemanticModel.GetDeclaredSymbol(indexer, context.CancellationToken) is IPropertySymbol { SetMethod: not null } or { ReturnsByRef: true } => - context.SemanticModel.GetDeclaredSymbol(indexer, context.CancellationToken), - _ => null, - }; + case FieldDeclarationSyntax field when !field.Modifiers.Any(SyntaxKind.ReadOnlyKeyword): + mutableMemberName = string.Join(", ", field.Declaration.Variables.Select(variable => variable.Identifier.ValueText)); + break; + case PropertyDeclarationSyntax property: + IPropertySymbol? propertySymbol = context.SemanticModel.GetDeclaredSymbol(property, context.CancellationToken); + mutableMemberName = propertySymbol is { SetMethod: not null } || propertySymbol is { ReturnsByRef: true } ? propertySymbol.Name : null; + break; + case IndexerDeclarationSyntax indexer: + IPropertySymbol? indexerSymbol = context.SemanticModel.GetDeclaredSymbol(indexer, context.CancellationToken); + mutableMemberName = indexerSymbol is { SetMethod: not null } || indexerSymbol is { ReturnsByRef: true } ? indexerSymbol.Name : null; + break; + default: + mutableMemberName = null; + break; + } - if (mutableMember is not null) + if (mutableMemberName is not null) { - context.ReportDiagnostic(Diagnostic.Create(InvalidCompletedTaskAttributeDescriptor, attribute.GetLocation(), mutableMember.Name)); + context.ReportDiagnostic(Diagnostic.Create(InvalidCompletedTaskAttributeDescriptor, attribute.GetLocation(), mutableMemberName)); } } private void AnalyzeArrowExpressionClause(SyntaxNodeAnalysisContext context) { var arrowExpressionClause = (ArrowExpressionClauseSyntax)context.Node; - if (arrowExpressionClause.Parent is MethodDeclarationSyntax methodDeclaration) + ISymbol? containingSymbol = arrowExpressionClause.Parent switch { - if (IsSymbolAlwaysOkToAwait(context.SemanticModel.GetDeclaredSymbol(methodDeclaration, context.CancellationToken))) - { - return; - } + MethodDeclarationSyntax method => context.SemanticModel.GetDeclaredSymbol(method, context.CancellationToken), + LocalFunctionStatementSyntax localFunction => context.SemanticModel.GetDeclaredSymbol(localFunction, context.CancellationToken), + PropertyDeclarationSyntax property => context.SemanticModel.GetDeclaredSymbol(property, context.CancellationToken), + IndexerDeclarationSyntax indexer => context.SemanticModel.GetDeclaredSymbol(indexer, context.CancellationToken), + AccessorDeclarationSyntax { Parent.Parent: PropertyDeclarationSyntax property } => context.SemanticModel.GetDeclaredSymbol(property, context.CancellationToken), + AccessorDeclarationSyntax { Parent.Parent: IndexerDeclarationSyntax indexer } => context.SemanticModel.GetDeclaredSymbol(indexer, context.CancellationToken), + _ => null, + }; - Diagnostic? diagnostic = this.AnalyzeAwaitedOrReturnedExpression(arrowExpressionClause.Expression, context, context.CancellationToken); - if (diagnostic is object) - { - context.ReportDiagnostic(diagnostic); - } + if (containingSymbol is null || IsSymbolAlwaysOkToAwait(containingSymbol)) + { + return; + } + + Diagnostic? diagnostic = this.AnalyzeAwaitedOrReturnedExpression(arrowExpressionClause.Expression, context, context.CancellationToken); + if (diagnostic is object) + { + context.ReportDiagnostic(diagnostic); } } diff --git a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD003UseJtfRunAsyncAnalyzerTests.cs b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD003UseJtfRunAsyncAnalyzerTests.cs index d4b9b5d8e..05d32e912 100644 --- a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD003UseJtfRunAsyncAnalyzerTests.cs +++ b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD003UseJtfRunAsyncAnalyzerTests.cs @@ -1099,13 +1099,22 @@ internal sealed class CompletedTaskAttribute : Attribute class Tests { - private static Task task; + private static Task task = Task.WhenAll(Task.CompletedTask); [CompletedTask] - private static readonly Task CompletedField = Task.Delay(1); + private static readonly Task CompletedField = Task.WhenAll(Task.CompletedTask); + + [CompletedTask] + private static Task CompletedProperty { get; } = Task.WhenAll(Task.CompletedTask); + + [CompletedTask] + private static Task CompletedExpressionProperty => task; [CompletedTask] - private static Task CompletedProperty { get; } = Task.Delay(1); + private static Task CompletedExpressionGetter + { + get => task; + } [CompletedTask] private static Task CompletedBlockProperty @@ -1119,16 +1128,20 @@ private static Task CompletedBlockProperty [CompletedTask] private static Task ReturnCompletedTask(Task task) { - return task; + return Task.WhenAll(Task.CompletedTask); } [CompletedTask] - private static Task ReturnCompletedTaskExpression(Task task) => task; + private static Task ReturnCompletedTaskExpression(Task task) => Task.WhenAll(Task.CompletedTask); public Task GetField() => CompletedField; public Task GetProperty() => CompletedProperty; + public Task GetExpressionProperty() => CompletedExpressionProperty; + + public Task GetExpressionGetter() => CompletedExpressionGetter; + public Task GetBlockProperty() => CompletedBlockProperty; public Task GetMethodResult(Task task) => ReturnCompletedTask(task); @@ -1140,7 +1153,7 @@ public Task GetLocalFunctionResult(Task task) [CompletedTask] static Task ReturnCompletedTaskLocal(Task task) { - return task; + return Task.WhenAll(Task.CompletedTask); } return ReturnCompletedTaskLocal(task); @@ -1170,17 +1183,19 @@ internal sealed class CompletedTaskAttribute : Attribute class Tests { [{|#0:CompletedTask|}] - private static Task CompletedField = Task.Delay(1); + private static Task CompletedField1 = Task.WhenAll(Task.CompletedTask), CompletedField2 = Task.WhenAll(Task.CompletedTask); [{|#1:CompletedTask|}] - private static Task CompletedProperty { get; set; } = Task.Delay(1); + private static Task CompletedProperty { get; set; } = Task.WhenAll(Task.CompletedTask); - private static Task task = Task.Delay(1); + private static Task task = Task.WhenAll(Task.CompletedTask); [{|#2:CompletedTask|}] private static ref Task CompletedRefProperty => ref task; - public Task GetField() => CompletedField; + public Task GetField1() => CompletedField1; + + public Task GetField2() => CompletedField2; public Task GetProperty() => CompletedProperty; @@ -1190,13 +1205,41 @@ class Tests DiagnosticResult[] expected = { - InvalidCompletedTaskAttributeDiagnostic().WithLocation(0).WithArguments("CompletedField"), + InvalidCompletedTaskAttributeDiagnostic().WithLocation(0).WithArguments("CompletedField1, CompletedField2"), InvalidCompletedTaskAttributeDiagnostic().WithLocation(1).WithArguments("CompletedProperty"), InvalidCompletedTaskAttributeDiagnostic().WithLocation(2).WithArguments("CompletedRefProperty"), }; await CSVerify.VerifyAnalyzerAsync(test, expected); } + [Fact] + public async Task ReportWarningForForeignTasksReturnedFromExpressionBodiedMembers() + { + string test = """ + using System.Threading.Tasks; + + class Tests + { + private static Task task = Task.WhenAll(Task.CompletedTask); + + private static Task ExpressionProperty => {|VSTHRD003:task|}; + + private static Task ExpressionGetter + { + get => {|VSTHRD003:task|}; + } + + private static Task GetTask() + { + Task LocalFunction() => {|VSTHRD003:task|}; + return LocalFunction(); + } + } + """; + + await CSVerify.VerifyAnalyzerAsync(test); + } + [Fact] public async Task ReportWarningForNestedCompletedTaskAttributeLookalike() { From f1399a69461f068434adfdf49e09ff0e75bf43e6 Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Fri, 21 Aug 2026 11:54:01 -0600 Subject: [PATCH 6/7] Remove obsolete Gitter badge Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/README.md b/README.md index cabe45111..575ba3d16 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,6 @@ # vs-threading [![Build Status](https://dev.azure.com/azure-public/vside/_apis/build/status/vs-threading)](https://dev.azure.com/azure-public/vside/_build/latest?definitionId=12) -[![Join the chat at https://gitter.im/vs-threading/Lobby](https://badges.gitter.im/vs-threading/Lobby.svg)](https://gitter.im/vs-threading/Lobby?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge) ## Microsoft.VisualStudio.Threading From 93fb82ccfa1e128ffd3c0bd481704867a9f468bd Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Fri, 21 Aug 2026 11:54:46 -0600 Subject: [PATCH 7/7] Remove legacy VSTHRD013 documentation redirect Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- doc/analyzers/VSTHRD013.md | 1 - 1 file changed, 1 deletion(-) delete mode 100644 doc/analyzers/VSTHRD013.md diff --git a/doc/analyzers/VSTHRD013.md b/doc/analyzers/VSTHRD013.md deleted file mode 100644 index 5bba1e001..000000000 --- a/doc/analyzers/VSTHRD013.md +++ /dev/null @@ -1 +0,0 @@ -This content has been moved to [GitHub Pages](https://microsoft.github.io/vs-threading/analyzers/VSTHRD013.html).