Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Callback cannot modify captured variable if it is passed to set up method as ref parameter #466

Closed
stakx opened this issue Oct 3, 2017 · 3 comments

Comments

@stakx
Copy link
Contributor

stakx commented Oct 3, 2017

Stumbled upon the following while implementing #105.

The following test sets up a Callback for a method having a ref parameter. Inside that callback, it tries to modify a captured variable (named capturedVariable). It turns out that the modification only succeeds if the captured variable isn't the one passed by-ref to the mocked method:

public interface IFrobbler
{
    void Frobble(ref string arg);
}

[Theory]
[InlineData(false)] // passes
[InlineData(true)]  // fails
public void CallbackCanModifyCapturedVariable(bool passCapturedVariableByRefToMockedMethod)
{
    string valueThatTriggersCallback = "original";

    string capturedVariable = valueThatTriggersCallback;
    string someOtherVariable = valueThatTriggersCallback;

    var mock = new Mock<IFrobbler>();
    mock.Setup(f => f.Frobble(ref valueThatTriggersCallback))
        .Callback(() =>
        {
            capturedVariable = "changed";
        });

    if (passCapturedVariableByRefToMockedMethod)
    {
        mock.Object.Frobble(ref capturedVariable);
    }
    else
    {
        mock.Object.Frobble(ref someOtherVariable);
    }

    Assert.Equal("changed", capturedVariable);
}

I am not sure yet whether this qualifies as a bug, but at the very least this behaviour is somewhat unintuitive and puzzling from a user's perspective.

@stakx stakx self-assigned this Oct 5, 2017
@stakx
Copy link
Contributor Author

stakx commented Oct 7, 2017

Yes, this probably classifies as a bug. Plain C# has no problem changing a captured variable if that variable is also passed to a method by-ref, so people will likely expect this to work in Moq, too:

[Fact] // passes
public static void CanModifyCapturedVariableEvenWhenPassedAsByRefArgument()
{
    int value = 0;
    ModifyAction action = (ref int arg) => ++arg;

    action.Invoke(ref value);

    Assert.Equal(1, value);
}

delegate void ModifyAction(ref int x);

@stakx stakx added the bug label Oct 7, 2017
@stakx
Copy link
Contributor Author

stakx commented Oct 7, 2017

Moq's behaviour here seems to be directly related to Castle DynamicProxy:

[Fact] // does not pass
public static void InterceptorCanModifyCapturedVariableIfPassedByRefToInterceptedMethod()
{
    int value = 0;
    var interceptor = new InterceptionAction(invocation =>
    {
        value = 1;
    });
    var generator = new ProxyGenerator();
    var proxy = generator.CreateInterfaceProxyWithoutTarget<IFoo>(interceptor);

    proxy.Do(ref value);

    Assert.Equal(1, value);
}

public class InterceptionAction : IInterceptor
{
    private Action<IInvocation> callback;

    public InterceptionAction(Action<IInvocation> callback)
    {
        this.callback = callback;
    }

    public void Intercept(IInvocation invocation)
    {
        this.callback?.Invoke(invocation);
    }
}

public interface IFoo
{
    void Do(ref int arg);
}

See also castleproject/Core#311.

@stakx stakx added this to the v4.8.0 milestone Oct 11, 2017
@stakx
Copy link
Contributor Author

stakx commented Oct 13, 2017

We've decided over at castleproject/Core#316 that it's probably not a good idea for DynamicProxy to support this particular scenario, so Moq likewise won't allow it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant