fix: Moq1203 false positives for ReturnsAsync and Callback chaining

docs/rules/Moq1203.md

@@ -7,9 +7,9 @@
 | CodeFix  | False   |
 ---
 
-Method setups that have a return type should specify what value to return using `Returns()` or `Throws()`.
+Method setups that have a return type should specify what value to return using `Returns()`, `ReturnsAsync()`, `Throws()`, or `ThrowsAsync()`.
 
-When setting up methods that return values, Moq requires explicit specification of what should be returned or thrown. Without this specification, the setup is incomplete and may not behave as expected during testing.
+When setting up methods that return values, Moq requires explicit specification of what should be returned or thrown. Without this specification, the setup is incomplete and may not behave as expected during testing. The analyzer walks the full method chain, so intermediate calls like `Callback()` do not interfere with detection.
 
 ## Examples of patterns that are flagged by this analyzer
 
@@ -18,43 +18,52 @@ interface IFoo
 {
     bool DoSomething(string value);
     int GetValue();
-    string Calculate(int a, int b);
+    Task<int> BarAsync();
 }
 
 var mock = new Mock<IFoo>();
 
 // Moq1203: Method setup should specify a return value
 mock.Setup(x => x.DoSomething("test"));
 
-// Moq1203: Method setup should specify a return value  
+// Moq1203: Method setup should specify a return value
 mock.Setup(x => x.GetValue());
 
-// Moq1203: Method setup should specify a return value
-mock.Setup(x => x.Calculate(1, 2));
+// Moq1203: Async methods also need a return value specification
+mock.Setup(x => x.BarAsync());
+
+// Moq1203: Callback alone does not satisfy the requirement
+mock.Setup(x => x.GetValue()).Callback(() => { });
 ```
 
 ## Solution
 
-Specify what the method should return using `Returns()` or what exception should be thrown using `Throws()`:
+Specify what the method should return using `Returns()` or `ReturnsAsync()`, or what exception should be thrown using `Throws()` or `ThrowsAsync()`. These methods can appear anywhere in the chain, including after `Callback()`.
 
 ```csharp
 interface IFoo
 {
     bool DoSomething(string value);
     int GetValue();
-    string Calculate(int a, int b);
+    Task<int> BarAsync();
 }
 
 var mock = new Mock<IFoo>();
 
 // Specify return value
 mock.Setup(x => x.DoSomething("test")).Returns(true);
 mock.Setup(x => x.GetValue()).Returns(42);
-mock.Setup(x => x.Calculate(1, 2)).Returns("result");
 
-// Or specify exception to throw
+// Async methods can use ReturnsAsync
+mock.Setup(x => x.BarAsync()).ReturnsAsync(1);
+
+// Callback before Returns is valid
+mock.Setup(x => x.DoSomething("test")).Callback(() => { }).Returns(true);
+mock.Setup(x => x.BarAsync()).Callback(() => { }).ReturnsAsync(1);
+
+// Specify exception to throw
 mock.Setup(x => x.DoSomething("test")).Throws<InvalidOperationException>();
