From b33edbc1e8dbf84114aa7bbdcf53db8b72bb4d41 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Jun 2025 02:45:38 +0000 Subject: [PATCH 1/4] Initial plan for issue From 19e83e9a1d5c907743f5cef5ab37d699e05404bd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Jun 2025 02:54:41 +0000 Subject: [PATCH 2/4] Implement fix for VSTHRD110 firing in expression-valued scenarios Co-authored-by: AArnott <3548+AArnott@users.noreply.github.com> --- ...HRD110ObserveResultOfAsyncCallsAnalyzer.cs | 84 +++++++++++++++++++ ...0ObserveResultOfAsyncCallsAnalyzerTests.cs | 69 +++++++++++++++ 2 files changed, 153 insertions(+) diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers/AbstractVSTHRD110ObserveResultOfAsyncCallsAnalyzer.cs b/src/Microsoft.VisualStudio.Threading.Analyzers/AbstractVSTHRD110ObserveResultOfAsyncCallsAnalyzer.cs index 864c3f5e4..b4de84fea 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers/AbstractVSTHRD110ObserveResultOfAsyncCallsAnalyzer.cs +++ b/src/Microsoft.VisualStudio.Threading.Analyzers/AbstractVSTHRD110ObserveResultOfAsyncCallsAnalyzer.cs @@ -57,6 +57,13 @@ private void AnalyzeInvocation(OperationAnalysisContext context, CommonInterest. return; } + // Check if this invocation is within a lambda that's being converted to an Expression<> + if (IsWithinExpressionLambda(operation)) + { + // This invocation is within a lambda converted to an expression tree, so it's not actually being invoked. + return; + } + // Only consider invocations that are direct statements (or are statements through limited steps). // Otherwise, we assume their result is awaited, assigned, or otherwise consumed. IOperation? parentOperation = operation.Parent; @@ -86,4 +93,81 @@ private void AnalyzeInvocation(OperationAnalysisContext context, CommonInterest. context.ReportDiagnostic(Diagnostic.Create(Descriptor, operation.Syntax.GetLocation())); } } + + /// + /// Determines whether an invocation is within a lambda expression that is being converted to an Expression tree. + /// + /// The invocation operation to check. + /// True if the invocation is within a lambda converted to an Expression; false otherwise. + private static bool IsWithinExpressionLambda(IInvocationOperation operation) + { + // Walk up the operation tree to find the containing lambda + IOperation? current = operation.Parent; + while (current is not null) + { + if (current is IAnonymousFunctionOperation lambda) + { + // Found a lambda, now check if it's being converted to an Expression<> + return IsLambdaConvertedToExpression(lambda); + } + + current = current.Parent; + } + + return false; + } + + /// + /// Determines whether a lambda is being converted to an Expression tree type. + /// + /// The lambda operation to check. + /// True if the lambda is being converted to an Expression; false otherwise. + private static bool IsLambdaConvertedToExpression(IAnonymousFunctionOperation lambda) + { + // Check if the lambda's parent is a conversion operation + if (lambda.Parent is IConversionOperation conversion) + { + // Check if the target type is Expression<> or a related expression tree type + return IsExpressionTreeType(conversion.Type); + } + + // Check if the lambda is being passed as an argument to a method expecting Expression<> + if (lambda.Parent is IArgumentOperation argument && + argument.Parameter?.Type is INamedTypeSymbol parameterType) + { + return IsExpressionTreeType(parameterType); + } + + return false; + } + + /// + /// Determines whether a type is an Expression tree type (Expression<T> or related types). + /// + /// The type to check. + /// True if the type is an Expression tree type; false otherwise. + private static bool IsExpressionTreeType(ITypeSymbol? type) + { + if (type is not INamedTypeSymbol namedType) + { + return false; + } + + // Check for System.Linq.Expressions.Expression + if (namedType.Name == "Expression" && + namedType.ContainingNamespace?.ToDisplayString() == "System.Linq.Expressions" && + namedType.IsGenericType) + { + return true; + } + + // Check for LambdaExpression and other expression types + if (namedType.ContainingNamespace?.ToDisplayString() == "System.Linq.Expressions" && + (namedType.Name == "LambdaExpression" || namedType.Name.EndsWith("Expression"))) + { + return true; + } + + return false; + } } diff --git a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD110ObserveResultOfAsyncCallsAnalyzerTests.cs b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD110ObserveResultOfAsyncCallsAnalyzerTests.cs index 95cdeffad..8bdd4cbf6 100644 --- a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD110ObserveResultOfAsyncCallsAnalyzerTests.cs +++ b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD110ObserveResultOfAsyncCallsAnalyzerTests.cs @@ -542,4 +542,73 @@ void DoOperation() await CSVerify.VerifyAnalyzerAsync(test); } + + [Fact] + public async Task ExpressionLambda_ProducesNoDiagnostic() + { + string test = """ + using System; + using System.Linq.Expressions; + using System.Threading.Tasks; + + interface ILogger + { + Task InfoAsync(string message); + } + + class MockVerifier + { + public static void Verify(Expression> expression) + { + } + } + + class Test + { + void TestMethod() + { + var logger = new MockLogger(); + MockVerifier.Verify(x => x.InfoAsync("test")); + } + } + + class MockLogger : ILogger + { + public Task InfoAsync(string message) => Task.CompletedTask; + } + """; + + await CSVerify.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task ExpressionFuncLambda_ProducesNoDiagnostic() + { + string test = """ + using System; + using System.Linq.Expressions; + using System.Threading.Tasks; + + class Test + { + void TestMethod() + { + SomeMethod(x => x.InfoAsync("test")); + } + + void SomeMethod(Expression> expression) + { + } + + Task InfoAsync(string message) => Task.CompletedTask; + } + + interface ILogger + { + Task InfoAsync(string message); + } + """; + + await CSVerify.VerifyAnalyzerAsync(test); + } } From b54e5a6afcb8e2254f74af50edd53c04f3195e39 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Jun 2025 02:57:51 +0000 Subject: [PATCH 3/4] Enhanced fix for VSTHRD110 with improved detection and additional tests Co-authored-by: AArnott <3548+AArnott@users.noreply.github.com> --- ...HRD110ObserveResultOfAsyncCallsAnalyzer.cs | 35 +++++--- ...0ObserveResultOfAsyncCallsAnalyzerTests.cs | 85 +++++++++++++++++++ 2 files changed, 110 insertions(+), 10 deletions(-) diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers/AbstractVSTHRD110ObserveResultOfAsyncCallsAnalyzer.cs b/src/Microsoft.VisualStudio.Threading.Analyzers/AbstractVSTHRD110ObserveResultOfAsyncCallsAnalyzer.cs index b4de84fea..0c2d968c4 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers/AbstractVSTHRD110ObserveResultOfAsyncCallsAnalyzer.cs +++ b/src/Microsoft.VisualStudio.Threading.Analyzers/AbstractVSTHRD110ObserveResultOfAsyncCallsAnalyzer.cs @@ -124,18 +124,33 @@ private static bool IsWithinExpressionLambda(IInvocationOperation operation) /// True if the lambda is being converted to an Expression; false otherwise. private static bool IsLambdaConvertedToExpression(IAnonymousFunctionOperation lambda) { - // Check if the lambda's parent is a conversion operation - if (lambda.Parent is IConversionOperation conversion) + // Walk up from the lambda to find conversion or argument operations + IOperation? current = lambda.Parent; + while (current is not null) { - // Check if the target type is Expression<> or a related expression tree type - return IsExpressionTreeType(conversion.Type); - } + // Check if the lambda's parent is a conversion operation + if (current is IConversionOperation conversion) + { + // Check if the target type is Expression<> or a related expression tree type + return IsExpressionTreeType(conversion.Type); + } - // Check if the lambda is being passed as an argument to a method expecting Expression<> - if (lambda.Parent is IArgumentOperation argument && - argument.Parameter?.Type is INamedTypeSymbol parameterType) - { - return IsExpressionTreeType(parameterType); + // Check if the lambda is being passed as an argument to a method expecting Expression<> + if (current is IArgumentOperation argument && + argument.Parameter?.Type is INamedTypeSymbol parameterType) + { + return IsExpressionTreeType(parameterType); + } + + // Allow certain operations to be skipped (like parentheses) + if (current is IParenthesizedOperation) + { + current = current.Parent; + continue; + } + + // Stop walking up at other operation types to avoid false positives + break; } return false; diff --git a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD110ObserveResultOfAsyncCallsAnalyzerTests.cs b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD110ObserveResultOfAsyncCallsAnalyzerTests.cs index 8bdd4cbf6..c9fca9dd6 100644 --- a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD110ObserveResultOfAsyncCallsAnalyzerTests.cs +++ b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD110ObserveResultOfAsyncCallsAnalyzerTests.cs @@ -611,4 +611,89 @@ interface ILogger await CSVerify.VerifyAnalyzerAsync(test); } + + [Fact] + public async Task MoqLikeScenario_ProducesNoDiagnostic() + { + string test = """ + using System; + using System.Linq.Expressions; + using System.Threading.Tasks; + + interface ILogger + { + Task InfoAsync(string message); + } + + class Mock + { + public void Verify(Expression> expression, Times times, string message) + { + } + } + + enum Times + { + Never + } + + class Test + { + void TestMethod() + { + var mock = new Mock(); + mock.Verify(x => x.InfoAsync("test"), Times.Never, "No Log should have been written"); + } + } + """; + + await CSVerify.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task DirectTaskCall_StillProducesDiagnostic() + { + string test = """ + using System.Threading.Tasks; + + class Test + { + void TestMethod() + { + // This should still trigger VSTHRD110 - direct call not in expression + [|TaskReturningMethod()|]; + } + + Task TaskReturningMethod() => Task.CompletedTask; + } + """; + + await CSVerify.VerifyAnalyzerAsync(test); + } + + [Fact] + public async Task ExpressionAssignment_ProducesNoDiagnostic() + { + string test = """ + using System; + using System.Linq.Expressions; + using System.Threading.Tasks; + + interface ILogger + { + Task InfoAsync(string message); + } + + class Test + { + void TestMethod() + { + // Assignment to Expression<> variable should not trigger VSTHRD110 + Expression> expr = x => x.InfoAsync("test"); + } + } + """; + + await CSVerify.VerifyAnalyzerAsync(test); + } } From f986ae9cf63722824e2685deb905e9503a67d8e7 Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Wed, 11 Jun 2025 06:12:36 -0600 Subject: [PATCH 4/4] Reorder members --- ...HRD110ObserveResultOfAsyncCallsAnalyzer.cs | 102 +++++++++--------- 1 file changed, 51 insertions(+), 51 deletions(-) diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers/AbstractVSTHRD110ObserveResultOfAsyncCallsAnalyzer.cs b/src/Microsoft.VisualStudio.Threading.Analyzers/AbstractVSTHRD110ObserveResultOfAsyncCallsAnalyzer.cs index 0c2d968c4..1bc7a2004 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers/AbstractVSTHRD110ObserveResultOfAsyncCallsAnalyzer.cs +++ b/src/Microsoft.VisualStudio.Threading.Analyzers/AbstractVSTHRD110ObserveResultOfAsyncCallsAnalyzer.cs @@ -43,57 +43,6 @@ public override void Initialize(AnalysisContext context) }); } - private void AnalyzeInvocation(OperationAnalysisContext context, CommonInterest.AwaitableTypeTester awaitableTypes) - { - var operation = (IInvocationOperation)context.Operation; - if (operation.Type is null) - { - return; - } - - if (operation.GetContainingFunction() is { } function && this.LanguageUtils.IsAsyncMethod(function.Syntax)) - { - // CS4014 should already take care of this case. - return; - } - - // Check if this invocation is within a lambda that's being converted to an Expression<> - if (IsWithinExpressionLambda(operation)) - { - // This invocation is within a lambda converted to an expression tree, so it's not actually being invoked. - return; - } - - // Only consider invocations that are direct statements (or are statements through limited steps). - // Otherwise, we assume their result is awaited, assigned, or otherwise consumed. - IOperation? parentOperation = operation.Parent; - while (parentOperation is not null) - { - if (parentOperation is IExpressionStatementOperation) - { - // This expression is directly used in a statement. - break; - } - - // This check is where we allow for specific operation types that may appear between the invocation - // and the statement that don't disqualify the invocation search for an invalid pattern. - if (parentOperation is IConditionalAccessOperation) - { - parentOperation = parentOperation.Parent; - } - else - { - // This expression is not directly used in a statement. - return; - } - } - - if (awaitableTypes.IsAwaitableType(operation.Type)) - { - context.ReportDiagnostic(Diagnostic.Create(Descriptor, operation.Syntax.GetLocation())); - } - } - /// /// Determines whether an invocation is within a lambda expression that is being converted to an Expression tree. /// @@ -185,4 +134,55 @@ private static bool IsExpressionTreeType(ITypeSymbol? type) return false; } + + private void AnalyzeInvocation(OperationAnalysisContext context, CommonInterest.AwaitableTypeTester awaitableTypes) + { + var operation = (IInvocationOperation)context.Operation; + if (operation.Type is null) + { + return; + } + + if (operation.GetContainingFunction() is { } function && this.LanguageUtils.IsAsyncMethod(function.Syntax)) + { + // CS4014 should already take care of this case. + return; + } + + // Check if this invocation is within a lambda that's being converted to an Expression<> + if (IsWithinExpressionLambda(operation)) + { + // This invocation is within a lambda converted to an expression tree, so it's not actually being invoked. + return; + } + + // Only consider invocations that are direct statements (or are statements through limited steps). + // Otherwise, we assume their result is awaited, assigned, or otherwise consumed. + IOperation? parentOperation = operation.Parent; + while (parentOperation is not null) + { + if (parentOperation is IExpressionStatementOperation) + { + // This expression is directly used in a statement. + break; + } + + // This check is where we allow for specific operation types that may appear between the invocation + // and the statement that don't disqualify the invocation search for an invalid pattern. + if (parentOperation is IConditionalAccessOperation) + { + parentOperation = parentOperation.Parent; + } + else + { + // This expression is not directly used in a statement. + return; + } + } + + if (awaitableTypes.IsAwaitableType(operation.Type)) + { + context.ReportDiagnostic(Diagnostic.Create(Descriptor, operation.Syntax.GetLocation())); + } + } }