From 04a1051e66bcbba506ead09df524d9bac9194d40 Mon Sep 17 00:00:00 2001 From: Richard Murillo <6811113+rjmurillo@users.noreply.github.com> Date: Sat, 28 Feb 2026 17:03:48 -0800 Subject: [PATCH 01/10] feat: add Moq1208 analyzer for Returns() delegate type mismatch on async methods Detects when .Returns() is called with a delegate whose return type does not match the async method's return type (e.g., returns int instead of Task), which causes a runtime MockException. Includes a code fix to transform Returns to ReturnsAsync. Closes #767 Co-Authored-By: Claude Opus 4.6 --- docs/rules/Moq1208.md | 77 ++++++ src/Analyzers/AnalyzerReleases.Unshipped.md | 1 + ...ReturnsDelegateShouldReturnTaskAnalyzer.cs | 227 ++++++++++++++++++ .../ReturnsDelegateShouldReturnTaskFixer.cs | 73 ++++++ src/Common/DiagnosticIds.cs | 1 + ...nsDelegateShouldReturnTaskAnalyzerTests.cs | 141 +++++++++++ ...turnsDelegateShouldReturnTaskFixerTests.cs | 86 +++++++ 7 files changed, 606 insertions(+) create mode 100644 docs/rules/Moq1208.md create mode 100644 src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs create mode 100644 src/CodeFixes/ReturnsDelegateShouldReturnTaskFixer.cs create mode 100644 tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs create mode 100644 tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskFixerTests.cs diff --git a/docs/rules/Moq1208.md b/docs/rules/Moq1208.md new file mode 100644 index 000000000..60072a712 --- /dev/null +++ b/docs/rules/Moq1208.md @@ -0,0 +1,77 @@ +# Moq1208: Returns() delegate type mismatch on async method setup + +| Item | Value | +| -------- | ----- | +| Enabled | True | +| Severity | Warning | +| CodeFix | True | +--- + +When `.Returns()` is called with a delegate that returns a non-Task type on an async method setup, Moq throws a runtime `MockException`. The delegate return type must match the method return type. For async methods returning `Task` or `ValueTask`, the delegate must return `Task` or `ValueTask`, not `T` directly. + +``` +MockException: Invalid callback. Setup on method with return type 'Task' cannot invoke callback with return type 'int'. +``` + +## Examples of patterns that are flagged by this analyzer + +```csharp +public interface IService +{ + Task GetValueAsync(); + Task GetNameAsync(); + ValueTask GetValueTaskAsync(); +} + +var mock = new Mock(); + +// These patterns are flagged: +mock.Setup(x => x.GetValueAsync()).Returns(() => 42); // Moq1208: Returns() delegate type mismatch on async method setup +mock.Setup(x => x.GetNameAsync()).Returns(() => "hello"); // Moq1208: Returns() delegate type mismatch on async method setup +mock.Setup(x => x.GetValueTaskAsync()).Returns(() => 42); // Moq1208: Returns() delegate type mismatch on async method setup +``` + +## Solution + +```csharp +public interface IService +{ + Task GetValueAsync(); + Task GetNameAsync(); + ValueTask GetValueTaskAsync(); +} + +var mock = new Mock(); + +// Use ReturnsAsync instead (recommended): +mock.Setup(x => x.GetValueAsync()).ReturnsAsync(42); +mock.Setup(x => x.GetValueAsync()).ReturnsAsync(() => 42); + +// Or return Task explicitly: +mock.Setup(x => x.GetValueAsync()).Returns(() => Task.FromResult(42)); +mock.Setup(x => x.GetNameAsync()).Returns(() => Task.FromResult("hello")); +mock.Setup(x => x.GetValueTaskAsync()).Returns(() => new ValueTask(42)); +``` + +## Suppress a warning + +If you just want to suppress a single violation, add preprocessor directives to +your source file to disable and then re-enable the rule. + +```csharp +#pragma warning disable Moq1208 +mock.Setup(x => x.GetValueAsync()).Returns(() => 42); // Moq1208: Returns() delegate type mismatch on async method setup +#pragma warning restore Moq1208 +``` + +To disable the rule for a file, folder, or project, set its severity to `none` +in the +[configuration file](https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/configuration-files). + +```ini +[*.{cs,vb}] +dotnet_diagnostic.Moq1208.severity = none +``` + +For more information, see +[How to suppress code analysis warnings](https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/suppress-warnings). diff --git a/src/Analyzers/AnalyzerReleases.Unshipped.md b/src/Analyzers/AnalyzerReleases.Unshipped.md index 80da4abd8..adc5e931c 100644 --- a/src/Analyzers/AnalyzerReleases.Unshipped.md +++ b/src/Analyzers/AnalyzerReleases.Unshipped.md @@ -17,6 +17,7 @@ Moq1204 | Usage | Warning | RaisesEventArgumentsShouldMatchEventSignatureAnalyze Moq1205 | Usage | Warning | EventSetupHandlerShouldMatchEventTypeAnalyzer (updated category from Moq to Usage) Moq1206 | Usage | Warning | ReturnsAsyncShouldBeUsedForAsyncMethodsAnalyzer (updated category from Moq to Usage) Moq1207 | Usage | Error | SetupSequenceShouldBeUsedOnlyForOverridableMembersAnalyzer (updated category from Moq to Usage) +Moq1208 | Usage | Warning | ReturnsDelegateShouldReturnTaskAnalyzer Moq1210 | Usage | Error | VerifyShouldBeUsedOnlyForOverridableMembersAnalyzer (updated category from Moq to Usage) Moq1300 | Usage | Error | AsShouldBeUsedOnlyForInterfaceAnalyzer (updated category from Moq to Usage) Moq1301 | Usage | Warning | Mock.Get() should not take literals (updated category from Moq to Usage) diff --git a/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs b/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs new file mode 100644 index 000000000..f141e0c3b --- /dev/null +++ b/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs @@ -0,0 +1,227 @@ +using System.Diagnostics.CodeAnalysis; + +namespace Moq.Analyzers; + +/// +/// Returns() delegate on async method setup should return Task/ValueTask to match the mocked method's return type. +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public class ReturnsDelegateShouldReturnTaskAnalyzer : DiagnosticAnalyzer +{ + private static readonly LocalizableString Title = "Moq: Returns() delegate type mismatch on async method"; + private static readonly LocalizableString Message = "Returns() delegate for async method '{0}' should return a Task type, not '{1}'. Use ReturnsAsync() or return Task.FromResult()."; + private static readonly LocalizableString Description = "Returns() delegate on async method setup should return Task/ValueTask. Use ReturnsAsync() or wrap with Task.FromResult()."; + + private static readonly DiagnosticDescriptor Rule = new( + DiagnosticIds.ReturnsDelegateMismatchOnAsyncMethod, + Title, + Message, + DiagnosticCategory.Usage, + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: Description, + helpLinkUri: $"https://github.com/rjmurillo/moq.analyzers/blob/{ThisAssembly.GitCommitId}/docs/rules/{DiagnosticIds.ReturnsDelegateMismatchOnAsyncMethod}.md"); + + /// + public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create(Rule); + + /// + public override void Initialize(AnalysisContext context) + { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.RegisterSyntaxNodeAction(Analyze, SyntaxKind.InvocationExpression); + } + + private static void Analyze(SyntaxNodeAnalysisContext context) + { + MoqKnownSymbols knownSymbols = new(context.SemanticModel.Compilation); + + InvocationExpressionSyntax invocation = (InvocationExpressionSyntax)context.Node; + + if (!IsReturnsMethodCallWithSyncDelegate(invocation, context.SemanticModel, knownSymbols)) + { + return; + } + + InvocationExpressionSyntax? setupInvocation = FindSetupInvocation(invocation, context.SemanticModel, knownSymbols); + if (setupInvocation == null) + { + return; + } + + if (!TryGetMismatchInfo(setupInvocation, invocation, context.SemanticModel, knownSymbols, out string? methodName, out ITypeSymbol? delegateReturnType)) + { + return; + } + + // Report diagnostic spanning from "Returns" identifier through the closing paren + MemberAccessExpressionSyntax memberAccess = (MemberAccessExpressionSyntax)invocation.Expression; + int startPos = memberAccess.Name.SpanStart; + int endPos = invocation.Span.End; + Microsoft.CodeAnalysis.Text.TextSpan span = Microsoft.CodeAnalysis.Text.TextSpan.FromBounds(startPos, endPos); + Location location = Location.Create(invocation.SyntaxTree, span); + + Diagnostic diagnostic = location.CreateDiagnostic(Rule, methodName, delegateReturnType.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat)); + context.ReportDiagnostic(diagnostic); + } + + private static bool IsReturnsMethodCallWithSyncDelegate(InvocationExpressionSyntax invocation, SemanticModel semanticModel, MoqKnownSymbols knownSymbols) + { + if (invocation.Expression is not MemberAccessExpressionSyntax) + { + return false; + } + + // Query the invocation (not the MemberAccessExpressionSyntax) so Roslyn has argument context + // for overload resolution. Fall back to CandidateSymbols for delegate overloads. + SymbolInfo symbolInfo = semanticModel.GetSymbolInfo(invocation); + bool isReturnsMethod = symbolInfo.Symbol is IMethodSymbol method + ? method.IsMoqReturnsMethod(knownSymbols) + : symbolInfo.CandidateSymbols + .OfType() + .Any(m => m.IsMoqReturnsMethod(knownSymbols)); + + if (!isReturnsMethod) + { + return false; + } + + return HasSyncDelegateArgument(invocation); + } + + private static bool HasSyncDelegateArgument(InvocationExpressionSyntax invocation) + { + if (invocation.ArgumentList.Arguments.Count == 0) + { + return false; + } + + ExpressionSyntax firstArgument = invocation.ArgumentList.Arguments[0].Expression; + + // Must be a lambda or delegate. Raw values (not delegates) are a different overload. + if (firstArgument is not LambdaExpressionSyntax lambda) + { + return false; + } + + // Exclude async lambdas. Those are Moq1206's domain. + return !lambda.AsyncKeyword.IsKind(SyntaxKind.AsyncKeyword); + } + + private static InvocationExpressionSyntax? FindSetupInvocation(InvocationExpressionSyntax returnsInvocation, SemanticModel semanticModel, MoqKnownSymbols knownSymbols) + { + // Walk up the fluent chain to find Setup. Handles patterns like: + // mock.Setup(...).Returns(...) + // mock.Setup(...).Callback(...).Returns(...) + if (returnsInvocation.Expression is not MemberAccessExpressionSyntax memberAccess) + { + return null; + } + + ExpressionSyntax current = memberAccess.Expression; + + while (true) + { + ExpressionSyntax unwrapped = current.WalkDownParentheses(); + + if (unwrapped is not InvocationExpressionSyntax candidateInvocation) + { + return null; + } + + if (candidateInvocation.Expression is not MemberAccessExpressionSyntax candidateMemberAccess) + { + return null; + } + + SymbolInfo symbolInfo = semanticModel.GetSymbolInfo(candidateMemberAccess); + if (symbolInfo.Symbol != null && symbolInfo.Symbol.IsMoqSetupMethod(knownSymbols)) + { + return candidateInvocation; + } + + // Continue walking up the chain (past Callback, etc.) + current = candidateMemberAccess.Expression; + } + } + + private static bool TryGetMismatchInfo( + InvocationExpressionSyntax setupInvocation, + InvocationExpressionSyntax returnsInvocation, + SemanticModel semanticModel, + MoqKnownSymbols knownSymbols, + [NotNullWhen(true)] out string? methodName, + [NotNullWhen(true)] out ITypeSymbol? delegateReturnType) + { + methodName = null; + delegateReturnType = null; + + // Get the mocked method from the Setup lambda + ExpressionSyntax? mockedMemberExpression = setupInvocation.FindMockedMemberExpressionFromSetupMethod(); + if (mockedMemberExpression == null) + { + return false; + } + + SymbolInfo mockedSymbolInfo = semanticModel.GetSymbolInfo(mockedMemberExpression); + if (mockedSymbolInfo.Symbol is not IMethodSymbol mockedMethod) + { + return false; + } + + // The mocked method must return Task or ValueTask (generic variants). + // Non-generic Task/ValueTask have no inner type to mismatch against. + ITypeSymbol returnType = mockedMethod.ReturnType; + if (returnType is not INamedTypeSymbol { IsGenericType: true }) + { + return false; + } + + if (!returnType.IsTaskOrValueTaskType(knownSymbols)) + { + return false; + } + + // Get the delegate's return type from the lambda argument + delegateReturnType = GetDelegateReturnType(returnsInvocation, semanticModel); + if (delegateReturnType == null) + { + return false; + } + + // If the delegate already returns a Task/ValueTask type, no mismatch + if (delegateReturnType.IsTaskOrValueTaskType(knownSymbols)) + { + return false; + } + + methodName = mockedMethod.Name; + return true; + } + + private static ITypeSymbol? GetDelegateReturnType(InvocationExpressionSyntax returnsInvocation, SemanticModel semanticModel) + { + if (returnsInvocation.ArgumentList.Arguments.Count == 0) + { + return null; + } + + ExpressionSyntax firstArgument = returnsInvocation.ArgumentList.Arguments[0].Expression; + + if (firstArgument is not LambdaExpressionSyntax lambda) + { + return null; + } + + // Get the type info of the lambda's body expression to determine what type it returns + ExpressionSyntax? bodyExpression = lambda.Body as ExpressionSyntax; + if (bodyExpression == null) + { + return null; + } + + TypeInfo typeInfo = semanticModel.GetTypeInfo(bodyExpression); + return typeInfo.Type; + } +} diff --git a/src/CodeFixes/ReturnsDelegateShouldReturnTaskFixer.cs b/src/CodeFixes/ReturnsDelegateShouldReturnTaskFixer.cs new file mode 100644 index 000000000..141d7615f --- /dev/null +++ b/src/CodeFixes/ReturnsDelegateShouldReturnTaskFixer.cs @@ -0,0 +1,73 @@ +using System.Composition; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; + +namespace Moq.CodeFixes; + +/// +/// Fixes for (Moq1208). +/// Replaces .Returns(delegate) with .ReturnsAsync(delegate) when the +/// mocked method is async and the delegate returns the unwrapped type. +/// +[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(ReturnsDelegateShouldReturnTaskFixer))] +[Shared] +public sealed class ReturnsDelegateShouldReturnTaskFixer : CodeFixProvider +{ + private static readonly string Title = "Use ReturnsAsync instead of Returns"; + + /// + public override ImmutableArray FixableDiagnosticIds { get; } = + ImmutableArray.Create(DiagnosticIds.ReturnsDelegateMismatchOnAsyncMethod); + + /// + public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + /// + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + SyntaxNode? root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + if (root == null) + { + return; + } + + Diagnostic diagnostic = context.Diagnostics[0]; + + // The diagnostic span starts at the "Returns" identifier. Use FindToken to land + // on that token, then walk up to the enclosing MemberAccessExpressionSyntax. + SyntaxToken token = root.FindToken(diagnostic.Location.SourceSpan.Start); + MemberAccessExpressionSyntax? memberAccess = token.Parent?.FirstAncestorOrSelf(); + if (memberAccess == null) + { + return; + } + + if (!string.Equals(memberAccess.Name.Identifier.ValueText, "Returns", StringComparison.Ordinal)) + { + return; + } + + context.RegisterCodeFix( + CodeAction.Create( + title: Title, + createChangedDocument: _ => ReplaceReturnsWithReturnsAsync(context.Document, root, memberAccess), + equivalenceKey: Title), + diagnostic); + } + + private static Task ReplaceReturnsWithReturnsAsync( + Document document, + SyntaxNode root, + MemberAccessExpressionSyntax memberAccess) + { + SimpleNameSyntax oldName = memberAccess.Name; + IdentifierNameSyntax newName = SyntaxFactory.IdentifierName("ReturnsAsync") + .WithLeadingTrivia(oldName.GetLeadingTrivia()) + .WithTrailingTrivia(oldName.GetTrailingTrivia()); + + MemberAccessExpressionSyntax newMemberAccess = memberAccess.WithName(newName); + SyntaxNode newRoot = root.ReplaceNode(memberAccess, newMemberAccess); + + return Task.FromResult(document.WithSyntaxRoot(newRoot)); + } +} diff --git a/src/Common/DiagnosticIds.cs b/src/Common/DiagnosticIds.cs index 9af16ce95..10c69a74a 100644 --- a/src/Common/DiagnosticIds.cs +++ b/src/Common/DiagnosticIds.cs @@ -17,6 +17,7 @@ internal static class DiagnosticIds internal const string EventSetupHandlerShouldMatchEventType = "Moq1205"; internal const string ReturnsAsyncShouldBeUsedForAsyncMethods = "Moq1206"; internal const string SetupSequenceOnlyUsedForOverridableMembers = "Moq1207"; + internal const string ReturnsDelegateMismatchOnAsyncMethod = "Moq1208"; internal const string VerifyOnlyUsedForOverridableMembers = "Moq1210"; internal const string AsShouldOnlyBeUsedForInterfacesRuleId = "Moq1300"; internal const string MockGetShouldNotTakeLiterals = "Moq1301"; diff --git a/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs b/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs new file mode 100644 index 000000000..6ee6e9f36 --- /dev/null +++ b/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs @@ -0,0 +1,141 @@ +using Verifier = Moq.Analyzers.Test.Helpers.AnalyzerVerifier; + +namespace Moq.Analyzers.Test; + +public class ReturnsDelegateShouldReturnTaskAnalyzerTests(ITestOutputHelper output) +{ + public static IEnumerable ValidTestData() + { + IEnumerable data = new object[][] + { + // Delegate returns Task (correct) + ["""new Mock().Setup(c => c.GetValueAsync()).Returns(() => Task.FromResult(42));"""], + + // Uses ReturnsAsync (correct, different overload) + ["""new Mock().Setup(c => c.GetValueAsync()).ReturnsAsync(42);"""], + + // Direct value, not a delegate + ["""new Mock().Setup(c => c.GetSync()).Returns(42);"""], + + // Non-async method with sync delegate (no mismatch) + ["""new Mock().Setup(c => c.GetSync()).Returns(() => 42);"""], + + // Async lambda (Moq1206's domain, not ours) + ["""new Mock().Setup(c => c.GetValueAsync()).Returns(async () => 42);"""], + + // Setup without Returns call + ["""new Mock().Setup(c => c.GetValueAsync());"""], + + // Delegate returns ValueTask (correct) + ["""new Mock().Setup(c => c.GetValueTaskAsync()).Returns(() => ValueTask.FromResult(42));"""], + + // Chained Callback with correct Task return + ["""new Mock().Setup(c => c.GetValueAsync()).Callback(() => { }).Returns(() => Task.FromResult(42));"""], + + // Non-generic Task method with sync delegate (no mismatch, Task has no inner type) + ["""new Mock().Setup(c => c.DoAsync()).Returns(() => Task.CompletedTask);"""], + + // Parenthesized Setup with ReturnsAsync (valid) + ["""(new Mock().Setup(c => c.GetValueAsync())).ReturnsAsync(42);"""], + }; + + return data.WithNamespaces().WithMoqReferenceAssemblyGroups(); + } + + public static IEnumerable InvalidTestData() + { + IEnumerable data = new object[][] + { + // Sync delegate returning int on Task method + ["""new Mock().Setup(c => c.GetValueAsync()).{|Moq1208:Returns(() => 42)|};"""], + + // Sync delegate returning string on Task method + ["""new Mock().Setup(c => c.GetNameAsync()).{|Moq1208:Returns(() => "hello")|};"""], + + // Sync delegate returning int on ValueTask method + ["""new Mock().Setup(c => c.GetValueTaskAsync()).{|Moq1208:Returns(() => 42)|};"""], + + // Parenthesized Setup with sync delegate mismatch + ["""(new Mock().Setup(c => c.GetValueAsync())).{|Moq1208:Returns(() => 42)|};"""], + + // Chained Callback then Returns with sync delegate mismatch + ["""new Mock().Setup(c => c.GetValueAsync()).Callback(() => { }).{|Moq1208:Returns(() => 42)|};"""], + }; + + return data.WithNamespaces().WithMoqReferenceAssemblyGroups(); + } + + public static IEnumerable DelegateOverloadTestData() + { + IEnumerable data = new object[][] + { + // Sync delegate with parameter returning wrong type on Task method + ["""new Mock().Setup(c => c.ProcessAsync(It.IsAny())).{|Moq1208:Returns((string x) => x.Length)|};"""], + }; + + return data.WithNamespaces().WithMoqReferenceAssemblyGroups(); + } + + [Theory] + [MemberData(nameof(ValidTestData))] + public async Task ShouldNotTriggerOnValidPatterns(string referenceAssemblyGroup, string @namespace, string mock) + { + await VerifyAsync(referenceAssemblyGroup, @namespace, mock); + } + + [Theory] + [MemberData(nameof(InvalidTestData))] + public async Task ShouldTriggerOnSyncDelegateMismatch(string referenceAssemblyGroup, string @namespace, string mock) + { + await VerifyAsync(referenceAssemblyGroup, @namespace, mock); + } + + [Theory] + [MemberData(nameof(DelegateOverloadTestData))] + public async Task ShouldFlagSyncDelegateLambdaWithParameterInReturns(string referenceAssemblyGroup, string @namespace, string mock) + { + await VerifyAsync(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 VerifyAsync(string referenceAssemblyGroup, string @namespace, string mock) + { + string source = + $$""" + {{@namespace}} + + public class AsyncService + { + public virtual Task GetValueAsync() => Task.FromResult(0); + public virtual Task GetNameAsync() => Task.FromResult(string.Empty); + public virtual ValueTask GetValueTaskAsync() => ValueTask.FromResult(0); + public virtual Task DoAsync() => Task.CompletedTask; + public virtual int GetSync() => 0; + public virtual Task ProcessAsync(string input) => Task.FromResult(input.Length); + } + + internal class UnitTest + { + private void Test() + { + {{mock}} + } + } + """; + + output.WriteLine(source); + + await Verifier.VerifyAnalyzerAsync( + source, + referenceAssemblyGroup) + .ConfigureAwait(false); + } +} diff --git a/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskFixerTests.cs b/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskFixerTests.cs new file mode 100644 index 000000000..30a5ef922 --- /dev/null +++ b/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskFixerTests.cs @@ -0,0 +1,86 @@ +using Verifier = Moq.Analyzers.Test.Helpers.CodeFixVerifier; + +namespace Moq.Analyzers.Test; + +public class ReturnsDelegateShouldReturnTaskFixerTests +{ + private readonly ITestOutputHelper _output; + + public ReturnsDelegateShouldReturnTaskFixerTests(ITestOutputHelper output) + { + _output = output; + } + + public static IEnumerable TestData() + { + return new object[][] + { + // Task with parameterless lambda returning int + [ + """new Mock().Setup(s => s.GetValueAsync()).{|Moq1208:Returns(() => 42)|};""", + """new Mock().Setup(s => s.GetValueAsync()).ReturnsAsync(() => 42);""", + ], + + // Task with parameterless lambda returning string + [ + """new Mock().Setup(s => s.GetNameAsync()).{|Moq1208:Returns(() => "hello")|};""", + """new Mock().Setup(s => s.GetNameAsync()).ReturnsAsync(() => "hello");""", + ], + + // ValueTask with parameterless lambda returning int + [ + """new Mock().Setup(s => s.GetValueTaskAsync()).{|Moq1208:Returns(() => 42)|};""", + """new Mock().Setup(s => s.GetValueTaskAsync()).ReturnsAsync(() => 42);""", + ], + + // Delegate with parameter + [ + """new Mock().Setup(s => s.ProcessAsync(It.IsAny())).{|Moq1208:Returns((string x) => x.Length)|};""", + """new Mock().Setup(s => s.ProcessAsync(It.IsAny())).ReturnsAsync((string x) => x.Length);""", + ], + + // Parenthesized Setup expression + [ + """(new Mock().Setup(s => s.GetValueAsync())).{|Moq1208:Returns(() => 42)|};""", + """(new Mock().Setup(s => s.GetValueAsync())).ReturnsAsync(() => 42);""", + ], + }.WithNamespaces().WithMoqReferenceAssemblyGroups(); + } + + [Theory] + [MemberData(nameof(TestData))] + public async Task ShouldReplaceReturnsWithReturnsAsync(string referenceAssemblyGroup, string @namespace, string original, string quickFix) + { + static string Template(string ns, string mock) => + $$""" + {{ns}} + + public class AsyncService + { + public virtual Task GetValueAsync() => Task.FromResult(0); + public virtual Task GetNameAsync() => Task.FromResult(string.Empty); + public virtual ValueTask GetValueTaskAsync() => new ValueTask(0); + public virtual Task ProcessAsync(string input) => Task.FromResult(input.Length); + } + + internal class UnitTest + { + private void Test() + { + {{mock}} + } + } + """; + + string o = Template(@namespace, original); + string f = Template(@namespace, quickFix); + + _output.WriteLine("Original:"); + _output.WriteLine(o); + _output.WriteLine(string.Empty); + _output.WriteLine("Fixed:"); + _output.WriteLine(f); + + await Verifier.VerifyCodeFixAsync(o, f, referenceAssemblyGroup); + } +} From 5b4f3a43afcbffcb238ab3f9718c79dce8022987 Mon Sep 17 00:00:00 2001 From: Richard Murillo <6811113+rjmurillo@users.noreply.github.com> Date: Sat, 28 Feb 2026 17:25:27 -0800 Subject: [PATCH 02/10] fix: address BCL compliance and coverage gaps in Moq1208 - Fix false negative: block-bodied lambdas (() => { return 42; }) now detected alongside expression-bodied lambdas - Replace unbounded while(true) with depth-guarded for loop in FindSetupInvocation to prevent runaway iteration - Extract FindFirstReturnStatement to avoid boxing (ECS0900) - Reword comment to fix Sonar S125 false positive - Revert const back to static readonly per project convention (ECS0200) - Use primary constructor in fixer tests for consistency - Fix SA1505 blank line after opening brace - Add block-bodied lambda tests for both analyzer and code fix Co-Authored-By: Claude Opus 4.6 --- ...ReturnsDelegateShouldReturnTaskAnalyzer.cs | 45 ++++++++++++++++--- ...nsDelegateShouldReturnTaskAnalyzerTests.cs | 6 +++ ...turnsDelegateShouldReturnTaskFixerTests.cs | 25 +++++------ 3 files changed, 56 insertions(+), 20 deletions(-) diff --git a/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs b/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs index f141e0c3b..1bb1f9271 100644 --- a/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs +++ b/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs @@ -119,9 +119,12 @@ private static bool HasSyncDelegateArgument(InvocationExpressionSyntax invocatio return null; } + // Moq fluent chains are short (Setup.Callback.Returns at most 3-4 deep). + // Guard against pathological syntax trees. + const int maxChainDepth = 10; ExpressionSyntax current = memberAccess.Expression; - while (true) + for (int depth = 0; depth < maxChainDepth; depth++) { ExpressionSyntax unwrapped = current.WalkDownParentheses(); @@ -144,6 +147,8 @@ private static bool HasSyncDelegateArgument(InvocationExpressionSyntax invocatio // Continue walking up the chain (past Callback, etc.) current = candidateMemberAccess.Expression; } + + return null; } private static bool TryGetMismatchInfo( @@ -214,14 +219,40 @@ private static bool TryGetMismatchInfo( return null; } - // Get the type info of the lambda's body expression to determine what type it returns - ExpressionSyntax? bodyExpression = lambda.Body as ExpressionSyntax; - if (bodyExpression == null) + // Expression-bodied lambda: () => 42 + if (lambda.Body is ExpressionSyntax bodyExpression) { - return null; + TypeInfo typeInfo = semanticModel.GetTypeInfo(bodyExpression); + return typeInfo.Type; + } + + // Block-bodied lambda with explicit return statement + if (lambda.Body is BlockSyntax block) + { + ReturnStatementSyntax? returnStatement = FindFirstReturnStatement(block); + + if (returnStatement?.Expression == null) + { + return null; + } + + TypeInfo returnTypeInfo = semanticModel.GetTypeInfo(returnStatement.Expression); + return returnTypeInfo.Type; + } + + return null; + } + + private static ReturnStatementSyntax? FindFirstReturnStatement(BlockSyntax block) + { + foreach (StatementSyntax statement in block.Statements) + { + if (statement is ReturnStatementSyntax returnStatement) + { + return returnStatement; + } } - TypeInfo typeInfo = semanticModel.GetTypeInfo(bodyExpression); - return typeInfo.Type; + return null; } } diff --git a/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs b/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs index 6ee6e9f36..a71992187 100644 --- a/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs +++ b/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs @@ -37,6 +37,9 @@ public static IEnumerable ValidTestData() // Parenthesized Setup with ReturnsAsync (valid) ["""(new Mock().Setup(c => c.GetValueAsync())).ReturnsAsync(42);"""], + + // Block-bodied lambda returning Task (correct) + ["""new Mock().Setup(c => c.GetValueAsync()).Returns(() => { return Task.FromResult(42); });"""], }; return data.WithNamespaces().WithMoqReferenceAssemblyGroups(); @@ -60,6 +63,9 @@ public static IEnumerable InvalidTestData() // Chained Callback then Returns with sync delegate mismatch ["""new Mock().Setup(c => c.GetValueAsync()).Callback(() => { }).{|Moq1208:Returns(() => 42)|};"""], + + // Block-bodied lambda returning wrong type on Task method + ["""new Mock().Setup(c => c.GetValueAsync()).{|Moq1208:Returns(() => { return 42; })|};"""], }; return data.WithNamespaces().WithMoqReferenceAssemblyGroups(); diff --git a/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskFixerTests.cs b/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskFixerTests.cs index 30a5ef922..642390c08 100644 --- a/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskFixerTests.cs +++ b/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskFixerTests.cs @@ -2,15 +2,8 @@ namespace Moq.Analyzers.Test; -public class ReturnsDelegateShouldReturnTaskFixerTests +public class ReturnsDelegateShouldReturnTaskFixerTests(ITestOutputHelper output) { - private readonly ITestOutputHelper _output; - - public ReturnsDelegateShouldReturnTaskFixerTests(ITestOutputHelper output) - { - _output = output; - } - public static IEnumerable TestData() { return new object[][] @@ -44,6 +37,12 @@ public static IEnumerable TestData() """(new Mock().Setup(s => s.GetValueAsync())).{|Moq1208:Returns(() => 42)|};""", """(new Mock().Setup(s => s.GetValueAsync())).ReturnsAsync(() => 42);""", ], + + // Block-bodied lambda returning wrong type + [ + """new Mock().Setup(s => s.GetValueAsync()).{|Moq1208:Returns(() => { return 42; })|};""", + """new Mock().Setup(s => s.GetValueAsync()).ReturnsAsync(() => { return 42; });""", + ], }.WithNamespaces().WithMoqReferenceAssemblyGroups(); } @@ -75,11 +74,11 @@ private void Test() string o = Template(@namespace, original); string f = Template(@namespace, quickFix); - _output.WriteLine("Original:"); - _output.WriteLine(o); - _output.WriteLine(string.Empty); - _output.WriteLine("Fixed:"); - _output.WriteLine(f); + output.WriteLine("Original:"); + output.WriteLine(o); + output.WriteLine(string.Empty); + output.WriteLine("Fixed:"); + output.WriteLine(f); await Verifier.VerifyCodeFixAsync(o, f, referenceAssemblyGroup); } From bd782b283d9ed82b52e996ddb885198febcb102e Mon Sep 17 00:00:00 2001 From: Richard Murillo <6811113+rjmurillo@users.noreply.github.com> Date: Sat, 28 Feb 2026 17:35:02 -0800 Subject: [PATCH 03/10] docs: improve Moq1208 documentation for junior developers and add to rule indexes Rewrote Moq1208.md to explain core Moq concepts (Setup/Returns), why the type mismatch causes a runtime error, what ReturnsAsync does internally, and how this rule differs from Moq1206. Added Moq1208 to both README.md and docs/rules/README.md rule tables. Co-Authored-By: Claude Opus 4.6 --- README.md | 1 + docs/rules/Moq1208.md | 76 ++++++++++++++++++++++++++++++++----------- docs/rules/README.md | 1 + 3 files changed, 59 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 6bc13ee99..1e8182c67 100644 --- a/README.md +++ b/README.md @@ -28,6 +28,7 @@ something is wrong with your Moq configuration. | [Moq1205](docs/rules/Moq1205.md) | Correctness | Event setup handler type should match event delegate type | | [Moq1206](docs/rules/Moq1206.md) | Correctness | Async method setups should use ReturnsAsync instead of Returns with async lambda | | [Moq1207](docs/rules/Moq1207.md) | Correctness | SetupSequence should be used only for overridable members | +| [Moq1208](docs/rules/Moq1208.md) | Correctness | Returns() delegate type mismatch on async method setup | | [Moq1210](docs/rules/Moq1210.md) | Correctness | Verify should be used only for overridable members | | [Moq1300](docs/rules/Moq1300.md) | Usage | `Mock.As()` should take interfaces only | | [Moq1301](docs/rules/Moq1301.md) | Usage | Mock.Get() should not take literals | diff --git a/docs/rules/Moq1208.md b/docs/rules/Moq1208.md index 60072a712..38e8742cf 100644 --- a/docs/rules/Moq1208.md +++ b/docs/rules/Moq1208.md @@ -7,47 +7,85 @@ | CodeFix | True | --- -When `.Returns()` is called with a delegate that returns a non-Task type on an async method setup, Moq throws a runtime `MockException`. The delegate return type must match the method return type. For async methods returning `Task` or `ValueTask`, the delegate must return `Task` or `ValueTask`, not `T` directly. +## What this rule checks + +In Moq, `.Setup()` defines what a mocked method should do when called. +`.Returns()` specifies the value the method gives back. For example: + +```csharp +mock.Setup(x => x.GetName()).Returns(() => "Alice"); +// ^^^^^ "when GetName is called" ^^^^^^^^^ "return Alice" +``` + +This rule fires when the function passed to `.Returns()` gives back a plain +value like `int` or `string`, but the mocked method is async and returns +`Task` or `ValueTask`. Moq requires the types to match exactly. + +### Why this matters + +The code compiles without errors, but the test fails at runtime with this +exception: ``` -MockException: Invalid callback. Setup on method with return type 'Task' cannot invoke callback with return type 'int'. +MockException: Invalid callback. Setup on method with return type 'Task' +cannot invoke callback with return type 'int'. ``` +This analyzer catches the mismatch at compile time so you don't have to debug +a failing test to find it. + +### How this differs from Moq1206 + +[Moq1206](./Moq1206.md) flags `async` lambdas in `.Returns()`, such as +`Returns(async () => 42)`. Moq1208 flags regular (non-async) lambdas that +return the wrong type, such as `Returns(() => 42)` on a `Task` method. + ## Examples of patterns that are flagged by this analyzer ```csharp public interface IService { - Task GetValueAsync(); - Task GetNameAsync(); - ValueTask GetValueTaskAsync(); + Task GetValueAsync(); // Returns Task + Task GetNameAsync(); // Returns Task + ValueTask GetValueTaskAsync(); // Returns ValueTask } var mock = new Mock(); -// These patterns are flagged: -mock.Setup(x => x.GetValueAsync()).Returns(() => 42); // Moq1208: Returns() delegate type mismatch on async method setup -mock.Setup(x => x.GetNameAsync()).Returns(() => "hello"); // Moq1208: Returns() delegate type mismatch on async method setup -mock.Setup(x => x.GetValueTaskAsync()).Returns(() => 42); // Moq1208: Returns() delegate type mismatch on async method setup +// GetValueAsync returns Task, but the lambda returns int. +mock.Setup(x => x.GetValueAsync()).Returns(() => 42); // Moq1208 + +// GetNameAsync returns Task, but the lambda returns string. +mock.Setup(x => x.GetNameAsync()).Returns(() => "hello"); // Moq1208 + +// GetValueTaskAsync returns ValueTask, but the lambda returns int. +mock.Setup(x => x.GetValueTaskAsync()).Returns(() => 42); // Moq1208 ``` ## Solution -```csharp -public interface IService -{ - Task GetValueAsync(); - Task GetNameAsync(); - ValueTask GetValueTaskAsync(); -} +### Option 1: Use ReturnsAsync (recommended) +`.ReturnsAsync()` wraps the value in `Task.FromResult()` for you. This is the +simplest fix and what the built-in code fix applies automatically. + +```csharp var mock = new Mock(); -// Use ReturnsAsync instead (recommended): +// Pass a plain value. Moq wraps it in Task.FromResult() internally. mock.Setup(x => x.GetValueAsync()).ReturnsAsync(42); + +// Or pass a lambda. Moq wraps the lambda's return value the same way. mock.Setup(x => x.GetValueAsync()).ReturnsAsync(() => 42); +``` + +### Option 2: Wrap the value yourself + +If you need more control, keep `.Returns()` and wrap the value explicitly. + +```csharp +var mock = new Mock(); -// Or return Task explicitly: mock.Setup(x => x.GetValueAsync()).Returns(() => Task.FromResult(42)); mock.Setup(x => x.GetNameAsync()).Returns(() => Task.FromResult("hello")); mock.Setup(x => x.GetValueTaskAsync()).Returns(() => new ValueTask(42)); @@ -60,7 +98,7 @@ your source file to disable and then re-enable the rule. ```csharp #pragma warning disable Moq1208 -mock.Setup(x => x.GetValueAsync()).Returns(() => 42); // Moq1208: Returns() delegate type mismatch on async method setup +mock.Setup(x => x.GetValueAsync()).Returns(() => 42); #pragma warning restore Moq1208 ``` diff --git a/docs/rules/README.md b/docs/rules/README.md index 374af21e4..beed4660e 100644 --- a/docs/rules/README.md +++ b/docs/rules/README.md @@ -15,6 +15,7 @@ | [Moq1205](./Moq1205.md) | Correctness | Event setup handler type should match event delegate type | [EventSetupHandlerShouldMatchEventTypeAnalyzer.cs](../../src/Analyzers/EventSetupHandlerShouldMatchEventTypeAnalyzer.cs) | | [Moq1206](./Moq1206.md) | Correctness | Async method setups should use ReturnsAsync instead of Returns with async lambda | [ReturnsAsyncShouldBeUsedForAsyncMethodsAnalyzer.cs](../../src/Analyzers/ReturnsAsyncShouldBeUsedForAsyncMethodsAnalyzer.cs) | | [Moq1207](./Moq1207.md) | Correctness | SetupSequence should be used only for overridable members | [SetupSequenceShouldBeUsedOnlyForOverridableMembersAnalyzer.cs](../../src/Analyzers/SetupSequenceShouldBeUsedOnlyForOverridableMembersAnalyzer.cs) | +| [Moq1208](./Moq1208.md) | Correctness | Returns() delegate type mismatch on async method setup | [ReturnsDelegateShouldReturnTaskAnalyzer.cs](../../src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs) | | [Moq1210](./Moq1210.md) | Correctness | Verify should be used only for overridable members | [VerifyShouldBeUsedOnlyForOverridableMembersAnalyzer.cs](../../src/Analyzers/VerifyShouldBeUsedOnlyForOverridableMembersAnalyzer.cs) | | [Moq1300](./Moq1300.md) | Usage | `Mock.As()` should take interfaces only | [AsShouldBeUsedOnlyForInterfaceAnalyzer.cs](../../src/Analyzers/AsShouldBeUsedOnlyForInterfaceAnalyzer.cs) | | [Moq1301](./Moq1301.md) | Usage | Mock.Get() should not take literals | [MockGetShouldNotTakeLiteralsAnalyzer.cs](../../src/Analyzers/MockGetShouldNotTakeLiteralsAnalyzer.cs) | From 6a07ef914e9134699f2bcace0003c4ca06fb1d0c Mon Sep 17 00:00:00 2001 From: Richard Murillo <6811113+rjmurillo@users.noreply.github.com> Date: Sat, 28 Feb 2026 17:40:52 -0800 Subject: [PATCH 04/10] fix: address PR review feedback for Moq1208 - Simplify GetDelegateReturnType using IMethodSymbol.ReturnType instead of manually inspecting expression/block bodies (handles all lambda forms) - Include expected return type in diagnostic message for clarity - Preserve generic type arguments in code fix (Returns -> ReturnsAsync) - Fix markdown blank line before horizontal rule separator Co-Authored-By: Claude Opus 4.6 --- docs/rules/Moq1208.md | 1 + ...ReturnsDelegateShouldReturnTaskAnalyzer.cs | 46 +++++-------------- .../ReturnsDelegateShouldReturnTaskFixer.cs | 10 +++- 3 files changed, 22 insertions(+), 35 deletions(-) diff --git a/docs/rules/Moq1208.md b/docs/rules/Moq1208.md index 38e8742cf..1da1cdc23 100644 --- a/docs/rules/Moq1208.md +++ b/docs/rules/Moq1208.md @@ -5,6 +5,7 @@ | Enabled | True | | Severity | Warning | | CodeFix | True | + --- ## What this rule checks diff --git a/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs b/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs index 1bb1f9271..cd171f63e 100644 --- a/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs +++ b/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs @@ -9,7 +9,7 @@ namespace Moq.Analyzers; public class ReturnsDelegateShouldReturnTaskAnalyzer : DiagnosticAnalyzer { private static readonly LocalizableString Title = "Moq: Returns() delegate type mismatch on async method"; - private static readonly LocalizableString Message = "Returns() delegate for async method '{0}' should return a Task type, not '{1}'. Use ReturnsAsync() or return Task.FromResult()."; + private static readonly LocalizableString Message = "Returns() delegate for async method '{0}' should return '{2}', not '{1}'. Use ReturnsAsync() or wrap with Task.FromResult()."; private static readonly LocalizableString Description = "Returns() delegate on async method setup should return Task/ValueTask. Use ReturnsAsync() or wrap with Task.FromResult()."; private static readonly DiagnosticDescriptor Rule = new( @@ -50,7 +50,7 @@ private static void Analyze(SyntaxNodeAnalysisContext context) return; } - if (!TryGetMismatchInfo(setupInvocation, invocation, context.SemanticModel, knownSymbols, out string? methodName, out ITypeSymbol? delegateReturnType)) + if (!TryGetMismatchInfo(setupInvocation, invocation, context.SemanticModel, knownSymbols, out string? methodName, out ITypeSymbol? expectedReturnType, out ITypeSymbol? delegateReturnType)) { return; } @@ -62,7 +62,9 @@ private static void Analyze(SyntaxNodeAnalysisContext context) Microsoft.CodeAnalysis.Text.TextSpan span = Microsoft.CodeAnalysis.Text.TextSpan.FromBounds(startPos, endPos); Location location = Location.Create(invocation.SyntaxTree, span); - Diagnostic diagnostic = location.CreateDiagnostic(Rule, methodName, delegateReturnType.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat)); + string actualDisplay = delegateReturnType.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat); + string expectedDisplay = expectedReturnType.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat); + Diagnostic diagnostic = location.CreateDiagnostic(Rule, methodName, actualDisplay, expectedDisplay); context.ReportDiagnostic(diagnostic); } @@ -157,9 +159,11 @@ private static bool TryGetMismatchInfo( SemanticModel semanticModel, MoqKnownSymbols knownSymbols, [NotNullWhen(true)] out string? methodName, + [NotNullWhen(true)] out ITypeSymbol? expectedReturnType, [NotNullWhen(true)] out ITypeSymbol? delegateReturnType) { methodName = null; + expectedReturnType = null; delegateReturnType = null; // Get the mocked method from the Setup lambda @@ -202,6 +206,7 @@ private static bool TryGetMismatchInfo( } methodName = mockedMethod.Name; + expectedReturnType = returnType; return true; } @@ -219,38 +224,11 @@ private static bool TryGetMismatchInfo( return null; } - // Expression-bodied lambda: () => 42 - if (lambda.Body is ExpressionSyntax bodyExpression) + // Use the lambda's semantic symbol to get its return type. + // This handles both expression-bodied (() => 42) and block-bodied (() => { return 42; }) lambdas. + if (semanticModel.GetSymbolInfo(lambda).Symbol is IMethodSymbol lambdaSymbol) { - TypeInfo typeInfo = semanticModel.GetTypeInfo(bodyExpression); - return typeInfo.Type; - } - - // Block-bodied lambda with explicit return statement - if (lambda.Body is BlockSyntax block) - { - ReturnStatementSyntax? returnStatement = FindFirstReturnStatement(block); - - if (returnStatement?.Expression == null) - { - return null; - } - - TypeInfo returnTypeInfo = semanticModel.GetTypeInfo(returnStatement.Expression); - return returnTypeInfo.Type; - } - - return null; - } - - private static ReturnStatementSyntax? FindFirstReturnStatement(BlockSyntax block) - { - foreach (StatementSyntax statement in block.Statements) - { - if (statement is ReturnStatementSyntax returnStatement) - { - return returnStatement; - } + return lambdaSymbol.ReturnType; } return null; diff --git a/src/CodeFixes/ReturnsDelegateShouldReturnTaskFixer.cs b/src/CodeFixes/ReturnsDelegateShouldReturnTaskFixer.cs index 141d7615f..a863a4117 100644 --- a/src/CodeFixes/ReturnsDelegateShouldReturnTaskFixer.cs +++ b/src/CodeFixes/ReturnsDelegateShouldReturnTaskFixer.cs @@ -61,7 +61,15 @@ private static Task ReplaceReturnsWithReturnsAsync( MemberAccessExpressionSyntax memberAccess) { SimpleNameSyntax oldName = memberAccess.Name; - IdentifierNameSyntax newName = SyntaxFactory.IdentifierName("ReturnsAsync") + + // Preserve generic type arguments if present (e.g., Returns(...) -> ReturnsAsync(...)) + SimpleNameSyntax newName = oldName is GenericNameSyntax genericName + ? SyntaxFactory.GenericName( + SyntaxFactory.Identifier("ReturnsAsync"), + genericName.TypeArgumentList) + : (SimpleNameSyntax)SyntaxFactory.IdentifierName("ReturnsAsync"); + + newName = newName .WithLeadingTrivia(oldName.GetLeadingTrivia()) .WithTrailingTrivia(oldName.GetTrailingTrivia()); From 4c5e83ad2d9565de37ed3fcc927e64d73076acb5 Mon Sep 17 00:00:00 2001 From: Richard Murillo <6811113+rjmurillo@users.noreply.github.com> Date: Sun, 1 Mar 2026 09:09:02 -0600 Subject: [PATCH 05/10] feat: extend Moq1208 to detect anonymous method and method group mismatches The analyzer previously only flagged lambda expressions. Anonymous methods (`delegate { return 42; }`) and method groups (`Returns(GetInt)`) on async method setups produce the same runtime MockException but were not detected. Broadens delegate detection from LambdaExpressionSyntax to AnonymousFunctionExpressionSyntax and adds method group resolution via GetSymbolInfo/CandidateSymbols. Includes PR review hardening: defensive casts, CandidateReason guards, FindSetupInvocation CandidateSymbols fallback, ambiguous candidate consensus check, and anonymous method body analysis for accurate return type extraction. Co-Authored-By: Claude Opus 4.6 --- docs/rules/Moq1208.md | 29 +++- ...ReturnsDelegateShouldReturnTaskAnalyzer.cs | 125 ++++++++++++++---- .../Helpers/CodeFixVerifier.cs | 13 +- ...nsDelegateShouldReturnTaskAnalyzerTests.cs | 62 +++++++-- ...turnsDelegateShouldReturnTaskFixerTests.cs | 78 ++++++++--- 5 files changed, 247 insertions(+), 60 deletions(-) diff --git a/docs/rules/Moq1208.md b/docs/rules/Moq1208.md index 1da1cdc23..26754db29 100644 --- a/docs/rules/Moq1208.md +++ b/docs/rules/Moq1208.md @@ -18,10 +18,16 @@ mock.Setup(x => x.GetName()).Returns(() => "Alice"); // ^^^^^ "when GetName is called" ^^^^^^^^^ "return Alice" ``` -This rule fires when the function passed to `.Returns()` gives back a plain +This rule fires when a delegate passed to `.Returns()` gives back a plain value like `int` or `string`, but the mocked method is async and returns `Task` or `ValueTask`. Moq requires the types to match exactly. +The rule detects three forms of delegates: + +- **Lambdas**: `Returns(() => 42)` +- **Anonymous methods**: `Returns(delegate { return 42; })` +- **Method groups**: `Returns(GetInt)` where `GetInt` returns `int` + ### Why this matters The code compiles without errors, but the test fails at runtime with this @@ -37,8 +43,8 @@ a failing test to find it. ### How this differs from Moq1206 -[Moq1206](./Moq1206.md) flags `async` lambdas in `.Returns()`, such as -`Returns(async () => 42)`. Moq1208 flags regular (non-async) lambdas that +[Moq1206](./Moq1206.md) flags `async` delegates in `.Returns()`, such as +`Returns(async () => 42)`. Moq1208 flags regular (non-async) delegates that return the wrong type, such as `Returns(() => 42)` on a `Task` method. ## Examples of patterns that are flagged by this analyzer @@ -53,13 +59,20 @@ public interface IService var mock = new Mock(); -// GetValueAsync returns Task, but the lambda returns int. +// Lambda returning int on a Task method. mock.Setup(x => x.GetValueAsync()).Returns(() => 42); // Moq1208 -// GetNameAsync returns Task, but the lambda returns string. +// Lambda returning string on a Task method. mock.Setup(x => x.GetNameAsync()).Returns(() => "hello"); // Moq1208 -// GetValueTaskAsync returns ValueTask, but the lambda returns int. +// Anonymous method returning int on a Task method. +mock.Setup(x => x.GetValueAsync()).Returns(delegate { return 42; }); // Moq1208 + +// Method group returning int on a Task method. +mock.Setup(x => x.GetValueAsync()).Returns(GetInt); // Moq1208 +// where: static int GetInt() => 42; + +// ValueTask with wrong return type. mock.Setup(x => x.GetValueTaskAsync()).Returns(() => 42); // Moq1208 ``` @@ -78,6 +91,10 @@ mock.Setup(x => x.GetValueAsync()).ReturnsAsync(42); // Or pass a lambda. Moq wraps the lambda's return value the same way. mock.Setup(x => x.GetValueAsync()).ReturnsAsync(() => 42); + +// Anonymous methods and method groups work the same way. +mock.Setup(x => x.GetValueAsync()).ReturnsAsync(delegate { return 42; }); +mock.Setup(x => x.GetValueAsync()).ReturnsAsync(GetInt); ``` ### Option 2: Wrap the value yourself diff --git a/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs b/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs index cd171f63e..638588adf 100644 --- a/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs +++ b/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs @@ -39,24 +39,22 @@ private static void Analyze(SyntaxNodeAnalysisContext context) InvocationExpressionSyntax invocation = (InvocationExpressionSyntax)context.Node; - if (!IsReturnsMethodCallWithSyncDelegate(invocation, context.SemanticModel, knownSymbols)) + if (!IsReturnsMethodCallWithSyncDelegate(invocation, context.SemanticModel, knownSymbols, out InvocationExpressionSyntax? setupInvocation)) { return; } - InvocationExpressionSyntax? setupInvocation = FindSetupInvocation(invocation, context.SemanticModel, knownSymbols); - if (setupInvocation == null) + if (!TryGetMismatchInfo(setupInvocation, invocation, context.SemanticModel, knownSymbols, out string? methodName, out ITypeSymbol? expectedReturnType, out ITypeSymbol? delegateReturnType)) { return; } - if (!TryGetMismatchInfo(setupInvocation, invocation, context.SemanticModel, knownSymbols, out string? methodName, out ITypeSymbol? expectedReturnType, out ITypeSymbol? delegateReturnType)) + // Report diagnostic spanning from "Returns" identifier through the closing paren + if (invocation.Expression is not MemberAccessExpressionSyntax memberAccess) { return; } - // Report diagnostic spanning from "Returns" identifier through the closing paren - MemberAccessExpressionSyntax memberAccess = (MemberAccessExpressionSyntax)invocation.Expression; int startPos = memberAccess.Name.SpanStart; int endPos = invocation.Span.End; Microsoft.CodeAnalysis.Text.TextSpan span = Microsoft.CodeAnalysis.Text.TextSpan.FromBounds(startPos, endPos); @@ -68,9 +66,15 @@ private static void Analyze(SyntaxNodeAnalysisContext context) context.ReportDiagnostic(diagnostic); } - private static bool IsReturnsMethodCallWithSyncDelegate(InvocationExpressionSyntax invocation, SemanticModel semanticModel, MoqKnownSymbols knownSymbols) + private static bool IsReturnsMethodCallWithSyncDelegate( + InvocationExpressionSyntax invocation, + SemanticModel semanticModel, + MoqKnownSymbols knownSymbols, + [NotNullWhen(true)] out InvocationExpressionSyntax? setupInvocation) { - if (invocation.Expression is not MemberAccessExpressionSyntax) + setupInvocation = null; + + if (invocation.Expression is not MemberAccessExpressionSyntax memberAccess) { return false; } @@ -80,19 +84,38 @@ private static bool IsReturnsMethodCallWithSyncDelegate(InvocationExpressionSynt SymbolInfo symbolInfo = semanticModel.GetSymbolInfo(invocation); bool isReturnsMethod = symbolInfo.Symbol is IMethodSymbol method ? method.IsMoqReturnsMethod(knownSymbols) - : symbolInfo.CandidateSymbols - .OfType() - .Any(m => m.IsMoqReturnsMethod(knownSymbols)); + : symbolInfo.CandidateReason is CandidateReason.OverloadResolutionFailure + && symbolInfo.CandidateSymbols + .OfType() + .Any(m => m.IsMoqReturnsMethod(knownSymbols)); + + // Parameterless anonymous methods can cause complete overload resolution failure, + // leaving Symbol null and CandidateSymbols empty. Fall back to verifying the method + // name is "Returns", the argument is an anonymous method, and the call is in a Moq Setup chain. + if (!isReturnsMethod + && string.Equals(memberAccess.Name.Identifier.Text, "Returns", StringComparison.Ordinal) + && invocation.ArgumentList.Arguments.Count > 0 + && invocation.ArgumentList.Arguments[0].Expression is AnonymousMethodExpressionSyntax) + { + setupInvocation = FindSetupInvocation(invocation, semanticModel, knownSymbols); + isReturnsMethod = setupInvocation != null; + } if (!isReturnsMethod) { return false; } - return HasSyncDelegateArgument(invocation); + if (!HasSyncDelegateArgument(invocation, semanticModel)) + { + return false; + } + + setupInvocation ??= FindSetupInvocation(invocation, semanticModel, knownSymbols); + return setupInvocation != null; } - private static bool HasSyncDelegateArgument(InvocationExpressionSyntax invocation) + private static bool HasSyncDelegateArgument(InvocationExpressionSyntax invocation, SemanticModel semanticModel) { if (invocation.ArgumentList.Arguments.Count == 0) { @@ -101,14 +124,28 @@ private static bool HasSyncDelegateArgument(InvocationExpressionSyntax invocatio ExpressionSyntax firstArgument = invocation.ArgumentList.Arguments[0].Expression; - // Must be a lambda or delegate. Raw values (not delegates) are a different overload. - if (firstArgument is not LambdaExpressionSyntax lambda) + // Lambdas and anonymous methods share AnonymousFunctionExpressionSyntax, + // which exposes AsyncKeyword for sync/async detection. + if (firstArgument is AnonymousFunctionExpressionSyntax anonymousFunction) { - return false; + return !anonymousFunction.AsyncKeyword.IsKind(SyntaxKind.AsyncKeyword); } - // Exclude async lambdas. Those are Moq1206's domain. - return !lambda.AsyncKeyword.IsKind(SyntaxKind.AsyncKeyword); + // Method groups require semantic resolution to distinguish from raw values. + return IsMethodGroupExpression(firstArgument, semanticModel); + } + + private static bool IsMethodGroupExpression(ExpressionSyntax expression, SemanticModel semanticModel) + { + SymbolInfo symbolInfo = semanticModel.GetSymbolInfo(expression); + if (symbolInfo.Symbol is IMethodSymbol) + { + return true; + } + + // Method groups with overloads fail resolution when no single overload matches the expected delegate type + return symbolInfo.CandidateReason is CandidateReason.OverloadResolutionFailure or CandidateReason.MemberGroup + && symbolInfo.CandidateSymbols.OfType().Any(); } private static InvocationExpressionSyntax? FindSetupInvocation(InvocationExpressionSyntax returnsInvocation, SemanticModel semanticModel, MoqKnownSymbols knownSymbols) @@ -140,12 +177,18 @@ private static bool HasSyncDelegateArgument(InvocationExpressionSyntax invocatio return null; } - SymbolInfo symbolInfo = semanticModel.GetSymbolInfo(candidateMemberAccess); + SymbolInfo symbolInfo = semanticModel.GetSymbolInfo(candidateInvocation); if (symbolInfo.Symbol != null && symbolInfo.Symbol.IsMoqSetupMethod(knownSymbols)) { return candidateInvocation; } + if (symbolInfo.CandidateReason is CandidateReason.OverloadResolutionFailure + && symbolInfo.CandidateSymbols.Any(s => s.IsMoqSetupMethod(knownSymbols))) + { + return candidateInvocation; + } + // Continue walking up the chain (past Callback, etc.) current = candidateMemberAccess.Expression; } @@ -192,7 +235,7 @@ private static bool TryGetMismatchInfo( return false; } - // Get the delegate's return type from the lambda argument + // Get the delegate's return type from the Returns() argument delegateReturnType = GetDelegateReturnType(returnsInvocation, semanticModel); if (delegateReturnType == null) { @@ -219,18 +262,48 @@ private static bool TryGetMismatchInfo( ExpressionSyntax firstArgument = returnsInvocation.ArgumentList.Arguments[0].Expression; - if (firstArgument is not LambdaExpressionSyntax lambda) + // For anonymous methods, prefer body analysis. Roslyn may infer the return type + // from the target delegate type (e.g., Task) for parameterless anonymous methods, + // masking the actual body return type (e.g., int). + if (firstArgument is AnonymousMethodExpressionSyntax { Body: BlockSyntax block }) { - return null; + return GetReturnTypeFromBlock(block, semanticModel); } - // Use the lambda's semantic symbol to get its return type. - // This handles both expression-bodied (() => 42) and block-bodied (() => { return 42; }) lambdas. - if (semanticModel.GetSymbolInfo(lambda).Symbol is IMethodSymbol lambdaSymbol) + // GetSymbolInfo resolves lambdas to IMethodSymbol even when type conversion fails. + // Raw values resolve to ILocalSymbol/IFieldSymbol/etc., filtered by the type check. + SymbolInfo symbolInfo = semanticModel.GetSymbolInfo(firstArgument); + if (symbolInfo.Symbol is IMethodSymbol methodSymbol) { - return lambdaSymbol.ReturnType; + return methodSymbol.ReturnType; + } + + // Method groups with type conversion errors may not resolve via Symbol. + // Fall back to CandidateSymbols only when all candidates agree on the return type. + IMethodSymbol[] candidates = symbolInfo.CandidateSymbols.OfType().ToArray(); + if (candidates.Length > 0 + && candidates.All(c => SymbolEqualityComparer.Default.Equals(c.ReturnType, candidates[0].ReturnType))) + { + return candidates[0].ReturnType; } return null; } + + private static ITypeSymbol? GetReturnTypeFromBlock(BlockSyntax block, SemanticModel semanticModel) + { + // Find the first return statement in this block, + // pruning nested anonymous functions so we don't pick up their returns. + ReturnStatementSyntax? returnStatement = block + .DescendantNodes(node => node is not AnonymousFunctionExpressionSyntax) + .OfType() + .FirstOrDefault(); + + if (returnStatement?.Expression == null) + { + return null; + } + + return semanticModel.GetTypeInfo(returnStatement.Expression).Type; + } } diff --git a/tests/Moq.Analyzers.Test/Helpers/CodeFixVerifier.cs b/tests/Moq.Analyzers.Test/Helpers/CodeFixVerifier.cs index 5be586f8b..9004adb1c 100644 --- a/tests/Moq.Analyzers.Test/Helpers/CodeFixVerifier.cs +++ b/tests/Moq.Analyzers.Test/Helpers/CodeFixVerifier.cs @@ -7,15 +7,22 @@ internal static class CodeFixVerifier where TAnalyzer : DiagnosticAnalyzer, new() where TCodeFixProvider : CodeFixProvider, new() { - public static async Task VerifyCodeFixAsync(string originalSource, string fixedSource, string referenceAssemblyGroup) + public static async Task VerifyCodeFixAsync(string originalSource, string fixedSource, string referenceAssemblyGroup, CompilerDiagnostics? compilerDiagnostics = null) { ReferenceAssemblies referenceAssemblies = ReferenceAssemblyCatalog.Catalog[referenceAssemblyGroup]; - await new Test + Test test = new Test { TestCode = originalSource, FixedCode = fixedSource, ReferenceAssemblies = referenceAssemblies, - }.RunAsync().ConfigureAwait(false); + }; + + if (compilerDiagnostics.HasValue) + { + test.CompilerDiagnostics = compilerDiagnostics.Value; + } + + await test.RunAsync().ConfigureAwait(false); } } diff --git a/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs b/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs index a71992187..cc633a126 100644 --- a/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs +++ b/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs @@ -1,3 +1,4 @@ +using Microsoft.CodeAnalysis.Testing; using Verifier = Moq.Analyzers.Test.Helpers.AnalyzerVerifier; namespace Moq.Analyzers.Test; @@ -32,6 +33,24 @@ public static IEnumerable ValidTestData() // Chained Callback with correct Task return ["""new Mock().Setup(c => c.GetValueAsync()).Callback(() => { }).Returns(() => Task.FromResult(42));"""], + // Anonymous method returning Task.FromResult (correct) + ["""new Mock().Setup(c => c.GetValueAsync()).Returns(delegate { return Task.FromResult(42); });"""], + + // Async anonymous method (Moq1206's domain, not ours) + ["""new Mock().Setup(c => c.GetValueAsync()).Returns(async delegate { return 42; });"""], + + // Method group returning Task (correct) + ["""new Mock().Setup(c => c.GetValueAsync()).Returns(GetIntAsync);"""], + + // Anonymous method on sync method (no mismatch) + ["""new Mock().Setup(c => c.GetSync()).Returns(delegate { return 42; });"""], + + // Method group on sync method (no mismatch) + ["""new Mock().Setup(c => c.GetSync()).Returns(GetInt);"""], + + // Direct value on async method (not a delegate, different Returns overload) + ["""new Mock().Setup(c => c.GetValueAsync()).Returns(Task.FromResult(42));"""], + // Non-generic Task method with sync delegate (no mismatch, Task has no inner type) ["""new Mock().Setup(c => c.DoAsync()).Returns(() => Task.CompletedTask);"""], @@ -66,17 +85,37 @@ public static IEnumerable InvalidTestData() // Block-bodied lambda returning wrong type on Task method ["""new Mock().Setup(c => c.GetValueAsync()).{|Moq1208:Returns(() => { return 42; })|};"""], + + // Sync delegate with parameter returning wrong type on Task method + ["""new Mock().Setup(c => c.ProcessAsync(It.IsAny())).{|Moq1208:Returns((string x) => x.Length)|};"""], }; return data.WithNamespaces().WithMoqReferenceAssemblyGroups(); } - public static IEnumerable DelegateOverloadTestData() + /// + /// Anonymous methods and method groups with type mismatches produce compiler errors + /// (CS0029/CS1662), unlike lambdas. We suppress compiler diagnostics to isolate the analyzer. + /// + /// Test data with compiler diagnostic suppression for anonymous delegate and method group cases. + public static IEnumerable InvalidAnonymousDelegateAndMethodGroupTestData() { IEnumerable data = new object[][] { - // Sync delegate with parameter returning wrong type on Task method - ["""new Mock().Setup(c => c.ProcessAsync(It.IsAny())).{|Moq1208:Returns((string x) => x.Length)|};"""], + // Anonymous method returning int on Task method + ["""new Mock().Setup(c => c.GetValueAsync()).{|Moq1208:Returns(delegate { return 42; })|};"""], + + // Anonymous method returning int on ValueTask method + ["""new Mock().Setup(c => c.GetValueTaskAsync()).{|Moq1208:Returns(delegate { return 42; })|};"""], + + // Anonymous method with parameter returning wrong type on Task method + ["""new Mock().Setup(c => c.ProcessAsync(It.IsAny())).{|Moq1208:Returns(delegate (string x) { return x.Length; })|};"""], + + // Method group returning int on Task method + ["""new Mock().Setup(c => c.GetValueAsync()).{|Moq1208:Returns(GetInt)|};"""], + + // Method group returning string on Task method + ["""new Mock().Setup(c => c.GetNameAsync()).{|Moq1208:Returns(GetString)|};"""], }; return data.WithNamespaces().WithMoqReferenceAssemblyGroups(); @@ -97,10 +136,10 @@ public async Task ShouldTriggerOnSyncDelegateMismatch(string referenceAssemblyGr } [Theory] - [MemberData(nameof(DelegateOverloadTestData))] - public async Task ShouldFlagSyncDelegateLambdaWithParameterInReturns(string referenceAssemblyGroup, string @namespace, string mock) + [MemberData(nameof(InvalidAnonymousDelegateAndMethodGroupTestData))] + public async Task ShouldFlagAnonymousDelegateAndMethodGroupMismatch(string referenceAssemblyGroup, string @namespace, string mock) { - await VerifyAsync(referenceAssemblyGroup, @namespace, mock); + await VerifyAsync(referenceAssemblyGroup, @namespace, mock, CompilerDiagnostics.None); } [Theory] @@ -112,7 +151,7 @@ await Verifier.VerifyAnalyzerAsync( ReferenceAssemblyCatalog.Net80WithNewMoq); } - private async Task VerifyAsync(string referenceAssemblyGroup, string @namespace, string mock) + private async Task VerifyAsync(string referenceAssemblyGroup, string @namespace, string mock, CompilerDiagnostics? compilerDiagnostics = null) { string source = $$""" @@ -130,6 +169,10 @@ public class AsyncService internal class UnitTest { + private static int GetInt() => 42; + private static string GetString() => "hello"; + private static Task GetIntAsync() => Task.FromResult(42); + private void Test() { {{mock}} @@ -141,7 +184,10 @@ private void Test() await Verifier.VerifyAnalyzerAsync( source, - referenceAssemblyGroup) + referenceAssemblyGroup, + configFileName: null, + configContent: null, + compilerDiagnostics) .ConfigureAwait(false); } } diff --git a/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskFixerTests.cs b/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskFixerTests.cs index 642390c08..0f5a61a1a 100644 --- a/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskFixerTests.cs +++ b/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskFixerTests.cs @@ -1,3 +1,4 @@ +using Microsoft.CodeAnalysis.Testing; using Verifier = Moq.Analyzers.Test.Helpers.CodeFixVerifier; namespace Moq.Analyzers.Test; @@ -46,31 +47,74 @@ public static IEnumerable TestData() }.WithNamespaces().WithMoqReferenceAssemblyGroups(); } + /// + /// Anonymous methods and method groups with type mismatches produce compiler errors + /// (CS0029/CS1662), unlike lambdas. We suppress compiler diagnostics to isolate the fixer. + /// + /// Test data with compiler diagnostic suppression for anonymous delegate and method group cases. + public static IEnumerable AnonymousDelegateAndMethodGroupTestData() + { + return new object[][] + { + // Anonymous method returning int on Task method + [ + """new Mock().Setup(s => s.GetValueAsync()).{|Moq1208:Returns(delegate { return 42; })|};""", + """new Mock().Setup(s => s.GetValueAsync()).ReturnsAsync(delegate { return 42; });""", + ], + + // Anonymous method with parameter returning wrong type on Task method + [ + """new Mock().Setup(s => s.ProcessAsync(It.IsAny())).{|Moq1208:Returns(delegate (string x) { return x.Length; })|};""", + """new Mock().Setup(s => s.ProcessAsync(It.IsAny())).ReturnsAsync(delegate (string x) { return x.Length; });""", + ], + + // Method group returning int on Task method + [ + """new Mock().Setup(s => s.GetValueAsync()).{|Moq1208:Returns(GetInt)|};""", + """new Mock().Setup(s => s.GetValueAsync()).ReturnsAsync(GetInt);""", + ], + }.WithNamespaces().WithMoqReferenceAssemblyGroups(); + } + [Theory] [MemberData(nameof(TestData))] public async Task ShouldReplaceReturnsWithReturnsAsync(string referenceAssemblyGroup, string @namespace, string original, string quickFix) { - static string Template(string ns, string mock) => - $$""" - {{ns}} + await VerifyAsync(referenceAssemblyGroup, @namespace, original, quickFix); + } - public class AsyncService - { - public virtual Task GetValueAsync() => Task.FromResult(0); - public virtual Task GetNameAsync() => Task.FromResult(string.Empty); - public virtual ValueTask GetValueTaskAsync() => new ValueTask(0); - public virtual Task ProcessAsync(string input) => Task.FromResult(input.Length); - } + [Theory] + [MemberData(nameof(AnonymousDelegateAndMethodGroupTestData))] + public async Task ShouldReplaceReturnsWithReturnsAsyncForAnonymousDelegateAndMethodGroup(string referenceAssemblyGroup, string @namespace, string original, string quickFix) + { + await VerifyAsync(referenceAssemblyGroup, @namespace, original, quickFix, CompilerDiagnostics.None); + } - internal class UnitTest + private static string Template(string ns, string mock) => + $$""" + {{ns}} + + public class AsyncService + { + public virtual Task GetValueAsync() => Task.FromResult(0); + public virtual Task GetNameAsync() => Task.FromResult(string.Empty); + public virtual ValueTask GetValueTaskAsync() => new ValueTask(0); + public virtual Task ProcessAsync(string input) => Task.FromResult(input.Length); + } + + internal class UnitTest + { + private static int GetInt() => 42; + + private void Test() { - private void Test() - { - {{mock}} - } + {{mock}} } - """; + } + """; + private async Task VerifyAsync(string referenceAssemblyGroup, string @namespace, string original, string quickFix, CompilerDiagnostics? compilerDiagnostics = null) + { string o = Template(@namespace, original); string f = Template(@namespace, quickFix); @@ -80,6 +124,6 @@ private void Test() output.WriteLine("Fixed:"); output.WriteLine(f); - await Verifier.VerifyCodeFixAsync(o, f, referenceAssemblyGroup); + await Verifier.VerifyCodeFixAsync(o, f, referenceAssemblyGroup, compilerDiagnostics).ConfigureAwait(false); } } From 3ee20905fc390b5b09b36b7021bb0b0b0116d164 Mon Sep 17 00:00:00 2001 From: Richard Murillo <6811113+rjmurillo@users.noreply.github.com> Date: Sun, 1 Mar 2026 09:55:45 -0600 Subject: [PATCH 06/10] fix: improve Moq1208 coverage by removing dead code and adding edge case tests Remove proven-dead code paths identified by coverage analysis: - Name-based anonymous method fallback (symbol resolution handles this) - CandidateSymbols fallback in FindSetupInvocation (never triggered) - Zero-argument guards where callers guarantee arguments exist - Duplicate MemberAccessExpressionSyntax check in Analyze - GenericNameSyntax branch in fixer (Moq's Returns is never generic) Merge two pattern matches in FindSetupInvocation into a single compound pattern for both InvocationExpression and MemberAccess. Add tests for uncovered but reachable paths: - Generic non-Task return type (IList) - Property setup (IPropertySymbol vs IMethodSymbol) - Split setup/returns (variable reference breaks chain walk) - Expression variable setup (non-lambda Setup argument) - Void anonymous delegate (null delegate return type) - Non-MemberAccess invocation (early exit path) Analyzer: 98.6% line, 90.9% branch (2 unreachable defensive guards remain) Fixer: 100% line/branch on ReplaceReturnsWithReturnsAsync Co-Authored-By: Claude Opus 4.6 --- ...ReturnsDelegateShouldReturnTaskAnalyzer.cs | 66 +++++-------------- .../ReturnsDelegateShouldReturnTaskFixer.cs | 10 +-- ...nsDelegateShouldReturnTaskAnalyzerTests.cs | 50 ++++++++++++++ 3 files changed, 66 insertions(+), 60 deletions(-) diff --git a/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs b/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs index 638588adf..781183c5f 100644 --- a/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs +++ b/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs @@ -39,7 +39,7 @@ private static void Analyze(SyntaxNodeAnalysisContext context) InvocationExpressionSyntax invocation = (InvocationExpressionSyntax)context.Node; - if (!IsReturnsMethodCallWithSyncDelegate(invocation, context.SemanticModel, knownSymbols, out InvocationExpressionSyntax? setupInvocation)) + if (!IsReturnsMethodCallWithSyncDelegate(invocation, context.SemanticModel, knownSymbols, out MemberAccessExpressionSyntax? memberAccess, out InvocationExpressionSyntax? setupInvocation)) { return; } @@ -50,11 +50,6 @@ private static void Analyze(SyntaxNodeAnalysisContext context) } // Report diagnostic spanning from "Returns" identifier through the closing paren - if (invocation.Expression is not MemberAccessExpressionSyntax memberAccess) - { - return; - } - int startPos = memberAccess.Name.SpanStart; int endPos = invocation.Span.End; Microsoft.CodeAnalysis.Text.TextSpan span = Microsoft.CodeAnalysis.Text.TextSpan.FromBounds(startPos, endPos); @@ -70,11 +65,13 @@ private static bool IsReturnsMethodCallWithSyncDelegate( InvocationExpressionSyntax invocation, SemanticModel semanticModel, MoqKnownSymbols knownSymbols, + [NotNullWhen(true)] out MemberAccessExpressionSyntax? memberAccess, [NotNullWhen(true)] out InvocationExpressionSyntax? setupInvocation) { + memberAccess = null; setupInvocation = null; - if (invocation.Expression is not MemberAccessExpressionSyntax memberAccess) + if (invocation.Expression is not MemberAccessExpressionSyntax access) { return false; } @@ -89,39 +86,28 @@ private static bool IsReturnsMethodCallWithSyncDelegate( .OfType() .Any(m => m.IsMoqReturnsMethod(knownSymbols)); - // Parameterless anonymous methods can cause complete overload resolution failure, - // leaving Symbol null and CandidateSymbols empty. Fall back to verifying the method - // name is "Returns", the argument is an anonymous method, and the call is in a Moq Setup chain. - if (!isReturnsMethod - && string.Equals(memberAccess.Name.Identifier.Text, "Returns", StringComparison.Ordinal) - && invocation.ArgumentList.Arguments.Count > 0 - && invocation.ArgumentList.Arguments[0].Expression is AnonymousMethodExpressionSyntax) + if (!isReturnsMethod) { - setupInvocation = FindSetupInvocation(invocation, semanticModel, knownSymbols); - isReturnsMethod = setupInvocation != null; + return false; } - if (!isReturnsMethod) + if (!HasSyncDelegateArgument(invocation, semanticModel)) { return false; } - if (!HasSyncDelegateArgument(invocation, semanticModel)) + setupInvocation = FindSetupInvocation(access.Expression, semanticModel, knownSymbols); + if (setupInvocation == null) { return false; } - setupInvocation ??= FindSetupInvocation(invocation, semanticModel, knownSymbols); - return setupInvocation != null; + memberAccess = access; + return true; } private static bool HasSyncDelegateArgument(InvocationExpressionSyntax invocation, SemanticModel semanticModel) { - if (invocation.ArgumentList.Arguments.Count == 0) - { - return false; - } - ExpressionSyntax firstArgument = invocation.ArgumentList.Arguments[0].Expression; // Lambdas and anonymous methods share AnonymousFunctionExpressionSyntax, @@ -148,31 +134,20 @@ private static bool IsMethodGroupExpression(ExpressionSyntax expression, Semanti && symbolInfo.CandidateSymbols.OfType().Any(); } - private static InvocationExpressionSyntax? FindSetupInvocation(InvocationExpressionSyntax returnsInvocation, SemanticModel semanticModel, MoqKnownSymbols knownSymbols) + private static InvocationExpressionSyntax? FindSetupInvocation(ExpressionSyntax receiver, SemanticModel semanticModel, MoqKnownSymbols knownSymbols) { // Walk up the fluent chain to find Setup. Handles patterns like: // mock.Setup(...).Returns(...) // mock.Setup(...).Callback(...).Returns(...) - if (returnsInvocation.Expression is not MemberAccessExpressionSyntax memberAccess) - { - return null; - } + ExpressionSyntax current = receiver; // Moq fluent chains are short (Setup.Callback.Returns at most 3-4 deep). // Guard against pathological syntax trees. - const int maxChainDepth = 10; - ExpressionSyntax current = memberAccess.Expression; - - for (int depth = 0; depth < maxChainDepth; depth++) + for (int depth = 0; depth < 10; depth++) { ExpressionSyntax unwrapped = current.WalkDownParentheses(); - if (unwrapped is not InvocationExpressionSyntax candidateInvocation) - { - return null; - } - - if (candidateInvocation.Expression is not MemberAccessExpressionSyntax candidateMemberAccess) + if (unwrapped is not InvocationExpressionSyntax { Expression: MemberAccessExpressionSyntax candidateMemberAccess } candidateInvocation) { return null; } @@ -183,12 +158,6 @@ private static bool IsMethodGroupExpression(ExpressionSyntax expression, Semanti return candidateInvocation; } - if (symbolInfo.CandidateReason is CandidateReason.OverloadResolutionFailure - && symbolInfo.CandidateSymbols.Any(s => s.IsMoqSetupMethod(knownSymbols))) - { - return candidateInvocation; - } - // Continue walking up the chain (past Callback, etc.) current = candidateMemberAccess.Expression; } @@ -255,11 +224,6 @@ private static bool TryGetMismatchInfo( private static ITypeSymbol? GetDelegateReturnType(InvocationExpressionSyntax returnsInvocation, SemanticModel semanticModel) { - if (returnsInvocation.ArgumentList.Arguments.Count == 0) - { - return null; - } - ExpressionSyntax firstArgument = returnsInvocation.ArgumentList.Arguments[0].Expression; // For anonymous methods, prefer body analysis. Roslyn may infer the return type diff --git a/src/CodeFixes/ReturnsDelegateShouldReturnTaskFixer.cs b/src/CodeFixes/ReturnsDelegateShouldReturnTaskFixer.cs index a863a4117..05ea04666 100644 --- a/src/CodeFixes/ReturnsDelegateShouldReturnTaskFixer.cs +++ b/src/CodeFixes/ReturnsDelegateShouldReturnTaskFixer.cs @@ -61,15 +61,7 @@ private static Task ReplaceReturnsWithReturnsAsync( MemberAccessExpressionSyntax memberAccess) { SimpleNameSyntax oldName = memberAccess.Name; - - // Preserve generic type arguments if present (e.g., Returns(...) -> ReturnsAsync(...)) - SimpleNameSyntax newName = oldName is GenericNameSyntax genericName - ? SyntaxFactory.GenericName( - SyntaxFactory.Identifier("ReturnsAsync"), - genericName.TypeArgumentList) - : (SimpleNameSyntax)SyntaxFactory.IdentifierName("ReturnsAsync"); - - newName = newName + SimpleNameSyntax newName = SyntaxFactory.IdentifierName("ReturnsAsync") .WithLeadingTrivia(oldName.GetLeadingTrivia()) .WithTrailingTrivia(oldName.GetTrailingTrivia()); diff --git a/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs b/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs index cc633a126..3519debe8 100644 --- a/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs +++ b/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs @@ -59,6 +59,28 @@ public static IEnumerable ValidTestData() // Block-bodied lambda returning Task (correct) ["""new Mock().Setup(c => c.GetValueAsync()).Returns(() => { return Task.FromResult(42); });"""], + + // Generic non-Task return type (IList is generic but not Task/ValueTask) + ["""new Mock().Setup(c => c.GetItems()).Returns(() => new List());"""], + + // Property setup (resolves to IPropertySymbol, not IMethodSymbol) + ["""new Mock().Setup(c => c.Value).Returns(() => Task.FromResult(42));"""], + + // Split setup/returns (FindSetupInvocation can't walk past variable reference) + [ + """ + var setup = new Mock().Setup(c => c.GetValueAsync()); + setup.Returns(() => Task.FromResult(42)); + """, + ], + + // Expression variable setup (Setup argument is not a lambda, so mocked member can't be extracted) + [ + """ + System.Linq.Expressions.Expression>> expr = c => c.GetValueAsync(); + new Mock().Setup(expr).Returns(() => Task.FromResult(42)); + """, + ], }; return data.WithNamespaces().WithMoqReferenceAssemblyGroups(); @@ -121,6 +143,22 @@ public static IEnumerable InvalidAnonymousDelegateAndMethodGroupTestDa return data.WithNamespaces().WithMoqReferenceAssemblyGroups(); } + /// + /// Valid patterns that produce compiler errors (CS0029/CS1662) but should not trigger the analyzer. + /// We suppress compiler diagnostics to isolate the analyzer. + /// + /// Test data with compiler diagnostic suppression. + public static IEnumerable ValidWithCompilerSuppression() + { + IEnumerable data = new object[][] + { + // Void anonymous delegate on async method (delegate return type is null, no mismatch to report) + ["""new Mock().Setup(c => c.GetValueAsync()).Returns(delegate { });"""], + }; + + return data.WithNamespaces().WithMoqReferenceAssemblyGroups(); + } + [Theory] [MemberData(nameof(ValidTestData))] public async Task ShouldNotTriggerOnValidPatterns(string referenceAssemblyGroup, string @namespace, string mock) @@ -128,6 +166,13 @@ public async Task ShouldNotTriggerOnValidPatterns(string referenceAssemblyGroup, await VerifyAsync(referenceAssemblyGroup, @namespace, mock); } + [Theory] + [MemberData(nameof(ValidWithCompilerSuppression))] + public async Task ShouldNotTriggerOnValidPatternsWithCompilerSuppression(string referenceAssemblyGroup, string @namespace, string mock) + { + await VerifyAsync(referenceAssemblyGroup, @namespace, mock, CompilerDiagnostics.None); + } + [Theory] [MemberData(nameof(InvalidTestData))] public async Task ShouldTriggerOnSyncDelegateMismatch(string referenceAssemblyGroup, string @namespace, string mock) @@ -165,6 +210,8 @@ public class AsyncService public virtual Task DoAsync() => Task.CompletedTask; public virtual int GetSync() => 0; public virtual Task ProcessAsync(string input) => Task.FromResult(input.Length); + public virtual IList GetItems() => new List(); + public virtual Task Value { get; set; } = Task.FromResult(0); } internal class UnitTest @@ -173,6 +220,9 @@ internal class UnitTest private static string GetString() => "hello"; private static Task GetIntAsync() => Task.FromResult(42); + // Non-MemberAccess invocation exercises analyzer's early-exit path + private static int CallHelper() => GetInt(); + private void Test() { {{mock}} From e4cf645d12ccaa5175337032d09b17866296e477 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 1 Mar 2026 16:09:08 +0000 Subject: [PATCH 07/10] fix(Moq1208): guard against empty argument list and preserve generic type arguments - Add argument count guard to HasSyncDelegateArgument to prevent IndexOutOfRangeException - Add argument count guard to GetDelegateReturnType for consistency - Preserve GenericNameSyntax type arguments when replacing Returns with ReturnsAsync in code fix --- .../ReturnsDelegateShouldReturnTaskAnalyzer.cs | 10 ++++++++++ .../ReturnsDelegateShouldReturnTaskFixer.cs | 14 +++++++++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs b/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs index 781183c5f..745e20f53 100644 --- a/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs +++ b/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs @@ -108,6 +108,11 @@ private static bool IsReturnsMethodCallWithSyncDelegate( private static bool HasSyncDelegateArgument(InvocationExpressionSyntax invocation, SemanticModel semanticModel) { + if (invocation.ArgumentList.Arguments.Count == 0) + { + return false; + } + ExpressionSyntax firstArgument = invocation.ArgumentList.Arguments[0].Expression; // Lambdas and anonymous methods share AnonymousFunctionExpressionSyntax, @@ -224,6 +229,11 @@ private static bool TryGetMismatchInfo( private static ITypeSymbol? GetDelegateReturnType(InvocationExpressionSyntax returnsInvocation, SemanticModel semanticModel) { + if (returnsInvocation.ArgumentList.Arguments.Count == 0) + { + return null; + } + ExpressionSyntax firstArgument = returnsInvocation.ArgumentList.Arguments[0].Expression; // For anonymous methods, prefer body analysis. Roslyn may infer the return type diff --git a/src/CodeFixes/ReturnsDelegateShouldReturnTaskFixer.cs b/src/CodeFixes/ReturnsDelegateShouldReturnTaskFixer.cs index 05ea04666..8e269d034 100644 --- a/src/CodeFixes/ReturnsDelegateShouldReturnTaskFixer.cs +++ b/src/CodeFixes/ReturnsDelegateShouldReturnTaskFixer.cs @@ -61,7 +61,19 @@ private static Task ReplaceReturnsWithReturnsAsync( MemberAccessExpressionSyntax memberAccess) { SimpleNameSyntax oldName = memberAccess.Name; - SimpleNameSyntax newName = SyntaxFactory.IdentifierName("ReturnsAsync") + SimpleNameSyntax newName; + if (oldName is GenericNameSyntax genericName) + { + newName = SyntaxFactory.GenericName( + SyntaxFactory.Identifier("ReturnsAsync"), + genericName.TypeArgumentList); + } + else + { + newName = SyntaxFactory.IdentifierName("ReturnsAsync"); + } + + newName = newName .WithLeadingTrivia(oldName.GetLeadingTrivia()) .WithTrailingTrivia(oldName.GetTrailingTrivia()); From da575a8aaeee69f29e86dceff3da224e3ee63611 Mon Sep 17 00:00:00 2001 From: Richard Murillo <6811113+rjmurillo@users.noreply.github.com> Date: Sun, 1 Mar 2026 12:11:29 -0600 Subject: [PATCH 08/10] fix: resolve two false positives in Moq1208 from PR review Fix 1: GetReturnTypeFromBlock now prunes LocalFunctionStatementSyntax in addition to AnonymousFunctionExpressionSyntax when searching for return statements. Previously, a local function inside an anonymous delegate (e.g., delegate { int Local() { return 1; } return Task.FromResult(Local()); }) would cause the analyzer to find the local function's "return 1" instead of the outer "return Task.FromResult(...)". Fix 2: IsMethodGroupExpression now filters out InvocationExpressionSyntax before checking symbol info. Previously, Returns(GetInt()) was misclassified as a delegate because GetSymbolInfo resolves invocations to IMethodSymbol (the called method). Invocations are values, not method groups. Both bugs confirmed by regression tests that fail without the fix. Co-Authored-By: Claude Opus 4.6 --- .../ReturnsDelegateShouldReturnTaskAnalyzer.cs | 12 ++++++++++-- .../ReturnsDelegateShouldReturnTaskAnalyzerTests.cs | 6 ++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs b/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs index 745e20f53..9fa82ee51 100644 --- a/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs +++ b/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs @@ -128,6 +128,12 @@ private static bool HasSyncDelegateArgument(InvocationExpressionSyntax invocatio private static bool IsMethodGroupExpression(ExpressionSyntax expression, SemanticModel semanticModel) { + // Invocations (e.g., GetInt()) resolve to IMethodSymbol but are values, not method groups. + if (expression is InvocationExpressionSyntax) + { + return false; + } + SymbolInfo symbolInfo = semanticModel.GetSymbolInfo(expression); if (symbolInfo.Symbol is IMethodSymbol) { @@ -267,9 +273,11 @@ private static bool TryGetMismatchInfo( private static ITypeSymbol? GetReturnTypeFromBlock(BlockSyntax block, SemanticModel semanticModel) { // Find the first return statement in this block, - // pruning nested anonymous functions so we don't pick up their returns. + // pruning nested functions so we don't pick up their returns. ReturnStatementSyntax? returnStatement = block - .DescendantNodes(node => node is not AnonymousFunctionExpressionSyntax) + .DescendantNodes(node => + node is not AnonymousFunctionExpressionSyntax + && node is not LocalFunctionStatementSyntax) .OfType() .FirstOrDefault(); diff --git a/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs b/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs index 3519debe8..41723cf43 100644 --- a/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs +++ b/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs @@ -154,6 +154,12 @@ public static IEnumerable ValidWithCompilerSuppression() { // Void anonymous delegate on async method (delegate return type is null, no mismatch to report) ["""new Mock().Setup(c => c.GetValueAsync()).Returns(delegate { });"""], + + // Anonymous delegate with local function: outer returns Task.FromResult, local returns int (GH PR #942 review thread) + ["""new Mock().Setup(c => c.GetValueAsync()).Returns(delegate { int Local() { return 1; } return Task.FromResult(Local()); });"""], + + // Invocation as value, not delegate: GetInt() is a call, not a method group (GH PR #942 review thread) + ["""new Mock().Setup(c => c.GetValueAsync()).Returns(GetInt());"""], }; return data.WithNamespaces().WithMoqReferenceAssemblyGroups(); From f4df58f0c333aeef1dd309be9d1735326df75af1 Mon Sep 17 00:00:00 2001 From: Richard Murillo <6811113+rjmurillo@users.noreply.github.com> Date: Sun, 1 Mar 2026 13:41:28 -0600 Subject: [PATCH 09/10] fix: resolve markdown lint errors in Moq1208 documentation Co-Authored-By: Claude Opus 4.6 --- docs/rules/Moq1208.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/rules/Moq1208.md b/docs/rules/Moq1208.md index 26754db29..4718ae7cb 100644 --- a/docs/rules/Moq1208.md +++ b/docs/rules/Moq1208.md @@ -1,10 +1,10 @@ # Moq1208: Returns() delegate type mismatch on async method setup -| Item | Value | -| -------- | ----- | -| Enabled | True | +| Item | Value | +| -------- | ------- | +| Enabled | True | | Severity | Warning | -| CodeFix | True | +| CodeFix | True | --- @@ -33,7 +33,7 @@ The rule detects three forms of delegates: The code compiles without errors, but the test fails at runtime with this exception: -``` +```text MockException: Invalid callback. Setup on method with return type 'Task' cannot invoke callback with return type 'int'. ``` From c5f829ab76d9abe773b6fbfa4d90a041e2829052 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 1 Mar 2026 19:48:15 +0000 Subject: [PATCH 10/10] refactor: extract FindSetupInvocation into shared extension method - Move robust FindSetupInvocation implementation from Moq1208 analyzer to InvocationExpressionSyntaxExtensions - Update Moq1206 (ReturnsAsyncShouldBeUsedForAsyncMethodsAnalyzer) to use the shared method - Update Moq1208 (ReturnsDelegateShouldReturnTaskAnalyzer) to use the shared method - Moq1206 now correctly detects Setup(...).Callback(...).Returns(async () => ...) patterns - Update tests to expect diagnostic for Callback-chained async Returns patterns --- ...syncShouldBeUsedForAsyncMethodsAnalyzer.cs | 21 +--------- ...ReturnsDelegateShouldReturnTaskAnalyzer.cs | 33 +--------------- .../InvocationExpressionSyntaxExtensions.cs | 39 ++++++++++++++++++- ...houldBeUsedForAsyncMethodsAnalyzerTests.cs | 7 ++-- 4 files changed, 44 insertions(+), 56 deletions(-) diff --git a/src/Analyzers/ReturnsAsyncShouldBeUsedForAsyncMethodsAnalyzer.cs b/src/Analyzers/ReturnsAsyncShouldBeUsedForAsyncMethodsAnalyzer.cs index e93ed2939..a491e16c5 100644 --- a/src/Analyzers/ReturnsAsyncShouldBeUsedForAsyncMethodsAnalyzer.cs +++ b/src/Analyzers/ReturnsAsyncShouldBeUsedForAsyncMethodsAnalyzer.cs @@ -44,7 +44,8 @@ private static void Analyze(SyntaxNodeAnalysisContext context) } // Find the Setup call that this Returns is chained from - InvocationExpressionSyntax? setupInvocation = FindSetupInvocation(invocation); + MemberAccessExpressionSyntax memberAccess = (MemberAccessExpressionSyntax)invocation.Expression; + InvocationExpressionSyntax? setupInvocation = memberAccess.Expression.FindSetupInvocation(context.SemanticModel, knownSymbols); if (setupInvocation == null) { return; @@ -58,9 +59,6 @@ private static void Analyze(SyntaxNodeAnalysisContext context) } // Report diagnostic on just the Returns(...) method call - // We can safely cast here because IsReturnsMethodCallWithAsyncLambda already verified this is a MemberAccessExpressionSyntax - MemberAccessExpressionSyntax memberAccess = (MemberAccessExpressionSyntax)invocation.Expression; - // Create a span from the Returns identifier through the end of the invocation int startPos = memberAccess.Name.SpanStart; int endPos = invocation.Span.End; @@ -96,21 +94,6 @@ private static bool IsReturnsMethodCallWithAsyncLambda(InvocationExpressionSynta return HasAsyncLambdaArgument(invocation); } - private static InvocationExpressionSyntax? FindSetupInvocation(InvocationExpressionSyntax returnsInvocation) - { - // The pattern is: mock.Setup(...).Returns(...) - // The returnsInvocation is the entire chain, so we need to examine its structure - if (returnsInvocation.Expression is MemberAccessExpressionSyntax memberAccess && - memberAccess.Expression.WalkDownParentheses() is InvocationExpressionSyntax setupInvocation && - setupInvocation.Expression is MemberAccessExpressionSyntax setupMemberAccess && - string.Equals(setupMemberAccess.Name.Identifier.ValueText, "Setup", StringComparison.Ordinal)) - { - return setupInvocation; - } - - return null; - } - private static bool HasAsyncLambdaArgument(InvocationExpressionSyntax invocation) { if (invocation.ArgumentList.Arguments.Count == 0) diff --git a/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs b/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs index 9fa82ee51..042c8d748 100644 --- a/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs +++ b/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs @@ -96,7 +96,7 @@ private static bool IsReturnsMethodCallWithSyncDelegate( return false; } - setupInvocation = FindSetupInvocation(access.Expression, semanticModel, knownSymbols); + setupInvocation = access.Expression.FindSetupInvocation(semanticModel, knownSymbols); if (setupInvocation == null) { return false; @@ -145,37 +145,6 @@ private static bool IsMethodGroupExpression(ExpressionSyntax expression, Semanti && symbolInfo.CandidateSymbols.OfType().Any(); } - private static InvocationExpressionSyntax? FindSetupInvocation(ExpressionSyntax receiver, SemanticModel semanticModel, MoqKnownSymbols knownSymbols) - { - // Walk up the fluent chain to find Setup. Handles patterns like: - // mock.Setup(...).Returns(...) - // mock.Setup(...).Callback(...).Returns(...) - ExpressionSyntax current = receiver; - - // Moq fluent chains are short (Setup.Callback.Returns at most 3-4 deep). - // Guard against pathological syntax trees. - for (int depth = 0; depth < 10; depth++) - { - ExpressionSyntax unwrapped = current.WalkDownParentheses(); - - if (unwrapped is not InvocationExpressionSyntax { Expression: MemberAccessExpressionSyntax candidateMemberAccess } candidateInvocation) - { - return null; - } - - SymbolInfo symbolInfo = semanticModel.GetSymbolInfo(candidateInvocation); - if (symbolInfo.Symbol != null && symbolInfo.Symbol.IsMoqSetupMethod(knownSymbols)) - { - return candidateInvocation; - } - - // Continue walking up the chain (past Callback, etc.) - current = candidateMemberAccess.Expression; - } - - return null; - } - private static bool TryGetMismatchInfo( InvocationExpressionSyntax setupInvocation, InvocationExpressionSyntax returnsInvocation, diff --git a/src/Common/InvocationExpressionSyntaxExtensions.cs b/src/Common/InvocationExpressionSyntaxExtensions.cs index 7daf30f30..9b53723e0 100644 --- a/src/Common/InvocationExpressionSyntaxExtensions.cs +++ b/src/Common/InvocationExpressionSyntaxExtensions.cs @@ -1,4 +1,4 @@ -namespace Moq.Analyzers.Common; +namespace Moq.Analyzers.Common; /// /// Extension methods for s. @@ -17,6 +17,43 @@ internal static class InvocationExpressionSyntaxExtensions return setupLambdaArgument?.Body as ExpressionSyntax; } + /// + /// Walks up the Moq fluent chain to find the Setup invocation. + /// Handles patterns like mock.Setup(...).Returns(...) and + /// mock.Setup(...).Callback(...).Returns(...). + /// + /// The receiver expression to start walking from (typically the expression before the Returns call). + /// The semantic model for symbol resolution. + /// The known Moq symbols for type checking. + /// The Setup invocation if found; otherwise, . + internal static InvocationExpressionSyntax? FindSetupInvocation(this ExpressionSyntax receiver, SemanticModel semanticModel, MoqKnownSymbols knownSymbols) + { + ExpressionSyntax current = receiver; + + // Moq fluent chains are short (Setup.Callback.Returns at most 3-4 deep). + // Guard against pathological syntax trees. + for (int depth = 0; depth < 10; depth++) + { + ExpressionSyntax unwrapped = current.WalkDownParentheses(); + + if (unwrapped is not InvocationExpressionSyntax { Expression: MemberAccessExpressionSyntax candidateMemberAccess } candidateInvocation) + { + return null; + } + + SymbolInfo symbolInfo = semanticModel.GetSymbolInfo(candidateInvocation); + if (symbolInfo.Symbol != null && symbolInfo.Symbol.IsMoqSetupMethod(knownSymbols)) + { + return candidateInvocation; + } + + // Continue walking up the chain (past Callback, etc.) + current = candidateMemberAccess.Expression; + } + + return null; + } + /// /// Determines if an invocation is a Raises method call using symbol-based detection. /// This method verifies the method belongs to IRaiseable or IRaiseableAsync. diff --git a/tests/Moq.Analyzers.Test/ReturnsAsyncShouldBeUsedForAsyncMethodsAnalyzerTests.cs b/tests/Moq.Analyzers.Test/ReturnsAsyncShouldBeUsedForAsyncMethodsAnalyzerTests.cs index 8cca0806a..6c7990d83 100644 --- a/tests/Moq.Analyzers.Test/ReturnsAsyncShouldBeUsedForAsyncMethodsAnalyzerTests.cs +++ b/tests/Moq.Analyzers.Test/ReturnsAsyncShouldBeUsedForAsyncMethodsAnalyzerTests.cs @@ -26,10 +26,6 @@ public static IEnumerable TestData() // Setup without Returns call should not be affected ["""new Mock().Setup(c => c.GetAsync());"""], - // Callback chained before Returns: FindSetupInvocation returns null because - // the expression before .Returns is Callback, not Setup - ["""new Mock().Setup(c => c.GetAsync()).Callback(() => { }).Returns(async () => "value");"""], - // Parenthesized Setup with ReturnsAsync (valid) ["""(new Mock().Setup(c => c.GetAsync())).ReturnsAsync("value");"""], @@ -52,6 +48,9 @@ public static IEnumerable TestData() // Async lambda in Returns for ValueTask method ["""new Mock().Setup(c => c.DoValueTaskAsync()).{|Moq1206:Returns(async () => { })|};"""], + // Callback chained before Returns should still detect the Setup call + ["""new Mock().Setup(c => c.GetAsync()).Callback(() => { }).{|Moq1206:Returns(async () => "value")|};"""], + // Parenthesized Setup with async Returns (invalid) ["""(new Mock().Setup(c => c.GetAsync())).{|Moq1206:Returns(async () => "value")|};"""],