-mock.Setup(x => x.GetValue()).Throws(new ArgumentException());
+mock.Setup(x => x.BarAsync()).ThrowsAsync(new InvalidOperationException());
 ```
 
 ## When this rule does not apply

src/Analyzers/MethodSetupShouldSpecifyReturnValueAnalyzer.cs

@@ -3,14 +3,15 @@
 namespace Moq.Analyzers;
 
 /// <summary>
-/// Method setups that return a value should specify a return value using Returns() or Throws().
+/// Method setups that return a value should specify a return value using
+/// Returns(), ReturnsAsync(), Throws(), or ThrowsAsync().
 /// </summary>
 [DiagnosticAnalyzer(LanguageNames.CSharp)]
 public class MethodSetupShouldSpecifyReturnValueAnalyzer : DiagnosticAnalyzer
 {
     private static readonly LocalizableString Title = "Method setup should specify a return value";
     private static readonly LocalizableString Message = "Method setup for '{0}' should specify a return value";
-    private static readonly LocalizableString Description = "Method setups that return a value should use Returns() or Throws() to specify a return value.";
+    private static readonly LocalizableString Description = "Method setups that return a value should use Returns(), ReturnsAsync(), Throws(), or ThrowsAsync() to specify a return value.";
 
     private static readonly DiagnosticDescriptor Rule = new(
         DiagnosticIds.MethodSetupShouldSpecifyReturnValue,
@@ -19,10 +20,11 @@
         DiagnosticCategory.Usage,
         DiagnosticSeverity.Warning,
         isEnabledByDefault: true,
-        description: Description);
+        description: Description,
+        helpLinkUri: $"https://github.com/rjmurillo/moq.analyzers/blob/{ThisAssembly.GitCommitId}/docs/rules/{DiagnosticIds.MethodSetupShouldSpecifyReturnValue}.md");
 
     /// <inheritdoc />
-    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Rule);
 
     /// <inheritdoc />
     public override void Initialize(AnalysisContext context)
@@ -34,7 +36,7 @@
 
     private static void AnalyzeInvocation(OperationAnalysisContext context)
     {
-        if (!TryGetSetupInvocation(context, out IInvocationOperation? setupInvocation, out _))
+        if (!TryGetSetupInvocation(context, out IInvocationOperation? setupInvocation))
         {
             return;
         }
@@ -44,23 +46,20 @@
             return;
         }
 
-        if (HasReturnValueSpecification(setupInvocation))
+        if (HasReturnValueSpecification(setupInvocation, context.CancellationToken))
         {
             return;
         }
 
-        // Report diagnostic for methods with return types that don't specify a return value
         Diagnostic diagnostic = setupInvocation.Syntax.CreateDiagnostic(Rule, mockedMethod.ToDisplayString());
         context.ReportDiagnostic(diagnostic);
     }
 
     private static bool TryGetSetupInvocation(
         OperationAnalysisContext context,
-        out IInvocationOperation setupInvocation,
-        out MoqKnownSymbols knownSymbols)
+        out IInvocationOperation setupInvocation)
     {
         setupInvocation = null!;
-        knownSymbols = null!;
 
         if (context.Operation is not IInvocationOperation invocationOperation)
         {
@@ -73,7 +72,7 @@
             return false;
         }
 
-        knownSymbols = new MoqKnownSymbols(semanticModel.Compilation);
+        MoqKnownSymbols knownSymbols = new(semanticModel.Compilation);
         IMethodSymbol targetMethod = invocationOperation.TargetMethod;
 
         if (!targetMethod.IsMoqSetupMethod(knownSymbols))
@@ -133,10 +132,14 @@
     }
 
     /// <summary>
-    /// Checks if the setup invocation is followed by a Returns() or Throws() call.
-    /// This uses semantic analysis to verify the method being called.
+    /// Checks if the setup invocation is followed by a return value specification
+    /// anywhere in the method chain (e.g., .Setup().Callback().Returns()).
+    /// Uses semantic analysis to resolve method symbols, including candidate symbols
+    /// when overload resolution fails (common with Moq extension methods).
     /// </summary>
-    private static bool HasReturnValueSpecification(IInvocationOperation setupInvocation)
+    private static bool HasReturnValueSpecification(
+        IInvocationOperation setupInvocation,
+        CancellationToken cancellationToken)
     {
         SyntaxNode setupSyntax = setupInvocation.Syntax;
         SemanticModel? semanticModel = setupInvocation.SemanticModel;
@@ -146,17 +149,48 @@
             return false;
         }
 
-        // Check if the setup call is the target of a member access (method chaining)
-        if (setupSyntax.Parent is Microsoft.CodeAnalysis.CSharp.Syntax.MemberAccessExpressionSyntax memberAccess)
+        SyntaxNode? current = setupSyntax;
+        while (current?.Parent is MemberAccessExpressionSyntax memberAccess)
         {
-            // Get the symbol information for the method being accessed
-            SymbolInfo symbolInfo = semanticModel.GetSymbolInfo(memberAccess.Name);
+            cancellationToken.ThrowIfCancellationRequested();
+
+            SymbolInfo symbolInfo = semanticModel.GetSymbolInfo(memberAccess, cancellationToken);
 
-            // Check if it's a method call and if the method name is Returns or Throws
-            return symbolInfo.Symbol is IMethodSymbol method &&
-                   (string.Equals(method.Name, "Returns", StringComparison.Ordinal) || string.Equals(method.Name, "Throws", StringComparison.Ordinal));
+            if (HasReturnValueSymbol(symbolInfo))
+            {
+                return true;
+            }
+
+            current = memberAccess.Parent as InvocationExpressionSyntax;
         }
 
         return false;
     }
+
+    private static bool HasReturnValueSymbol(SymbolInfo symbolInfo)
+    {
+        if (symbolInfo.CandidateReason == CandidateReason.OverloadResolutionFailure)
+        {
+            foreach (ISymbol candidate in symbolInfo.CandidateSymbols)
+            {
+                if (candidate is IMethodSymbol method && IsReturnValueMethod(method.Name))
+                {
+                    return true;
+                }
+            }
+
+            return false;
+        }
+
+        return symbolInfo.CandidateReason == CandidateReason.None
+            && symbolInfo.Symbol is IMethodSymbol resolved
+            && IsReturnValueMethod(resolved.Name);
+    }
+
+    private static bool IsReturnValueMethod(string methodName) =>
+        methodName switch
+        {
+            "Returns" or "ReturnsAsync" or "Throws" or "ThrowsAsync" => true,
+            _ => false,
+        };
 }

tests/Moq.Analyzers.Test/MethodSetupShouldSpecifyReturnValueAnalyzerTests.cs

@@ -45,14 +45,90 @@ public static IEnumerable<object[]> TestData()
             // Property setups should not trigger this analyzer
             ["""new Mock<IFoo>().Setup(x => x.Name);"""],
             ["""new Mock<IFoo>().SetupGet(x => x.Name);"""],
+
+            // Async method without return specification should flag
+            ["""{|Moq1203:new Mock<IFoo>().Setup(x => x.BarAsync())|};"""],
         ];
 
         return both.Union(edge).WithNamespaces().WithMoqReferenceAssemblyGroups();
     }
 
+    // Regression test data for https://github.com/rjmurillo/moq.analyzers/issues/849
+    // Tests valid return value specifications that should NOT trigger Moq1203.
+    public static IEnumerable<object[]> Issue849_FalsePositiveTestData()
+    {
+        IEnumerable<object[]> data =
+        [
+
+            // ReturnsAsync recognized as a return value specification
+            ["""new Mock<IFoo>().Setup(x => x.BarAsync()).ReturnsAsync(1);"""],
+
+            // ThrowsAsync recognized as a return value specification
+            ["""new Mock<IFoo>().Setup(x => x.BarAsync()).ThrowsAsync(new InvalidOperationException());"""],
+
+            // Callback before Returns should not cause false positive
+            ["""new Mock<IFoo>().Setup(x => x.BarAsync()).Callback(() => { }).Returns(Task.FromResult(1));"""],
+
+            // Callback before ReturnsAsync should not cause false positive
+            ["""new Mock<IFoo>().Setup(x => x.BarAsync()).Callback(() => { }).ReturnsAsync(1);"""],
+
+            // Callback before ThrowsAsync should not cause false positive
+            ["""new Mock<IFoo>().Setup(x => x.BarAsync()).Callback(() => { }).ThrowsAsync(new InvalidOperationException());"""],
+
+            // Callback before Returns on sync method should not cause false positive
+            ["""new Mock<IFoo>().Setup(x => x.DoSomething("test")).Callback(() => { }).Returns(true);"""],
+
+            // Callback before Throws should not cause false positive
+            ["""new Mock<IFoo>().Setup(x => x.DoSomething("test")).Callback(() => { }).Throws<InvalidOperationException>();"""],
+        ];
+
+        return data.WithNamespaces().WithMoqReferenceAssemblyGroups();
+    }
+
+    // Callback-only tests require newer Moq (4.18.4+) API shapes.
+    public static IEnumerable<object[]> CallbackOnlyNewMoqTestData()
+    {
+        IEnumerable<object[]> data =
+        [
+
+            // Callback alone should still require return value specification
+            ["""{|Moq1203:new Mock<IFoo>().Setup(x => x.GetValue())|}.Callback(() => { });"""],
+        ];
+
+        return data.WithNamespaces().WithNewMoqReferenceAssemblyGroups();
+    }
+
     [Theory]
     [MemberData(nameof(TestData))]
     public async Task ShouldAnalyzeMethodSetupReturnValue(string referenceAssemblyGroup, string @namespace, string mock)
+    {
+        await VerifyMock(referenceAssemblyGroup, @namespace, mock);
+    }
+
+    [Theory]
+    [MemberData(nameof(Issue849_FalsePositiveTestData))]
+    public async Task ShouldNotFlagSetupWithReturnsAsyncOrCallbackChaining(string referenceAssemblyGroup, string @namespace, string mock)
+    {
+        await VerifyMock(referenceAssemblyGroup, @namespace, mock);
+    }
+
+    [Theory]
+    [MemberData(nameof(CallbackOnlyNewMoqTestData))]
+    public async Task ShouldFlagCallbackOnlySetupOnNewMoq(string referenceAssemblyGroup, string @namespace, string mock)
+    {
+        await VerifyMock(referenceAssemblyGroup, @namespace, mock);
+    }
+
+    [Theory]
+    [MemberData(nameof(DoppelgangerTestHelper.GetAllCustomMockData), MemberType = typeof(DoppelgangerTestHelper))]
+    public async Task ShouldPassIfCustomMockClassIsUsed(string mockCode)
+    {
+        await Verifier.VerifyAnalyzerAsync(
+            DoppelgangerTestHelper.CreateTestCode(mockCode),
+            ReferenceAssemblyCatalog.Net80WithNewMoq);
+    }
+
+    private async Task VerifyMock(string referenceAssemblyGroup, string @namespace, string mock)
     {
         string source = $$"""
             {{@namespace}}
@@ -62,6 +138,7 @@ public interface IFoo
                 bool DoSomething(string value);
                 int GetValue();
                 int Calculate(int a, int b);
+                Task<int> BarAsync();
                 void DoVoidMethod();
                 void ProcessData(string data);
                 string Name { get; set; }
@@ -80,15 +157,6 @@ private void Test()
 
         await Verifier.VerifyAnalyzerAsync(
             source,
-            referenceAssemblyGroup);
-    }
-
-    [Theory]
-    [MemberData(nameof(DoppelgangerTestHelper.GetAllCustomMockData), MemberType = typeof(DoppelgangerTestHelper))]
-    public async Task ShouldPassIfCustomMockClassIsUsed(string mockCode)
-    {
-        await Verifier.VerifyAnalyzerAsync(
-            DoppelgangerTestHelper.CreateTestCode(mockCode),
-            ReferenceAssemblyCatalog.Net80WithNewMoq);
+            referenceAssemblyGroup).ConfigureAwait(false);
     }
 }