Refactor SetupShouldOnlyBeUsedForOverridableMembersAnalyzer to IOpera…

…tion (#340)

Updates `SetupShouldOnlyBeUsedForOverridableMembersAnalyzer` to use
`IOperation` pattern found in explainer #118

With the IOperation implementation, the symbol specificity is broader on
the detected text span, so the test cases where updated to reflect that
with a marker move to the beginning and end of the inserted symbol.

Additional related updates include:

- Adding utility methods for handling code operations and symbol
retrieval
- Updated symbol overridability detection logic

Fixes #339

src/Analyzers/SetupShouldBeUsedOnlyForOverridableMembersAnalyzer.cs

@@ -1,4 +1,5 @@
 using System.Diagnostics.CodeAnalysis;
+using Microsoft.CodeAnalysis.Operations;
 
 namespace Moq.Analyzers;
 
@@ -28,75 +29,130 @@ public override void Initialize(AnalysisContext context)
     {
         context.EnableConcurrentExecution();
         context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
-        context.RegisterSyntaxNodeAction(Analyze, SyntaxKind.InvocationExpression);
+        context.RegisterOperationAction(AnalyzeInvocation, OperationKind.Invocation);
     }
 
     [SuppressMessage("Design", "MA0051:Method is too long", Justification = "Should be fixed. Ignoring for now to avoid additional churn as part of larger refactor.")]
-    private static void Analyze(SyntaxNodeAnalysisContext context)
+    private static void AnalyzeInvocation(OperationAnalysisContext context)
     {
-        InvocationExpressionSyntax setupInvocation = (InvocationExpressionSyntax)context.Node;
+        if (context.Operation is not IInvocationOperation invocationOperation)
+        {
+            return;
+        }
 
-        MoqKnownSymbols knownSymbols = new(context.SemanticModel.Compilation);
+        SemanticModel? semanticModel = invocationOperation.SemanticModel;
 
-        if (setupInvocation.Expression is not MemberAccessExpressionSyntax memberAccessExpression)
+        if (semanticModel == null)
         {
             return;
         }
 
-        SymbolInfo memberAccessSymbolInfo = context.SemanticModel.GetSymbolInfo(memberAccessExpression, context.CancellationToken);
-        if (memberAccessSymbolInfo.Symbol is null || !context.SemanticModel.IsMoqSetupMethod(knownSymbols, memberAccessSymbolInfo.Symbol, context.CancellationToken))
+        MoqKnownSymbols knownSymbols = new(semanticModel.Compilation);
+        IMethodSymbol targetMethod = invocationOperation.TargetMethod;
+
+        // 1. Check if the invoked method is a Moq Setup method
+        if (!targetMethod.IsMoqSetupMethod(knownSymbols))
         {
             return;
         }
 
-        ExpressionSyntax? mockedMemberExpression = setupInvocation.FindMockedMemberExpressionFromSetupMethod();
-        if (mockedMemberExpression == null)
+        // 2. Attempt to locate the member reference from the Setup expression argument.
+        //    Typically, Moq setup calls have a single lambda argument like x => x.SomeMember.
+        //    We'll extract that member reference or invocation to see whether it is overridable.
+        ISymbol? mockedMemberSymbol = TryGetMockedMemberSymbol(invocationOperation);
+        if (mockedMemberSymbol == null)
         {
             return;
         }
 
-        SymbolInfo symbolInfo = context.SemanticModel.GetSymbolInfo(mockedMemberExpression, context.CancellationToken);
-        ISymbol? symbol = symbolInfo.Symbol;
-
-        if (symbol is null)
+        // 3. Skip if the symbol is part of an interface, those are always "overridable".
+        if (mockedMemberSymbol.ContainingType?.TypeKind == TypeKind.Interface)
         {
             return;
         }
 
-        // Skip if it's part of an interface
-        if (symbol.ContainingType.TypeKind == TypeKind.Interface)
+        // 4. Check if symbol is a property or method, and if it is overridable or is returning a Task (which Moq allows).
+        if (IsPropertyOrMethod(mockedMemberSymbol, knownSymbols))
         {
             return;
         }
 
-        switch (symbol)
+        // 5. If we reach here, the member is neither overridable nor allowed by Moq
+        //    So we report the diagnostic.
+        //
+        // NOTE: The location is on the invocationOperation, which is fairly broad
+        Diagnostic diagnostic = invocationOperation.Syntax.CreateDiagnostic(Rule);
+        context.ReportDiagnostic(diagnostic);
+    }
+
+    private static bool IsPropertyOrMethod(ISymbol mockedMemberSymbol, MoqKnownSymbols knownSymbols)
+    {
+        switch (mockedMemberSymbol)
         {
             case IPropertySymbol propertySymbol:
-                // Check if the property is Task<T>.Result and skip diagnostic if it is
+                // If the property is Task<T>.Result, skip diagnostic
                 if (IsTaskResultProperty(propertySymbol, knownSymbols))
                 {
-                    return;
+                    return true;
                 }
 
                 if (propertySymbol.IsOverridable() || propertySymbol.IsMethodReturnTypeTask())
                 {
-                    return;
+                    return true;
                 }
 
                 break;
+
             case IMethodSymbol methodSymbol:
                 if (methodSymbol.IsOverridable() || methodSymbol.IsMethodReturnTypeTask())
                 {
-                    return;
+                    return true;
                 }
 
                 break;
+
+            default:
+                // If it's not a property or method, we do not issue a diagnostic
+                return true;
         }
 
-        Diagnostic diagnostic = mockedMemberExpression.CreateDiagnostic(Rule);
-        context.ReportDiagnostic(diagnostic);
+        return false;
+    }
+
+    /// <summary>
+    /// Attempts to resolve the symbol representing the member (property or method)
+    /// being referenced in the Setup(...) call. Returns null if it cannot be determined.
+    /// </summary>
+    private static ISymbol? TryGetMockedMemberSymbol(IInvocationOperation moqSetupInvocation)
+    {
+        // Usually the first argument to a Moq Setup(...) is a lambda expression like x => x.Property
+        // or x => x.Method(...). We can look at moqSetupInvocation.Arguments[0].Value to see this.
+        //
+        // In almost all Moq setups, the first argument is the expression (lambda) to be analyzed.
+        if (moqSetupInvocation.Arguments.Length == 0)
+        {
+            return null;
+        }
+
+        IOperation argumentOperation = moqSetupInvocation.Arguments[0].Value;
+
+        // 1) Unwrap conversions (Roslyn often wraps lambdas in IConversionOperation).
+        argumentOperation = argumentOperation.WalkDownImplicitConversion();
+
+        if (argumentOperation is IAnonymousFunctionOperation lambdaOperation)
+        {
+            // If it's a simple lambda of the form x => x.SomeMember,
+            // the body often ends up as an IPropertyReferenceOperation or IInvocationOperation.
+            return lambdaOperation.Body.GetReferencedMemberSymbolFromLambda();
+        }
+
+        // Sometimes it might be a delegate creation or something else. Handle other patterns if needed.
+        return null;
     }
 
+    /// <summary>
+    /// Checks if a property is the 'Result' property on <see cref="Task{TResult}"/>.
+    /// </summary>
     private static bool IsTaskResultProperty(IPropertySymbol propertySymbol, MoqKnownSymbols knownSymbols)
     {
         // Check if the property is named "Result"

src/Analyzers/SetupShouldNotIncludeAsyncResultAnalyzer.cs

@@ -50,7 +50,7 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
         }
 
         SymbolInfo memberAccessSymbolInfo = context.SemanticModel.GetSymbolInfo(memberAccessExpression, context.CancellationToken);
-        if (memberAccessSymbolInfo.Symbol is null || !context.SemanticModel.IsMoqSetupMethod(knownSymbols, memberAccessSymbolInfo.Symbol, context.CancellationToken))
+        if (memberAccessSymbolInfo.Symbol is null || !memberAccessSymbolInfo.Symbol.IsMoqSetupMethod(knownSymbols))
         {
             return;
         }

src/Analyzers/SquiggleCop.Baseline.yaml

@@ -755,7 +755,7 @@
 - {Id: RCS1138, Title: Add summary to documentation comment, Category: Roslynator, DefaultSeverity: Warning, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false}
 - {Id: RCS1139, Title: Add summary element to documentation comment, Category: Roslynator, DefaultSeverity: Warning, IsEnabledByDefault: true, EffectiveSeverities: [Error], IsEverSuppressed: false}
 - {Id: RCS1140, Title: Add exception to documentation comment, Category: Roslynator, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Note], IsEverSuppressed: true}
