diff --git a/README.md b/README.md index b8c7ddb1a..2976629be 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 new file mode 100644 index 000000000..4718ae7cb --- /dev/null +++ b/docs/rules/Moq1208.md @@ -0,0 +1,133 @@ +# Moq1208: Returns() delegate type mismatch on async method setup + +| Item | Value | +| -------- | ------- | +| Enabled | True | +| Severity | Warning | +| CodeFix | True | + +--- + +## 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 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 +exception: + +```text +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` 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 + +```csharp +public interface IService +{ + Task GetValueAsync(); // Returns Task + Task GetNameAsync(); // Returns Task + ValueTask GetValueTaskAsync(); // Returns ValueTask +} + +var mock = new Mock(); + +// Lambda returning int on a Task method. +mock.Setup(x => x.GetValueAsync()).Returns(() => 42); // Moq1208 + +// Lambda returning string on a Task method. +mock.Setup(x => x.GetNameAsync()).Returns(() => "hello"); // Moq1208 + +// 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 +``` + +## Solution + +### 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(); + +// 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); + +// 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 + +If you need more control, keep `.Returns()` and wrap the value explicitly. + +```csharp +var mock = new Mock(); + +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); +#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/docs/rules/README.md b/docs/rules/README.md index 4fea2fc44..b6587232f 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) | 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/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 new file mode 100644 index 000000000..042c8d748 --- /dev/null +++ b/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs @@ -0,0 +1,260 @@ +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 '{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( + 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, out MemberAccessExpressionSyntax? memberAccess, out InvocationExpressionSyntax? setupInvocation)) + { + return; + } + + if (!TryGetMismatchInfo(setupInvocation, invocation, context.SemanticModel, knownSymbols, out string? methodName, out ITypeSymbol? expectedReturnType, out ITypeSymbol? delegateReturnType)) + { + return; + } + + // Report diagnostic spanning from "Returns" identifier through the closing paren + 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); + + string actualDisplay = delegateReturnType.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat); + string expectedDisplay = expectedReturnType.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat); + Diagnostic diagnostic = location.CreateDiagnostic(Rule, methodName, actualDisplay, expectedDisplay); + context.ReportDiagnostic(diagnostic); + } + + 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 access) + { + 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.CandidateReason is CandidateReason.OverloadResolutionFailure + && symbolInfo.CandidateSymbols + .OfType() + .Any(m => m.IsMoqReturnsMethod(knownSymbols)); + + if (!isReturnsMethod) + { + return false; + } + + if (!HasSyncDelegateArgument(invocation, semanticModel)) + { + return false; + } + + setupInvocation = access.Expression.FindSetupInvocation(semanticModel, knownSymbols); + if (setupInvocation == null) + { + return false; + } + + 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, + // which exposes AsyncKeyword for sync/async detection. + if (firstArgument is AnonymousFunctionExpressionSyntax anonymousFunction) + { + return !anonymousFunction.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) + { + // 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) + { + 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 bool TryGetMismatchInfo( + InvocationExpressionSyntax setupInvocation, + InvocationExpressionSyntax returnsInvocation, + 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 + 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 Returns() 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; + expectedReturnType = returnType; + 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; + + // 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 GetReturnTypeFromBlock(block, semanticModel); + } + + // 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 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 functions so we don't pick up their returns. + ReturnStatementSyntax? returnStatement = block + .DescendantNodes(node => + node is not AnonymousFunctionExpressionSyntax + && node is not LocalFunctionStatementSyntax) + .OfType() + .FirstOrDefault(); + + if (returnStatement?.Expression == null) + { + return null; + } + + return semanticModel.GetTypeInfo(returnStatement.Expression).Type; + } +} diff --git a/src/CodeFixes/ReturnsDelegateShouldReturnTaskFixer.cs b/src/CodeFixes/ReturnsDelegateShouldReturnTaskFixer.cs new file mode 100644 index 000000000..8e269d034 --- /dev/null +++ b/src/CodeFixes/ReturnsDelegateShouldReturnTaskFixer.cs @@ -0,0 +1,85 @@ +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; + 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()); + + 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/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/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/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")|};"""], diff --git a/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs b/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs new file mode 100644 index 000000000..41723cf43 --- /dev/null +++ b/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs @@ -0,0 +1,249 @@ +using Microsoft.CodeAnalysis.Testing; +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));"""], + + // 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);"""], + + // 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); });"""], + + // 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(); + } + + 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)|};"""], + + // 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(); + } + + /// + /// 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[][] + { + // 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(); + } + + /// + /// 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 { });"""], + + // 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(); + } + + [Theory] + [MemberData(nameof(ValidTestData))] + public async Task ShouldNotTriggerOnValidPatterns(string referenceAssemblyGroup, string @namespace, string mock) + { + 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) + { + await VerifyAsync(referenceAssemblyGroup, @namespace, mock); + } + + [Theory] + [MemberData(nameof(InvalidAnonymousDelegateAndMethodGroupTestData))] + public async Task ShouldFlagAnonymousDelegateAndMethodGroupMismatch(string referenceAssemblyGroup, string @namespace, string mock) + { + await VerifyAsync(referenceAssemblyGroup, @namespace, mock, CompilerDiagnostics.None); + } + + [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, CompilerDiagnostics? compilerDiagnostics = null) + { + 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); + public virtual IList GetItems() => new List(); + public virtual Task Value { get; set; } = Task.FromResult(0); + } + + internal class UnitTest + { + private static int GetInt() => 42; + 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}} + } + } + """; + + output.WriteLine(source); + + await Verifier.VerifyAnalyzerAsync( + source, + referenceAssemblyGroup, + configFileName: null, + configContent: null, + compilerDiagnostics) + .ConfigureAwait(false); + } +} diff --git a/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskFixerTests.cs b/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskFixerTests.cs new file mode 100644 index 000000000..0f5a61a1a --- /dev/null +++ b/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskFixerTests.cs @@ -0,0 +1,129 @@ +using Microsoft.CodeAnalysis.Testing; +using Verifier = Moq.Analyzers.Test.Helpers.CodeFixVerifier; + +namespace Moq.Analyzers.Test; + +public class ReturnsDelegateShouldReturnTaskFixerTests(ITestOutputHelper 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);""", + ], + + // 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(); + } + + /// + /// 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) + { + await VerifyAsync(referenceAssemblyGroup, @namespace, original, quickFix); + } + + [Theory] + [MemberData(nameof(AnonymousDelegateAndMethodGroupTestData))] + public async Task ShouldReplaceReturnsWithReturnsAsyncForAnonymousDelegateAndMethodGroup(string referenceAssemblyGroup, string @namespace, string original, string quickFix) + { + await VerifyAsync(referenceAssemblyGroup, @namespace, original, quickFix, CompilerDiagnostics.None); + } + + 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() + { + {{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); + + output.WriteLine("Original:"); + output.WriteLine(o); + output.WriteLine(string.Empty); + output.WriteLine("Fixed:"); + output.WriteLine(f); + + await Verifier.VerifyCodeFixAsync(o, f, referenceAssemblyGroup, compilerDiagnostics).ConfigureAwait(false); + } +}