Skip to content

Commit b3c2d21

Browse files
authored
Merge pull request #1467 from microsoft/copilot/fix-1167
Fix VSTHRD110 firing in Expression-valued scenarios
1 parent b90980a commit b3c2d21

File tree

2 files changed

+253
-0
lines changed

2 files changed

+253
-0
lines changed

src/Microsoft.VisualStudio.Threading.Analyzers/AbstractVSTHRD110ObserveResultOfAsyncCallsAnalyzer.cs

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,98 @@ public override void Initialize(AnalysisContext context)
4343
});
4444
}
4545

46+
/// <summary>
47+
/// Determines whether an invocation is within a lambda expression that is being converted to an Expression tree.
48+
/// </summary>
49+
/// <param name="operation">The invocation operation to check.</param>
50+
/// <returns>True if the invocation is within a lambda converted to an Expression; false otherwise.</returns>
51+
private static bool IsWithinExpressionLambda(IInvocationOperation operation)
52+
{
53+
// Walk up the operation tree to find the containing lambda
54+
IOperation? current = operation.Parent;
55+
while (current is not null)
56+
{
57+
if (current is IAnonymousFunctionOperation lambda)
58+
{
59+
// Found a lambda, now check if it's being converted to an Expression<>
60+
return IsLambdaConvertedToExpression(lambda);
61+
}
62+
63+
current = current.Parent;
64+
}
65+
66+
return false;
67+
}
68+
69+
/// <summary>
70+
/// Determines whether a lambda is being converted to an Expression tree type.
71+
/// </summary>
72+
/// <param name="lambda">The lambda operation to check.</param>
73+
/// <returns>True if the lambda is being converted to an Expression; false otherwise.</returns>
74+
private static bool IsLambdaConvertedToExpression(IAnonymousFunctionOperation lambda)
75+
{
76+
// Walk up from the lambda to find conversion or argument operations
77+
IOperation? current = lambda.Parent;
78+
while (current is not null)
79+
{
80+
// Check if the lambda's parent is a conversion operation
81+
if (current is IConversionOperation conversion)
82+
{
83+
// Check if the target type is Expression<> or a related expression tree type
84+
return IsExpressionTreeType(conversion.Type);
85+
}
86+
87+
// Check if the lambda is being passed as an argument to a method expecting Expression<>
88+
if (current is IArgumentOperation argument &&
89+
argument.Parameter?.Type is INamedTypeSymbol parameterType)
90+
{
91+
return IsExpressionTreeType(parameterType);
92+
}
93+
94+
// Allow certain operations to be skipped (like parentheses)
95+
if (current is IParenthesizedOperation)
96+
{
97+
current = current.Parent;
98+
continue;
99+
}
100+
101+
// Stop walking up at other operation types to avoid false positives
102+
break;
103+
}
104+
105+
return false;
106+
}
107+
108+
/// <summary>
109+
/// Determines whether a type is an Expression tree type (Expression&lt;T&gt; or related types).
110+
/// </summary>
111+
/// <param name="type">The type to check.</param>
112+
/// <returns>True if the type is an Expression tree type; false otherwise.</returns>
113+
private static bool IsExpressionTreeType(ITypeSymbol? type)
114+
{
115+
if (type is not INamedTypeSymbol namedType)
116+
{
117+
return false;
118+
}
119+
120+
// Check for System.Linq.Expressions.Expression<T>
121+
if (namedType.Name == "Expression" &&
122+
namedType.ContainingNamespace?.ToDisplayString() == "System.Linq.Expressions" &&
123+
namedType.IsGenericType)
124+
{
125+
return true;
126+
}
127+
128+
// Check for LambdaExpression and other expression types
129+
if (namedType.ContainingNamespace?.ToDisplayString() == "System.Linq.Expressions" &&
130+
(namedType.Name == "LambdaExpression" || namedType.Name.EndsWith("Expression")))
131+
{
132+
return true;
133+
}
134+
135+
return false;
136+
}
137+
46138
private void AnalyzeInvocation(OperationAnalysisContext context, CommonInterest.AwaitableTypeTester awaitableTypes)
47139
{
48140
var operation = (IInvocationOperation)context.Operation;
@@ -57,6 +149,13 @@ private void AnalyzeInvocation(OperationAnalysisContext context, CommonInterest.
57149
return;
58150
}
59151

152+
// Check if this invocation is within a lambda that's being converted to an Expression<>
153+
if (IsWithinExpressionLambda(operation))
154+
{
155+
// This invocation is within a lambda converted to an expression tree, so it's not actually being invoked.
156+
return;
157+
}
158+
60159
// Only consider invocations that are direct statements (or are statements through limited steps).
61160
// Otherwise, we assume their result is awaited, assigned, or otherwise consumed.
62161
IOperation? parentOperation = operation.Parent;

test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD110ObserveResultOfAsyncCallsAnalyzerTests.cs

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -542,4 +542,158 @@ void DoOperation()
542542

543543
await CSVerify.VerifyAnalyzerAsync(test);
544544
}
545+
546+
[Fact]
547+
public async Task ExpressionLambda_ProducesNoDiagnostic()
548+
{
549+
string test = """
550+
using System;
551+
using System.Linq.Expressions;
552+
using System.Threading.Tasks;
553+
554+
interface ILogger
555+
{
556+
Task InfoAsync(string message);
557+
}
558+
559+
class MockVerifier
560+
{
561+
public static void Verify<T>(Expression<Func<T, Task>> expression)
562+
{
563+
}
564+
}
565+
566+
class Test
567+
{
568+
void TestMethod()
569+
{
570+
var logger = new MockLogger();
571+
MockVerifier.Verify<ILogger>(x => x.InfoAsync("test"));
572+
}
573+
}
574+
575+
class MockLogger : ILogger
576+
{
577+
public Task InfoAsync(string message) => Task.CompletedTask;
578+
}
579+
""";
580+
581+
await CSVerify.VerifyAnalyzerAsync(test);
582+
}
583+
584+
[Fact]
585+
public async Task ExpressionFuncLambda_ProducesNoDiagnostic()
586+
{
587+
string test = """
588+
using System;
589+
using System.Linq.Expressions;
590+
using System.Threading.Tasks;
591+
592+
class Test
593+
{
594+
void TestMethod()
595+
{
596+
SomeMethod(x => x.InfoAsync("test"));
597+
}
598+
599+
void SomeMethod(Expression<Func<ILogger, Task>> expression)
600+
{
601+
}
602+
603+
Task InfoAsync(string message) => Task.CompletedTask;
604+
}
605+
606+
interface ILogger
607+
{
608+
Task InfoAsync(string message);
609+
}
610+
""";
611+
612+
await CSVerify.VerifyAnalyzerAsync(test);
613+
}
614+
615+
[Fact]
616+
public async Task MoqLikeScenario_ProducesNoDiagnostic()
617+
{
618+
string test = """
619+
using System;
620+
using System.Linq.Expressions;
621+
using System.Threading.Tasks;
622+
623+
interface ILogger
624+
{
625+
Task InfoAsync(string message);
626+
}
627+
628+
class Mock<T>
629+
{
630+
public void Verify(Expression<Func<T, Task>> expression, Times times, string message)
631+
{
632+
}
633+
}
634+
635+
enum Times
636+
{
637+
Never
638+
}
639+
640+
class Test
641+
{
642+
void TestMethod()
643+
{
644+
var mock = new Mock<ILogger>();
645+
mock.Verify(x => x.InfoAsync("test"), Times.Never, "No Log should have been written");
646+
}
647+
}
648+
""";
649+
650+
await CSVerify.VerifyAnalyzerAsync(test);
651+
}
652+
653+
[Fact]
654+
public async Task DirectTaskCall_StillProducesDiagnostic()
655+
{
656+
string test = """
657+
using System.Threading.Tasks;
658+
659+
class Test
660+
{
661+
void TestMethod()
662+
{
663+
// This should still trigger VSTHRD110 - direct call not in expression
664+
[|TaskReturningMethod()|];
665+
}
666+
667+
Task TaskReturningMethod() => Task.CompletedTask;
668+
}
669+
""";
670+
671+
await CSVerify.VerifyAnalyzerAsync(test);
672+
}
673+
674+
[Fact]
675+
public async Task ExpressionAssignment_ProducesNoDiagnostic()
676+
{
677+
string test = """
678+
using System;
679+
using System.Linq.Expressions;
680+
using System.Threading.Tasks;
681+
682+
interface ILogger
683+
{
684+
Task InfoAsync(string message);
685+
}
686+
687+
class Test
688+
{
689+
void TestMethod()
690+
{
691+
// Assignment to Expression<> variable should not trigger VSTHRD110
692+
Expression<Func<ILogger, Task>> expr = x => x.InfoAsync("test");
693+
}
694+
}
695+
""";
696+
697+
await CSVerify.VerifyAnalyzerAsync(test);
698+
}
545699
}

0 commit comments

Comments
 (0)