-- {Id: RCS1141, Title: Add 'param' element to documentation comment, Category: Roslynator, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Note], IsEverSuppressed: false}
+- {Id: RCS1141, Title: Add 'param' element to documentation comment, Category: Roslynator, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Note], IsEverSuppressed: true}
 - {Id: RCS1142, Title: Add 'typeparam' element to documentation comment, Category: Roslynator, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Note], IsEverSuppressed: false}
 - {Id: RCS1143, Title: Simplify coalesce expression, Category: Roslynator, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Note], IsEverSuppressed: true}
 - {Id: RCS1145, Title: Remove redundant 'as' operator, Category: Roslynator, DefaultSeverity: Note, IsEnabledByDefault: true, EffectiveSeverities: [Note], IsEverSuppressed: true}

src/Common/IOperationExtensions.cs

@@ -18,4 +18,68 @@ public static IOperation WalkDownConversion(this IOperation operation)
 
         return operation;
     }
+
+    /// <summary>
+    /// Walks down consecutive implicit conversion operations until an operand is reached that isn't an implicit conversion operation.
+    /// Unlike WalkDownConversion, this method only traverses through implicit conversions, which is particularly useful for
+    /// handling Moq expression lambdas that are typically converted to Expression&lt;Func&lt;...&gt;&gt; or Func&lt;...&gt;.
+    /// </summary>
+    /// <param name="operation">The starting operation.</param>
+    /// <returns>The inner non-conversion operation or the starting operation if it wasn't an implicit conversion operation.</returns>
+    public static IOperation WalkDownImplicitConversion(this IOperation operation)
+    {
+        // Keep peeling off any IConversionOperation layers as long as the conversion is implicit or trivial.
+        // Typically, Moq expression lambdas are converted to Expression<Func<...>> or Func<...>.
+        while (operation is IConversionOperation { Conversion.IsImplicit: true, Operand: not null } conversion)
+        {
+            operation = conversion.Operand;
+        }
+
+        return operation;
+    }
+
+    /// <summary>
+    /// Extracts the referenced member symbol from a lambda operation, handling both block lambdas
+    /// (e.g., => { return x.Property; }) and expression lambdas (e.g., => x.Property).
+    /// </summary>
+    /// <param name="bodyOperation">The lambda body operation to analyze.</param>
+    /// <returns>The referenced member symbol, or <see langword="null" /> if not found or if the operation is <see langword="null" />.</returns>
+    internal static ISymbol? GetReferencedMemberSymbolFromLambda(this IOperation? bodyOperation)
+    {
+        if (bodyOperation is IBlockOperation { Operations.Length: 1 } blockOperation)
+        {
+            // If it's a block lambda (example: => { return x.Property; })
+            return blockOperation.Operations[0].GetSymbolFromOperation();
+        }
+
+        // If it's an expression lambda (example: => x.Property or => x.Method(...))
+        return bodyOperation.GetSymbolFromOperation();
+    }
+
+    /// <summary>
+    /// Extracts a <see cref="ISymbol"/> from an <see cref="IOperation"/>, handling return operations, property references,
+    /// method invocations, events, and fields.
+    /// </summary>
+    /// <param name="operation">The <see cref="IOperation"/> to analyze.</param>
+    /// <returns>The extracted symbol, or <see langword="null" /> if not found or if the <paramref name="operation"/> operation is <see langword="null" />.</returns>
+    private static ISymbol? GetSymbolFromOperation(this IOperation? operation)
+    {
+        switch (operation)
+        {
+            case null:
+                return null;
+            case IReturnOperation returnOp:
+                operation = returnOp.ReturnedValue;
+                break;
+        }
+
+        return operation switch
+        {
+            IPropertyReferenceOperation propertyRef => propertyRef.Property,
+            IInvocationOperation methodOp => methodOp.TargetMethod,
+            IEventReferenceOperation eventRef => eventRef.Event,
+            IFieldReferenceOperation fieldRef => fieldRef.Field,
+            _ => null,
+        };
+    }
 }

