Skip to content
31 changes: 20 additions & 11 deletions docs/rules/Moq1203.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
| CodeFix | False |
---

Method setups that have a return type should specify what value to return using `Returns()` or `Throws()`.
Method setups that have a return type should specify what value to return using `Returns()`, `ReturnsAsync()`, `Throws()`, or `ThrowsAsync()`.

When setting up methods that return values, Moq requires explicit specification of what should be returned or thrown. Without this specification, the setup is incomplete and may not behave as expected during testing.
When setting up methods that return values, Moq requires explicit specification of what should be returned or thrown. Without this specification, the setup is incomplete and may not behave as expected during testing. The analyzer walks the full method chain, so intermediate calls like `Callback()` do not interfere with detection.

## Examples of patterns that are flagged by this analyzer

Expand All @@ -18,43 +18,52 @@ interface IFoo
{
bool DoSomething(string value);
int GetValue();
string Calculate(int a, int b);
Task<int> BarAsync();
}

var mock = new Mock<IFoo>();

// Moq1203: Method setup should specify a return value
mock.Setup(x => x.DoSomething("test"));

// Moq1203: Method setup should specify a return value
// Moq1203: Method setup should specify a return value
mock.Setup(x => x.GetValue());

// Moq1203: Method setup should specify a return value
mock.Setup(x => x.Calculate(1, 2));
// Moq1203: Async methods also need a return value specification
mock.Setup(x => x.BarAsync());

// Moq1203: Callback alone does not satisfy the requirement
mock.Setup(x => x.GetValue()).Callback(() => { });
```

## Solution

Specify what the method should return using `Returns()` or what exception should be thrown using `Throws()`:
Specify what the method should return using `Returns()` or `ReturnsAsync()`, or what exception should be thrown using `Throws()` or `ThrowsAsync()`. These methods can appear anywhere in the chain, including after `Callback()`.

```csharp
interface IFoo
{
bool DoSomething(string value);
int GetValue();
string Calculate(int a, int b);
Task<int> BarAsync();
}

var mock = new Mock<IFoo>();

// Specify return value
mock.Setup(x => x.DoSomething("test")).Returns(true);
mock.Setup(x => x.GetValue()).Returns(42);
mock.Setup(x => x.Calculate(1, 2)).Returns("result");

// Or specify exception to throw
// Async methods can use ReturnsAsync
mock.Setup(x => x.BarAsync()).ReturnsAsync(1);

// Callback before Returns is valid
mock.Setup(x => x.DoSomething("test")).Callback(() => { }).Returns(true);
mock.Setup(x => x.BarAsync()).Callback(() => { }).ReturnsAsync(1);

