Skip to content

Commit d0c6e38

Browse files
jonorossijonorossi
authored
Merge pull request castleproject#619 from stakx/bugfix/covariant-returns
Add support for covariant returns
2 parents 0176c31 + 01c2c6a commit d0c6e38

File tree

7 files changed

+221
-3
lines changed

7 files changed

+221
-3
lines changed

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,12 @@
22

33
## Unreleased
44

5+
Enhancements:
6+
- Support for covariant method returns (@stakx, #619)
7+
58
Bugfixes:
69
- DynamicProxy emits invalid metadata for redeclared event (@stakx, #590)
10+
- Proxies using records with a base class broken using .NET 6 compiler (@ajcvickers, #601)
711

812
## 5.0.0 (2022-05-11)
913

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

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
// Copyright 2004-2022 Castle Project - http://www.castleproject.org/
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
#if NET5_0_OR_GREATER
16+
17+
using System;
18+
using System.Reflection;
19+
20+
using NUnit.Framework;
21+
22+
namespace Castle.DynamicProxy.Tests
23+
{
24+
[TestFixture]
25+
public class CovariantReturnsTestCase : BasePEVerifyTestCase
26+
{
27+
// DynamicProxy's current implementation for covariant returns support expects to see override methods
28+
// before the overridden methods. That is, we rely on a specific behavior of .NET Reflection, and this test
29+
// codifies that assumption. If it ever breaks, we'll need to adjust our implementation accordingly.
30+
[Test]
31+
public void Reflection_returns_methods_from_a_derived_class_before_methods_from_its_base_class()
32+
{
33+
var derivedType = typeof(DerivedClassWithStringInsteadOfObject);
34+
var baseType = typeof(BaseClassWithObject);
35+
Assume.That(derivedType.BaseType == baseType);
36+
37+
var derivedMethod = derivedType.GetMethod("Method");
38+
var baseMethod = baseType.GetMethod("Method");
39+
Assume.That(derivedMethod != baseMethod);
40+
41+
var methods = derivedType.GetMethods(BindingFlags.Public | BindingFlags.Instance);
42+
var derivedMethodIndex = Array.FindIndex(methods, m => m.Name == "Method" && m.DeclaringType == derivedType);
43+
var baseMethodIndex = Array.FindIndex(methods, m => m.Name == "Method" && m.DeclaringType == baseType);
44+
Assume.That(derivedMethodIndex >= 0);
45+
Assume.That(baseMethodIndex >= 0);
46+
47+
Assert.IsTrue(derivedMethodIndex < baseMethodIndex);
48+
}
49+
50+
[Theory]
51+
[TestCase(typeof(DerivedClassWithInterfaceInsteadOfObject))]
52+
[TestCase(typeof(DerivedClassWithStringInsteadOfObject))]
53+
[TestCase(typeof(DerivedClassWithStringInsteadOfInterface))]
54+
[TestCase(typeof(DerivedClassWithStringInsteadOfGenericArg))]
55+
public void Can_proxy_type_having_method_with_covariant_return(Type classToProxy)
56+
{
57+
_ = generator.CreateClassProxy(classToProxy, new StandardInterceptor());
58+
}
59+
60+
[Theory]
61+
[TestCase(typeof(DerivedClassWithInterfaceInsteadOfObject), typeof(IComparable))]
62+
[TestCase(typeof(DerivedClassWithStringInsteadOfObject), typeof(string))]
63+
[TestCase(typeof(DerivedClassWithStringInsteadOfInterface), typeof(string))]
64+
[TestCase(typeof(DerivedClassWithStringInsteadOfGenericArg), typeof(string))]
65+
public void Proxied_method_has_correct_return_type(Type classToProxy, Type expectedMethodReturnType)
66+
{
67+
var proxy = generator.CreateClassProxy(classToProxy, new StandardInterceptor());
68+
var method = proxy.GetType().GetMethod("Method");
69+
Assert.AreEqual(expectedMethodReturnType, method.ReturnType);
70+
}
71+
72+
[Theory]
73+
[TestCase(typeof(DerivedClassWithInterfaceInsteadOfObject))]
74+
[TestCase(typeof(DerivedClassWithStringInsteadOfObject))]
75+
[TestCase(typeof(DerivedClassWithStringInsteadOfInterface))]
76+
[TestCase(typeof(DerivedClassWithStringInsteadOfGenericArg))]
77+
public void Can_invoke_method_with_covariant_return(Type classToProxy)
78+
{
79+
var proxy = generator.CreateClassProxy(classToProxy, new StandardInterceptor());
80+
var method = proxy.GetType().GetMethod("Method");
81+
var returnValue = method.Invoke(proxy, null);
82+
Assert.AreEqual(expected: classToProxy.Name, returnValue);
83+
}
84+
85+
public class BaseClassWithObject
86+
{
87+
public virtual object Method() => nameof(BaseClassWithObject);
88+
}
89+
90+
public class DerivedClassWithInterfaceInsteadOfObject : BaseClassWithObject
91+
{
92+
public override IComparable Method() => nameof(DerivedClassWithInterfaceInsteadOfObject);
93+
}
94+
95+
public class DerivedClassWithStringInsteadOfObject : BaseClassWithObject
96+
{
97+
public override string Method() => nameof(DerivedClassWithStringInsteadOfObject);
98+
}
99+
100+
public class BaseClassWithInterface
101+
{
102+
public virtual IComparable Method() => nameof(BaseClassWithInterface);
103+
}
104+
105+
public class DerivedClassWithStringInsteadOfInterface : BaseClassWithInterface
106+
{
107+
public override string Method() => nameof(DerivedClassWithStringInsteadOfInterface);
108+
}
109+
110+
public class BaseClassWithGenericArg<T> where T : IComparable
111+
{
112+
public virtual T Method() => default(T);
113+
}
114+
115+
public class DerivedClassWithStringInsteadOfGenericArg : BaseClassWithGenericArg<IComparable>
116+
{
117+
public override string Method() => nameof(DerivedClassWithStringInsteadOfGenericArg);
118+
}
119+
}
120+
}
121+
122+
#endif

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Copyright 2004-2022 Castle Project - http://www.castleproject.org/
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
namespace Castle.DynamicProxy.Tests.Records
16+
{
17+
public record DerivedEmptyRecord : EmptyRecord
18+
{
19+
}
20+
}

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Copyright 2004-2022 Castle Project - http://www.castleproject.org/
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
namespace Castle.DynamicProxy.Tests.Records
16+
{
17+
public record EmptyRecord
18+
{
19+
}
20+
}

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// Copyright 2004-2022 Castle Project - http://www.castleproject.org/
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
using Castle.DynamicProxy.Tests.Records;
16+
17+
using NUnit.Framework;
18+
19+
namespace Castle.DynamicProxy.Tests
20+
{
21+
[TestFixture]
22+
public class RecordsTestCase : BasePEVerifyTestCase
23+
{
24+
[Test]
25+
public void Can_proxy_empty_record()
26+
{
27+
_ = generator.CreateClassProxy<EmptyRecord>(new StandardInterceptor());
28+
}
29+
30+
[Test]
31+
public void Can_proxy_derived_empty_record()
32+
{
33+
_ = generator.CreateClassProxy<DerivedEmptyRecord>(new StandardInterceptor());
34+
}
35+
}
36+
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public bool Equals(MetaMethod other)
6767
}
6868

6969
var comparer = MethodSignatureComparer.Instance;
70-
if (!comparer.EqualSignatureTypes(Method.ReturnType, other.Method.ReturnType))
70+
if (!comparer.EqualReturnTypes(Method, other.Method))
7171
{
7272
return false;
7373
}

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

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ internal class MethodSignatureComparer : IEqualityComparer<MethodInfo>
2222
{
2323
public static readonly MethodSignatureComparer Instance = new MethodSignatureComparer();
2424

25+
private static readonly Type preserveBaseOverridesAttribute = Type.GetType("System.Runtime.CompilerServices.PreserveBaseOverridesAttribute", throwOnError: false);
26+
2527
public bool EqualGenericParameters(MethodInfo x, MethodInfo y)
2628
{
2729
if (x.IsGenericMethod != y.IsGenericMethod)
@@ -77,7 +79,12 @@ public bool EqualParameters(MethodInfo x, MethodInfo y)
7779
return true;
7880
}
7981

80-
public bool EqualSignatureTypes(Type x, Type y)
82+
public bool EqualReturnTypes(MethodInfo x, MethodInfo y)
83+
{
84+
return EqualSignatureTypes(x.ReturnType, y.ReturnType, x, y);
85+
}
86+
87+
private bool EqualSignatureTypes(Type x, Type y, MethodInfo xm = null, MethodInfo ym = null)
8188
{
8289
if (x.IsGenericParameter != y.IsGenericParameter)
8390
{
@@ -122,6 +129,15 @@ public bool EqualSignatureTypes(Type x, Type y)
122129
{
123130
if (!x.Equals(y))
124131
{
132+
// This enables covariant method returns for .NET 5 and newer.
133+
// No need to check for runtime support, since such methods are marked with a custom attribute;
134+
// see https://github.com/dotnet/runtime/blob/main/docs/design/features/covariant-return-methods.md.
135+
if (preserveBaseOverridesAttribute != null)
136+
{
137+
return (xm != null && xm.IsDefined(preserveBaseOverridesAttribute, inherit: false) && y.IsAssignableFrom(x))
138+
|| (ym != null && ym.IsDefined(preserveBaseOverridesAttribute, inherit: false) && x.IsAssignableFrom(y));
139+
}
140+
125141
return false;
126142
}
127143
}
@@ -142,7 +158,7 @@ public bool Equals(MethodInfo x, MethodInfo y)
142158

143159
return EqualNames(x, y) &&
144160
EqualGenericParameters(x, y) &&
145-
EqualSignatureTypes(x.ReturnType, y.ReturnType) &&
161+
EqualReturnTypes(x, y) &&
146162
EqualParameters(x, y);
147163
}
148164

0 commit comments

Comments
 (0)