feat(mocks): handle ref struct parameters and return types#4999
feat(mocks): handle ref struct parameters and return types#4999
Conversation
The source generator now properly handles interfaces with ref struct members (e.g. ReadOnlySpan<byte>) instead of generating uncompilable code that tries to box them into object[] arrays. How it works: - Ref struct parameters are excluded from the args array and argument matchers. Non-ref-struct params on the same method still support full argument matching. - Methods with ref struct params route through the engine normally for setup (Returns, Throws, Callback) and verification (WasCalled). - Ref struct return types use the void dispatch path (HandleCall) since they can't be generic type arguments, then return default. - Properties with ref struct types are handled in MockImpl but excluded from MockMembers extensions (can't create PropertyMockCall<RefStruct>).
There was a problem hiding this comment.
Code Review:
This is a solid foundational fix for a real limitation in the mock source generator. The approach is correct — detect IsRefLikeType at model creation time and propagate that through the code gen pipeline to avoid boxing and generic type argument constraints. Test coverage is good and the snapshot test appropriately locks in the generated output.
That said, I found a few issues ranging from a potential compile-time bug to design concerns worth discussing.
Bug: GenerateWrapProperty and GeneratePartialProperty are missing the IsRefStructReturn check
The PR correctly fixes GenerateInterfaceProperty to use HandleCall + return default for ref struct properties. However, the equivalent fixes were not applied to:
GenerateWrapProperty(line ~265 inMockImplBuilder.cs) — handles wrap mocks of classes with virtual/abstract ref struct propertiesGeneratePartialProperty(line ~655) — handles partial mocks with abstract ref struct properties
Both still emit:
get => _engine.HandleCallWithReturn<{prop.ReturnType}>(...);
// or
if (_engine.TryHandleCallWithReturn<{prop.ReturnType}>(..., out var __result))If prop.ReturnType is a ref struct (e.g., ReadOnlySpan<byte>), these lines won't compile because ref structs can't be generic type arguments. The fix is the same pattern already used in GenerateInterfaceProperty:
if (prop.IsRefStructReturn)
{
// Abstract getter
writer.AppendLine("get");
writer.OpenBrace();
writer.AppendLine($"_engine.HandleCall({prop.MemberId}, \"get_{prop.Name}\", global::System.Array.Empty<object?>());");
writer.AppendLine("return default;");
writer.CloseBrace();
}
else
{
// existing path
}Similarly, the wrap property setter still tries to box the ref struct value:
// Bug: value can't be boxed
set => _engine.HandleCall({prop.SetterMemberId}, "set_{prop.Name}", new object?[] { value });Design Concern: Silent behavior for ref struct returns is surprising and undiagnosable
When a method returns a ref struct, the generated mock always returns default. But from the user's perspective, there's no way to know this at the point of setup — they'll get an extension method returning VoidMockMethodCall, which prevents calling .Returns(...), but there's no diagnostic telling them why this limitation exists or that the return value is always default.
A Roslyn analyzer diagnostic here would be much more user-friendly than discovering the behaviour implicitly:
warning TUnit_MOCK_001: Method 'Parse' returns ref struct 'ReadOnlySpan<char>' which cannot be
configured via Returns(). Mock will always return default(ReadOnlySpan<char>).
Similarly for the setter: the set accessor silently drops the value (empty args array is passed instead of the actual value). A user who sets up a Callback on a ref struct property setter expecting to receive the value will be confused. This should also be surfaced as a diagnostic.
Inconsistency: IsValueType is not set for method parameters
In MemberDiscovery.CreateMethodModel, the MockParameterModel is constructed without setting IsValueType:
// CreateMethodModel — IsValueType is never set (defaults to false)
IsRefStruct = p.Type.IsRefLikeTypeBut in DiscoverConstructors, IsValueType is set while IsRefStruct is not:
// DiscoverConstructors — IsRefStruct is never set (defaults to false)
IsValueType = p.Type.IsValueTypeSince all ref structs are value types, IsRefStruct = true implies IsValueType = true. This is currently harmless because IsValueType isn't used in the builder paths, but it's an internal inconsistency worth fixing in the same PR to prevent future bugs where someone adds an IsValueType check that produces wrong results for ref struct params.
Minor: IsRefStruct not set in constructor parameters
In DiscoverConstructors, IsRefStruct defaults to false. While constructors with ref struct parameters are rare (ref structs can't be stored as fields), it's still an inconsistency. This is low priority but aligns with the above cleanup.
What's Good
- Correct use of
IsRefLikeTypeto detect ref struct types at compile/generation time — this is the right API - Filtering ref struct params from
GetArgsArrayExpressionandGetArgParameterListis clean and correct - The
EmitOutRefReadbackskip forIsRefStructparams is exactly right - Mixed-param argument matching (
Compute(int id, ReadOnlySpan<byte> data)→ match onintonly) is a sensible design decision - Test coverage is solid for interface mocks and mixed-param scenarios
- Snapshot test properly locks in the generated output
The main outstanding issue is the missing ref struct handling in GenerateWrapProperty and GeneratePartialProperty, which could cause compile errors for wrap/partial mocks with ref struct properties. The rest are improvements to make the limitation more discoverable.
There was a problem hiding this comment.
Code Review: feat(mocks): handle ref struct parameters and return types
This is a solid foundational fix for a real limitation in the mock source generator. The approach is correct — detect IsRefLikeType at model creation time and propagate that through the code gen pipeline to avoid boxing and generic type argument constraints. Test coverage is good and the snapshot test appropriately locks in the generated output.
That said, I found a few issues ranging from a potential compile-time bug to design concerns worth discussing.
Bug: GenerateWrapProperty and GeneratePartialProperty are missing the IsRefStructReturn check
The PR correctly fixes GenerateInterfaceProperty to use HandleCall + return default for ref struct properties. However, the equivalent fixes were not applied to:
GenerateWrapProperty(~line 265 inMockImplBuilder.cs) — handles wrap mocks of classes with virtual/abstract ref struct propertiesGeneratePartialProperty(~line 655) — handles partial mocks with abstract ref struct properties
Both still emit:
get => _engine.HandleCallWithReturn<{prop.ReturnType}>(...);
// or
if (_engine.TryHandleCallWithReturn<{prop.ReturnType}>(..., out var __result))If prop.ReturnType is a ref struct (e.g., ReadOnlySpan<byte>), these lines won't compile because ref structs can't be generic type arguments. The fix is the same pattern already used in GenerateInterfaceProperty:
if (prop.IsRefStructReturn)
{
writer.AppendLine("get");
writer.OpenBrace();
writer.AppendLine($"_engine.HandleCall({prop.MemberId}, \"get_{prop.Name}\", global::System.Array.Empty<object?>());");
writer.AppendLine("return default;");
writer.CloseBrace();
}
else
{
// existing path
}Similarly, GenerateWrapProperty's setter still tries to box the ref struct value (new object?[] { value }) for abstract setter members — that also won't compile.
Design Concern: Silent behavior for ref struct returns is surprising and undiagnosable
When a method returns a ref struct, the generated mock always returns default. The user gets a VoidMockMethodCall extension (which correctly prevents .Returns(...) from compiling), but there's no diagnostic explaining why or that return values are always default.
A Roslyn analyzer diagnostic would be much more user-friendly:
warning TUnit_MOCK_001: Method 'Parse' returns ref struct 'ReadOnlySpan<char>' which cannot be
configured via Returns(). Mock will always return default(ReadOnlySpan<char>).
The same applies to the property setter — it silently drops the value (passes empty args array instead of the actual set value). A user who sets up a Callback on a ref struct property setter expecting to receive the value will be confused.
Inconsistency: IsValueType and IsRefStruct are not set symmetrically across code paths
In MemberDiscovery.CreateMethodModel, MockParameterModel is built without setting IsValueType:
// CreateMethodModel — IsValueType never set (defaults to false)
IsRefStruct = p.Type.IsRefLikeTypeBut in DiscoverConstructors, IsValueType is set while IsRefStruct is not:
// DiscoverConstructors — IsRefStruct never set (defaults to false)
IsValueType = p.Type.IsValueTypeSince all ref structs are value types, IsRefStruct = true implies IsValueType = true. This is currently harmless because IsValueType isn't checked in the method builder paths, but it's an internal inconsistency that could cause subtle bugs if someone later adds an IsValueType guard expecting it to cover ref structs.
What's Good
- Correct use of
IsRefLikeTypeto detect ref struct types at generation time — the right API - Filtering ref struct params from
GetArgsArrayExpressionandGetArgParameterListis clean and correct - The
EmitOutRefReadbackskip forIsRefStructparams is exactly right - Mixed-param argument matching (
Compute(int id, ReadOnlySpan<byte> data)→ match onintonly) is a sensible design decision, well-tested - Snapshot test properly locks in the generated output
- Solid runtime test coverage for the interface mock scenarios
The main structural concern is the missing ref struct handling in GenerateWrapProperty and GeneratePartialProperty — these could cause compile errors for wrap/partial mocks with ref struct properties. Everything else is either a minor inconsistency or an improvement to user-facing discoverability.
- Fix GenerateWrapProperty and GeneratePartialProperty to handle IsRefStructReturn (was missing, would cause compile errors for wrap/partial mocks with ref struct properties) - Set IsValueType on method parameters (was only set on constructor params) and IsRefStruct on constructor parameters (was only set on method params) for symmetry
There was a problem hiding this comment.
Code Review (Follow-up): feat(mocks): handle ref struct parameters and return types
Previous Review Issues — All Addressed ✅
The second commit (2775de05) correctly addresses all the feedback from the prior review:
GenerateWrapPropertyref struct getter/setter — now usesHandleCall+return defaultfor both abstract and virtual cases, and the setter correctly passesArray.Empty<object?>()instead of boxing the value. ✅GeneratePartialPropertyref struct getter/setter — same fix applied symmetrically. ✅IsValueTypenot set inCreateMethodModel— now set alongsideIsRefStruct. ✅IsRefStructnot set inDiscoverConstructors— now set. ✅
Remaining Minor Observations
1. Dummy assertions in tests
Several tests end with await Assert.That(true).IsTrue(); purely as an async placeholder after a WasCalled() verification call:
// RefStructTests.cs – Void_RefStruct_Method_Verification_Works
mock.Process().WasCalled(Times.Exactly(2));
await Assert.That(true).IsTrue(); // not neededIf WasCalled throws synchronously on failure, the dummy assertion provides no value and obscures test intent. The test method can simply be marked public void instead of public async Task, or the call to WasCalled can be the terminal statement. This is a minor code-smell but worth cleaning up so the test structure is self-documenting.
2. Design: silent default return for ref struct results (previously raised)
The generated mock silently returns default(ReadOnlySpan<byte>) etc. with no way to configure the return value. Users only discover this because the extension method returns VoidMockMethodCall (no .Returns(...)), but there's no explanation of why. A Roslyn diagnostic here would be a net improvement for DX — but this is an enhancement, not a blocker.
Overall
The core ref struct support is well-implemented, the symmetry across all four code-gen paths (interface, wrap, partial, engine-dispatch) is now correct, and the test coverage is solid. The two remaining items above are minor. This PR is ready to merge.
Summary
ReadOnlySpan<byte>) instead of generating uncompilable code that tries to box them intoobject[]arraysmock.Send("server-a")matches on thestringparam while ignoring theReadOnlySpan<byte>param)Returns,Throws,Callback) and verification (WasCalled) through the normal engine dispatchHandleCall) since they can't be generic type arguments, returningdefaultChanges
Source generator:
MockParameterModel— newIsRefStructflag per parameterMockMemberModel— newIsRefStructReturnflag for return typesMemberDiscovery— sets both flags viaITypeSymbol.IsRefLikeTypeMockImplBuilder— filters ref struct params fromobject?[]args arrays; ref struct returns useHandleCall+return defaultMockMembersBuilder— filters ref struct params fromArg<T>extension method signatures and matchers; ref struct returns useVoidMockMethodCallTests:
ReadOnlySpan<byte>andReadOnlySpan<char>methodsTest plan