Skip to content

Commit 781c529

Browse files
rjmurilloclaude
andcommitted
fix: correct XML docs, remove dead symbol comparisons, add wrapper tests (#1067)
Remove dead comparisons against IReturns (arity 0) and IReturns1 (arity 1) from IsMatchingFluentSymbol. These types resolve to null in Moq 4.18+ and the comparisons always evaluated to false. Fix XML doc comments for accuracy: - IsMatchingFluentSymbol: correct type parameter names, clarify arity-0/1 resolve to null rather than claiming non-existence - ImplementsMoqFluentInterface: describe behavior, not caller intent - IsCallbackRaisesMethod: include ISetupGetter and ISetupSetter in summary - IsFluentChainWrapperMethod: state assumption rather than assertion - IsWrapperMethod: mention raises methods in exclusion list - MockRepositoryCreate: fix doc that said "Of" instead of "Create" Add test coverage for ReturnsAsync wrapper, ValueTask wrapper, and NoOpPassthrough (documenting accepted false-negative tradeoff). Deduplicate MoqKnownSymbolsTests added by upstream merge. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 41e6fd5 commit 781c529

5 files changed

Lines changed: 106 additions & 58 deletions

File tree

src/Analyzers/MethodSetupShouldSpecifyReturnValueAnalyzer.cs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -189,9 +189,9 @@ private static bool HasReturnValueSpecification(
189189
}
190190

191191
// When a non-Moq method (e.g., an extension method) appears in the chain,
192-
// check if its return type implements a Moq fluent interface. A return type
193-
// like IReturns<TMock, TResult> indicates the method wraps the setup
194-
// configuration internally and preserves the fluent chain.
192+
// check if its return type implements a Moq fluent interface. The analyzer
193+
// assumes such methods handle return value specification internally or pass
194+
// the chain through for further configuration.
195195
if (IsFluentChainWrapperMethod(symbolInfo, knownSymbols))
196196
{
197197
return true;
@@ -241,8 +241,9 @@ private static bool HasReturnValueSymbol(SymbolInfo symbolInfo, MoqKnownSymbols
241241

242242
/// <summary>
243243
/// Determines whether a method in the chain is a wrapper (e.g., an extension method)
244-
/// whose return type implements a Moq fluent interface. Such methods wrap the setup
245-
/// configuration internally (calling Returns/Throws inside) and return the fluent chain.
244+
/// whose return type implements a Moq fluent interface. Such methods are assumed to
245+
/// handle return value specification internally, as indicated by their Moq fluent
246+
/// return type.
246247
/// </summary>
247248
private static bool IsFluentChainWrapperMethod(SymbolInfo symbolInfo, MoqKnownSymbols knownSymbols)
248249
{
@@ -261,9 +262,9 @@ private static bool IsFluentChainWrapperMethod(SymbolInfo symbolInfo, MoqKnownSy
261262

262263
/// <summary>
263264
/// Returns true when <paramref name="method"/> is a user-defined wrapper whose return
264-
/// type implements a Moq fluent interface. Known Moq return-value and callback methods
265-
/// are excluded because they are already handled by <see cref="HasReturnValueSymbol"/>
266-
/// and <see cref="IsKnownReturnValueMethodName"/>.
265+
/// type implements a Moq fluent interface. Known Moq return-value, callback, and raises
266+
/// methods are excluded because they are already handled by earlier checks in the
267+
/// chain walk.
267268
/// </summary>
268269
private static bool IsWrapperMethod(IMethodSymbol method, MoqKnownSymbols knownSymbols)
269270
{

src/Common/ISymbolExtensions.Moq.cs

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -188,15 +188,13 @@ internal static bool IsMoqRaisesMethod(this ISymbol symbol, MoqKnownSymbols know
188188
}
189189

190190
/// <summary>
191-
/// Determines whether a type symbol implements any Moq fluent interface from the
192-
/// <c>Moq.Language</c> or <c>Moq.Language.Flow</c> namespaces. Used to detect
193-
/// wrapper methods (e.g., extension methods) that internally configure the setup
194-
/// chain and return a Moq fluent type.
191+
/// Determines whether a type symbol is or implements any Moq fluent interface from
192+
/// the <c>Moq.Language</c> or <c>Moq.Language.Flow</c> namespaces.
195193
/// </summary>
196-
/// <param name="typeSymbol">The return type of the method to check.</param>
194+
/// <param name="typeSymbol">The type symbol to check.</param>
197195
/// <param name="knownSymbols">Known Moq symbols resolved from the compilation.</param>
198196
/// <returns>
199-
/// <see langword="true"/> if the type implements or is a Moq fluent interface;
197+
/// <see langword="true"/> if the type is or implements a Moq fluent interface;
200198
/// otherwise, <see langword="false"/>.
201199
/// </returns>
202200
internal static bool ImplementsMoqFluentInterface(this ITypeSymbol typeSymbol, MoqKnownSymbols knownSymbols)
@@ -226,11 +224,11 @@ private static bool IsConcreteSetupPhraseRaisesMethod(ISymbol symbol, MoqKnownSy
226224
}
227225

228226
/// <summary>
229-
/// Checks if the symbol is a Raises method from ICallback interfaces.
227+
/// Checks if the symbol is a Raises method from ICallback, ISetupGetter, or ISetupSetter interfaces.
230228
/// </summary>
231229
/// <param name="symbol">The symbol to check.</param>
232230
/// <param name="knownSymbols">The known symbols for type checking.</param>
233-
/// <returns>True if the symbol is a callback Raises method; otherwise false.</returns>
231+
/// <returns>True if the symbol is a Raises method from one of these interfaces; otherwise false.</returns>
234232
private static bool IsCallbackRaisesMethod(ISymbol symbol, MoqKnownSymbols knownSymbols)
235233
{
236234
return symbol.IsInstanceOf(knownSymbols.ICallbackRaises) ||
@@ -319,23 +317,29 @@ private static bool HasMoqFluentInterfaceInHierarchy(ITypeSymbol typeSymbol, Moq
319317

320318
/// <summary>
321319
/// Compares a type symbol against the set of known Moq fluent interface symbols
322-
/// that indicate a return value has been or can be configured.
320+
/// that indicate the fluent chain is active and a return value can be configured.
323321
/// </summary>
324322
/// <remarks>
325323
/// <para>
326324
/// This method intentionally excludes <c>ICallback</c>, <c>ICallback{TMock}</c>, and
327325
/// <c>ICallback{TMock, TResult}</c> because these interfaces do not inherit from
328-
/// <c>IReturns</c> and do not indicate that a return value has been configured.
329-
/// Types like <c>ISetup{TMock, TResult}</c> that inherit from <c>IReturns</c> are
330-
/// still caught via <see cref="HasMoqFluentInterfaceInHierarchy"/> when checking
326+
/// <c>IReturns{TMock, TResult}</c> and do not indicate that a return value can be
327+
/// configured. Types like <c>ISetup{TMock, TResult}</c> that inherit from
328+
/// <c>IReturns{TMock, TResult}</c> are still caught via
329+
/// <see cref="HasMoqFluentInterfaceInHierarchy"/> when checking
331330
/// <see cref="ITypeSymbol.AllInterfaces"/>.
332331
/// </para>
333332
/// <para>
334-
/// Only symbols that exist in Moq are checked. <c>Moq.Language.IReturns</c> (arity 0)
335-
/// and <c>Moq.Language.IReturns`1</c> (arity 1) do not exist in Moq; only
336-
/// <c>Moq.Language.IReturns`2</c> exists. <c>IReturnsResult{TMock}</c> is the actual
337-
/// return type of <c>.Returns()</c> and <c>.ReturnsAsync()</c>, and
338-
/// <c>IThrowsResult</c> is the return type of <c>.Throws()</c>.
333+
/// Only <c>IReturns{TMock, TResult}</c> (arity 2) is checked. <c>Moq.Language.IReturns</c>
334+
/// (arity 0) and <c>Moq.Language.IReturns{T}</c> (arity 1) resolve to null in Moq 4.18+
335+
/// and are not part of the fluent setup chain.
336+
/// </para>
337+
/// <para>
338+
/// <c>IThrows</c>, <c>ISetup{TResult}</c>, <c>ISetupGetter{TMock, TProperty}</c>, and
339+
/// <c>ISetupSetter{TMock, TProperty}</c> are included because a wrapper method may
340+
/// return these types directly. <c>IReturnsResult{TMock}</c> is the return type of
341+
/// <c>.Returns()</c> and <c>.ReturnsAsync()</c>. <c>IThrowsResult</c> is the return
342+
/// type of <c>.Throws()</c>.
339343
/// </para>
340344
/// </remarks>
341345
private static bool IsMatchingFluentSymbol(INamedTypeSymbol type, MoqKnownSymbols knownSymbols)

src/Common/WellKnown/MoqKnownSymbols.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ internal MoqKnownSymbols(Compilation compilation)
246246
internal INamedTypeSymbol? MockRepository => TypeProvider.GetOrCreateTypeByMetadataName("Moq.MockRepository");
247247

248248
/// <summary>
249-
/// Gets the methods for <c>Moq.MockRepository.Of</c>.
249+
/// Gets the methods for <c>Moq.MockRepository.Create</c>.
250250
/// </summary>
251251
/// <remarks>
252252
/// <c>MockRepository</c> is a subclass of <c>MockFactory</c>.

tests/Moq.Analyzers.Test/Common/MoqKnownSymbolsTests.cs

Lines changed: 32 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -158,20 +158,6 @@ public void IThrows_WithoutMoqReference_ReturnsNull()
158158
Assert.Null(symbols.IThrows);
159159
}
160160

161-
[Fact]
162-
public void IReturnsResult1_WithoutMoqReference_ReturnsNull()
163-
{
164-
MoqKnownSymbols symbols = CreateSymbolsWithoutMoq();
165-
Assert.Null(symbols.IReturnsResult1);
166-
}
167-
168-
[Fact]
169-
public void IThrowsResult_WithoutMoqReference_ReturnsNull()
170-
{
171-
MoqKnownSymbols symbols = CreateSymbolsWithoutMoq();
172-
Assert.Null(symbols.IThrowsResult);
173-
}
174-
175161
[Fact]
176162
public void ReturnsExtensions_WithoutMoqReference_ReturnsNull()
177163
{
@@ -235,6 +221,20 @@ public void NonVoidSetupPhrase2_WithoutMoqReference_ReturnsNull()
235221
Assert.Null(symbols.NonVoidSetupPhrase2);
236222
}
237223

224+
[Fact]
225+
public void IReturnsResult1_WithoutMoqReference_ReturnsNull()
226+
{
227+
MoqKnownSymbols symbols = CreateSymbolsWithoutMoq();
228+
Assert.Null(symbols.IReturnsResult1);
229+
}
230+
231+
[Fact]
232+
public void IThrowsResult_WithoutMoqReference_ReturnsNull()
233+
{
234+
MoqKnownSymbols symbols = CreateSymbolsWithoutMoq();
235+
Assert.Null(symbols.IThrowsResult);
236+
}
237+
238238
[Fact]
239239
public void IRaiseable_WithoutMoqReference_ReturnsNull()
240240
{
@@ -840,23 +840,6 @@ public async Task IThrows_WithMoqReference_ReturnsNamedTypeSymbol()
840840
Assert.Equal("IThrows", symbols.IThrows!.Name);
841841
}
842842

843-
[Fact]
844-
public async Task IReturnsResult1_WithMoqReference_ReturnsNamedTypeSymbol()
845-
{
846-
MoqKnownSymbols symbols = await CreateSymbolsWithMoqAsync();
847-
Assert.NotNull(symbols.IReturnsResult1);
848-
Assert.Equal("IReturnsResult", symbols.IReturnsResult1!.Name);
849-
Assert.Equal(1, symbols.IReturnsResult1.Arity);
850-
}
851-
852-
[Fact]
853-
public async Task IThrowsResult_WithMoqReference_ReturnsNamedTypeSymbol()
854-
{
855-
MoqKnownSymbols symbols = await CreateSymbolsWithMoqAsync();
856-
Assert.NotNull(symbols.IThrowsResult);
857-
Assert.Equal("IThrowsResult", symbols.IThrowsResult!.Name);
858-
}
859-
860843
[Fact]
861844
public async Task IThrowsThrows_WithMoqReference_ReturnsNonEmpty()
862845
{
@@ -933,6 +916,24 @@ public async Task IRaise1_WithMoqReference_ReturnsNamedTypeSymbol()
933916
Assert.Equal(1, symbols.IRaise1!.Arity);
934917
}
935918

919+
[Fact]
920+
public async Task IReturnsResult1_WithMoqReference_ReturnsNamedTypeSymbol()
921+
{
922+
MoqKnownSymbols symbols = await CreateSymbolsWithMoqAsync();
923+
Assert.NotNull(symbols.IReturnsResult1);
924+
Assert.Equal("IReturnsResult", symbols.IReturnsResult1!.Name);
925+
Assert.Equal(1, symbols.IReturnsResult1.Arity);
926+
}
927+
928+
[Fact]
929+
public async Task IThrowsResult_WithMoqReference_ReturnsNamedTypeSymbol()
930+
{
931+
MoqKnownSymbols symbols = await CreateSymbolsWithMoqAsync();
932+
Assert.NotNull(symbols.IThrowsResult);
933+
Assert.Equal("IThrowsResult", symbols.IThrowsResult!.Name);
934+
Assert.Equal(0, symbols.IThrowsResult.Arity);
935+
}
936+
936937
[Fact]
937938
public void Task_WithCoreReferences_ReturnsNamedTypeSymbol()
938939
{

tests/Moq.Analyzers.Test/MethodSetupShouldSpecifyReturnValueAnalyzerTests.cs

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -442,7 +442,7 @@ public static IEnumerable<object[]> Issue1067_WrappedSetupTestData()
442442
new Mock<IFoo>().Setup(x => x.DoSomething("test")).ReturnsTrue();
443443
"""],
444444

445-
// Extension method on async setup returning Task result wrapper
445+
// Extension method returning IReturns<TMock, Task<int>> that calls Returns internally
446446
["""
447447
var moq = new Mock<IFoo>();
448448
moq.Setup(x => x.BarAsync()).ReturnsOneAsync();
@@ -459,6 +459,27 @@ public static IEnumerable<object[]> Issue1067_WrappedSetupTestData()
459459
var moq = new Mock<IFoo>();
460460
moq.Setup(x => x.GetValue()).ThrowsInvalidOp();
461461
"""],
462+
463+
// Extension method on async setup that calls ReturnsAsync internally
464+
["""
465+
var moq = new Mock<IFoo>();
466+
moq.Setup(x => x.BarAsync()).ReturnsOneAsyncViaReturnsAsync();
467+
"""],
468+
469+
// Extension method on ValueTask setup using Returns(new ValueTask<T>(...))
470+
["""
471+
var moq = new Mock<IFoo>();
472+
moq.Setup(x => x.BazValueTaskAsync()).ReturnsOneValueTask();
473+
"""],
474+
475+
// Accepted false negative: no-op passthrough returning a fluent type without
476+
// calling Returns. The analyzer cannot inspect the wrapper body, so it assumes
477+
// methods returning fluent types handle configuration internally. Documented
478+
// tradeoff: this suppresses Moq1203 even though no return value is configured.
479+
["""
480+
var moq = new Mock<IFoo>();
481+
moq.Setup(x => x.GetValue()).NoOpPassthrough();
482+
"""],
462483
];
463484

464485
return data.WithNamespaces().WithNewMoqReferenceAssemblyGroups();
@@ -653,6 +674,7 @@ public interface IFoo
653674
int GetValue();
654675
int Calculate(int a, int b);
655676
Task<int> BarAsync();
677+
ValueTask<int> BazValueTaskAsync();
656678
void DoVoidMethod();
657679
void ProcessData(string data);
658680
string Name { get; set; }
@@ -710,6 +732,26 @@ public static Task AsTask<TMock, TResult>(this ISetup<TMock, TResult> setup)
710732
{
711733
return Task.CompletedTask;
712734
}
735+
736+
public static IReturns<TMock, Task<int>> ReturnsOneAsyncViaReturnsAsync<TMock>(this IReturns<TMock, Task<int>> mock)
737+
where TMock : class
738+
{
739+
mock.ReturnsAsync(1);
740+
return mock;
741+
}
742+
743+
public static IReturns<TMock, ValueTask<int>> ReturnsOneValueTask<TMock>(this IReturns<TMock, ValueTask<int>> mock)
744+
where TMock : class
745+
{
746+
mock.Returns(new ValueTask<int>(1));
747+
return mock;
748+
}
749+
750+
public static ISetup<TMock, TResult> NoOpPassthrough<TMock, TResult>(this ISetup<TMock, TResult> setup)
751+
where TMock : class
752+
{
753+
return setup;
754+
}
713755
}
714756
715757
internal class UnitTest

0 commit comments

Comments
 (0)