Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 50 additions & 23 deletions src/Analyzers/LinqToMocksExpressionShouldBeValidAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ private static void AnalyzeInvocation(OperationAnalysisContext context)
}

// Analyze lambda expressions in the arguments
AnalyzeMockOfArguments(context, invocationOperation);
AnalyzeMockOfArguments(context, invocationOperation, knownSymbols);
}

/// <summary>
Expand All @@ -75,7 +75,7 @@ private static bool IsValidMockOfInvocation(IInvocationOperation invocation, Moq
targetMethod.ContainingType.Equals(knownSymbols.Mock, SymbolEqualityComparer.Default);
}

private static void AnalyzeMockOfArguments(OperationAnalysisContext context, IInvocationOperation invocationOperation)
private static void AnalyzeMockOfArguments(OperationAnalysisContext context, IInvocationOperation invocationOperation, MoqKnownSymbols knownSymbols)
{
// Look for lambda expressions in the arguments (LINQ to Mocks expressions)
foreach (IArgumentOperation argument in invocationOperation.Arguments)
Expand All @@ -84,19 +84,19 @@ private static void AnalyzeMockOfArguments(OperationAnalysisContext context, IIn

if (argumentValue is IAnonymousFunctionOperation lambdaOperation)
{
AnalyzeLambdaExpression(context, lambdaOperation);
AnalyzeLambdaExpression(context, lambdaOperation, knownSymbols);
}
}
}

private static void AnalyzeLambdaExpression(OperationAnalysisContext context, IAnonymousFunctionOperation lambdaOperation)
private static void AnalyzeLambdaExpression(OperationAnalysisContext context, IAnonymousFunctionOperation lambdaOperation, MoqKnownSymbols knownSymbols)
{
// For LINQ to Mocks, we need to handle more complex expressions like: x => x.Property == "value"
// The lambda body is often a binary expression where the left operand is the member we want to check
AnalyzeLambdaBody(context, lambdaOperation, lambdaOperation.Body);
AnalyzeLambdaBody(context, lambdaOperation, lambdaOperation.Body, knownSymbols);
}

