Exception on `MakeGenericMethod` called on a static interface method from `GetInterfaceMap`'s `TargetMethods`

[hamarb123]

Description

This issue is for adding the tests, see Other information

When you use MakeGenericMethod on a static generic method, which comes from GetInterfaceMap's TargetMethods, it throws an InvalidOperationException. It doesn't throw when you get the method directly from the type.

Gotta love static interface methods :)

Originally found by @Joe4evr on the C# discord, just thought I'd report it so it doesn't get lost.

Reproduction Steps

using System;
using System.Linq;
using System.Reflection;
public class C
{
 public static void Main()
 {
 var x1 = typeof(C1).GetMethod("I1.A", (BindingFlags)(-1));
 var x2 = typeof(C1).GetInterfaceMap(typeof(I1)).TargetMethods.Single((x) => x.Name == "I1.A"); //doesn't occur on InterfaceMethods
 Console.WriteLine(x1.MakeGenericMethod(typeof(int)).ContainsGenericParameters);
 Console.WriteLine(x2.MakeGenericMethod(typeof(int)).ContainsGenericParameters); //exception is here (on call to MakeGenericMethod)
 }
}

public interface I1
{
 static abstract void A<T>();
}

public class C1 : I1
{
 static void I1.A<T>() { } //doesn't need to be an explicit implementation
}

Expected behavior

Prints:

False
False

Actual behavior

Prints:

False
Exception
System.InvalidOperationException: Void I1.A[T]() is not a GenericMethodDefinition. MakeGenericMethod may only be called on a method for which MethodBase.IsGenericMethodDefinition is true.
 at System.Reflection.RuntimeMethodInfo.MakeGenericMethod(Type[] methodInstantiation)
 at C.Main()
 at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
 at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)

Regression?

No response

Known Workarounds

No response

Configuration

I tested this in sharplab.io
I have also reproduced locally on macOS 13.4.1 x64 with .NET 7.0.10

Other information

The above described issue was fixed with other changes. This issue is now for adding tests to ensure we do not regress.
I also found another instance of the original issue: #90351 (comment).

Tagging subscribers to this area: @dotnet/area-system-reflection
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

When you use MakeGenericMethod on a static generic method, which comes from GetInterfaceMap's TargetMethods, it throws an InvalidOperationException. It doesn't throw when you get the method directly from the type.

Gotta love static interface methods :)

Originally found by @Joe4evr on the C# discord, just thought I'd report it so it doesn't get lost.

Reproduction Steps

using System;
using System.Linq;
using System.Reflection;
public class C
{
 public static void Main()
 {
 var x1 = typeof(C1).GetMethod("I1.A", (BindingFlags)(-1));
 var x2 = typeof(C1).GetInterfaceMap(typeof(I1)).TargetMethods.Single((x) => x.Name == "I1.A"); //doesn't occur on InterfaceMethods
 Console.WriteLine(x1.MakeGenericMethod(typeof(int)).ContainsGenericParameters);
 Console.WriteLine(x2.MakeGenericMethod(typeof(int)).ContainsGenericParameters); //exception is here (on call to MakeGenericMethod)
 }
}

public interface I1
{
 static abstract void A<T>();
}

public class C1 : I1
{
 static void I1.A<T>() { } //doesn't need to be an explicit implementation
}

Expected behavior

Prints:

False
False

Actual behavior

Prints:

False
Exception
System.InvalidOperationException: Void I1.A[T]() is not a GenericMethodDefinition. MakeGenericMethod may only be called on a method for which MethodBase.IsGenericMethodDefinition is true.
 at System.Reflection.RuntimeMethodInfo.MakeGenericMethod(Type[] methodInstantiation)
 at C.Main()
 at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
 at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)

Regression?

No response

Known Workarounds

No response

Configuration

I tested this in sharplab.io
I have also reproduced locally on macOS 13.4.1 x64 with .NET 7.0.10

Other information

No response

Author:	hamarb123
Assignees:	-
Labels:	area-System.Reflection, untriaged
Milestone:	-

[ericstj]

Thank you for the report. This appears to be an existing bug - I can repro on 7.0 as well. Moving this to 9.0.0 for us to fix

[Joe4evr]

@hamarb123 You were a bit quick slow on the draw here. This seems to already be fixed in the nightly builds (rc1).

[jkotas]

Yes, this appears to be fixed as part of other fixes for static methods. We should add a test for it to make sure that it does not regress.

[hamarb123]

I'd be happy to make a PR to test it. What directory/file would it go in?

