SteveDunn / Vogen

A semi-opinionated library which is a source generator and a code analyser. It Source generates Value Objects
Apache License 2.0
888 stars 46 forks source link

Struct-based ValueObjects doesn't work in `Nsubstitute`'s `Arg.Any` or `Arg.Is` methods #707

Closed KennethHoff closed 6 days ago

KennethHoff commented 1 week ago

Describe the bug

If you try to mock methods containing struct-based ValueObjects using the NSubstitute library, NSubstitute throws an exception. Class-based ValueObjects work.

Exception for the StructString_ArgAny test in the Repro:

NSubstitute.Exceptions.RedundantArgumentMatcherException: Some argument specifications (e.g. Arg.Is, Arg.Any) were left over after the last call.

NSubstitute.Exceptions.RedundantArgumentMatcherException
Some argument specifications (e.g. Arg.Is, Arg.Any) were left over after the last call.

This is often caused by using an argument spec with a call to a member NSubstitute does not handle (such as a non-virtual member or a call to an instance which is not a substitute), or for a purpose other than specifying a call (such as using an arg spec as a return value). For example:

    var sub = Substitute.For<SomeClass>();
    var realType = new MyRealType(sub);
    // INCORRECT, arg spec used on realType, not a substitute:
    realType.SomeMethod(Arg.Any<int>()).Returns(2);
    // INCORRECT, arg spec used as a return value, not to specify a call:
    sub.VirtualMethod(2).Returns(Arg.Any<int>());
    // INCORRECT, arg spec used with a non-virtual method:
    sub.NonVirtualMethod(Arg.Any<int>()).Returns(2);
    // CORRECT, arg spec used to specify virtual call on a substitute:
    sub.VirtualMethod(Arg.Any<int>()).Returns(2);

To fix this make sure you only use argument specifications with calls to substitutes. If your substitute is a class, make sure the member is virtual.

Another possible cause is that the argument spec type does not match the actual argument type, but code compiles due to an implicit cast. For example, Arg.Any<int>() was used, but Arg.Any<double>() was required.

NOTE: the cause of this exception can be in a previously executed test. Use the diagnostics below to see the types of any redundant arg specs, then work out where they are being created.

Diagnostic information:

Remaining (non-bound) argument specifications:
    any StructString

