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. Intermediate calls like `Callback()` between `Setup()` and the return value specification do not affect the diagnostic.

## 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 follow `Setup()` at any position in the fluent 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
61 changes: 44 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.
/// </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,38 @@ 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;
}

/// <summary>
/// Determines whether the given <see cref="SymbolInfo"/> resolves to a Moq return value
/// specification method. Checks the resolved symbol first, then falls back to scanning
/// candidate symbols when Roslyn cannot complete overload resolution.
/// </summary>
private static bool HasReturnValueSymbol(SymbolInfo symbolInfo, MoqKnownSymbols knownSymbols)
{
if (symbolInfo.Symbol is IMethodSymbol resolved)
{
return resolved.IsMoqReturnValueSpecificationMethod(knownSymbols);
}

return symbolInfo.CandidateSymbols
.OfType<IMethodSymbol>()
.Any(method => method.IsMoqReturnValueSpecificationMethod(knownSymbols));
}
}
62 changes: 58 additions & 4 deletions 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 @@ -179,15 +187,62 @@ internal static bool IsMoqVerificationMethod(this ISymbol symbol, MoqKnownSymbol
/// </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 Returns method from Moq.Language.IReturns; otherwise false.</returns>
/// <returns>True if the symbol is a Returns method from any Moq.Language.IReturns interface; otherwise false.</returns>
internal static bool IsMoqReturnsMethod(this ISymbol symbol, MoqKnownSymbols knownSymbols)
{
// Check if this method symbol matches any of the known Returns methods
return symbol.IsInstanceOf(knownSymbols.IReturnsReturns) ||
symbol.IsInstanceOf(knownSymbols.IReturns1Returns) ||
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 All @@ -196,7 +251,6 @@ internal static bool IsMoqReturnsMethod(this ISymbol symbol, MoqKnownSymbols kno
/// <returns>True if the symbol is a Callback method from Moq.Language.ICallback; otherwise false.</returns>
internal static bool IsMoqCallbackMethod(this ISymbol symbol, MoqKnownSymbols knownSymbols)
{
// Check if this method symbol matches any of the known Callback methods
return symbol.IsInstanceOf(knownSymbols.ICallbackCallback) ||
symbol.IsInstanceOf(knownSymbols.ICallback1Callback) ||
symbol.IsInstanceOf(knownSymbols.ICallback2Callback);
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>.
/// </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
9 changes: 5 additions & 4 deletions tests/Moq.Analyzers.Test/Helpers/AllAnalyzersVerifier.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Reflection;
using Microsoft.CodeAnalysis.Testing;

namespace Moq.Analyzers.Test.Helpers;

Expand Down Expand Up @@ -57,19 +58,19 @@ private static async Task VerifyAnalyzerDynamicallyAsync(Type analyzerType, stri
// Create AnalyzerVerifier<TAnalyzer> using reflection
Type analyzerVerifierType = typeof(AnalyzerVerifier<>).MakeGenericType(analyzerType);

// Get the VerifyAnalyzerAsync method
// Get the VerifyAnalyzerAsync method (5 params: source, group, configFileName, configContent, compilerDiagnostics?)
MethodInfo? verifyMethod = analyzerVerifierType.GetMethod(
nameof(AnalyzerVerifier<AsShouldBeUsedOnlyForInterfaceAnalyzer>.VerifyAnalyzerAsync),
BindingFlags.Static | BindingFlags.Public,
new[] { typeof(string), typeof(string), typeof(string), typeof(string) });
new[] { typeof(string), typeof(string), typeof(string), typeof(string), typeof(CompilerDiagnostics?) });

if (verifyMethod == null)
{
throw new InvalidOperationException($"Could not find VerifyAnalyzerAsync method for analyzer type {analyzerType.Name}");
}

// Invoke the method
object? task = verifyMethod.Invoke(null, new object?[] { source, referenceAssemblyGroup, configFileName, configContent });
// Invoke the method (pass null for the optional compilerDiagnostics parameter)
object? task = verifyMethod.Invoke(null, new object?[] { source, referenceAssemblyGroup, configFileName, configContent, null });
if (task is Task taskResult)
{
await taskResult.ConfigureAwait(false);
Expand Down
19 changes: 17 additions & 2 deletions tests/Moq.Analyzers.Test/Helpers/AnalyzerVerifier.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using Microsoft.CodeAnalysis.Testing;
using Microsoft.CodeAnalysis.Testing;

namespace Moq.Analyzers.Test.Helpers;

Expand All @@ -10,7 +10,12 @@ public static async Task VerifyAnalyzerAsync(string source, string referenceAsse
await VerifyAnalyzerAsync(source, referenceAssemblyGroup, configFileName: null, configContent: null).ConfigureAwait(false);
}

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

Expand All @@ -21,11 +26,21 @@ public static async Task VerifyAnalyzerAsync(string source, string referenceAsse
ReferenceAssemblies = referenceAssemblies,
};

if (compilerDiagnostics.HasValue)
{
test.CompilerDiagnostics = compilerDiagnostics.Value;
}

if (configFileName != null && configContent != null)
{
test.TestState.AnalyzerConfigFiles.Add((configFileName, configContent));
}

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

public static async Task VerifyAnalyzerAsync(string source, string referenceAssemblyGroup, CompilerDiagnostics compilerDiagnostics)
{
await VerifyAnalyzerAsync(source, referenceAssemblyGroup, configFileName: null, configContent: null, compilerDiagnostics).ConfigureAwait(false);
}
}
Loading
Loading