Support for ref returns

[metakirby5]

C# 7.0 added support for ref returns. I tried to mock the value of one such method via .Returns, but got the following:

System.InvalidProgramException : Invalid IL code in Castle.Proxies.IPlayerManagerProxy:get_PlayerInfo (): IL_0030: ret

Where we have:

  public struct PlayerInfo {
    // ...
  }

  public interface IPlayerManager {
    ref PlayerInfo PlayerInfo { get; }
  }

Is there a different way I should be mocking ref returns?

[dtchepak]

Hi @metakirby5 ,

Thanks for raising this. I tried to reproduce it but got this instead:

System.NullReferenceException : Object reference not set to an instance of an object.
   at Castle.Proxies.ObjectProxy.get_PlayerInfo()
   at NSubWorkshop.Tests...

This is with the following test (dotnet core 2, mac):

[Fact]
public void Example() {
    var sub = Substitute.For<IPlayerManager>();
    var result = sub.PlayerInfo;
}

Could you please let me know what .NET environment/platform you're running, and maybe show the full test as well?

[metakirby5]

Hi @dtchepak, thank you for the swift response!

Environment:

• Unity 2019.2.14f1
• NSubstitute 3.1.0.0

Below is a full self-contained repro. I actually forgot to add that I was working with ref readonly, my mistake.

using NSubstitute;
using NUnit.Framework;

namespace RefTestExample {
  public class RefTest {
    public readonly struct PlayerInfo {
      public readonly int Level;

      public PlayerInfo(int level) {
        Level = level;
      }
    }

    public interface IPlayerManager {
      ref readonly PlayerInfo PlayerInfo { get; }
    }

    [Test]
    public void GetRef() {
      var sub = Substitute.For<IPlayerManager>();
      sub.PlayerInfo.Returns(new PlayerInfo(5));
      Assert.AreEqual(5, sub.PlayerInfo.Level);
    }
  }
}

[dtchepak]

This seems to affect Moq (4.13.1) too:

    [Fact]
    public void MoqExample() {
        var mock = new Mock<IPlayerManager>().Object;
        var result = mock.PlayerInfo;
    }

Possibly related: https://github.com/moq/moq4/issues/829

//cc: @stakx (hope you have an awesome holiday! 😄 )

[dtchepak]

Thanks for the full repro @metakirby5.

I don't think this is going to be a quick fix, so you may need to manually mock this interface for now sorry. :(

[metakirby5]

@dtchepak no worries! The difference between using a class versus readonly struct in our case is imperceptible anyways, so we're erring on the side of testability for now and using class which doesn't have this problem. Thank you for taking a look!

[dtchepak]

@metakirby5 Thanks for your understanding. :)

I worry about compromising the code for testability here. There is nothing untestable about this from a design perspective I don't think; it is just a limitation of our library (and based on https://github.com/moq/moq4/issues/829, potentially a limit to what we can do with .NET reflection?).

If you want to keep the struct you could try something like this:

    public class TestPlayerManager : IPlayerManager {
        static PlayerInfo DefaultPlayer = new PlayerInfo();
        public ref PlayerInfo PlayerInfo {
            get {
                return ref DefaultPlayer;
            }
        }
        public void SetPlayerInfo(ref PlayerInfo info) => DefaultPlayer = info;
    }

If IPlayerManager has other members that don't rely on ref return you may be able to use partial subs to get a mix of both worlds:

    public interface IPlayerManager {
        ref PlayerInfo PlayerInfo { get; }
        void DoStuff();
    }

    public abstract class TestPlayerManager : IPlayerManager {
        static PlayerInfo DefaultPlayer = new PlayerInfo();
        // Non-virtual, so NSubstitute won't interfere with calls to this. 
        // Make sure to install NSubstitute.Analyzers which will warn you if you accidentally try 
        // to configure this with NSubstitute.
        public ref PlayerInfo PlayerInfo {
            get {
                return ref DefaultPlayer;
            }
        }
        // Non-virtual; used to configure `PlayerInfo` return value.
        public void SetPlayerInfo(ref PlayerInfo info) => DefaultPlayer = info;

        // This one is abstract/virtual so you can configure it with NSubstitute:
        public abstract void DoStuff();
    }

    static PlayerInfo info = new PlayerInfo();

    [Fact]
    public void NewExample() {
        var sub = Substitute.For<TestPlayerManager>();
        // Arrange
        sub.SetPlayerInfo(ref info);
        // Act (er, fake act. Pretend subject under test is calling `sub.DoStuff`)
        sub.DoStuff();
        // Assert
        sub.Received().DoStuff();
    }

If it really doesn't make a difference then I guess it is fine to leave as is. If you need to switch to struct then hopefully this option will work for you.

[metakirby5]

@dtchepak Oh, I didn't know about partial subs! That sounds like a great solution, thank you :)