All argument specifications:
    any StructString

   at NSubstitute.Core.Arguments.ArgumentSpecificationsFactory.Create(IList`1 argumentSpecs, Object[] arguments, IParameterInfo[] parameterInfos, MethodInfo methodInfo, MatchArgs matchArgs)
   at NSubstitute.Core.CallSpecificationFactory.CreateFrom(ICall call, MatchArgs matchArgs)
   at NSubstitute.Routing.Handlers.RecordCallSpecificationHandler.Handle(ICall call)
   at NSubstitute.Routing.Route.Handle(ICall call)
   at NSubstitute.Core.CallRouter.Route(ICall call)
   at NSubstitute.Proxies.CastleDynamicProxy.CastleForwardingInterceptor.Intercept(IInvocation invocation)
   at Castle.DynamicProxy.AbstractInvocation.Proceed()
   at NSubstitute.Proxies.CastleDynamicProxy.ProxyIdInterceptor.Intercept(IInvocation invocation)
   at Castle.DynamicProxy.AbstractInvocation.Proceed()
   at Castle.Proxies.ObjectProxy.GetStructString(StructString value)
   at TestProject.UnitTest1.StructString_ArgAny() in /Users/kennethhoff/Development/repros/VogenNsubstitute/TestProject/UnitTest1.cs:line 42
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

dotnet --info:

.NET SDK:
 Version:           9.0.100
 Commit:            59db016f11
 Workload version:  9.0.100-manifests.3068a692
 MSBuild version:   17.12.7+5b8665660

Runtime Environment:
 OS Name:     Mac OS X
 OS Version:  14.5
 OS Platform: Darwin
 RID:         osx-arm64
 Base Path:   /Users/kennethhoff/.dotnet/sdk/9.0.100/

.NET workloads installed:
There are no installed workloads to display.
Configured to use loose manifests when installing new manifests.

Host:
  Version:      9.0.0
  Architecture: arm64
  Commit:       9d5a6a9aa4

.NET SDKs installed:
  8.0.404 [/Users/kennethhoff/.dotnet/sdk]
  9.0.100 [/Users/kennethhoff/.dotnet/sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 8.0.11 [/Users/kennethhoff/.dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 9.0.0 [/Users/kennethhoff/.dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 8.0.11 [/Users/kennethhoff/.dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 9.0.0 [/Users/kennethhoff/.dotnet/shared/Microsoft.NETCore.App]

Other architectures found:
  None

Environment variables:
  Not set

global.json file:
  Not found

Learn more:
  https://aka.ms/dotnet/info

Download .NET:
  https://aka.ms/dotnet/download

Steps to reproduce

Repro: https://github.com/KennethHoff/Repros/tree/master/VogenNsubstitute

Vogen version: 5.0.5 NSubstitute version: 5.3.0 .Net version: 8.0.11 && 9.0.0

using NSubstitute;
using Vogen;

namespace TestProject;

[ValueObject<string>]
public readonly partial struct StructString;

[ValueObject<string>]
public sealed partial class ClassString;

[ValueObject<int>]
public readonly partial struct StructInt;

[ValueObject<int>]
public sealed partial class ClassInt;

public interface IService
{
    void GetStructString(StructString value);
    void GetClassString(ClassString value);
    void GetBothStrings(ClassString value, StructString other);

    void GetStructInt(StructInt value);
    void GetClassInt(ClassInt value);
    void GetBothInts(ClassInt value, StructInt other);
}

public class UnitTest1
{
    private IService _sut = Substitute.For<IService>();

    // This works.
    [Fact]
    public void ClassString_ArgAny()
    {
        _sut.GetClassString(Arg.Any<ClassString>());
    }

    // This doesn't work.
    [Fact]
    public void StructString_ArgAny()
    {
        _sut.GetStructString(Arg.Any<StructString>());
    }

    // This doesn't work.
    [Fact]
    public void BothStrings_ArgAny()
    {
        _sut.GetBothStrings(Arg.Any<ClassString>(),Arg.Any<StructString>());
    }

    // This works.
    [Fact]
    public void ClassString_ArgIs()
    {
        _sut.GetClassString(Arg.Is<ClassString>(_ => true));
    }

    // This doesn't work.
    [Fact]
    public void StructString_ArgIs()
    {
        _sut.GetStructString(Arg.Is<StructString>(_ => true));
    }

    // This doesn't work.
    [Fact]
    public void BothStrings_ArgIs()
    {
        _sut.GetBothStrings(Arg.Is<ClassString>(_ => true),Arg.Is<StructString>(_ => true));
    }

    // This works.
    [Fact]
    public void ClassInt_ArgAny()
    {
        _sut.GetClassInt(Arg.Any<ClassInt>());
    }

    // This doesn't work.
    [Fact]
    public void StructInt_ArgAny()
    {
        _sut.GetStructInt(Arg.Any<StructInt>());
    }

    // This doesn't work.
    [Fact]
    public void BothInts_ArgAny()
    {
        _sut.GetBothInts(Arg.Any<ClassInt>(),Arg.Any<StructInt>());
    }

    // This works.
    [Fact]
    public void ClassInt_ArgIs()
    {
        _sut.GetClassInt(Arg.Is<ClassInt>(_ => true));
    }

    // This doesn't work.
    [Fact]
    public void StructInt_ArgIs()
    {
        _sut.GetStructInt(Arg.Is<StructInt>(_ => true));
    }

    // This doesn't work.
    [Fact]
    public void BothInts_ArgIs()
    {
        _sut.GetBothInts(Arg.Is<ClassInt>(_ => true),Arg.Is<StructInt>(_ => true));
    }

}

Expected behaviour

class and struct ValueObjects should both work as type parameters for Arg.Any<{TYPE}> and Arg.Is<{TYPE}>.

SteveDunn commented 6 days ago

Thanks for the bug report and repro - very useful.

I found this on nsub: https://github.com/nsubstitute/NSubstitute/issues/766

In particular, this entry that says it's a bug in nsub: https://github.com/nsubstitute/NSubstitute/issues/766#issuecomment-1882436897

I don't know much about how nsub works, but I use it a fair bit and it's very useful. If I get time, I'll take a look to see if I can contribute the suggested fix in that thread.

But for now, I'll close this as it's not a bug in Vogen. I'l have a think of a workaround and let you know if I come up with anything.

Thanks again for the feedback!