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

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

Merged
merged 9 commits into from
Dec 29, 2022
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## Unreleased

Bugfixes:
- Proxies using records derived from a base generic record broken using .NET 6 compiler (@CesarD, #632)
stakx marked this conversation as resolved.
Show resolved Hide resolved
- 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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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());
Expand Down Expand Up @@ -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>);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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>
{
}
}
Original file line number Diff line number Diff line change
@@ -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
{
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

namespace Castle.DynamicProxy.Tests.Records
{
public record DerivedEmptyRecord : EmptyRecord
public record EmptyGenericRecord<T>
{
}
}
16 changes: 14 additions & 2 deletions src/Castle.Core.Tests/DynamicProxy.Tests/RecordsTestCase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
}
32 changes: 20 additions & 12 deletions src/Castle.Core/DynamicProxy/Generators/MethodSignatureComparer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;
}
}
Expand Down