// Specify exception to throw
mock.Setup(x => x.DoSomething("test")).Throws<InvalidOperationException>();
mock.Setup(x => x.GetValue()).Throws(new ArgumentException());
mock.Setup(x => x.BarAsync()).ThrowsAsync(new InvalidOperationException());
```

## When this rule does not apply
Expand Down
64 changes: 47 additions & 17 deletions src/Analyzers/MethodSetupShouldSpecifyReturnValueAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@
namespace Moq.Analyzers;

/// <summary>
/// Method setups that return a value should specify a return value using Returns() or Throws().
/// Method setups that return a value should specify a return value using
/// Returns(), ReturnsAsync(), Throws(), or ThrowsAsync().
/// </summary>
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class MethodSetupShouldSpecifyReturnValueAnalyzer : DiagnosticAnalyzer
{
private static readonly LocalizableString Title = "Method setup should specify a return value";
private static readonly LocalizableString Message = "Method setup for '{0}' should specify a return value";
private static readonly LocalizableString Description = "Method setups that return a value should use Returns() or Throws() to specify a return value.";
private static readonly LocalizableString Description = "Method setups that return a value should use Returns(), ReturnsAsync(), Throws(), or ThrowsAsync() to specify a return value.";

private static readonly DiagnosticDescriptor Rule = new(
DiagnosticIds.MethodSetupShouldSpecifyReturnValue,
Expand All @@ -19,10 +20,11 @@ public class MethodSetupShouldSpecifyReturnValueAnalyzer : DiagnosticAnalyzer
DiagnosticCategory.Usage,
DiagnosticSeverity.Warning,
isEnabledByDefault: true,
description: Description);
description: Description,
helpLinkUri: $"https://github.com/rjmurillo/moq.analyzers/blob/{ThisAssembly.GitCommitId}/docs/rules/{DiagnosticIds.MethodSetupShouldSpecifyReturnValue}.md");

/// <inheritdoc />
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Rule);

/// <inheritdoc />
public override void Initialize(AnalysisContext context)
Expand All @@ -34,7 +36,7 @@ public override void Initialize(AnalysisContext context)

private static void AnalyzeInvocation(OperationAnalysisContext context)
{
if (!TryGetSetupInvocation(context, out IInvocationOperation? setupInvocation, out _))
if (!TryGetSetupInvocation(context, out IInvocationOperation? setupInvocation, out MoqKnownSymbols? knownSymbols))
{
return;
}
Expand All @@ -44,12 +46,11 @@ private static void AnalyzeInvocation(OperationAnalysisContext context)
return;
}

if (HasReturnValueSpecification(setupInvocation))
if (HasReturnValueSpecification(setupInvocation, knownSymbols, context.CancellationToken))
{
return;
}

// Report diagnostic for methods with return types that don't specify a return value
Diagnostic diagnostic = setupInvocation.Syntax.CreateDiagnostic(Rule, mockedMethod.ToDisplayString());
context.ReportDiagnostic(diagnostic);
}
Expand Down Expand Up @@ -133,10 +134,15 @@ private static bool TryGetMockedMethodWithReturnValue(
}

/// <summary>
/// Checks if the setup invocation is followed by a Returns() or Throws() call.
/// This uses semantic analysis to verify the method being called.
/// Checks if the setup invocation is followed by a return value specification
/// anywhere in the method chain (e.g., .Setup().Callback().Returns()).
/// Uses semantic analysis to resolve method symbols, including candidate symbols
/// when overload resolution fails (common with Moq extension methods).
/// </summary>
private static bool HasReturnValueSpecification(IInvocationOperation setupInvocation)
private static bool HasReturnValueSpecification(
IInvocationOperation setupInvocation,
MoqKnownSymbols knownSymbols,
CancellationToken cancellationToken)
{
SyntaxNode setupSyntax = setupInvocation.Syntax;
SemanticModel? semanticModel = setupInvocation.SemanticModel;
Expand All @@ -146,17 +152,41 @@ private static bool HasReturnValueSpecification(IInvocationOperation setupInvoca
return false;
}

// Check if the setup call is the target of a member access (method chaining)
if (setupSyntax.Parent is Microsoft.CodeAnalysis.CSharp.Syntax.MemberAccessExpressionSyntax memberAccess)
SyntaxNode? current = setupSyntax;
while (current?.Parent is MemberAccessExpressionSyntax memberAccess)
{
// Get the symbol information for the method being accessed
SymbolInfo symbolInfo = semanticModel.GetSymbolInfo(memberAccess.Name);
cancellationToken.ThrowIfCancellationRequested();

SymbolInfo symbolInfo = semanticModel.GetSymbolInfo(memberAccess, cancellationToken);

if (HasReturnValueSymbol(symbolInfo, knownSymbols))
{
return true;
}

// Check if it's a method call and if the method name is Returns or Throws
return symbolInfo.Symbol is IMethodSymbol method &&
(string.Equals(method.Name, "Returns", StringComparison.Ordinal) || string.Equals(method.Name, "Throws", StringComparison.Ordinal));
current = memberAccess.Parent as InvocationExpressionSyntax;
}
Comment thread
rjmurillo marked this conversation as resolved.

return false;
}

private static bool HasReturnValueSymbol(SymbolInfo symbolInfo, MoqKnownSymbols knownSymbols)
{
if (symbolInfo.CandidateReason == CandidateReason.OverloadResolutionFailure)
{
foreach (ISymbol candidate in symbolInfo.CandidateSymbols)
{
if (candidate is IMethodSymbol method && method.IsMoqReturnValueSpecificationMethod(knownSymbols))
{
return true;
}
}

return false;
}

return symbolInfo.CandidateReason == CandidateReason.None
&& symbolInfo.Symbol is IMethodSymbol resolved
&& resolved.IsMoqReturnValueSpecificationMethod(knownSymbols);
}
}
58 changes: 57 additions & 1 deletion src/Common/ISymbolExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,15 @@ public static bool IsInstanceOf<TSymbol>(this ISymbol? symbol, TSymbol? other, S

if (symbol is IMethodSymbol method)
{
return symbolEqualityComparer.Equals(method.OriginalDefinition, other);
if (symbolEqualityComparer.Equals(method.OriginalDefinition, other))
{
return true;
}

// Reduced extension methods have a different OriginalDefinition than
// the static method on the containing type. Check the unreduced form.
return method.ReducedFrom != null
&& symbolEqualityComparer.Equals(method.ReducedFrom.OriginalDefinition, other);
}

if (symbol is IParameterSymbol parameter && other is IParameterSymbol otherParameter)
Expand Down Expand Up @@ -188,6 +196,54 @@ internal static bool IsMoqReturnsMethod(this ISymbol symbol, MoqKnownSymbols kno
symbol.IsInstanceOf(knownSymbols.IReturns2Returns);
}

/// <summary>
/// Determines whether a symbol is a Moq Throws method.
/// </summary>
/// <param name="symbol">The symbol to check.</param>
/// <param name="knownSymbols">The known symbols for type checking.</param>
/// <returns>True if the symbol is a Throws method from Moq.Language.IThrows; otherwise false.</returns>
internal static bool IsMoqThrowsMethod(this ISymbol symbol, MoqKnownSymbols knownSymbols)
{
return symbol.IsInstanceOf(knownSymbols.IThrowsThrows);
}

/// <summary>
/// Determines whether a symbol is a Moq ReturnsAsync extension method.
/// </summary>
/// <param name="symbol">The symbol to check.</param>
/// <param name="knownSymbols">The known symbols for type checking.</param>
/// <returns>True if the symbol is a ReturnsAsync method from Moq.ReturnsExtensions; otherwise false.</returns>
internal static bool IsMoqReturnsAsyncMethod(this ISymbol symbol, MoqKnownSymbols knownSymbols)
{
return symbol.IsInstanceOf(knownSymbols.ReturnsExtensionsReturnsAsync);
}

/// <summary>
/// Determines whether a symbol is a Moq ThrowsAsync extension method.
/// </summary>
/// <param name="symbol">The symbol to check.</param>
/// <param name="knownSymbols">The known symbols for type checking.</param>
/// <returns>True if the symbol is a ThrowsAsync method from Moq.ReturnsExtensions; otherwise false.</returns>
internal static bool IsMoqThrowsAsyncMethod(this ISymbol symbol, MoqKnownSymbols knownSymbols)
{
return symbol.IsInstanceOf(knownSymbols.ReturnsExtensionsThrowsAsync);
}

/// <summary>
/// Determines whether a symbol is any Moq method that specifies a return value
/// (Returns, ReturnsAsync, Throws, or ThrowsAsync).
/// </summary>
/// <param name="symbol">The symbol to check.</param>
/// <param name="knownSymbols">The known symbols for type checking.</param>
/// <returns>True if the symbol is a return value specification method; otherwise false.</returns>
internal static bool IsMoqReturnValueSpecificationMethod(this ISymbol symbol, MoqKnownSymbols knownSymbols)
{
return symbol.IsMoqReturnsMethod(knownSymbols) ||
symbol.IsMoqThrowsMethod(knownSymbols) ||
symbol.IsMoqReturnsAsyncMethod(knownSymbols) ||
symbol.IsMoqThrowsAsyncMethod(knownSymbols);
}

/// <summary>
/// Determines whether a symbol is a Moq Callback method.
/// </summary>
Expand Down
25 changes: 25 additions & 0 deletions src/Common/WellKnown/MoqKnownSymbols.cs
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,31 @@ internal MoqKnownSymbols(Compilation compilation)
/// </summary>
internal ImmutableArray<IMethodSymbol> IReturns2Returns => IReturns2?.GetMembers("Returns").OfType<IMethodSymbol>().ToImmutableArray() ?? ImmutableArray<IMethodSymbol>.Empty;

/// <summary>
/// Gets the interface <c>Moq.Language.IThrows</c>.
/// </summary>
internal INamedTypeSymbol? IThrows => TypeProvider.GetOrCreateTypeByMetadataName("Moq.Language.IThrows");

/// <summary>
/// Gets the methods for <c>Moq.Language.IThrows.Throws</c>.
/// </summary>
internal ImmutableArray<IMethodSymbol> IThrowsThrows => IThrows?.GetMembers("Throws").OfType<IMethodSymbol>().ToImmutableArray() ?? ImmutableArray<IMethodSymbol>.Empty;

/// <summary>
/// Gets the class <c>Moq.ReturnsExtensions</c> (contains ReturnsAsync and ThrowsAsync extension methods).
/// </summary>
internal INamedTypeSymbol? ReturnsExtensions => TypeProvider.GetOrCreateTypeByMetadataName("Moq.ReturnsExtensions");

/// <summary>
/// Gets the methods for <c>Moq.ReturnsExtensions.ReturnsAsync</c>.
/// </summary>
internal ImmutableArray<IMethodSymbol> ReturnsExtensionsReturnsAsync => ReturnsExtensions?.GetMembers("ReturnsAsync").OfType<IMethodSymbol>().ToImmutableArray() ?? ImmutableArray<IMethodSymbol>.Empty;

/// <summary>
/// Gets the methods for <c>Moq.ReturnsExtensions.ThrowsAsync</c>.
/// </summary>
internal ImmutableArray<IMethodSymbol> ReturnsExtensionsThrowsAsync => ReturnsExtensions?.GetMembers("ThrowsAsync").OfType<IMethodSymbol>().ToImmutableArray() ?? ImmutableArray<IMethodSymbol>.Empty;

/// <summary>
/// Gets the methods for <c>Moq.Language.ICallback.Callback</c>.
/// </summary>
Expand Down
15 changes: 15 additions & 0 deletions tests/Moq.Analyzers.Test/Helpers/AnalyzerVerifier.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,19 @@ public static async Task VerifyAnalyzerAsync(string source, string referenceAsse

await test.RunAsync().ConfigureAwait(false);
}

public static async Task VerifyAnalyzerAsync(string source, string referenceAssemblyGroup, CompilerDiagnostics compilerDiagnostics)
{
ReferenceAssemblies referenceAssemblies = ReferenceAssemblyCatalog.Catalog[referenceAssemblyGroup];

Test<TAnalyzer, EmptyCodeFixProvider> test = new Test<TAnalyzer, EmptyCodeFixProvider>
{
TestCode = source,
FixedCode = source,
ReferenceAssemblies = referenceAssemblies,
CompilerDiagnostics = compilerDiagnostics,
};

await test.RunAsync().ConfigureAwait(false);
}
}
Loading
Loading