src/Common/ISymbolExtensions.cs

@@ -109,6 +109,17 @@ public static bool IsMethodReturnTypeTask(this ISymbol methodSymbol)
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public static bool IsOverridable(this ISymbol symbol)
     {
-        return !symbol.IsSealed && (symbol.IsVirtual || symbol.IsAbstract || symbol.IsOverride);
+        return symbol switch
+        {
+            IMethodSymbol { IsStatic: true } or IPropertySymbol { IsStatic: true } => false,
+            _ when symbol.ContainingType?.TypeKind == TypeKind.Interface => true,
+            _ => !symbol.IsSealed &&
+                  (symbol.IsVirtual || symbol.IsAbstract || symbol is { IsOverride: true, IsSealed: false }),
+        };
+    }
+
+    internal static bool IsMoqSetupMethod(this ISymbol symbol, MoqKnownSymbols knownSymbols)
+    {
+        return symbol.IsInstanceOf(knownSymbols.Mock1Setup) && symbol is IMethodSymbol { IsGenericMethod: true };
     }
 }

src/Common/SemanticModelExtensions.cs

@@ -21,7 +21,7 @@ internal static class SemanticModelExtensions
             return null;
         }
 
-        if (semanticModel.IsMoqSetupMethod(knownSymbols, symbolInfo.Symbol, cancellationToken))
+        if (symbolInfo.Symbol.IsMoqSetupMethod(knownSymbols))
         {
             return invocation;
         }
