Skip to content

Commit 70320e7

Browse files
committed
Update documentation why MethodFinder is needed
The only remaining test in `MethodFinderTestCase` suggests that `GetAll- InstanceMethods` and the underlying `GetMethods` are interchangeable. Add a reference to an existing test from another fixture, as well as a new test, to demonstrate why this is not always the case. Update the XML doc comment accordingly.
1 parent b24f248 commit 70320e7

File tree

2 files changed

+35
-2
lines changed

2 files changed

+35
-2
lines changed

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
namespace Castle.DynamicProxy.Tests
1616
{
17+
using System.Linq;
1718
using System.Reflection;
1819

1920
using Castle.DynamicProxy.Generators;
@@ -32,5 +33,37 @@ public void GetMethodsForPublicAndNonPublic()
3233
typeof(object).GetMethods(BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public);
3334
CollectionAssert.AreEquivalent(realMethods, methods);
3435
}
36+
37+
// The test above suggests that `MethodFinder.GetAllInstanceMethods` and `Type.GetMethods`
38+
// can be used interchangeably, but this is not always the case. See the test(s) below and
39+
// `GenericInterfaceProxyTestCase.MethodFinderIsStable` for cases where the two methods
40+
// may produce different results.
41+
42+
#if NET5_0_OR_GREATER
43+
44+
[Test]
45+
public void NoDuplicatesForMethodWithCovariantReturnType()
46+
{
47+
MethodInfo[] methods =
48+
MethodFinder.GetAllInstanceMethods(typeof(Derived));
49+
MethodInfo[] realMethods =
50+
typeof(Derived).GetMethods(BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public);
51+
CollectionAssert.AreNotEquivalent(realMethods, methods);
52+
CollectionAssert.IsSubsetOf(subset: methods, superset: realMethods);
53+
Assert.AreEqual(2, realMethods.Where(m => m.Name == nameof(Derived.Method)).Count());
54+
Assert.AreEqual(1, methods.Where(m => m.Name == nameof(Derived.Method)).Count());
55+
}
56+
57+
public abstract class Base
58+
{
59+
public abstract Base Method();
60+
}
61+
62+
public abstract class Derived : Base
63+
{
64+
public override abstract Derived Method();
65+
}
66+
67+
#endif
3568
}
3669
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ namespace Castle.DynamicProxy.Generators
2020
using System.Reflection;
2121

2222
/// <summary>
23-
/// Returns the methods implemented by a type. Use this instead of Type.GetMethods() to work around a CLR issue
24-
/// where duplicate MethodInfos are returned by Type.GetMethods() after a token of a generic type's method was loaded.
23+
/// Returns the methods implemented by a type. Use this instead of Type.GetMethods to filter out duplicate MethodInfos
24+
/// sometimes reported by the latter. The test suite documents cases where such duplicates may occur.
2525
/// </summary>
2626
internal class MethodFinder
2727
{

0 commit comments

Comments
 (0)