Skip to content

Commit

Permalink
Fix CallBase regression with explicitly implemented interface metho…
Browse files Browse the repository at this point in the history
…ds (#558)

* Add failing regression test

which shows that `CallBase` no longer works for explicitly implemented
interface methods (which appear e. g. when `mock.As<TInterface>()` is
used).

* Fix `CallBase` for explicit interface method impls.

While we could just revert 5837c53, that would reintroduce a slow and
hard to understand multi-line `if` condition. Let's instead rewrite
the original conditions such that no superfluous checks are performed,
and such that fast checks are performed first.

(Notably, checking whether a type is an interface is faster than
checking whether it is a class.)

* Update the changelog
  • Loading branch information
stakx authored Dec 26, 2017
1 parent f6a8be6 commit 7e12f34
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 3 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file.
The format is loosely based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).


## Unreleased

#### Fixed

* `CallBase` regression with explicitly implemented interface methods (@stakx, #558)


## 4.8.0 (2017-12-24)

Same as 4.8.0-rc1 (see below), plus some significant speed improvements.
Expand Down
28 changes: 28 additions & 0 deletions Moq.Tests/Regressions/IssueReportsFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1748,6 +1748,34 @@ private static void RegisterInvocations(IMock<ITest> m, System.Collections.Concu

#endregion

#region 557

public sealed class Issue557
{
[Fact]
public void CallBase_works_when_called_on_AsInterface_mock()
{
var mock = new Mock<MyClass>().As<IMyClass>();
mock.CallBase = true;
Assert.NotNull(mock.Object.DoSomething());
}

public interface IMyClass
{
object DoSomething();
}

public class MyClass : IMyClass
{
public object DoSomething()
{
return new object();
}
}
}

#endregion

// Old @ Google Code

#region #47
Expand Down
40 changes: 37 additions & 3 deletions Source/Interception/InterceptionAspects.cs
Original file line number Diff line number Diff line change
Expand Up @@ -329,11 +329,45 @@ public override InterceptionAction Handle(Invocation invocation, Mock mock)

var method = invocation.Method;

if (mock.CallBase && !method.IsAbstract)
if (mock.CallBase)
{
invocation.ReturnBase();
var declaringType = method.DeclaringType;
if (declaringType.GetTypeInfo().IsInterface)
{
if (mock.TargetType.GetTypeInfo().IsInterface)
{
// Case 1: Interface method of an interface proxy.
// There is no base method to call, so fall through.
}
else
{
Debug.Assert(mock.TargetType.GetTypeInfo().IsClass);
Debug.Assert(mock.ImplementsInterface(declaringType));

// Case 2: Explicitly implemented interface method of a class proxy.
// Only call base method if it isn't an event accessor.
if (!method.LooksLikeEventAttach() && !method.LooksLikeEventDetach())
{
invocation.ReturnBase();
return InterceptionAction.Stop;
}
}
}
else
{
Debug.Assert(declaringType.GetTypeInfo().IsClass);

// Case 3: Non-interface method of a class proxy.
// Only call base method if it isn't abstract.
if (!method.IsAbstract)
{
invocation.ReturnBase();
return InterceptionAction.Stop;
}
}
}
else if (method.ReturnType == typeof(void))

if (method.ReturnType == typeof(void))
{
invocation.Return();
}
Expand Down

0 comments on commit 7e12f34

Please sign in to comment.