diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs b/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs index 99c2f6b1..81528776 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs +++ b/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs @@ -390,6 +390,151 @@ internal static bool IsApplicableAsyncAlternative( return SymbolEqualityComparer.Default.Equals(applicableDefinition, candidateDefinition); } + /// + /// Determines whether a method carries async-compatible values from its parameters into its return value. + /// + /// + /// Synchronous higher-order methods may compose asynchronous work without synchronously blocking on it. + /// For example, Enumerable.Select can project values into an IEnumerable<Task>. + /// In such cases, an Async-suffixed method is not necessarily a preferable alternative. + /// + internal static bool ReturnsAsyncCompatibleValuesFromParameters(IMethodSymbol method) + { + IMethodSymbol constructedMethod = method; + IMethodSymbol methodDefinition = constructedMethod.OriginalDefinition; + foreach (ITypeParameterSymbol typeParameter in methodDefinition.TypeParameters) + { + if (IsAsyncCompatibleTypeParameterFlow(typeParameter)) + { + return true; + } + } + + for (INamedTypeSymbol? containingType = methodDefinition.ContainingType; containingType is object; containingType = containingType.ContainingType) + { + foreach (ITypeParameterSymbol typeParameter in containingType.TypeParameters) + { + if (IsAsyncCompatibleTypeParameterFlow(typeParameter)) + { + return true; + } + } + } + + return false; + + bool IsAsyncCompatibleTypeParameterFlow(ITypeParameterSymbol typeParameter) + => ContainsTypeParameterInOutputPosition(methodDefinition.ReturnType, typeParameter) + && IsUsedByParameter(typeParameter) + && GetConstructedTypeArgument(constructedMethod, typeParameter) is { } typeArgument + && ContainsAsyncCompatibleType(typeArgument); + + bool IsUsedByParameter(ITypeParameterSymbol typeParameter) + { + if (methodDefinition.Parameters.Any( + parameter => parameter.RefKind != RefKind.Out + && ContainsTypeParameterInOutputPosition(parameter.Type, typeParameter))) + { + return true; + } + + IMethodSymbol? unreducedDefinition = method.ReducedFrom?.OriginalDefinition; + return unreducedDefinition is object + && typeParameter.TypeParameterKind == TypeParameterKind.Method + && unreducedDefinition.Parameters.Length > 0 + && ContainsTypeParameterInOutputPosition( + unreducedDefinition.Parameters[0].Type, + unreducedDefinition.TypeParameters[typeParameter.Ordinal]); + } + + static bool ContainsAsyncCompatibleType(ITypeSymbol type, VarianceKind variance = VarianceKind.Out) + { + if (variance != VarianceKind.In && type.IsAsyncCompatibleReturnType()) + { + return true; + } + + if (type is IArrayTypeSymbol arrayType) + { + return ContainsAsyncCompatibleType(arrayType.ElementType, variance); + } + + if (type is INamedTypeSymbol namedType) + { + for (int i = 0; i < namedType.TypeArguments.Length; i++) + { + VarianceKind typeArgumentVariance = i < namedType.OriginalDefinition.TypeParameters.Length + ? namedType.OriginalDefinition.TypeParameters[i].Variance + : VarianceKind.None; + if (ContainsAsyncCompatibleType(namedType.TypeArguments[i], ComposeVariance(variance, typeArgumentVariance))) + { + return true; + } + } + } + + return false; + } + + static bool ContainsTypeParameterInOutputPosition( + ITypeSymbol type, + ITypeParameterSymbol typeParameter, + VarianceKind variance = VarianceKind.Out) + { + if (variance != VarianceKind.In && SymbolEqualityComparer.Default.Equals(type, typeParameter)) + { + return true; + } + + if (type is IArrayTypeSymbol arrayType) + { + return ContainsTypeParameterInOutputPosition(arrayType.ElementType, typeParameter, variance); + } + + if (type is INamedTypeSymbol namedType) + { + for (int i = 0; i < namedType.TypeArguments.Length; i++) + { + VarianceKind typeArgumentVariance = i < namedType.OriginalDefinition.TypeParameters.Length + ? namedType.OriginalDefinition.TypeParameters[i].Variance + : VarianceKind.None; + if (ContainsTypeParameterInOutputPosition( + namedType.TypeArguments[i], + typeParameter, + ComposeVariance(variance, typeArgumentVariance))) + { + return true; + } + } + } + + return false; + } + + static VarianceKind ComposeVariance(VarianceKind outer, VarianceKind inner) + => outer == VarianceKind.None || inner == VarianceKind.None + ? VarianceKind.None + : outer == inner ? VarianceKind.Out : VarianceKind.In; + + static ITypeSymbol? GetConstructedTypeArgument(IMethodSymbol constructedMethod, ITypeParameterSymbol typeParameter) + { + if (typeParameter.TypeParameterKind == TypeParameterKind.Method) + { + return constructedMethod.TypeArguments[typeParameter.Ordinal]; + } + + for (INamedTypeSymbol? containingType = constructedMethod.ContainingType; containingType is object; containingType = containingType.ContainingType) + { + if (SymbolEqualityComparer.Default.Equals(containingType.OriginalDefinition, typeParameter.ContainingSymbol)) + { + return containingType.TypeArguments[typeParameter.Ordinal]; + } + } + + return null; + } + } + /// /// Determines whether a blocking member access has a receiver that is provably complete. /// diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD002UseJtfRunAnalyzer.cs b/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD002UseJtfRunAnalyzer.cs index 09c179b8..19215891 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD002UseJtfRunAnalyzer.cs +++ b/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD002UseJtfRunAnalyzer.cs @@ -172,6 +172,7 @@ private static void AnalyzeInvocation( && !methodsExcludedFromVSTHRD103.Contains(methodDefinition) && !invokedMethod.Name.EndsWith(VSTHRD200UseAsyncNamingConventionAnalyzer.MandatoryAsyncSuffix, StringComparison.CurrentCulture) && !invokedMethod.HasAsyncCompatibleReturnType() + && !CSharpCommonInterest.ReturnsAsyncCompatibleValuesFromParameters(invokedMethod) && IsInTaskReturningMethodOrDelegate(context) && HasAsyncAlternative(context, invocationExpressionSyntax, invokedMethod); if (!isBuiltInSyncBlockingMethod diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD103UseAsyncOptionAnalyzer.cs b/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD103UseAsyncOptionAnalyzer.cs index 3d76461b..788fdc60 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD103UseAsyncOptionAnalyzer.cs +++ b/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD103UseAsyncOptionAnalyzer.cs @@ -145,7 +145,8 @@ MemberBindingExpressionSyntax memberBinding when invocationExpressionSyntax.Firs // Also consider all method calls to check for Async-suffixed alternatives. SymbolInfo symbolInfo = context.SemanticModel.GetSymbolInfo(invocationExpressionSyntax, context.CancellationToken); if (symbolInfo.Symbol is IMethodSymbol methodSymbol && !methodSymbol.Name.EndsWith(VSTHRD200UseAsyncNamingConventionAnalyzer.MandatoryAsyncSuffix, StringComparison.CurrentCulture) && - !methodSymbol.HasAsyncCompatibleReturnType()) + !methodSymbol.HasAsyncCompatibleReturnType() && + !CSharpCommonInterest.ReturnsAsyncCompatibleValuesFromParameters(methodSymbol)) { string asyncMethodName = methodSymbol.Name + VSTHRD200UseAsyncNamingConventionAnalyzer.MandatoryAsyncSuffix; diff --git a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD002UseJtfRunAnalyzerTests.cs b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD002UseJtfRunAnalyzerTests.cs index 9b3d5a93..94ff6a47 100644 --- a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD002UseJtfRunAnalyzerTests.cs +++ b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD002UseJtfRunAnalyzerTests.cs @@ -1345,6 +1345,44 @@ Task FAsync(Task task) { await verifyTest.RunAsync(); } + [Fact] + public async Task ConfiguredTaskFlowMethodInTaskReturningMethodReports() + { + string test = """ + using System; + using System.Collections.Generic; + using System.Threading.Tasks; + + static class Helpers + { + internal static IEnumerable Project( + IEnumerable source, + Func selector) => throw null; + + internal static Task> ProjectAsync( + IEnumerable source, + Func selector) => throw null; + } + + class Test + { + Task FAsync(IEnumerable values) + { + IEnumerable tasks = Helpers.[|Project|](values, value => Task.CompletedTask); + return Task.WhenAll(tasks); + } + } + """; + + var verifyTest = new CSVerify.Test + { + TestCode = test, + FixedCode = test, + }; + verifyTest.TestState.AdditionalFiles.Add(("vs-threading.SyncBlockingMethods.txt", "[Helpers]::Project")); + await verifyTest.RunAsync(); + } + [Fact] public async Task ConditionalTaskWaitInSynchronousMethodReports() { diff --git a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD103UseAsyncOptionAnalyzerTests.cs b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD103UseAsyncOptionAnalyzerTests.cs index 677b716f..4bac4f3a 100644 --- a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD103UseAsyncOptionAnalyzerTests.cs +++ b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD103UseAsyncOptionAnalyzerTests.cs @@ -1983,6 +1983,135 @@ await CSVerify.VerifyAnalyzerAsync( CSVerify.Diagnostic(Descriptor).WithLocation(0).WithArguments("Join", "JoinAsync")); } + [Fact] + public async Task TaskProducingProjectionDoesNotGenerateWarning() + { + string test = """ + using System; + using System.Collections.Generic; + using System.Linq; + using System.Threading.Tasks; + + class Item { } + + class Test + { + async Task ProcessAllAsync(IEnumerable items) + { + await Task.WhenAll(items.Select(ProcessAsync)); + + IEnumerable tasks = items.Select(ProcessAsync); + await Task.WhenAll(tasks); + } + + Task ProcessAsync(Item item) => Task.CompletedTask; + } + + static class AsyncEnumerableExtensions + { + public static Task> SelectAsync( + this IEnumerable source, + Func selector) => Task.FromResult(source.Select(selector)); + } + """; + + await CSVerify.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task TaskLikeValuesFlowingThroughContainersDoNotGenerateWarning() + { + string test = """ + using System; + using System.Collections.Generic; + using System.Threading.Tasks; + + class Test + { + Task ProcessAllAsync(IEnumerable items, Task existingTask) + { + _ = Map(items, ProcessAsync); + _ = Wrap(existingTask); + _ = WrapInArray(existingTask); + _ = existingTask.WrapInArray(); + return Task.CompletedTask; + } + + ValueTask ProcessAsync(int item) => new ValueTask(item); + + static IReadOnlyList Map( + IEnumerable source, + Func selector) => throw null; + + static Task> MapAsync( + IEnumerable source, + Func selector) => throw null; + + static (T, int) Wrap(T value) => (value, 0); + + static Task<(T, int)> WrapAsync(T value) => Task.FromResult((value, 0)); + + static T[] WrapInArray(T value) => new[] { value }; + + static Task WrapInArrayAsync(T value) => Task.FromResult(new[] { value }); + } + + static class TaskCarryingExtensions + { + internal static T[] WrapInArray(this T value) => new[] { value }; + + internal static Task WrapInArrayAsync(this T value) => Task.FromResult(new[] { value }); + } + """; + + await CSVerify.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task AsyncAlternativeStillWarnsWhenTaskLikeValuesDoNotFlowToReturnValue() + { + string test = """ + using System; + using System.Collections.Generic; + using System.Threading.Tasks; + + class Test + { + Task ProcessAllAsync() + { + _ = {|#0:GetTasks|}(); + {|#1:Run|}(() => Task.CompletedTask); + _ = {|#2:Wrap|}(Task.CompletedTask); + _ = {|#3:Load|}(task => { }); + return Task.CompletedTask; + } + + static IEnumerable GetTasks() => throw null; + + static Task> GetTasksAsync() => throw null; + + static void Run(Func action) { } + + static Task RunAsync(Func action) => action(); + + static (Task, int) Wrap(Task task) => (task, 0); + + static Task<(Task, int)> WrapAsync(Task task) => Task.FromResult((task, 0)); + + static IEnumerable Load(Action consumer) => throw null; + + static Task> LoadAsync(Action consumer) => throw null; + } + """; + + await CSVerify.VerifyAnalyzerAsync( + test, + CSVerify.Diagnostic(Descriptor).WithLocation(0).WithArguments("GetTasks", "GetTasksAsync"), + CSVerify.Diagnostic(Descriptor).WithLocation(1).WithArguments("Run", "RunAsync"), + CSVerify.Diagnostic(Descriptor).WithLocation(2).WithArguments("Wrap", "WrapAsync"), + CSVerify.Diagnostic(Descriptor).WithLocation(3).WithArguments("Load", "LoadAsync")); + } + [Fact] public async Task TaskGetAwaiterGetResultInTaskReturningMethodGeneratesWarning() {