Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 32 additions & 3 deletions src/Analyzers/MethodSetupShouldSpecifyReturnValueAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,19 @@ private static bool HasReturnValueSpecification(
{
cancellationToken.ThrowIfCancellationRequested();

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

if (HasReturnValueSymbol(symbolInfo, knownSymbols))
InvocationExpressionSyntax? invocation = memberAccess.Parent as InvocationExpressionSyntax;
SymbolInfo symbolInfo = invocation != null
? semanticModel.GetSymbolInfo(invocation, cancellationToken)
: semanticModel.GetSymbolInfo(memberAccess, cancellationToken);
Comment thread
rjmurillo marked this conversation as resolved.

// First try semantic symbol matching (exact). Then fall back to method
// name matching when Roslyn resolves a symbol that IsInstanceOf cannot
// match (e.g., delegate-based overloads with constructed generic types).
// The name-based fallback is safe because this walk only visits methods
// chained from a verified Setup() call, and we verify the named method
// exists in the Moq compilation.
if (HasReturnValueSymbol(symbolInfo, knownSymbols)
|| IsKnownReturnValueMethodName(memberAccess.Name.Identifier.ValueText, knownSymbols))
{
Comment thread
rjmurillo marked this conversation as resolved.
return true;
}
Expand All @@ -177,6 +187,25 @@ private static bool HasReturnValueSpecification(
return false;
}

/// <summary>
/// Checks whether a method name corresponds to a known Moq return value specification
/// method that exists in the compilation. Used as a last-resort fallback when Roslyn
/// cannot resolve symbols at all (e.g., delegate-based overloads with failed type inference).
/// </summary>
private static bool IsKnownReturnValueMethodName(string methodName, MoqKnownSymbols knownSymbols)
{
return methodName switch
{
"Returns" => !knownSymbols.IReturnsReturns.IsEmpty
|| !knownSymbols.IReturns1Returns.IsEmpty
|| !knownSymbols.IReturns2Returns.IsEmpty,
"ReturnsAsync" => !knownSymbols.ReturnsExtensionsReturnsAsync.IsEmpty,
"Throws" => !knownSymbols.IThrowsThrows.IsEmpty,
"ThrowsAsync" => !knownSymbols.ReturnsExtensionsThrowsAsync.IsEmpty,
_ => 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,19 +73,21 @@ private static void Analyze(SyntaxNodeAnalysisContext context)

private static bool IsReturnsMethodCallWithAsyncLambda(InvocationExpressionSyntax invocation, SemanticModel semanticModel, MoqKnownSymbols knownSymbols)
{
if (invocation.Expression is not MemberAccessExpressionSyntax memberAccess)
if (invocation.Expression is not MemberAccessExpressionSyntax)
{
return false;
}

// Check if this is a Returns method call
SymbolInfo symbolInfo = semanticModel.GetSymbolInfo(memberAccess);
if (symbolInfo.Symbol is not IMethodSymbol method)
{
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 (!method.IsMoqReturnsMethod(knownSymbols))
if (!isReturnsMethod)
{
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,17 @@ public static IEnumerable<object[]> CustomReturnTypeTestData()
mock.Setup(x => x.GetAsync()).ThrowsAsync(new InvalidOperationException());
""", "public record MyValue(string Name);", "Task<MyValue> GetAsync();"],

// Delegate-based ReturnsAsync with MockBehavior.Strict
["""
var databaseMock = new Mock<IDatabase>(MockBehavior.Strict);
databaseMock.Setup(mock => mock.SaveAsync(It.IsAny<MyValue>())).ReturnsAsync((MyValue val) => val);
""", "public record MyValue;", "Task<MyValue> SaveAsync(MyValue value);"],

// Delegate-based ReturnsAsync with default MockBehavior, inline
["""
new Mock<IDatabase>().Setup(x => x.SaveAsync(It.IsAny<MyValue>())).ReturnsAsync((MyValue val) => val);
""", "public record MyValue;", "Task<MyValue> SaveAsync(MyValue value);"],
Comment thread
rjmurillo marked this conversation as resolved.

];

return data.WithNamespaces().WithMoqReferenceAssemblyGroups();
Expand Down Expand Up @@ -393,20 +404,6 @@ public static IEnumerable<object[]> CustomReturnTypeMissingReturnValueTestData()
var mock = new Mock<IDatabase>(MockBehavior.Strict);
{|Moq1203:mock.Setup(x => x.SaveAsync(It.IsAny<MyValue>()))|};
""", "public record MyValue;", "Task<MyValue> SaveAsync(MyValue value);"],

// Known false positive: delegate-based ReturnsAsync IS a return value specification,
// but the analyzer does not recognize it. Reported by DamienCassou:
// https://github.com/rjmurillo/moq.analyzers/issues/849#issuecomment-3925720443
// When fixed, move these to CustomReturnTypeTestData without the diagnostic markup.
["""
var databaseMock = new Mock<IDatabase>(MockBehavior.Strict);
{|Moq1203:databaseMock.Setup(mock => mock.SaveAsync(It.IsAny<MyValue>()))|}.ReturnsAsync((MyValue val) => val);
""", "public record MyValue;", "Task<MyValue> SaveAsync(MyValue value);"],

// Same false positive with default MockBehavior, no variable
["""
{|Moq1203:new Mock<IDatabase>().Setup(x => x.SaveAsync(It.IsAny<MyValue>()))|}.ReturnsAsync((MyValue val) => val);
""", "public record MyValue;", "Task<MyValue> SaveAsync(MyValue value);"],
];

return data.WithNamespaces().WithMoqReferenceAssemblyGroups();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,19 @@ public static IEnumerable<object[]> TestData()
return valid.Concat(invalid);
}

// Delegate-typed async lambdas cause overload resolution failure in Roslyn.
// The analyzer must fall back to CandidateSymbols to detect Returns usage.
public static IEnumerable<object[]> DelegateOverloadTestData()
{
IEnumerable<object[]> data = new object[][]
{
// Async delegate lambda in Returns for Task<T> method with parameter (should flag)
["""new Mock<AsyncClient>().Setup(c => c.ProcessAsync(It.IsAny<string>())).{|Moq1206:Returns(async (string x) => x)|};"""],
};

return data.WithNamespaces().WithMoqReferenceAssemblyGroups();
}

[Theory]
[MemberData(nameof(TestData))]
public async Task ShouldAnalyzeReturnsAsyncUsage(string referenceAssemblyGroup, string @namespace, string mock)
Expand Down Expand Up @@ -95,6 +108,40 @@ await Verifier.VerifyAnalyzerAsync(
referenceAssemblyGroup);
}

[Theory]
[MemberData(nameof(DelegateOverloadTestData))]
public async Task ShouldFlagAsyncDelegateLambdaInReturns(string referenceAssemblyGroup, string @namespace, string mock)
{
string source =
$$"""
{{@namespace}}

public class AsyncClient
{
public virtual Task DoAsync() => Task.CompletedTask;
public virtual Task<string> GetAsync() => Task.FromResult(string.Empty);
public virtual ValueTask DoValueTaskAsync() => ValueTask.CompletedTask;
public virtual ValueTask<string> GetValueTaskAsync() => ValueTask.FromResult(string.Empty);
public virtual string GetSync() => string.Empty;
public virtual Task<string> ProcessAsync(string input) => Task.FromResult(input);
}

internal class UnitTest
{
private void Test()
{
{{mock}}
}
}
""";

output.WriteLine(source);

await Verifier.VerifyAnalyzerAsync(
source,
referenceAssemblyGroup);
Comment thread
rjmurillo marked this conversation as resolved.
}

[Theory]
[MemberData(nameof(DoppelgangerTestHelper.GetAllCustomMockData), MemberType = typeof(DoppelgangerTestHelper))]
public async Task ShouldPassIfCustomMockClassIsUsed(string mockCode)
Expand Down
Loading