diff --git a/CHANGELOG.md b/CHANGELOG.md index e35c05566..7e0449e90 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,9 +6,14 @@ The format is loosely based on [Keep a Changelog](http://keepachangelog.com/en/1 ## Unreleased +#### Added + +* `CallBase` can now be used with interface methods that have a default interface implementation. It will call [the most specific override](https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-8.0/default-interface-methods#the-most-specific-override-rule). (@stakx, #1130) + #### Fixed -* Newly introduced: `AmbiguousMatchException` raised when interface has property indexer besides property in VB. (@mujdatdinc, #1129) +* `AmbiguousMatchException` raised when interface has property indexer besides property in VB. (@mujdatdinc, #1129) +* Interface default methods are ignored (@hahn-kev, #972) ## 4.16.0 (2021-01-16) diff --git a/src/Moq/Behaviors/ReturnBaseOrDefaultValue.cs b/src/Moq/Behaviors/ReturnBaseOrDefaultValue.cs index d39a24acf..068a209ae 100644 --- a/src/Moq/Behaviors/ReturnBaseOrDefaultValue.cs +++ b/src/Moq/Behaviors/ReturnBaseOrDefaultValue.cs @@ -26,13 +26,24 @@ public override void Execute(Invocation invocation) if (this.mock.CallBase) { + +#if FEATURE_DEFAULT_INTERFACE_IMPLEMENTATIONS + var tryCallDefaultInterfaceImplementation = false; +#endif + var declaringType = method.DeclaringType; if (declaringType.IsInterface) { if (this.mock.MockedType.IsInterface) { // Case 1: Interface method of an interface proxy. + +#if FEATURE_DEFAULT_INTERFACE_IMPLEMENTATIONS + // Fall through to invoke default implementation (if one exists). + tryCallDefaultInterfaceImplementation = true; +#else // There is no base method to call, so fall through. +#endif } else { @@ -56,7 +67,13 @@ public override void Execute(Invocation invocation) Debug.Assert(this.mock.AdditionalInterfaces.Contains(declaringType)); // Case 2b: Additional interface. + +#if FEATURE_DEFAULT_INTERFACE_IMPLEMENTATIONS + // Fall through to invoke default implementation (if one exists). + tryCallDefaultInterfaceImplementation = true; +#else // There is no base method to call, so fall through. +#endif } } } @@ -72,6 +89,16 @@ public override void Execute(Invocation invocation) return; } } + +#if FEATURE_DEFAULT_INTERFACE_IMPLEMENTATIONS + if (tryCallDefaultInterfaceImplementation && !method.IsAbstract) + { + // Invoke default implementation. + invocation.ReturnValue = invocation.CallBase(); + return; + } +#endif + } if (method.ReturnType != typeof(void)) diff --git a/src/Moq/Moq.csproj b/src/Moq/Moq.csproj index 6027a1629..28485bbaf 100644 --- a/src/Moq/Moq.csproj +++ b/src/Moq/Moq.csproj @@ -16,7 +16,11 @@ Moq true 4 - 8.0 + 9.0 + + + + $(DefineConstants);FEATURE_DEFAULT_INTERFACE_IMPLEMENTATIONS diff --git a/src/Moq/Pair.cs b/src/Moq/Pair.cs index 5f411d794..fc184b2c5 100644 --- a/src/Moq/Pair.cs +++ b/src/Moq/Pair.cs @@ -1,9 +1,11 @@ // Copyright (c) 2007, Clarius Consulting, Manas Technology Solutions, InSTEDD, and Contributors. // All rights reserved. Licensed under the BSD 3-Clause License; see License.txt. +using System; + namespace Moq { - internal readonly struct Pair + internal readonly struct Pair : IEquatable> { public readonly T1 Item1; public readonly T2 Item2; @@ -19,5 +21,21 @@ public void Deconstruct(out T1 item1, out T2 item2) item1 = this.Item1; item2 = this.Item2; } + + public bool Equals(Pair other) + { + return object.Equals(this.Item1, other.Item1) + && object.Equals(this.Item2, other.Item2); + } + + public override bool Equals(object obj) + { + return obj is Pair other && this.Equals(other); + } + + public override int GetHashCode() + { + return unchecked(1001 * this.Item1?.GetHashCode() ?? 101 + this.Item2?.GetHashCode() ?? 11); + } } } diff --git a/src/Moq/ProxyFactories/CastleProxyFactory.cs b/src/Moq/ProxyFactories/CastleProxyFactory.cs index bf4ece1ef..0e994c312 100644 --- a/src/Moq/ProxyFactories/CastleProxyFactory.cs +++ b/src/Moq/ProxyFactories/CastleProxyFactory.cs @@ -2,12 +2,18 @@ // All rights reserved. Licensed under the BSD 3-Clause License; see License.txt. using System; -using System.Collections.Generic; using System.Diagnostics; -using System.Globalization; -using System.Linq; using System.Reflection; +#if FEATURE_DEFAULT_INTERFACE_IMPLEMENTATIONS +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Linq; +using System.Reflection.Emit; +using System.Runtime; +using System.Text; +#endif + using Castle.DynamicProxy; using Moq.Internals; @@ -126,6 +132,17 @@ protected internal override object CallBase() { Debug.Assert(this.underlying != null); +#if FEATURE_DEFAULT_INTERFACE_IMPLEMENTATIONS + var method = this.Method; + if (method.DeclaringType.IsInterface && !method.IsAbstract) + { + // As of version 4.4.0, DynamicProxy cannot proceed to default method implementations of interfaces. + // we need to find and call those manually. + var mostSpecificOverride = FindMostSpecificOverride(method, this.underlying.Proxy.GetType()); + return DynamicInvokeNonVirtually(mostSpecificOverride, this.underlying.Proxy, this.Arguments); + } +#endif + this.underlying.Proceed(); return this.underlying.ReturnValue; } @@ -136,6 +153,207 @@ public void DetachFromUnderlying() } } +#if FEATURE_DEFAULT_INTERFACE_IMPLEMENTATIONS + // Finding and calling default interface implementations currently involves a lot of reflection, + // we are using two caches to speed up these operations for repeated calls. + private static ConcurrentDictionary, MethodInfo> mostSpecificOverrides; + private static ConcurrentDictionary> nonVirtualInvocationThunks; + + static CastleProxyFactory() + { + mostSpecificOverrides = new ConcurrentDictionary, MethodInfo>(); + nonVirtualInvocationThunks = new ConcurrentDictionary>(); + } + + /// + /// Attempts to find the most specific override for the given method + /// in the type chains (base class, interfaces) of the given . + /// + public static MethodInfo FindMostSpecificOverride(MethodInfo declaration, Type proxyType) + { + return mostSpecificOverrides.GetOrAdd(new Pair(declaration, proxyType), static key => + { + // This follows the rule specified in: + // https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-8.0/default-interface-methods#the-most-specific-override-rule. + + var (declaration, proxyType) = key; + + var genericParameterCount = declaration.IsGenericMethod ? declaration.GetGenericArguments().Length : 0; + var returnType = declaration.ReturnType; + var parameterTypes = declaration.GetParameterTypes().ToArray(); + var declaringType = declaration.DeclaringType; + + // If the base class has a method implementation, then by rule (2) it will be more specific + // than any candidate method from an implemented interface: + var baseClass = proxyType.BaseType; + if (baseClass != null && declaringType.IsAssignableFrom(baseClass)) + { + var map = baseClass.GetInterfaceMap(declaringType); + var index = Array.IndexOf(map.InterfaceMethods, declaration); + return map.TargetMethods[index]; + } + + // Otherwise, we need to look for candidates in all directly or indirectly implemented interfaces: + var implementedInterfaces = proxyType.GetInterfaces(); + var candidateMethods = new HashSet(); + foreach (var implementedInterface in implementedInterfaces.Where(i => declaringType.IsAssignableFrom(i))) + { + // Search for an implicit override: + var candidateMethod = implementedInterface.GetMethod(declaration.Name, genericParameterCount, BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance, null, parameterTypes, null); + + // Search for an explicit override: + if (candidateMethod?.GetBaseDefinition() != declaration) + { + // Unfortunately, we cannot use `.GetInterfaceMap` to find out whether an interface method + // overrides another base interface method, i.e. whether they share the same vtbl slot. + // It appears that the best thing we can do is to look for a non-public method having + // the right name and parameter types, and hope for the best: + var name = new StringBuilder(); + name.Append(declaringType.FullName); + name.Replace('+', '.'); + name.Append('.'); + name.Append(declaration.Name); + candidateMethod = implementedInterface.GetMethod(name.ToString(), genericParameterCount, BindingFlags.NonPublic | BindingFlags.Instance, null, parameterTypes, null); + } + + if (candidateMethod == null) continue; + + // Now we have a candidate override. We need to check if it is less specific than any others + // that we have already found earlier: + if (candidateMethods.Any(cm => implementedInterface.IsAssignableFrom(cm.DeclaringType))) continue; + + // No, it is the most specific override so far. Add it to the list, but before doing so, + // remove all less specific overrides from it: + candidateMethods.ExceptWith(candidateMethods.Where(cm => cm.DeclaringType.IsAssignableFrom(implementedInterface)).ToArray()); + candidateMethods.Add(candidateMethod); + } + + var candidateCount = candidateMethods.Count(); + if (candidateCount > 1) + { + throw new AmbiguousImplementationException(); + } + else if (candidateCount == 1) + { + return candidateMethods.First(); + } + else + { + return declaration; + } + }); + } + + /// + /// Performs a non-virtual (non-polymorphic) call to the given + /// using the specified object and . + /// + public static object DynamicInvokeNonVirtually(MethodInfo method, object instance, object[] arguments) + { + // There are a couple of probable alternatives to the following implementation that + // unfortunately don't work in practice: + // + // * We could try `method.Invoke(instance, InvokeMethod | DeclaredOnly, arguments)`, + // unfortunately that doesn't work. `DeclaredOnly` does not have the desired effect. + // + // * We could get a function pointer via `method.MethodHandle.GetFunctionPointer()`, + // then construct a delegate for it (see ECMA-335 �II.14.4). This does not work + // because the delegate signature would have to have a matching parameter list, + // not just an untyped `object[]`. It also doesn't work because we don't always have + // a suitable delegate type ready (e.g. when a method has by-ref parameters). + // + // So we end up having to create a dynamic method that transforms the `object[]`array + // to a properly typed argument list and then invokes the method using the IL `call` + // instruction. + + var thunk = nonVirtualInvocationThunks.GetOrAdd(method, static method => + { + var originalParameterTypes = method.GetParameterTypes(); + var n = originalParameterTypes.Count; + + var dynamicMethod = new DynamicMethod(string.Empty, returnType: typeof(object), parameterTypes: new[] { typeof(object), typeof(object[]) }); + dynamicMethod.InitLocals = true; + var il = dynamicMethod.GetILGenerator(); + + var arguments = new LocalBuilder[n]; + var returnValue = il.DeclareLocal(typeof(object)); + + // Erase by-ref-ness of parameter types to get at the actual type of value. + // We need this because we are handed `invocation.Arguments` as an `object[]` array. + var parameterTypes = originalParameterTypes.ToArray(); + for (var i = 0; i < n; ++i) + { + if (parameterTypes[i].IsByRef) + { + parameterTypes[i] = parameterTypes[i].GetElementType(); + } + } + + // Transfer `invocation.Arguments` into appropriately typed local variables. + // This involves unboxing value-typed arguments, and possibly down-casting others from `object`. + // The `unbox.any` instruction will do the right thing in both cases. + for (var i = 0; i < n; ++i) + { + arguments[i] = il.DeclareLocal(parameterTypes[i]); + + il.Emit(OpCodes.Ldarg_1); + il.Emit(OpCodes.Ldc_I4, i); + il.Emit(OpCodes.Ldelem_Ref); + il.Emit(OpCodes.Unbox_Any, parameterTypes[i]); + il.Emit(OpCodes.Stloc, arguments[i]); + } + + // Now we're going to call the actual default implementation. + + // We do this inside a `try` block because we need to write back possibly modified + // arguments to `invocation.Arguments` even if the called method throws. + var returnLabel = il.DefineLabel(); + il.BeginExceptionBlock(); + + // Perform the actual call. + il.Emit(OpCodes.Ldarg_0); + il.Emit(OpCodes.Castclass, method.DeclaringType); + for (var i = 0; i < n; ++i) + { + il.Emit(originalParameterTypes[i].IsByRef ? OpCodes.Ldloca : OpCodes.Ldloc, arguments[i]); + } + il.Emit(OpCodes.Call, method); + + // Put the return value in a local variable for later retrieval. + if (method.ReturnType != typeof(void)) + { + il.Emit(OpCodes.Box, method.ReturnType); + il.Emit(OpCodes.Castclass, typeof(object)); + il.Emit(OpCodes.Stloc, returnValue); + } + il.Emit(OpCodes.Leave, returnLabel); + + il.BeginFinallyBlock(); + + // Write back possibly modified arguments to `invocation.Arguments`. + for (var i = 0; i < n; ++i) + { + il.Emit(OpCodes.Ldarg_1); + il.Emit(OpCodes.Ldc_I4, i); + il.Emit(OpCodes.Ldloc, arguments[i]); + il.Emit(OpCodes.Box, arguments[i].LocalType); + il.Emit(OpCodes.Stelem_Ref); + } + il.Emit(OpCodes.Endfinally); + + il.EndExceptionBlock(); + il.MarkLabel(returnLabel); + + il.Emit(OpCodes.Ldloc, returnValue); + il.Emit(OpCodes.Ret); + + return (Func)dynamicMethod.CreateDelegate(typeof(Func)); + }); + + return thunk.Invoke(instance, arguments); + } +#endif + /// /// This hook tells Castle DynamicProxy to proxy the default methods it suggests, /// plus some of the methods defined by , e.g. so we can intercept diff --git a/tests/Moq.Tests/CallBaseDefaultInterfaceImplementationsFixture.cs b/tests/Moq.Tests/CallBaseDefaultInterfaceImplementationsFixture.cs new file mode 100644 index 000000000..035f5f34b --- /dev/null +++ b/tests/Moq.Tests/CallBaseDefaultInterfaceImplementationsFixture.cs @@ -0,0 +1,145 @@ +// Copyright (c) 2007, Clarius Consulting, Manas Technology Solutions, InSTEDD, and Contributors. +// All rights reserved. Licensed under the BSD 3-Clause License; see License.txt. + +#if FEATURE_DEFAULT_INTERFACE_IMPLEMENTATIONS + +using System; + +using Xunit; + +namespace Moq.Tests +{ + public class CallBaseDefaultInterfaceImplementationsFixture + { + [Fact] + public void CallBase__configured_on_mock__succeeds() + { + var mock = new Mock() { CallBase = true }; + mock.Object.Inert(); + } + + [Fact] + public void CallBase__configured_on_setup__succeeds() + { + var mock = new Mock(); + mock.Setup(m => m.Inert()).CallBase(); + mock.Object.Inert(); + } + + [Fact] + public void Reference_typed_return_value__is_returned() + { + var mock = new Mock() { CallBase = true }; + var returnValue = mock.Object.ReturnsHello(); + Assert.Equal("Hello", returnValue); + } + + [Fact] + public void Value_typed_return_value__is_returned() + { + var mock = new Mock() { CallBase = true }; + var returnValue = mock.Object.ReturnsFortyTwo(); + Assert.Equal(42, returnValue); + } + + [Fact] + public void Can_deal_with__value_typed_argument() + { + var mock = new Mock() { CallBase = true }; + var returnValue = mock.Object.ReturnsIntArg(42); + Assert.Equal(42, returnValue); + } + + [Fact] + public void Can_deal_with__reference_typed_argument() + { + var mock = new Mock() { CallBase = true }; + var returnValue = mock.Object.ReturnsStringArg("Echo"); + Assert.Equal("Echo", returnValue); + } + + [Fact] + public void Can_deal_with__generic_argument() + { + var mock = new Mock() { CallBase = true }; + var returnValue = mock.Object.ReturnsArg(true); + Assert.True(returnValue); + } + + [Fact] + public void Can_deal_with__value_typed_by_ref_argument() + { + var mock = new Mock() { CallBase = true }; + var arg = 42; + var returnValue = mock.Object.Increment(ref arg); + Assert.Equal(42, returnValue); + Assert.Equal(43, arg); + } + + [Fact] + public void Can_deal_with__reference_typed_by_ref_argument() + { + var mock = new Mock() { CallBase = true }; + var arg = "C+"; + var returnValue = mock.Object.AppendPlus(ref arg); + Assert.Equal("C+", returnValue); + Assert.Equal("C++", arg); + } + + [Fact] + public void Does_not_lose_modified_arguments__when_exception_thrown() + { + var mock = new Mock() { CallBase = true }; + var arg = "C+"; + Assert.Throws(() => mock.Object.AppendPlusThenThrow(ref arg)); + Assert.Equal("C++", arg); + } + + [Fact] + public void Should_call__most_specific__most_derived__implementation() + { + var mock = new Mock() { CallBase = true }; + Assert.Equal(nameof(IZ), mock.Object.WhoAmI()); + } + + public interface IX + { + void Inert() + { + } + + int ReturnsFortyTwo() => 42; + string ReturnsHello() => "Hello"; + + int ReturnsIntArg(int arg) => arg; + string ReturnsStringArg(string arg) => arg; + T ReturnsArg(T arg) => arg; + + int Increment(ref int arg) => arg++; + string AppendPlus(ref string arg) + { + var argBefore = arg; + arg += "+"; + return argBefore; + } + + string AppendPlusThenThrow(ref string arg) + { + arg += "+"; + throw new ArithmeticException(); + } + } + + public interface IY + { + string WhoAmI() => nameof(IY); + } + + public interface IZ : IY + { + string IY.WhoAmI() => nameof(IZ); + } + } +} + +#endif diff --git a/tests/Moq.Tests/Moq.Tests.csproj b/tests/Moq.Tests/Moq.Tests.csproj index 72c3f3f48..326ca5a3b 100644 --- a/tests/Moq.Tests/Moq.Tests.csproj +++ b/tests/Moq.Tests/Moq.Tests.csproj @@ -18,7 +18,7 @@ $(DefineConstants);FEATURE_DYNAMICPROXY_SERIALIZABLE_PROXIES;FEATURE_EF;FEATURE_SYSTEM_WEB;FEATURE_SYSTEM_WINDOWS_FORMS - $(DefineConstants) + $(DefineConstants);FEATURE_DEFAULT_INTERFACE_IMPLEMENTATIONS diff --git a/tests/Moq.Tests/ProxyFactories/MostSpecificOverrideFixture.cs b/tests/Moq.Tests/ProxyFactories/MostSpecificOverrideFixture.cs new file mode 100644 index 000000000..969f622c5 --- /dev/null +++ b/tests/Moq.Tests/ProxyFactories/MostSpecificOverrideFixture.cs @@ -0,0 +1,74 @@ +// Copyright (c) 2007, Clarius Consulting, Manas Technology Solutions, InSTEDD, and Contributors. +// All rights reserved. Licensed under the BSD 3-Clause License; see License.txt. + +#if FEATURE_DEFAULT_INTERFACE_IMPLEMENTATIONS + +using System; +using System.Reflection; + +using Xunit; + +using static Moq.CastleProxyFactory; + +namespace Moq.Tests.ProxyFactories +{ + public class MostSpecificOverrideFixture + { + [Theory] + [InlineData(typeof(IA), "Method", typeof(IA), typeof(IA), "Method")] + [InlineData(typeof(IA), "Method", typeof(CA), typeof(IA), "Method")] + [InlineData(typeof(IA), "Method", typeof(CAimpl), typeof(CAimpl), "Method")] + [InlineData(typeof(IA), "Method", typeof(IBoverride), typeof(IBoverride), "Moq.Tests.ProxyFactories.MostSpecificOverrideFixture.IA.Method")] + [InlineData(typeof(IA), "Method", typeof(IBnew), typeof(IA), "Method")] + [InlineData(typeof(IA), "Method", typeof(ICfromBoverride), typeof(ICfromBoverride), "Moq.Tests.ProxyFactories.MostSpecificOverrideFixture.IA.Method")] + [InlineData(typeof(IA), "Method", typeof(ICfromBnew), typeof(ICfromBnew), "Moq.Tests.ProxyFactories.MostSpecificOverrideFixture.IA.Method")] + [InlineData(typeof(IBnew), "Method", typeof(ICfromBnew), typeof(ICfromBnew), "Moq.Tests.ProxyFactories.MostSpecificOverrideFixture.IBnew.Method")] + public void Finds_correct_most_specific_override(Type declarationType, string declarationName, Type proxiedType, Type overrideType, string overrideName) + { + var expected = overrideType.GetMethod(overrideName, BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.DeclaredOnly); + var proxy = typeof(Mock).GetMethod("Of", BindingFlags.Public | BindingFlags.Static, null, Type.EmptyTypes, null).MakeGenericMethod(proxiedType).Invoke(null, null); + var proxyType = proxy.GetType(); + var actual = CastleProxyFactory.FindMostSpecificOverride( + declaration: declarationType.GetMethod(declarationName, BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.DeclaredOnly), + proxyType); + Assert.Same(expected, actual); + } + + public interface IA + { + string Method() => "Method in IA"; + } + + public class CA : IA + { + } + + public class CAimpl : IA + { + public string Method() => "Method in CAimpl"; + } + + public interface IBoverride : IA + { + string IA.Method() => "IA.Method in IBoverride"; + } + + public interface IBnew : IA + { + new string Method() => "Method in IBnew"; + } + + public interface ICfromBoverride : IBoverride + { + string IA.Method() => "IA.Method in ICfromBoverride"; + } + + public interface ICfromBnew : IBnew + { + string IBnew.Method() => "IBnewMethod in ICfromBnew"; + string IA.Method() => "IA.Method in ICfromBnew"; + } + } +} + +#endif