feat: add Moq1208 analyzer for Returns() delegate mismatch on async methods#942
Merged
feat: add Moq1208 analyzer for Returns() delegate mismatch on async methods#942
Conversation
…ync 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<int>), which causes a runtime MockException. Includes a code fix to transform Returns to ReturnsAsync. Closes #767 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds analyzer Moq1208 that detects .Returns(...) delegates that don't return Task/ValueTask for async mocked members, a code fix replacing .Returns with .ReturnsAsync, documentation and README updates, release notes, and comprehensive analyzer/fixer tests. Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(240,248,255,0.5)
participant IDE as IDE/Compiler
participant Analyzer as ReturnsDelegateShouldReturnTaskAnalyzer
participant Diagnostic as DiagnosticEngine
participant CodeFix as ReturnsDelegateShouldReturnTaskFixer
participant Document as SourceDocument
end
IDE->>Analyzer: Analyze InvocationExpression + semantic model
Analyzer->>Analyzer: Locate Setup(...) chain and Returns(...) argument
Analyzer->>Analyzer: Resolve mocked member return type and delegate return type
Analyzer->>Diagnostic: Report Moq1208 diagnostic spanning Returns invocation
IDE->>CodeFix: Request fixes for Moq1208
CodeFix->>Document: Replace `Returns` with `ReturnsAsync` (preserve trivia/generics)
CodeFix->>IDE: Return updated document
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the Moq.Analyzers suite by adding a new diagnostic tool and an automated code fix. The primary goal is to prevent common runtime Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
rjmurillo
added
documentation
analyzers
![qltysh[bot]](https://avatars.githubusercontent.com/in/890766?s=60&v=4){kind=small}
tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskFixerTests.cs
Outdated
Show resolved
Hide resolved
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This is an excellent pull request that adds a valuable new analyzer and code fix. The changes are well-structured, the code is clean, and the test coverage is impressive. The documentation for the new rule is also very clear.
I've found one area for improvement in the analyzer's implementation. The GetDelegateReturnType method in ReturnsDelegateShouldReturnTaskAnalyzer.cs currently only supports expression-bodied lambdas. It should be updated to handle block-bodied lambdas as well to cover all cases. I've left a specific comment with a suggested implementation that uses semanticModel.GetSymbolInfo on the lambda to make it more robust.
Once the analyzer is updated, it would be great to add a new test case to both the analyzer and code fix tests to cover block-bodied lambdas (e.g., ...Returns(() => { return 42; })).
Overall, great work on this feature!
 |
|
Overall Grade Focus Area: Hygiene |
Security  Reliability Complexity  Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| C# |  |
Mar 1, 2026 7:48p.m. | Review ↗ |
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a new Roslyn analyzer (Moq1208) and code fix to catch Moq async setups that use .Returns(...) with a non-Task-returning delegate, and to automatically convert those calls to .ReturnsAsync(...).
Changes:
- Introduces Moq1208 diagnostic ID and release tracking entry.
- Adds
ReturnsDelegateShouldReturnTaskAnalyzerto detect invalid.Returns(<sync delegate>)onTask<T>/ValueTask<T>setups. - Adds
ReturnsDelegateShouldReturnTaskFixerplus new analyzer/code-fix tests and rule documentation.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Common/DiagnosticIds.cs | Adds the Moq1208 diagnostic ID constant. |
| src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs | New analyzer implementation for detecting Returns delegate mismatches on async methods. |
| src/CodeFixes/ReturnsDelegateShouldReturnTaskFixer.cs | New code fix to replace .Returns(...) with .ReturnsAsync(...). |
| src/Analyzers/AnalyzerReleases.Unshipped.md | Adds Moq1208 entry for release tracking. |
| tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs | Adds analyzer test coverage for valid/invalid patterns. |
| tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskFixerTests.cs | Adds code fix tests for converting .Returns to .ReturnsAsync. |
| docs/rules/Moq1208.md | Adds documentation for the new rule. |
- 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 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/rules/Moq1208.md`:
- Around line 3-8: The Markdown in the Moq1208 rule doc needs formatting: add a
blank line before and after the table (the block starting with "| Item | Value
|") so the table is separated from surrounding text, and convert the suggested
code block at the bottom into a fenced code block with an explicit language
(e.g., ```text) so lines 12-14 declare the fence language; update the block
around the MockException example to use the fenced syntax with the language
token and ensure there is a blank line before and after that fenced block as
well.
In `@src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs`:
- Around line 12-13: The diagnostic message and description (the static fields
Message and Description in ReturnsDelegateShouldReturnTaskAnalyzer) don't
include the expected async return type, making reports ambiguous; update those
strings to add a third placeholder for the expected return type (e.g. "...should
return a {2} type, not '{1}'..." and description to mention the expected type),
and then ensure wherever the diagnostic is constructed (the
Diagnostic.Create/Report call inside ReturnsDelegateShouldReturnTaskAnalyzer's
analysis method) you pass the computed expected return type as the third
argument to fill that placeholder so the diagnostic shows the exact expected
Task/ValueTask<T> type.
- Around line 20-21: The diagnostic for Moq1208 in
ReturnsDelegateShouldReturnTaskAnalyzer is currently created with
DiagnosticSeverity.Warning; change its DiagnosticDescriptor/creation to use
DiagnosticSeverity.Error (compile-time error) so the rule enforces an
error-level diagnostic, i.e., locate the DiagnosticDescriptor or
Diagnostic.Create call for Moq1208 in ReturnsDelegateShouldReturnTaskAnalyzer
and replace DiagnosticSeverity.Warning with DiagnosticSeverity.Error.
- Around line 93-110: HasSyncDelegateArgument currently only recognizes
expression-bodied lambdas and ignores block-bodied lambdas and anonymous
delegates; update it to accept LambdaExpressionSyntax (including
ParenthesizedLambdaExpressionSyntax), and AnonymousMethodExpressionSyntax, then
determine sync-ness by: for lambdas/anonymous methods first check AsyncKeyword
(reject async), then if body is an ExpressionSyntax treat as sync, and if body
is a BlockSyntax inspect the block for a single ReturnStatementSyntax whose
Expression is not an AwaitExpression (treat as sync); mirror this same logic in
the other analyzer locations referenced around lines 203–225/212–218 so
block-bodied lambdas and anonymous delegates are analyzed, and add regression
tests covering Returns(() => { return 42; }) and Returns(delegate { return 42;
}).
In `@src/CodeFixes/ReturnsDelegateShouldReturnTaskFixer.cs`:
- Around line 58-71: The ReplaceReturnsWithReturnsAsync fixer currently always
renames memberAccess.Name to "ReturnsAsync", which is invalid for ValueTask<T>;
update ReplaceReturnsWithReturnsAsync to examine the mocked method return type
(the setup's generic/expected return type) and branch: if the return type is
Task<T> keep the existing rename to ReturnsAsync (preserving trivia), but if
it's ValueTask<T> instead construct a new invocation that calls Returns and
wraps the original delegate result with a ValueTask<T>(...) expression (i.e.,
replace the memberAccess/argument expression so the delegate return is wrapped
in new ValueTask<T>(result)); ensure you update root via
root.ReplaceNode(document.WithSyntaxRoot(...)) the same way but creating the
correct MemberAccessExpressionSyntax/InvocationExpressionSyntax for each branch
and preserve existing trivia from the original memberAccess and its name.
In `@tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs`:
- Around line 79-84: The test ShouldNotTriggerOnValidPatterns is using
VerifyAsync but per guidelines no-diagnostic tests must call
AllAnalyzersVerifier.VerifyAllAnalyzersAsync; update the method to await
AllAnalyzersVerifier.VerifyAllAnalyzersAsync(referenceAssemblyGroup, `@namespace`,
mock) instead of awaiting VerifyAsync, keeping the same parameters and async
signature so the test continues to run asynchronously; locate the call in
ReturnsDelegateShouldReturnTaskAnalyzerTests.ShouldNotTriggerOnValidPatterns and
replace the invocation accordingly.
ℹ️ Review info
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
docs/rules/Moq1208.mdsrc/Analyzers/AnalyzerReleases.Unshipped.mdsrc/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cssrc/CodeFixes/ReturnsDelegateShouldReturnTaskFixer.cssrc/Common/DiagnosticIds.cstests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cstests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskFixerTests.cs
tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs
Show resolved
Hide resolved
tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskFixerTests.cs
Outdated
Show resolved
Hide resolved
…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 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Resolved by another fix: Nested return statements missed in block-bodied lambdas
- The FindFirstReturnStatement method was replaced with a semantic model approach that uses lambdaSymbol.ReturnType, which correctly handles nested returns in all control flow constructs.
Preview (4d6345d226)
diff --git a/README.md b/README.md
--- a/README.md
+++ b/README.md
@@ -28,6 +28,7 @@
| [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
--- /dev/null
+++ b/docs/rules/Moq1208.md
@@ -1,0 +1,116 @@
+# 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 the function passed to `.Returns()` gives back a plain
+value like `int` or `string`, but the mocked method is async and returns
+`Task<int>` or `ValueTask<string>`. 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<int>'
+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<int>` method.
+
+## Examples of patterns that are flagged by this analyzer
+
+```csharp
+public interface IService
+{
+ Task<int> GetValueAsync(); // Returns Task<int>
+ Task<string> GetNameAsync(); // Returns Task<string>
+ ValueTask<int> GetValueTaskAsync(); // Returns ValueTask<int>
+}
+
+var mock = new Mock<IService>();
+
+// GetValueAsync returns Task<int>, but the lambda returns int.
+mock.Setup(x => x.GetValueAsync()).Returns(() => 42); // Moq1208
+
+// GetNameAsync returns Task<string>, but the lambda returns string.
+mock.Setup(x => x.GetNameAsync()).Returns(() => "hello"); // Moq1208
+
+// GetValueTaskAsync returns ValueTask<int>, but the lambda returns int.
+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<IService>();
+
+// 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<IService>();
+
+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<int>(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
--- 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
--- a/src/Analyzers/AnalyzerReleases.Unshipped.md
+++ b/src/Analyzers/AnalyzerReleases.Unshipped.md
@@ -17,6 +17,7 @@
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
--- /dev/null
+++ b/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs
@@ -1,0 +1,236 @@
+using System.Diagnostics.CodeAnalysis;
+
+namespace Moq.Analyzers;
+
+/// <summary>
+/// Returns() delegate on async method setup should return Task/ValueTask to match the mocked method's return type.
+/// </summary>
+[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");
+
+ /// <inheritdoc />
+ public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Rule);
+
+ /// <inheritdoc />
+ 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? expectedReturnType, 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);
+
+ 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)
+ {
+ 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<IMethodSymbol>()
+ .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;
+ }
+
+ // 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++)
+ {
+ 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;
+ }
+
+ return null;
+ }
+
+ 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<T> or ValueTask<T> (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;
+ 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;
+
+ if (firstArgument is not LambdaExpressionSyntax lambda)
+ {
+ return null;
+ }
+
+ // 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)
+ {
+ return lambdaSymbol.ReturnType;
+ }
+
+ return null;
+ }
+}
diff --git a/src/CodeFixes/ReturnsDelegateShouldReturnTaskFixer.cs b/src/CodeFixes/ReturnsDelegateShouldReturnTaskFixer.cs
new file mode 100644
--- /dev/null
+++ b/src/CodeFixes/ReturnsDelegateShouldReturnTaskFixer.cs
@@ -1,0 +1,81 @@
+using System.Composition;
+using Microsoft.CodeAnalysis.CodeActions;
+using Microsoft.CodeAnalysis.CodeFixes;
+
+namespace Moq.CodeFixes;
+
+/// <summary>
+/// Fixes for <see cref="DiagnosticIds.ReturnsDelegateMismatchOnAsyncMethod"/> (Moq1208).
+/// Replaces <c>.Returns(delegate)</c> with <c>.ReturnsAsync(delegate)</c> when the
+/// mocked method is async and the delegate returns the unwrapped type.
+/// </summary>
+[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(ReturnsDelegateShouldReturnTaskFixer))]
+[Shared]
+public sealed class ReturnsDelegateShouldReturnTaskFixer : CodeFixProvider
+{
+ private static readonly string Title = "Use ReturnsAsync instead of Returns";
+
+ /// <inheritdoc />
+ public override ImmutableArray<string> FixableDiagnosticIds { get; } =
+ ImmutableArray.Create(DiagnosticIds.ReturnsDelegateMismatchOnAsyncMethod);
+
+ /// <inheritdoc />
+ public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;
+
+ /// <inheritdoc />
+ 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<MemberAccessExpressionSyntax>();
+ 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<Document> ReplaceReturnsWithReturnsAsync(
+ Document document,
+ SyntaxNode root,
+ MemberAccessExpressionSyntax memberAccess)
+ {
+ SimpleNameSyntax oldName = memberAccess.Name;
+
+ // Preserve generic type arguments if present (e.g., Returns<int>(...) -> ReturnsAsync<int>(...))
+ 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());
+
+ 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
--- a/src/Common/DiagnosticIds.cs
+++ b/src/Common/DiagnosticIds.cs
@@ -17,6 +17,7 @@
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
--- /dev/null
+++ b/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs
@@ -1,0 +1,147 @@
+using Verifier = Moq.Analyzers.Test.Helpers.AnalyzerVerifier<Moq.Analyzers.ReturnsDelegateShouldReturnTaskAnalyzer>;
+
+namespace Moq.Analyzers.Test;
+
+public class ReturnsDelegateShouldReturnTaskAnalyzerTests(ITestOutputHelper output)
+{
+ public static IEnumerable<object[]> ValidTestData()
+ {
+ IEnumerable<object[]> data = new object[][]
+ {
+ // Delegate returns Task<T> (correct)
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).Returns(() => Task.FromResult(42));"""],
+
+ // Uses ReturnsAsync (correct, different overload)
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).ReturnsAsync(42);"""],
+
+ // Direct value, not a delegate
+ ["""new Mock<AsyncService>().Setup(c => c.GetSync()).Returns(42);"""],
+
+ // Non-async method with sync delegate (no mismatch)
+ ["""new Mock<AsyncService>().Setup(c => c.GetSync()).Returns(() => 42);"""],
+
+ // Async lambda (Moq1206's domain, not ours)
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).Returns(async () => 42);"""],
+
+ // Setup without Returns call
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync());"""],
+
+ // Delegate returns ValueTask<T> (correct)
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueTaskAsync()).Returns(() => ValueTask.FromResult(42));"""],
+
+ // Chained Callback with correct Task return
+ ["""new Mock<AsyncService>().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<AsyncService>().Setup(c => c.DoAsync()).Returns(() => Task.CompletedTask);"""],
+
+ // Parenthesized Setup with ReturnsAsync (valid)
+ ["""(new Mock<AsyncService>().Setup(c => c.GetValueAsync())).ReturnsAsync(42);"""],
+
+ // Block-bodied lambda returning Task (correct)
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).Returns(() => { return Task.FromResult(42); });"""],
+ };
+
+ return data.WithNamespaces().WithMoqReferenceAssemblyGroups();
+ }
+
+ public static IEnumerable<object[]> InvalidTestData()
+ {
+ IEnumerable<object[]> data = new object[][]
+ {
+ // Sync delegate returning int on Task<int> method
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).{|Moq1208:Returns(() => 42)|};"""],
+
+ // Sync delegate returning string on Task<string> method
+ ["""new Mock<AsyncService>().Setup(c => c.GetNameAsync()).{|Moq1208:Returns(() => "hello")|};"""],
+
+ // Sync delegate returning int on ValueTask<int> method
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueTaskAsync()).{|Moq1208:Returns(() => 42)|};"""],
+
+ // Parenthesized Setup with sync delegate mismatch
+ ["""(new Mock<AsyncService>().Setup(c => c.GetValueAsync())).{|Moq1208:Returns(() => 42)|};"""],
+
+ // Chained Callback then Returns with sync delegate mismatch
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).Callback(() => { }).{|Moq1208:Returns(() => 42)|};"""],
+
+ // Block-bodied lambda returning wrong type on Task<int> method
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).{|Moq1208:Returns(() => { return 42; })|};"""],
+ };
+
+ return data.WithNamespaces().WithMoqReferenceAssemblyGroups();
+ }
+
+ public static IEnumerable<object[]> DelegateOverloadTestData()
+ {
+ IEnumerable<object[]> data = new object[][]
+ {
+ // Sync delegate with parameter returning wrong type on Task<int> method
+ ["""new Mock<AsyncService>().Setup(c => c.ProcessAsync(It.IsAny<string>())).{|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<int> GetValueAsync() => Task.FromResult(0);
+ public virtual Task<string> GetNameAsync() => Task.FromResult(string.Empty);
+ public virtual ValueTask<int> GetValueTaskAsync() => ValueTask.FromResult(0);
+ public virtual Task DoAsync() => Task.CompletedTask;
+ public virtual int GetSync() => 0;
+ public virtual Task<int> 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
--- /dev/null
+++ b/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskFixerTests.cs
@@ -1,0 +1,85 @@
+using Verifier = Moq.Analyzers.Test.Helpers.CodeFixVerifier<Moq.Analyzers.ReturnsDelegateShouldReturnTaskAnalyzer, Moq.CodeFixes.ReturnsDelegateShouldReturnTaskFixer>;
+
+namespace Moq.Analyzers.Test;
+
+public class ReturnsDelegateShouldReturnTaskFixerTests(ITestOutputHelper output)
+{
+ public static IEnumerable<object[]> TestData()
+ {
+ return new object[][]
+ {
+ // Task<int> with parameterless lambda returning int
+ [
+ """new Mock<AsyncService>().Setup(s => s.GetValueAsync()).{|Moq1208:Returns(() => 42)|};""",
+ """new Mock<AsyncService>().Setup(s => s.GetValueAsync()).ReturnsAsync(() => 42);""",
+ ],
+
+ // Task<string> with parameterless lambda returning string
+ [
+ """new Mock<AsyncService>().Setup(s => s.GetNameAsync()).{|Moq1208:Returns(() => "hello")|};""",
+ """new Mock<AsyncService>().Setup(s => s.GetNameAsync()).ReturnsAsync(() => "hello");""",
+ ],
+
+ // ValueTask<int> with parameterless lambda returning int
+ [
+ """new Mock<AsyncService>().Setup(s => s.GetValueTaskAsync()).{|Moq1208:Returns(() => 42)|};""",
+ """new Mock<AsyncService>().Setup(s => s.GetValueTaskAsync()).ReturnsAsync(() => 42);""",
+ ],
+
+ // Delegate with parameter
+ [
+ """new Mock<AsyncService>().Setup(s => s.ProcessAsync(It.IsAny<string>())).{|Moq1208:Returns((string x) => x.Length)|};""",
+ """new Mock<AsyncService>().Setup(s => s.ProcessAsync(It.IsAny<string>())).ReturnsAsync((string x) => x.Length);""",
+ ],
+
+ // Parenthesized Setup expression
+ [
+ """(new Mock<AsyncService>().Setup(s => s.GetValueAsync())).{|Moq1208:Returns(() => 42)|};""",
+ """(new Mock<AsyncService>().Setup(s => s.GetValueAsync())).ReturnsAsync(() => 42);""",
+ ],
+
+ // Block-bodied lambda returning wrong type
+ [
+ """new Mock<AsyncService>().Setup(s => s.GetValueAsync()).{|Moq1208:Returns(() => { return 42; })|};""",
+ """new Mock<AsyncService>().Setup(s => s.GetValueAsync()).ReturnsAsync(() => { return 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<int> GetValueAsync() => Task.FromResult(0);
+ public virtual Task<string> GetNameAsync() => Task.FromResult(string.Empty);
+ public virtual ValueTask<int> GetValueTaskAsync() => new ValueTask<int>(0);
+ public virtual Task<int> 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);
+ }
+}- 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<T> -> ReturnsAsync<T>) - Fix markdown blank line before horizontal rule separator Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
rjmurillo
commented
Mar 1, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
docs/rules/Moq1208.md (1)
30-33:⚠️ Potential issue | 🟡 MinorAdd language specifier to fenced code block.
The exception example block should specify a language for consistent formatting.
📝 Suggested fix
-``` +```text MockException: Invalid callback. Setup on method with return type 'Task<int>' cannot invoke callback with return type 'int'.</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/rules/Moq1208.mdaround lines 30 - 33, The fenced code block showing
the MockException example lacks a language specifier; update the fenced block
that contains "MockException: Invalid callback. Setup on method with return type
'Task' cannot invoke callback with return type 'int'." to include a
language tag (e.g., text) after the opening backticks so the exception is
consistently formatted.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs:
- Around line 95-112: The HasSyncDelegateArgument method currently only treats
firstArgument as LambdaExpressionSyntax and ignores anonymous delegates
(AnonymousMethodExpressionSyntax); either update HasSyncDelegateArgument to also
detect and handle AnonymousMethodExpressionSyntax (check for
AnonymousMethodExpressionSyntax and treat non-async anonymous methods as sync
delegates) or, if you want to postpone the implementation, add a clear TODO
comment above HasSyncDelegateArgument noting the known gap (anonymous delegates
not handled) and referencing AnonymousMethodExpressionSyntax so future
maintainers know what to change; keep the existing logic for excluding async
delegates by checking the AsyncKeyword in both LambdaExpressionSyntax and
AnonymousMethodExpressionSyntax cases.
Duplicate comments:
In@docs/rules/Moq1208.md:
- Around line 30-33: The fenced code block showing the MockException example
lacks a language specifier; update the fenced block that contains
"MockException: Invalid callback. Setup on method with return type 'Task'
cannot invoke callback with return type 'int'." to include a language tag (e.g.,
text) after the opening backticks so the exception is consistently formatted.</details> --- <details> <summary>ℹ️ Review info</summary> **Configuration used**: Repository UI **Review profile**: ASSERTIVE **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 04a1051e66bcbba506ead09df524d9bac9194d40 and 4d6345d226a968b3e703817f2c52e72361a4dc8e. </details> <details> <summary>📒 Files selected for processing (7)</summary> * `README.md` * `docs/rules/Moq1208.md` * `docs/rules/README.md` * `src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs` * `src/CodeFixes/ReturnsDelegateShouldReturnTaskFixer.cs` * `tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs` * `tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskFixerTests.cs` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
…atches
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 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
docs/rules/Moq1208.md (1)
36-39:⚠️ Potential issue | 🟡 MinorAdd a language identifier to the fenced runtime-exception block.
Line 36 uses an unlabeled fenced block, which violates MD040.
📝 Proposed fix
-``` +```text MockException: Invalid callback. Setup on method with return type 'Task<int>' cannot invoke callback with return type 'int'.</details> As per coding guidelines: All documentation and markdown files must pass formatting checks; use a markdown linter if available. <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/rules/Moq1208.mdaround lines 36 - 39, The fenced code block showing
the MockException message is unlabeled (violates MD040); update the fenced block
around the runtime-exception text (the block that starts with "MockException:
Invalid callback. Setup on method with return type 'Task'") to include a
language identifier such as text (e.g., add "text" after the opening backticks)
so the markdown linter accepts it.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs:
- Around line 29-34: Replace the syntax-node registration with an
operation-based registration: in Initialize() stop calling
context.RegisterSyntaxNodeAction(Analyze, SyntaxKind.InvocationExpression) and
instead call context.RegisterOperationAction(AnalyzeInvocation,
OperationKind.Invocation); implement a new
AnalyzeInvocation(OperationAnalysisContext context) that casts context.Operation
to IInvocationOperation and uses invocation.TargetMethod for symbol/type checks
instead of semanticModel.GetSymbolInfo()/GetTypeInfo; update or remove the old
Analyze(SyntaxNodeAnalysisContext) helper and adjust any references to rely on
IInvocationOperation, OperationAnalysisContext, and TargetMethod for the
existing symbol/type logic.- Around line 293-300: GetReturnTypeFromBlock currently prunes anonymous
functions but still descends into local functions, causing it to pick up returns
from LocalFunctionStatementSyntax; update the descendant traversal predicate in
GetReturnTypeFromBlock so it does not descend into LocalFunctionStatementSyntax
(along with AnonymousFunctionExpressionSyntax) when searching for
ReturnStatementSyntax, then add a regression test in
tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs where
an anonymous delegate contains a local function that returns a primitive while
the delegate returns Task.FromResult(...) to ensure the analyzer infers the
outer delegate return type correctly.
Duplicate comments:
In@docs/rules/Moq1208.md:
- Around line 36-39: The fenced code block showing the MockException message is
unlabeled (violates MD040); update the fenced block around the runtime-exception
text (the block that starts with "MockException: Invalid callback. Setup on
method with return type 'Task'") to include a language identifier such as
text (e.g., add "text" after the opening backticks) so the markdown linter
accepts it.</details> --- <details> <summary>ℹ️ Review info</summary> **Configuration used**: Repository UI **Review profile**: ASSERTIVE **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 4d6345d226a968b3e703817f2c52e72361a4dc8e and 4c5e83ad2d9565de37ed3fcc927e64d73076acb5. </details> <details> <summary>📒 Files selected for processing (5)</summary> * `docs/rules/Moq1208.md` * `src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs` * `tests/Moq.Analyzers.Test/Helpers/CodeFixVerifier.cs` * `tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs` * `tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskFixerTests.cs` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
…ase 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<int>) - 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 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Missing argument count guard causes potential analyzer crash
- Added argument count guards in both HasSyncDelegateArgument and GetDelegateReturnType methods to prevent IndexOutOfRangeException when the argument list is empty.
- ✅ Fixed: Code fix drops generic type arguments from Returns
- Modified ReplaceReturnsWithReturnsAsync to check if oldName is a GenericNameSyntax and preserve the TypeArgumentList when creating the ReturnsAsync replacement.
Preview (e4cf645d12)
diff --git a/README.md b/README.md
--- a/README.md
+++ b/README.md
@@ -28,6 +28,7 @@
| [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
--- /dev/null
+++ b/docs/rules/Moq1208.md
@@ -1,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<int>` or `ValueTask<string>`. 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:
+
+```
+MockException: Invalid callback. Setup on method with return type 'Task<int>'
+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<int>` method.
+
+## Examples of patterns that are flagged by this analyzer
+
+```csharp
+public interface IService
+{
+ Task<int> GetValueAsync(); // Returns Task<int>
+ Task<string> GetNameAsync(); // Returns Task<string>
+ ValueTask<int> GetValueTaskAsync(); // Returns ValueTask<int>
+}
+
+var mock = new Mock<IService>();
+
+// Lambda returning int on a Task<int> method.
+mock.Setup(x => x.GetValueAsync()).Returns(() => 42); // Moq1208
+
+// Lambda returning string on a Task<string> method.
+mock.Setup(x => x.GetNameAsync()).Returns(() => "hello"); // Moq1208
+
+// Anonymous method returning int on a Task<int> method.
+mock.Setup(x => x.GetValueAsync()).Returns(delegate { return 42; }); // Moq1208
+
+// Method group returning int on a Task<int> method.
+mock.Setup(x => x.GetValueAsync()).Returns(GetInt); // Moq1208
+// where: static int GetInt() => 42;
+
+// ValueTask<int> 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<IService>();
+
+// 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<IService>();
+
+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<int>(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
--- 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
--- a/src/Analyzers/AnalyzerReleases.Unshipped.md
+++ b/src/Analyzers/AnalyzerReleases.Unshipped.md
@@ -17,6 +17,7 @@
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
--- /dev/null
+++ b/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs
@@ -1,0 +1,283 @@
+using System.Diagnostics.CodeAnalysis;
+
+namespace Moq.Analyzers;
+
+/// <summary>
+/// Returns() delegate on async method setup should return Task/ValueTask to match the mocked method's return type.
+/// </summary>
+[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");
+
+ /// <inheritdoc />
+ public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Rule);
+
+ /// <inheritdoc />
+ 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<IMethodSymbol>()
+ .Any(m => m.IsMoqReturnsMethod(knownSymbols));
+
+ if (!isReturnsMethod)
+ {
+ return false;
+ }
+
+ if (!HasSyncDelegateArgument(invocation, semanticModel))
+ {
+ return false;
+ }
+
+ setupInvocation = FindSetupInvocation(access.Expression, 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)
+ {
+ 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<IMethodSymbol>().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,
+ 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<T> or ValueTask<T> (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<int>) 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<IMethodSymbol>().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<ReturnStatementSyntax>()
+ .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
--- /dev/null
+++ b/src/CodeFixes/ReturnsDelegateShouldReturnTaskFixer.cs
@@ -1,0 +1,85 @@
+using System.Composition;
+using Microsoft.CodeAnalysis.CodeActions;
+using Microsoft.CodeAnalysis.CodeFixes;
+
+namespace Moq.CodeFixes;
+
+/// <summary>
+/// Fixes for <see cref="DiagnosticIds.ReturnsDelegateMismatchOnAsyncMethod"/> (Moq1208).
+/// Replaces <c>.Returns(delegate)</c> with <c>.ReturnsAsync(delegate)</c> when the
+/// mocked method is async and the delegate returns the unwrapped type.
+/// </summary>
+[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(ReturnsDelegateShouldReturnTaskFixer))]
+[Shared]
+public sealed class ReturnsDelegateShouldReturnTaskFixer : CodeFixProvider
+{
+ private static readonly string Title = "Use ReturnsAsync instead of Returns";
+
+ /// <inheritdoc />
+ public override ImmutableArray<string> FixableDiagnosticIds { get; } =
+ ImmutableArray.Create(DiagnosticIds.ReturnsDelegateMismatchOnAsyncMethod);
+
+ /// <inheritdoc />
+ public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;
+
+ /// <inheritdoc />
+ 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<MemberAccessExpressionSyntax>();
+ 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<Document> 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
--- a/src/Common/DiagnosticIds.cs
+++ b/src/Common/DiagnosticIds.cs
@@ -17,6 +17,7 @@
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/Helpers/CodeFixVerifier.cs b/tests/Moq.Analyzers.Test/Helpers/CodeFixVerifier.cs
--- a/tests/Moq.Analyzers.Test/Helpers/CodeFixVerifier.cs
+++ b/tests/Moq.Analyzers.Test/Helpers/CodeFixVerifier.cs
@@ -7,15 +7,22 @@
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<TAnalyzer, TCodeFixProvider>
+ Test<TAnalyzer, TCodeFixProvider> test = new Test<TAnalyzer, TCodeFixProvider>
{
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
new file mode 100644
--- /dev/null
+++ b/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs
@@ -1,0 +1,243 @@
+using Microsoft.CodeAnalysis.Testing;
+using Verifier = Moq.Analyzers.Test.Helpers.AnalyzerVerifier<Moq.Analyzers.ReturnsDelegateShouldReturnTaskAnalyzer>;
+
+namespace Moq.Analyzers.Test;
+
+public class ReturnsDelegateShouldReturnTaskAnalyzerTests(ITestOutputHelper output)
+{
+ public static IEnumerable<object[]> ValidTestData()
+ {
+ IEnumerable<object[]> data = new object[][]
+ {
+ // Delegate returns Task<T> (correct)
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).Returns(() => Task.FromResult(42));"""],
+
+ // Uses ReturnsAsync (correct, different overload)
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).ReturnsAsync(42);"""],
+
+ // Direct value, not a delegate
+ ["""new Mock<AsyncService>().Setup(c => c.GetSync()).Returns(42);"""],
+
+ // Non-async method with sync delegate (no mismatch)
+ ["""new Mock<AsyncService>().Setup(c => c.GetSync()).Returns(() => 42);"""],
+
+ // Async lambda (Moq1206's domain, not ours)
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).Returns(async () => 42);"""],
+
+ // Setup without Returns call
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync());"""],
+
+ // Delegate returns ValueTask<T> (correct)
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueTaskAsync()).Returns(() => ValueTask.FromResult(42));"""],
+
+ // Chained Callback with correct Task return
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).Callback(() => { }).Returns(() => Task.FromResult(42));"""],
+
+ // Anonymous method returning Task.FromResult (correct)
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).Returns(delegate { return Task.FromResult(42); });"""],
+
+ // Async anonymous method (Moq1206's domain, not ours)
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).Returns(async delegate { return 42; });"""],
+
+ // Method group returning Task<int> (correct)
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).Returns(GetIntAsync);"""],
+
+ // Anonymous method on sync method (no mismatch)
+ ["""new Mock<AsyncService>().Setup(c => c.GetSync()).Returns(delegate { return 42; });"""],
+
+ // Method group on sync method (no mismatch)
+ ["""new Mock<AsyncService>().Setup(c => c.GetSync()).Returns(GetInt);"""],
+
+ // Direct value on async method (not a delegate, different Returns overload)
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).Returns(Task.FromResult(42));"""],
+
+ // Non-generic Task method with sync delegate (no mismatch, Task has no inner type)
+ ["""new Mock<AsyncService>().Setup(c => c.DoAsync()).Returns(() => Task.CompletedTask);"""],
+
+ // Parenthesized Setup with ReturnsAsync (valid)
+ ["""(new Mock<AsyncService>().Setup(c => c.GetValueAsync())).ReturnsAsync(42);"""],
+
+ // Block-bodied lambda returning Task (correct)
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).Returns(() => { return Task.FromResult(42); });"""],
+
+ // Generic non-Task return type (IList<int> is generic but not Task/ValueTask)
+ ["""new Mock<AsyncService>().Setup(c => c.GetItems()).Returns(() => new List<int>());"""],
+
+ // Property setup (resolves to IPropertySymbol, not IMethodSymbol)
+ ["""new Mock<AsyncService>().Setup(c => c.Value).Returns(() => Task.FromResult(42));"""],
+
+ // Split setup/returns (FindSetupInvocation can't walk past variable reference)
+ [
+ """
+ var setup = new Mock<AsyncService>().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<Func<AsyncService, Task<int>>> expr = c => c.GetValueAsync();
+ new Mock<AsyncService>().Setup(expr).Returns(() => Task.FromResult(42));
+ """,
+ ],
+ };
+
+ return data.WithNamespaces().WithMoqReferenceAssemblyGroups();
+ }
+
+ public static IEnumerable<object[]> InvalidTestData()
+ {
+ IEnumerable<object[]> data = new object[][]
+ {
+ // Sync delegate returning int on Task<int> method
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).{|Moq1208:Returns(() => 42)|};"""],
+
+ // Sync delegate returning string on Task<string> method
+ ["""new Mock<AsyncService>().Setup(c => c.GetNameAsync()).{|Moq1208:Returns(() => "hello")|};"""],
+
+ // Sync delegate returning int on ValueTask<int> method
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueTaskAsync()).{|Moq1208:Returns(() => 42)|};"""],
+
+ // Parenthesized Setup with sync delegate mismatch
+ ["""(new Mock<AsyncService>().Setup(c => c.GetValueAsync())).{|Moq1208:Returns(() => 42)|};"""],
+
+ // Chained Callback then Returns with sync delegate mismatch
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).Callback(() => { }).{|Moq1208:Returns(() => 42)|};"""],
+
+ // Block-bodied lambda returning wrong type on Task<int> method
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).{|Moq1208:Returns(() => { return 42; })|};"""],
+
+ // Sync delegate with parameter returning wrong type on Task<int> method
+ ["""new Mock<AsyncService>().Setup(c => c.ProcessAsync(It.IsAny<string>())).{|Moq1208:Returns((string x) => x.Length)|};"""],
+ };
+
+ return data.WithNamespaces().WithMoqReferenceAssemblyGroups();
+ }
+
+ /// <summary>
+ /// Anonymous methods and method groups with type mismatches produce compiler errors
+ /// (CS0029/CS1662), unlike lambdas. We suppress compiler diagnostics to isolate the analyzer.
+ /// </summary>
+ /// <returns>Test data with compiler diagnostic suppression for anonymous delegate and method group cases.</returns>
+ public static IEnumerable<object[]> InvalidAnonymousDelegateAndMethodGroupTestData()
+ {
+ IEnumerable<object[]> data = new object[][]
+ {
+ // Anonymous method returning int on Task<int> method
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).{|Moq1208:Returns(delegate { return 42; })|};"""],
+
+ // Anonymous method returning int on ValueTask<int> method
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueTaskAsync()).{|Moq1208:Returns(delegate { return 42; })|};"""],
+
+ // Anonymous method with parameter returning wrong type on Task<int> method
+ ["""new Mock<AsyncService>().Setup(c => c.ProcessAsync(It.IsAny<string>())).{|Moq1208:Returns(delegate (string x) { return x.Length; })|};"""],
+
+ // Method group returning int on Task<int> method
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).{|Moq1208:Returns(GetInt)|};"""],
+
+ // Method group returning string on Task<string> method
+ ["""new Mock<AsyncService>().Setup(c => c.GetNameAsync()).{|Moq1208:Returns(GetString)|};"""],
+ };
+
+ return data.WithNamespaces().WithMoqReferenceAssemblyGroups();
+ }
+
+ /// <summary>
+ /// Valid patterns that produce compiler errors (CS0029/CS1662) but should not trigger the analyzer.
+ /// We suppress compiler diagnostics to isolate the analyzer.
+ /// </summary>
+ /// <returns>Test data with compiler diagnostic suppression.</returns>
+ public static IEnumerable<object[]> ValidWithCompilerSuppression()
+ {
+ IEnumerable<object[]> data = new object[][]
+ {
+ // Void anonymous delegate on async method (delegate return type is null, no mismatch to report)
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).Returns(delegate { });"""],
+ };
+
+ 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);
+ }
+
... diff truncated: showing 800 of 980 lines
…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
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs (1)
271-273:⚠️ Potential issue | 🟠 MajorExclude local-function scopes when inferring anonymous delegate return type.
Line 271-273 prunes nested anonymous functions but still descends into local functions. This can pick
returnfrom a local function instead of the outer anonymous delegate, causing false positives.Proposed fix
ReturnStatementSyntax? returnStatement = block - .DescendantNodes(node => node is not AnonymousFunctionExpressionSyntax) + .DescendantNodes(node => + node is not AnonymousFunctionExpressionSyntax && + node is not LocalFunctionStatementSyntax) .OfType<ReturnStatementSyntax>() .FirstOrDefault();Please also add a regression test for:
Returns(delegate { int Local() { return 1; } return Task.FromResult(Local()); }).Based on learnings: anonymous-method return analysis must use the outer body return expression and avoid nested-function returns to prevent Roslyn target-type inference from masking the true body type.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs` around lines 271 - 273, The analyzer currently prunes nested anonymous functions but still descends into local functions, so update the DescendantNodes predicate in ReturnsDelegateShouldReturnTaskAnalyzer (the block traversal that builds returnStatement via .DescendantNodes(...).OfType<ReturnStatementSyntax>()) to also exclude LocalFunctionStatementSyntax (i.e., change the lambda to return false for LocalFunctionStatementSyntax as well as AnonymousFunctionExpressionSyntax) so nested local functions' return statements are not considered; then add a regression unit test asserting that the anonymous-method case `Returns(delegate { int Local() { return 1; } return Task.FromResult(Local()); })` is treated as returning a Task (no diagnostic), placed alongside the existing analyzer tests for anonymous delegates.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs`:
- Around line 129-139: IsMethodGroupExpression is treating invoked calls as
method groups; change it to first return false if expression is an
InvocationExpressionSyntax, and then only proceed to semanticModel.GetSymbolInfo
for expressions that are SimpleNameSyntax or MemberAccessExpressionSyntax (i.e.,
the real method-group syntaxes). Update the logic in IsMethodGroupExpression to
short-circuit on InvocationExpressionSyntax and restrict the symbol-based checks
(SymbolInfo.Symbol is IMethodSymbol and
CandidateSymbols.OfType<IMethodSymbol>()) to those allowed syntax kinds so
invocation results like GetInt() are not misclassified as delegates.
---
Duplicate comments:
In `@src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs`:
- Around line 271-273: The analyzer currently prunes nested anonymous functions
but still descends into local functions, so update the DescendantNodes predicate
in ReturnsDelegateShouldReturnTaskAnalyzer (the block traversal that builds
returnStatement via .DescendantNodes(...).OfType<ReturnStatementSyntax>()) to
also exclude LocalFunctionStatementSyntax (i.e., change the lambda to return
false for LocalFunctionStatementSyntax as well as
AnonymousFunctionExpressionSyntax) so nested local functions' return statements
are not considered; then add a regression unit test asserting that the
anonymous-method case `Returns(delegate { int Local() { return 1; } return
Task.FromResult(Local()); })` is treated as returning a Task (no diagnostic),
placed alongside the existing analyzer tests for anonymous delegates.
ℹ️ Review info
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cssrc/CodeFixes/ReturnsDelegateShouldReturnTaskFixer.cstests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs
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 <noreply@anthropic.com>
…delegate-mismatch # Conflicts: # docs/rules/README.md
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Duplicated FindSetupInvocation logic across sibling analyzers
- Extracted the robust FindSetupInvocation implementation from Moq1208 into a shared extension method in InvocationExpressionSyntaxExtensions, enabling both Moq1206 and Moq1208 analyzers to use semantic symbol-based detection that walks up to 10 levels through fluent chains.
Preview (c5f829ab76)
diff --git a/README.md b/README.md
--- a/README.md
+++ b/README.md
@@ -28,6 +28,7 @@
| [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
--- /dev/null
+++ b/docs/rules/Moq1208.md
@@ -1,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<int>` or `ValueTask<string>`. 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<int>'
+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<int>` method.
+
+## Examples of patterns that are flagged by this analyzer
+
+```csharp
+public interface IService
+{
+ Task<int> GetValueAsync(); // Returns Task<int>
+ Task<string> GetNameAsync(); // Returns Task<string>
+ ValueTask<int> GetValueTaskAsync(); // Returns ValueTask<int>
+}
+
+var mock = new Mock<IService>();
+
+// Lambda returning int on a Task<int> method.
+mock.Setup(x => x.GetValueAsync()).Returns(() => 42); // Moq1208
+
+// Lambda returning string on a Task<string> method.
+mock.Setup(x => x.GetNameAsync()).Returns(() => "hello"); // Moq1208
+
+// Anonymous method returning int on a Task<int> method.
+mock.Setup(x => x.GetValueAsync()).Returns(delegate { return 42; }); // Moq1208
+
+// Method group returning int on a Task<int> method.
+mock.Setup(x => x.GetValueAsync()).Returns(GetInt); // Moq1208
+// where: static int GetInt() => 42;
+
+// ValueTask<int> 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<IService>();
+
+// 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<IService>();
+
+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<int>(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
--- 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
--- a/src/Analyzers/AnalyzerReleases.Unshipped.md
+++ b/src/Analyzers/AnalyzerReleases.Unshipped.md
@@ -17,6 +17,7 @@
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
--- a/src/Analyzers/ReturnsAsyncShouldBeUsedForAsyncMethodsAnalyzer.cs
+++ b/src/Analyzers/ReturnsAsyncShouldBeUsedForAsyncMethodsAnalyzer.cs
@@ -44,7 +44,8 @@
}
// 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 @@
}
// 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 @@
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
--- /dev/null
+++ b/src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cs
@@ -1,0 +1,260 @@
+using System.Diagnostics.CodeAnalysis;
+
+namespace Moq.Analyzers;
+
+/// <summary>
+/// Returns() delegate on async method setup should return Task/ValueTask to match the mocked method's return type.
+/// </summary>
+[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");
+
+ /// <inheritdoc />
+ public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Rule);
+
+ /// <inheritdoc />
+ 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<IMethodSymbol>()
+ .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<IMethodSymbol>().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<T> or ValueTask<T> (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<int>) 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<IMethodSymbol>().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<ReturnStatementSyntax>()
+ .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
--- /dev/null
+++ b/src/CodeFixes/ReturnsDelegateShouldReturnTaskFixer.cs
@@ -1,0 +1,85 @@
+using System.Composition;
+using Microsoft.CodeAnalysis.CodeActions;
+using Microsoft.CodeAnalysis.CodeFixes;
+
+namespace Moq.CodeFixes;
+
+/// <summary>
+/// Fixes for <see cref="DiagnosticIds.ReturnsDelegateMismatchOnAsyncMethod"/> (Moq1208).
+/// Replaces <c>.Returns(delegate)</c> with <c>.ReturnsAsync(delegate)</c> when the
+/// mocked method is async and the delegate returns the unwrapped type.
+/// </summary>
+[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(ReturnsDelegateShouldReturnTaskFixer))]
+[Shared]
+public sealed class ReturnsDelegateShouldReturnTaskFixer : CodeFixProvider
+{
+ private static readonly string Title = "Use ReturnsAsync instead of Returns";
+
+ /// <inheritdoc />
+ public override ImmutableArray<string> FixableDiagnosticIds { get; } =
+ ImmutableArray.Create(DiagnosticIds.ReturnsDelegateMismatchOnAsyncMethod);
+
+ /// <inheritdoc />
+ public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;
+
+ /// <inheritdoc />
+ 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<MemberAccessExpressionSyntax>();
+ 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<Document> 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
--- a/src/Common/DiagnosticIds.cs
+++ b/src/Common/DiagnosticIds.cs
@@ -17,6 +17,7 @@
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
--- a/src/Common/InvocationExpressionSyntaxExtensions.cs
+++ b/src/Common/InvocationExpressionSyntaxExtensions.cs
@@ -18,6 +18,43 @@
}
/// <summary>
+ /// Walks up the Moq fluent chain to find the Setup invocation.
+ /// Handles patterns like <c>mock.Setup(...).Returns(...)</c> and
+ /// <c>mock.Setup(...).Callback(...).Returns(...)</c>.
+ /// </summary>
+ /// <param name="receiver">The receiver expression to start walking from (typically the expression before the Returns call).</param>
+ /// <param name="semanticModel">The semantic model for symbol resolution.</param>
+ /// <param name="knownSymbols">The known Moq symbols for type checking.</param>
+ /// <returns>The Setup invocation if found; otherwise, <see langword="null"/>.</returns>
+ 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;
+ }
+
+ /// <summary>
/// Determines if an invocation is a Raises method call using symbol-based detection.
/// This method verifies the method belongs to IRaiseable or IRaiseableAsync.
/// </summary>
diff --git a/tests/Moq.Analyzers.Test/Helpers/CodeFixVerifier.cs b/tests/Moq.Analyzers.Test/Helpers/CodeFixVerifier.cs
--- a/tests/Moq.Analyzers.Test/Helpers/CodeFixVerifier.cs
+++ b/tests/Moq.Analyzers.Test/Helpers/CodeFixVerifier.cs
@@ -7,15 +7,22 @@
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<TAnalyzer, TCodeFixProvider>
+ Test<TAnalyzer, TCodeFixProvider> test = new Test<TAnalyzer, TCodeFixProvider>
{
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
--- a/tests/Moq.Analyzers.Test/ReturnsAsyncShouldBeUsedForAsyncMethodsAnalyzerTests.cs
+++ b/tests/Moq.Analyzers.Test/ReturnsAsyncShouldBeUsedForAsyncMethodsAnalyzerTests.cs
@@ -26,10 +26,6 @@
// Setup without Returns call should not be affected
["""new Mock<AsyncClient>().Setup(c => c.GetAsync());"""],
- // Callback chained before Returns: FindSetupInvocation returns null because
- // the expression before .Returns is Callback, not Setup
- ["""new Mock<AsyncClient>().Setup(c => c.GetAsync()).Callback(() => { }).Returns(async () => "value");"""],
-
// Parenthesized Setup with ReturnsAsync (valid)
["""(new Mock<AsyncClient>().Setup(c => c.GetAsync())).ReturnsAsync("value");"""],
@@ -52,6 +48,9 @@
// Async lambda in Returns for ValueTask method
["""new Mock<AsyncClient>().Setup(c => c.DoValueTaskAsync()).{|Moq1206:Returns(async () => { })|};"""],
+ // Callback chained before Returns should still detect the Setup call
+ ["""new Mock<AsyncClient>().Setup(c => c.GetAsync()).Callback(() => { }).{|Moq1206:Returns(async () => "value")|};"""],
+
// Parenthesized Setup with async Returns (invalid)
["""(new Mock<AsyncClient>().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
--- /dev/null
+++ b/tests/Moq.Analyzers.Test/ReturnsDelegateShouldReturnTaskAnalyzerTests.cs
@@ -1,0 +1,249 @@
+using Microsoft.CodeAnalysis.Testing;
+using Verifier = Moq.Analyzers.Test.Helpers.AnalyzerVerifier<Moq.Analyzers.ReturnsDelegateShouldReturnTaskAnalyzer>;
+
+namespace Moq.Analyzers.Test;
+
+public class ReturnsDelegateShouldReturnTaskAnalyzerTests(ITestOutputHelper output)
+{
+ public static IEnumerable<object[]> ValidTestData()
+ {
+ IEnumerable<object[]> data = new object[][]
+ {
+ // Delegate returns Task<T> (correct)
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).Returns(() => Task.FromResult(42));"""],
+
+ // Uses ReturnsAsync (correct, different overload)
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).ReturnsAsync(42);"""],
+
+ // Direct value, not a delegate
+ ["""new Mock<AsyncService>().Setup(c => c.GetSync()).Returns(42);"""],
+
+ // Non-async method with sync delegate (no mismatch)
+ ["""new Mock<AsyncService>().Setup(c => c.GetSync()).Returns(() => 42);"""],
+
+ // Async lambda (Moq1206's domain, not ours)
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).Returns(async () => 42);"""],
+
+ // Setup without Returns call
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync());"""],
+
+ // Delegate returns ValueTask<T> (correct)
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueTaskAsync()).Returns(() => ValueTask.FromResult(42));"""],
+
+ // Chained Callback with correct Task return
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).Callback(() => { }).Returns(() => Task.FromResult(42));"""],
+
+ // Anonymous method returning Task.FromResult (correct)
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).Returns(delegate { return Task.FromResult(42); });"""],
+
+ // Async anonymous method (Moq1206's domain, not ours)
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).Returns(async delegate { return 42; });"""],
+
+ // Method group returning Task<int> (correct)
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).Returns(GetIntAsync);"""],
+
+ // Anonymous method on sync method (no mismatch)
+ ["""new Mock<AsyncService>().Setup(c => c.GetSync()).Returns(delegate { return 42; });"""],
+
+ // Method group on sync method (no mismatch)
+ ["""new Mock<AsyncService>().Setup(c => c.GetSync()).Returns(GetInt);"""],
+
+ // Direct value on async method (not a delegate, different Returns overload)
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).Returns(Task.FromResult(42));"""],
+
+ // Non-generic Task method with sync delegate (no mismatch, Task has no inner type)
+ ["""new Mock<AsyncService>().Setup(c => c.DoAsync()).Returns(() => Task.CompletedTask);"""],
+
+ // Parenthesized Setup with ReturnsAsync (valid)
+ ["""(new Mock<AsyncService>().Setup(c => c.GetValueAsync())).ReturnsAsync(42);"""],
+
+ // Block-bodied lambda returning Task (correct)
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).Returns(() => { return Task.FromResult(42); });"""],
+
+ // Generic non-Task return type (IList<int> is generic but not Task/ValueTask)
+ ["""new Mock<AsyncService>().Setup(c => c.GetItems()).Returns(() => new List<int>());"""],
+
+ // Property setup (resolves to IPropertySymbol, not IMethodSymbol)
+ ["""new Mock<AsyncService>().Setup(c => c.Value).Returns(() => Task.FromResult(42));"""],
+
+ // Split setup/returns (FindSetupInvocation can't walk past variable reference)
+ [
+ """
+ var setup = new Mock<AsyncService>().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<Func<AsyncService, Task<int>>> expr = c => c.GetValueAsync();
+ new Mock<AsyncService>().Setup(expr).Returns(() => Task.FromResult(42));
+ """,
+ ],
+ };
+
+ return data.WithNamespaces().WithMoqReferenceAssemblyGroups();
+ }
+
+ public static IEnumerable<object[]> InvalidTestData()
+ {
+ IEnumerable<object[]> data = new object[][]
+ {
+ // Sync delegate returning int on Task<int> method
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueAsync()).{|Moq1208:Returns(() => 42)|};"""],
+
+ // Sync delegate returning string on Task<string> method
+ ["""new Mock<AsyncService>().Setup(c => c.GetNameAsync()).{|Moq1208:Returns(() => "hello")|};"""],
+
+ // Sync delegate returning int on ValueTask<int> method
+ ["""new Mock<AsyncService>().Setup(c => c.GetValueTaskAsync()).{|Moq1208:Returns(() => 42)|};"""],
+
+ // Parenthesized Setup with sync delegate mismatch
... diff truncated: showing 800 of 1082 lines| } | ||
|
|
||
| return null; | ||
| } |
There was a problem hiding this comment.
Duplicated FindSetupInvocation logic across sibling analyzers
Low Severity
The new FindSetupInvocation in ReturnsDelegateShouldReturnTaskAnalyzer and the existing one in ReturnsAsyncShouldBeUsedForAsyncMethodsAnalyzer solve the same problem — locating the Setup call in a Moq fluent chain — with divergent approaches. The Moq1208 version is significantly more robust (semantic IsMoqSetupMethod checks, walks up to 10 fluent-chain levels, handles Callback chains), while the Moq1206 version is syntax-only, uses string comparison for "Setup", and only handles one level. This divergence means the Moq1206 analyzer silently misses Setup(...).Callback(...).Returns(async () => ...) patterns. Extracting the Moq1208 version into a shared utility would fix this gap and eliminate duplicated logic.
- 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
rjmurillo-bot
approved these changes
Mar 1, 2026
Collaborator
rjmurillo-bot
left a comment
rjmurillo-bot
left a comment
There was a problem hiding this comment.
All required CI checks pass. Moq1208 analyzer for Returns() delegate mismatch is ready.
This was referenced Mar 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.Returns(() => value)with a delegate whose return type does not match the async method's return type (e.g., returnsintinstead ofTask<int>), which causes a runtimeMockException.Returns(delegate)to.ReturnsAsync(delegate)Problem
Solution
The analyzer flags the mismatch at compile time. The code fix transforms:
Files changed
src/Common/DiagnosticIds.csReturnsDelegateMismatchOnAsyncMethod = "Moq1208"src/Analyzers/ReturnsDelegateShouldReturnTaskAnalyzer.cssrc/CodeFixes/ReturnsDelegateShouldReturnTaskFixer.cssrc/Analyzers/AnalyzerReleases.Unshipped.mdtests/.../ReturnsDelegateShouldReturnTaskAnalyzerTests.cstests/.../ReturnsDelegateShouldReturnTaskFixerTests.csdocs/rules/Moq1208.mdEdge cases handled
.Returns(value)(direct value, not delegate).Returns(() => Task.FromResult(7)).Returns(async () => 7).Returns(() => 7)Taskreturn type.Callback().Returns(() => 7)on async method(mock.Setup(...)).Returns(() => 7)(string x) => x.LengthTest plan
Closes #767
🤖 Generated with Claude Code
Summary by CodeRabbit