[jkotas]

It can be a new case under

runtime/src/libraries/System.Runtime/tests/System/Type/TypeTests.cs

Line 984 in 5549f72

public static IEnumerable<object[]> GetInterfaceMap_TestData()

[hamarb123]

I'm not sure if this is another bug (I suspect probably not, but still it's unexpected at least), but I've discovered some weird behaviour while writing the test:

void Print(InterfaceMapping map)
{
	for (int i = 0; i < map.InterfaceMethods.Length; i++)
	{
		Console.WriteLine($"{map.InterfaceMethods[i]} ({map.InterfaceMethods[i].DeclaringType}) maps to {map.TargetMethods[i]} ({map.TargetMethods[i].DeclaringType})");
	}
	Console.WriteLine();
}

Print(typeof(C2).GetInterfaceMap(typeof(I2)));
Print(typeof(C2).GetInterfaceMap(typeof(I1)));
Print(typeof(C1).GetInterfaceMap(typeof(I1)));

public interface I1
{
	static abstract void A();
	void B();
}

public class C1 : I1
{
	static void I1.A() { }
	void I1.B() { }
}

public interface I2 : I1
{
	static void I1.A() { }
	void I1.B() { }
}

public class C2 : I2
{
	static void I1.A() { }
	void I1.B() { }
}

->

Void I1.B() (I2) maps to Void I1.B() (I2)

Void B() (I1) maps to Void I1.B() (C2)
Void A() (I1) maps to Void I1.A() (C2)

Void B() (I1) maps to Void I1.B() (C1)
Void A() (I1) maps to Void I1.A() (C1)

the behaviour of the first set seems quite odd, is this what is expected? And if so, why?
Specifically, I would have expected any of the 3: 1. no entries, 2. entries for both A and B default implementations in I2, or 3. entries for the actual implementations in C2.

Option 1 or 2 probably makes the most sense, but it seems like static methods do it differently to instance methods for some reason. Option 1 (no entries) probably makes the most sense overall for both of them from a consumer point of view, since these methods are not supposed to be overriden themselves, but instead provide a default implementation for I1, but one could argue that from an IL point of view option 2 makes more sense for the instance method, since it's marked as virtual (but the instance method is also marked as final...).

EDIT:
actually, the behaviour is even weirder than I thought:

void Print(InterfaceMapping map)
{
	for (int i = 0; i < map.InterfaceMethods.Length; i++)
	{
		Console.WriteLine($"{map.InterfaceMethods[i]?.ToString() ?? "null"} ({map.InterfaceMethods[i]?.DeclaringType?.ToString() ?? "null"}) maps to {map.TargetMethods[i]?.ToString() ?? "null"} ({map.TargetMethods[i]?.DeclaringType?.ToString() ?? "null"})");
	}
	Console.WriteLine();
}

Print(typeof(C3).GetInterfaceMap(typeof(I3)));
Print(typeof(C3).GetInterfaceMap(typeof(I2)));
Print(typeof(C3).GetInterfaceMap(typeof(I1)));
Print(typeof(C2).GetInterfaceMap(typeof(I2)));
Print(typeof(C2).GetInterfaceMap(typeof(I1)));
Print(typeof(C1).GetInterfaceMap(typeof(I1)));

public interface I1
{
	static abstract void A();
	void B();
}

public class C1 : I1
{
	static void I1.A() { }
	void I1.B() { }
}

public interface I2 : I1
{
	static void I1.A() { }
	void I1.B() { }
}

public class C2 : I2
{
	static void I1.A() { }
	void I1.B() { }
}

public interface I3 : I2
{
	static abstract void I1.A();
	abstract void I1.B();
}

public class C3 : I3
{
	static void I1.A() { }
	void I1.B() { }
}

->

Void I1.B() (I3) maps to null (null)
Void I1.A() (I3) maps to null (null)

Void I1.B() (I2) maps to Void I1.B() (I2)

Void B() (I1) maps to Void I1.B() (C3)
Void A() (I1) maps to Void I1.A() (C3)

Void I1.B() (I2) maps to Void I1.B() (I2)

Void B() (I1) maps to Void I1.B() (C2)
Void A() (I1) maps to Void I1.A() (C2)

Void B() (I1) maps to Void I1.B() (C1)
Void A() (I1) maps to Void I1.A() (C1)

This seems even less ideal, since the arrays on GetInterfaceMap do not even have their element marked as nullable.
While this probably(?) is the behaviour the runtime should produce, it's certainly not what a user would probably expect here.