Merge pull request #619 from stakx/bugfix/covariant-returns

Add support for covariant returns

CHANGELOG.md

@@ -2,8 +2,12 @@
 
 ## Unreleased
 
+Enhancements:
+- Support for covariant method returns (@stakx, #619)
+
 Bugfixes:
 - DynamicProxy emits invalid metadata for redeclared event (@stakx, #590)
+- Proxies using records with a base class broken using .NET 6 compiler (@ajcvickers, #601)
 
 ## 5.0.0 (2022-05-11)
 

src/Castle.Core.Tests/DynamicProxy.Tests/CovariantReturnsTestCase.cs

@@ -0,0 +1,122 @@
+// Copyright 2004-2022 Castle Project - http://www.castleproject.org/
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#if NET5_0_OR_GREATER
+
+using System;
+using System.Reflection;
+
+using NUnit.Framework;
+
+namespace Castle.DynamicProxy.Tests
+{
+	[TestFixture]
+	public class CovariantReturnsTestCase : BasePEVerifyTestCase
+	{
+		// DynamicProxy's current implementation for covariant returns support expects to see override methods
+		// before the overridden methods. That is, we rely on a specific behavior of .NET Reflection, and this test
+		// codifies that assumption. If it ever breaks, we'll need to adjust our implementation accordingly.
+		[Test]
+		public void Reflection_returns_methods_from_a_derived_class_before_methods_from_its_base_class()
+		{
+			var derivedType = typeof(DerivedClassWithStringInsteadOfObject);
+			var baseType = typeof(BaseClassWithObject);
+			Assume.That(derivedType.BaseType == baseType);
+
+			var derivedMethod = derivedType.GetMethod("Method");
+			var baseMethod = baseType.GetMethod("Method");
+			Assume.That(derivedMethod != baseMethod);
+
+			var methods = derivedType.GetMethods(BindingFlags.Public | BindingFlags.Instance);
+			var derivedMethodIndex = Array.FindIndex(methods, m => m.Name == "Method" && m.DeclaringType == derivedType);
+			var baseMethodIndex = Array.FindIndex(methods, m => m.Name == "Method" && m.DeclaringType == baseType);
+			Assume.That(derivedMethodIndex >= 0);
+			Assume.That(baseMethodIndex >= 0);
+
+			Assert.IsTrue(derivedMethodIndex < baseMethodIndex);
+		}
+
+		[Theory]
+		[TestCase(typeof(DerivedClassWithInterfaceInsteadOfObject))]
+		[TestCase(typeof(DerivedClassWithStringInsteadOfObject))]
+		[TestCase(typeof(DerivedClassWithStringInsteadOfInterface))]
+		[TestCase(typeof(DerivedClassWithStringInsteadOfGenericArg))]
+		public void Can_proxy_type_having_method_with_covariant_return(Type classToProxy)
+		{
+			_ = generator.CreateClassProxy(classToProxy, new StandardInterceptor());
+		}
+
+		[Theory]
+		[TestCase(typeof(DerivedClassWithInterfaceInsteadOfObject), typeof(IComparable))]
+		[TestCase(typeof(DerivedClassWithStringInsteadOfObject), typeof(string))]
+		[TestCase(typeof(DerivedClassWithStringInsteadOfInterface), typeof(string))]
+		[TestCase(typeof(DerivedClassWithStringInsteadOfGenericArg), typeof(string))]
+		public void Proxied_method_has_correct_return_type(Type classToProxy, Type expectedMethodReturnType)
+		{
+			var proxy = generator.CreateClassProxy(classToProxy, new StandardInterceptor());
+			var method = proxy.GetType().GetMethod("Method");
+			Assert.AreEqual(expectedMethodReturnType, method.ReturnType);
+		}
+
+		[Theory]
+		[TestCase(typeof(DerivedClassWithInterfaceInsteadOfObject))]
+		[TestCase(typeof(DerivedClassWithStringInsteadOfObject))]
+		[TestCase(typeof(DerivedClassWithStringInsteadOfInterface))]
+		[TestCase(typeof(DerivedClassWithStringInsteadOfGenericArg))]
+		public void Can_invoke_method_with_covariant_return(Type classToProxy)
+		{
+			var proxy = generator.CreateClassProxy(classToProxy, new StandardInterceptor());
+			var method = proxy.GetType().GetMethod("Method");
+			var returnValue = method.Invoke(proxy, null);
+			Assert.AreEqual(expected: classToProxy.Name, returnValue);
+		}
+
+		public class BaseClassWithObject
+		{
+			public virtual object Method() => nameof(BaseClassWithObject);
+		}
+
+		public class DerivedClassWithInterfaceInsteadOfObject : BaseClassWithObject
+		{
+			public override IComparable Method() => nameof(DerivedClassWithInterfaceInsteadOfObject);
+		}
+
+		public class DerivedClassWithStringInsteadOfObject : BaseClassWithObject
+		{
+			public override string Method() => nameof(DerivedClassWithStringInsteadOfObject);
+		}
+
+		public class BaseClassWithInterface
+		{
+			public virtual IComparable Method() => nameof(BaseClassWithInterface);
+		}
+
+		public class DerivedClassWithStringInsteadOfInterface : BaseClassWithInterface
+		{
+			public override string Method() => nameof(DerivedClassWithStringInsteadOfInterface);
+		}
+
+		public class BaseClassWithGenericArg<T> where T : IComparable
+		{
+			public virtual T Method() => default(T);
+		}
+
+		public class DerivedClassWithStringInsteadOfGenericArg : BaseClassWithGenericArg<IComparable>
+		{
+			public override string Method() => nameof(DerivedClassWithStringInsteadOfGenericArg);
+		}
+	}
+}
+
+#endif

src/Castle.Core.Tests/DynamicProxy.Tests/Records/DerivedEmptyRecord.cs

@@ -0,0 +1,20 @@
+// Copyright 2004-2022 Castle Project - http://www.castleproject.org/
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+namespace Castle.DynamicProxy.Tests.Records
+{
+	public record DerivedEmptyRecord : EmptyRecord
+	{
+	}
+}

src/Castle.Core.Tests/DynamicProxy.Tests/Records/EmptyRecord.cs

@@ -0,0 +1,20 @@
+// Copyright 2004-2022 Castle Project - http://www.castleproject.org/
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+namespace Castle.DynamicProxy.Tests.Records
+{
+	public record EmptyRecord
+	{
+	}
+}

src/Castle.Core.Tests/DynamicProxy.Tests/RecordsTestCase.cs

@@ -0,0 +1,36 @@
+// Copyright 2004-2022 Castle Project - http://www.castleproject.org/
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+using Castle.DynamicProxy.Tests.Records;
+
+using NUnit.Framework;
+
+namespace Castle.DynamicProxy.Tests
+{
+	[TestFixture]
+	public class RecordsTestCase : BasePEVerifyTestCase
+	{
+		[Test]
+		public void Can_proxy_empty_record()
+		{
+			_ = generator.CreateClassProxy<EmptyRecord>(new StandardInterceptor());
+		}
+
+		[Test]
+		public void Can_proxy_derived_empty_record()
+		{
+			_ = generator.CreateClassProxy<DerivedEmptyRecord>(new StandardInterceptor());
+		}
+	}
+}

src/Castle.Core/DynamicProxy/Generators/MetaMethod.cs

@@ -67,7 +67,7 @@ public bool Equals(MetaMethod other)
 			}
 
 			var comparer = MethodSignatureComparer.Instance;
-			if (!comparer.EqualSignatureTypes(Method.ReturnType, other.Method.ReturnType))
+			if (!comparer.EqualReturnTypes(Method, other.Method))
 			{
 				return false;
 			}

src/Castle.Core/DynamicProxy/Generators/MethodSignatureComparer.cs

@@ -22,6 +22,8 @@ internal class MethodSignatureComparer : IEqualityComparer<MethodInfo>
 	{
 		public static readonly MethodSignatureComparer Instance = new MethodSignatureComparer();
 
+		private static readonly Type preserveBaseOverridesAttribute = Type.GetType("System.Runtime.CompilerServices.PreserveBaseOverridesAttribute", throwOnError: false);
+
 		public bool EqualGenericParameters(MethodInfo x, MethodInfo y)
 		{
 			if (x.IsGenericMethod != y.IsGenericMethod)
@@ -77,7 +79,12 @@ public bool EqualParameters(MethodInfo x, MethodInfo y)
 			return true;
 		}
 
-		public bool EqualSignatureTypes(Type x, Type y)
+		public bool EqualReturnTypes(MethodInfo x, MethodInfo y)
+		{
+			return EqualSignatureTypes(x.ReturnType, y.ReturnType, x, y);
+		}
+
+		private bool EqualSignatureTypes(Type x, Type y, MethodInfo xm = null, MethodInfo ym = null)
 		{
 			if (x.IsGenericParameter != y.IsGenericParameter)
 			{
@@ -122,6 +129,15 @@ public bool EqualSignatureTypes(Type x, Type y)
 			{
 				if (!x.Equals(y))
 				{
+					// This enables covariant method returns for .NET 5 and newer.
+					// No need to check for runtime support, since such methods are marked with a custom attribute;
+					// see https://github.com/dotnet/runtime/blob/main/docs/design/features/covariant-return-methods.md.
+					if (preserveBaseOverridesAttribute != null)
+					{
+						return (xm != null && xm.IsDefined(preserveBaseOverridesAttribute, inherit: false) && y.IsAssignableFrom(x))
+						    || (ym != null && ym.IsDefined(preserveBaseOverridesAttribute, inherit: false) && x.IsAssignableFrom(y));
+					}
+
 					return false;
 				}
 			}
@@ -142,7 +158,7 @@ public bool Equals(MethodInfo x, MethodInfo y)
 
 			return EqualNames(x, y) &&
 				   EqualGenericParameters(x, y) &&
-				   EqualSignatureTypes(x.ReturnType, y.ReturnType) &&
+				   EqualReturnTypes(x, y) &&
 				   EqualParameters(x, y);
 		}