Merge pull request #643 from stakx/bug/covariant-returns

Improve support for covariant returns (where generic types are involved)

Author: stakx

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## Unreleased
 
 Bugfixes:
+- Proxies using records derived from a base generic record broken using .NET 6 compiler (@CesarD, #632)
 - Failure proxying type that has a non-inheritable custom attribute type applied where `null` argument is given for array parameter  (@stakx, #637)
 - Nested custom attribute types do not get replicated (@stakx, #638)
 

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

@@ -52,6 +52,7 @@ public void Reflection_returns_methods_from_a_derived_class_before_methods_from_
 		[TestCase(typeof(DerivedClassWithStringInsteadOfObject))]
 		[TestCase(typeof(DerivedClassWithStringInsteadOfInterface))]
 		[TestCase(typeof(DerivedClassWithStringInsteadOfGenericArg))]
+		[TestCase(typeof(BottleOfWater))]
 		public void Can_proxy_type_having_method_with_covariant_return(Type classToProxy)
 		{
 			_ = generator.CreateClassProxy(classToProxy, new StandardInterceptor());
@@ -62,6 +63,7 @@ public void Can_proxy_type_having_method_with_covariant_return(Type classToProxy
 		[TestCase(typeof(DerivedClassWithStringInsteadOfObject), typeof(string))]
 		[TestCase(typeof(DerivedClassWithStringInsteadOfInterface), typeof(string))]
 		[TestCase(typeof(DerivedClassWithStringInsteadOfGenericArg), typeof(string))]
+		[TestCase(typeof(BottleOfWater), typeof(Glass<Water>))]
 		public void Proxied_method_has_correct_return_type(Type classToProxy, Type expectedMethodReturnType)
 		{
 			var proxy = generator.CreateClassProxy(classToProxy, new StandardInterceptor());
@@ -116,6 +118,22 @@ public class DerivedClassWithStringInsteadOfGenericArg : BaseClassWithGenericArg
 		{
 			public override string Method() => nameof(DerivedClassWithStringInsteadOfGenericArg);
 		}
+
+		public interface Glass<out T> { }
+
+		public class Liquid { }
+
+		public class Water : Liquid { }
+
+		public class BottleOfLiquid
+		{
+			public virtual Glass<Liquid> Method() => default(Glass<Liquid>);
+		}
+
+		public class BottleOfWater : BottleOfLiquid
+		{
+			public override Glass<Water> Method() => default(Glass<Water>);
+		}
 	}
 }
 

src/Castle.Core.Tests/DynamicProxy.Tests/Records/DerivedFromEmptyGenericRecord.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 DerivedFromEmptyGenericRecord : EmptyGenericRecord<object>
+	{
+	}
+}

src/Castle.Core.Tests/DynamicProxy.Tests/Records/DerivedFromEmptyRecord.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 DerivedFromEmptyRecord : EmptyRecord
+	{
+	}
+}

…roxy.Tests/Records/DerivedEmptyRecord.cs …roxy.Tests/Records/EmptyGenericRecord.cssrc/Castle.Core.Tests/DynamicProxy.Tests/Records/DerivedEmptyRecord.cs renamed to src/Castle.Core.Tests/DynamicProxy.Tests/Records/EmptyGenericRecord.cs src/Castle.Core.Tests/DynamicProxy.Tests/Records/DerivedEmptyRecord.cs renamed to src/Castle.Core.Tests/DynamicProxy.Tests/Records/EmptyGenericRecord.cs

@@ -14,7 +14,7 @@
 
 namespace Castle.DynamicProxy.Tests.Records
 {
-	public record DerivedEmptyRecord : EmptyRecord
+	public record EmptyGenericRecord<T>
 	{
 	}
 }

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

@@ -28,9 +28,21 @@ public void Can_proxy_empty_record()
 		}
 
 		[Test]
-		public void Can_proxy_derived_empty_record()
+		public void Can_proxy_record_derived_from_empty_record()
 		{
-			_ = generator.CreateClassProxy<DerivedEmptyRecord>(new StandardInterceptor());
+			_ = generator.CreateClassProxy<DerivedFromEmptyRecord>(new StandardInterceptor());
+		}
+
+		[Test]
+		public void Can_proxy_empty_generic_record()
+		{
+			_ = generator.CreateClassProxy<EmptyGenericRecord<object>>(new StandardInterceptor());
+		}
+
+		[Test]
+		public void Can_proxy_record_derived_from_empty_generic_record()
+		{
+			_ = generator.CreateClassProxy<DerivedFromEmptyGenericRecord>(new StandardInterceptor());
 		}
 	}
 }

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

@@ -81,10 +81,27 @@ public bool EqualParameters(MethodInfo x, MethodInfo y)
 
 		public bool EqualReturnTypes(MethodInfo x, MethodInfo y)
 		{
-			return EqualSignatureTypes(x.ReturnType, y.ReturnType, x, y);
+			var xr = x.ReturnType;
+			var yr = y.ReturnType;
+
+			if (EqualSignatureTypes(xr, yr))
+			{
+				return true;
+			}
+
+			// 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 (x.IsDefined(preserveBaseOverridesAttribute, inherit: false) && yr.IsAssignableFrom(xr))
+				    || (y.IsDefined(preserveBaseOverridesAttribute, inherit: false) && xr.IsAssignableFrom(yr));
+			}
+
+			return false;
 		}
 
-		private bool EqualSignatureTypes(Type x, Type y, MethodInfo xm = null, MethodInfo ym = null)
+		private bool EqualSignatureTypes(Type x, Type y)
 		{
 			if (x.IsGenericParameter != y.IsGenericParameter)
 			{
@@ -122,22 +139,13 @@ private bool EqualSignatureTypes(Type x, Type y, MethodInfo xm = null, MethodInf
 
 				for (var i = 0; i < xArgs.Length; ++i)
 				{
-					 if(!EqualSignatureTypes(xArgs[i], yArgs[i])) return false;
+					if(!EqualSignatureTypes(xArgs[i], yArgs[i])) return false;
 				}
 			}
 			else
 			{
 				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;
 				}
 			}