private static void AnalyzeLambdaBody(OperationAnalysisContext context, IAnonymousFunctionOperation lambdaOperation, IOperation? body)
private static void AnalyzeLambdaBody(OperationAnalysisContext context, IAnonymousFunctionOperation lambdaOperation, IOperation? body, MoqKnownSymbols knownSymbols)
{
if (body == null)
{
Expand All @@ -107,18 +107,20 @@ private static void AnalyzeLambdaBody(OperationAnalysisContext context, IAnonymo
{
case IBlockOperation blockOp when blockOp.Operations.Length == 1:
// Handle block lambdas with return statements
AnalyzeLambdaBody(context, lambdaOperation, blockOp.Operations[0]);
AnalyzeLambdaBody(context, lambdaOperation, blockOp.Operations[0], knownSymbols);
break;

case IReturnOperation returnOp:
// Handle return statements
AnalyzeLambdaBody(context, lambdaOperation, returnOp.ReturnedValue);
AnalyzeLambdaBody(context, lambdaOperation, returnOp.ReturnedValue, knownSymbols);
break;

case IBinaryOperation binaryOp:
// Handle binary expressions like equality comparisons
AnalyzeMemberOperations(context, lambdaOperation, binaryOp.LeftOperand);
AnalyzeMemberOperations(context, lambdaOperation, binaryOp.RightOperand);
// Analyze each operand independently. The IsRootedInLambdaParameter guard
// in AnalyzeMemberOperations filters out operands not rooted in the lambda
// parameter (e.g., static constants, enum values).
AnalyzeMemberOperations(context, lambdaOperation, binaryOp.LeftOperand, knownSymbols);
AnalyzeMemberOperations(context, lambdaOperation, binaryOp.RightOperand, knownSymbols);
Comment thread
cursor[bot] marked this conversation as resolved.
break;

case IPropertyReferenceOperation propertyRef:
Expand All @@ -137,35 +139,60 @@ private static void AnalyzeLambdaBody(OperationAnalysisContext context, IAnonymo
break;

default:
// For other complex expressions, try to recursively find member references
// Route children through AnalyzeMemberOperations so they pass through the
// IsRootedInLambdaParameter guard. Calling AnalyzeLambdaBody directly would
// bypass the guard for operation kinds not enumerated above (e.g.,
// IConditionalOperation, ICoalesceOperation).
foreach (IOperation childOperation in body.ChildOperations)
{
AnalyzeLambdaBody(context, lambdaOperation, childOperation);
AnalyzeMemberOperations(context, lambdaOperation, childOperation, knownSymbols);
}

break;
}
}

private static void AnalyzeMemberOperations(OperationAnalysisContext context, IAnonymousFunctionOperation lambdaOperation, IOperation? operation)
/// <summary>
/// Guards member analysis by filtering out operations not rooted in the lambda parameter,
/// then delegates to <see cref="AnalyzeLambdaBody"/> for recursive analysis.
/// </summary>
/// <remarks>
/// <para>
/// This method is the single entry point for all recursive member analysis. Every code path
/// in <see cref="AnalyzeLambdaBody"/> that descends into child operations must route through
/// this method. The <see cref="IOperationExtensions.IsRootedInLambdaParameter"/> guard is
/// applied only to leaf member operations (<see cref="IMemberReferenceOperation"/> and
/// <see cref="IInvocationOperation"/>). Composite operations (e.g., <c>IBinaryOperation</c>
/// for chained <c>&amp;&amp;</c>/<c>||</c>/<c>==</c>) pass through to
/// <see cref="AnalyzeLambdaBody"/> for decomposition. Blocking composite operations would
/// cause false negatives for chained comparisons (see GitHub issue #1010).
/// </para>
/// <para>
/// Nested <c>Mock.Of</c> calls are excluded to prevent false positives from inner mock
/// expressions that have their own lambda parameters.
/// </para>
/// </remarks>
private static void AnalyzeMemberOperations(OperationAnalysisContext context, IAnonymousFunctionOperation lambdaOperation, IOperation operation, MoqKnownSymbols knownSymbols)
{
if (operation == null)
// Don't recursively analyze nested Mock.Of calls to avoid false positives
if (operation is IInvocationOperation invocation && IsValidMockOfInvocation(invocation, knownSymbols))
{
return;
}

// Don't recursively analyze nested Mock.Of calls to avoid false positives
if (operation is IInvocationOperation invocation)
// Only apply the lambda-parameter guard to leaf member operations (property,
// field, event, method). Composite operations (IBinaryOperation for &&/||/==,
// IConditionalOperation, etc.) must pass through to AnalyzeLambdaBody for
// decomposition; blocking them here causes false negatives for chained
// comparisons like `c.Prop == "a" && c.Other == "b"`.
if (operation is (IMemberReferenceOperation or IInvocationOperation)
&& !operation.IsRootedInLambdaParameter(lambdaOperation))
{
MoqKnownSymbols knownSymbols = new(context.Operation.SemanticModel!.Compilation);
if (IsValidMockOfInvocation(invocation, knownSymbols))
{
return; // Skip analyzing nested Mock.Of calls
}
return;
}

// Recursively analyze the operation to find member references
AnalyzeLambdaBody(context, lambdaOperation, operation);
AnalyzeLambdaBody(context, lambdaOperation, operation, knownSymbols);
}

private static void AnalyzeMemberSymbol(OperationAnalysisContext context, ISymbol memberSymbol, IAnonymousFunctionOperation lambdaOperation)
Expand Down
70 changes: 70 additions & 0 deletions src/Common/IOperationExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,76 @@
internal static SyntaxNode? GetReferencedMemberSyntaxFromLambda(this IOperation? bodyOperation)
=> TraverseLambdaBody(bodyOperation, static op => op.GetSyntaxFromOperation());

/// <summary>
/// Determines whether an operation's receiver chain terminates in a parameter of the
/// given lambda. Walks instance receivers (property, method, field, event) and transparent
/// wrappers (conversion, parenthesized) until it reaches a
/// <see cref="IParameterReferenceOperation"/> or a terminal node.
/// </summary>
/// <remarks>
/// <para>
/// This method exists because <c>IAnonymousFunctionOperation.GetCaptures()</c> is an
/// internal Roslyn API and cannot be used by analyzers. Even if it were public, it
/// solves a different problem: it reports closed-over variables, not whether a member
/// access chain originates from the lambda parameter.
/// </para>
/// <para>
/// Use this method before flagging member accesses inside lambda expression analysis
/// to distinguish mock setup members (rooted in the lambda parameter) from value
/// expressions (static members, external locals, constants).
/// </para>
/// </remarks>
/// <param name="operation">The operation whose receiver chain to walk.</param>
/// <param name="lambdaOperation">The lambda whose parameter to match against.</param>
/// <returns>
/// <see langword="true"/> if the receiver chain terminates in the lambda parameter;
/// <see langword="false"/> otherwise.
/// </returns>
internal static bool IsRootedInLambdaParameter(

Check warning on line 88 in src/Common/IOperationExtensions.cs

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

src/Common/IOperationExtensions.cs#L88

Method IOperationExtensions::IsRootedInLambdaParameter has a cyclomatic complexity of 11 (limit is 8)
this IOperation operation,
IAnonymousFunctionOperation lambdaOperation)
{
IParameterSymbol? lambdaParameter = lambdaOperation.Symbol.Parameters.FirstOrDefault();
IOperation? current = operation;
while (true)
{
switch (current)
{
case IParameterReferenceOperation paramRef:
return lambdaParameter is not null &&
SymbolEqualityComparer.Default.Equals(paramRef.Parameter, lambdaParameter);

case IMemberReferenceOperation memberRef:
if (memberRef.Instance == null)
{
return false; // Static member access
}

current = memberRef.Instance;
break;

case IInvocationOperation invocationOp:
if (invocationOp.Instance == null)
{
return false; // Static method call
}

current = invocationOp.Instance;
break;

case IConversionOperation conversionOp:
current = conversionOp.Operand;
break;

default:
// IParenthesizedOperation is intentionally omitted. The C# compiler
// never emits it in IOperation trees (VB.NET only), and this analyzer
// targets C# exclusively via [DiagnosticAnalyzer(LanguageNames.CSharp)].
return false;
}
Comment thread
rjmurillo marked this conversation as resolved.
}
}

/// <summary>
/// Traverses a lambda body operation to extract a value. For block lambdas, iterates all
/// operations and returns the first non-null result (handling <see cref="System.Action{T}"/> lambdas with multiple
Expand Down
Loading
Loading