@@ -65,11 +65,6 @@ internal static bool IsCallbackOrReturnInvocation(this SemanticModel semanticMod
         };
     }
 
-    internal static bool IsMoqSetupMethod(this SemanticModel semanticModel, MoqKnownSymbols knownSymbols, ISymbol symbol, CancellationToken cancellationToken)
-    {
-        return symbol.IsInstanceOf(knownSymbols.Mock1Setup) && symbol is IMethodSymbol { IsGenericMethod: true };
-    }
-
     private static List<T> GetAllMatchingSymbols<T>(this SemanticModel semanticModel, ExpressionSyntax expression)
         where T : class
     {

tests/Moq.Analyzers.Test/SetupShouldBeUsedOnlyForOverridableMembersAnalyzerTests.cs

@@ -8,15 +8,16 @@ public static IEnumerable<object[]> TestData()
     {
         return new object[][]
         {
-            ["""new Mock<BaseSampleClass>().Setup(x => {|Moq1200:x.Calculate()|});"""],
-            ["""new Mock<SampleClass>().Setup(x => {|Moq1200:x.Property|});"""],
-            ["""new Mock<SampleClass>().Setup(x => {|Moq1200:x.Calculate(It.IsAny<int>(), It.IsAny<int>(), It.IsAny<int>())|});"""],
+            ["""{|Moq1200:new Mock<BaseSampleClass>().Setup(x => x.Calculate())|};"""],
+            ["""{|Moq1200:new Mock<SampleClass>().Setup(x => x.Property)|};"""],
+            ["""{|Moq1200:new Mock<SampleClass>().Setup(x => x.Calculate(It.IsAny<int>(), It.IsAny<int>(), It.IsAny<int>()))|};"""],
             ["""new Mock<BaseSampleClass>().Setup(x => x.Calculate(It.IsAny<int>(), It.IsAny<int>()));"""],
             ["""new Mock<ISampleInterface>().Setup(x => x.Calculate(It.IsAny<int>(), It.IsAny<int>()));"""],
             ["""new Mock<ISampleInterface>().Setup(x => x.TestProperty);"""],
             ["""new Mock<SampleClass>().Setup(x => x.Calculate(It.IsAny<int>(), It.IsAny<int>()));"""],
             ["""new Mock<SampleClass>().Setup(x => x.DoSth());"""],
             ["""new Mock<IParameterlessAsyncMethod>().Setup(x => x.DoSomethingAsync().Result).Returns(true);"""],
+            ["""new Mock<SampleClass>().Setup(x => x.Field);"""],
         }.WithNamespaces().WithMoqReferenceAssemblyGroups();
     }
 
@@ -51,6 +52,7 @@ public class SampleClass : BaseSampleClass
                                     public sealed override int Calculate(int a, int b, int c) => 0;
                                     public virtual int DoSth() => 0;
                                     public int Property { get; set; }
+                                    public int Field;
                                 }
 
                                 internal class UnitTest

tests/Moq.Analyzers.Test/SetupShouldNotIncludeAsyncResultAnalyzerTests.cs

@@ -29,7 +29,7 @@ public async Task ShouldAnalyzeSetupForAsyncResult(string referenceAssemblyGroup
               public class AsyncClient
               {
                   public virtual Task TaskAsync() => Task.CompletedTask;
-              
+
                   public virtual Task<string> GenericTaskAsync() => Task.FromResult(string.Empty);
               }