Importer create ClassSig rather than GenericInstSig for generic type

[CreateAndInject]

I don't know if it's a .NET runtime bug or dnlib bug.
.NET runtime indicate different IsGenericTypeDefinition for the same type Task<>
dnlib create ClassSig since IsGenericType and IsGenericTypeDefinition are both True;

			if (a.IsGenericType && !a.IsGenericTypeDefinition)
				return ElementType.GenericInst;

using System;
using System.Reflection;
using System.Threading.Tasks;
using dnlib.DotNet;

class Program {
	static void Main() {
		var methodInfo1 = typeof(Task<string>).GetMethod("ContinueWith", new Type[] { typeof(Action<Task<string>>) });
		Show(methodInfo1); // True

		var methodInfo2 = typeof(Demo2<string>).GetMethod("Test");
		Show(methodInfo2); // False

		var moule = ModuleDefMD.Load(typeof(Program).Module);
		var method1 = moule.Import(methodInfo1); // System.Threading.Tasks.Task System.Threading.Tasks.Task`1<System.String>::ContinueWith(System.Action`1<System.Threading.Tasks.Task`1>)
		Show(method1); // Class
		var method2 = moule.Import(methodInfo2); // System.Void Demo2`1<System.String>::Test(System.Action`1<System.Threading.Tasks.Task`1<System.String>>)
		Show(method2); // GenericInst
		Console.ReadKey();
	}

	static void Show(MethodInfo method) {
		var origMethod = method.Module.ResolveMethod(method.MetadataToken);
		var type = origMethod.GetParameters()[0].ParameterType.GenericTypeArguments[0];
		Console.WriteLine(type.IsGenericTypeDefinition);
	}

	static void Show(IMethod method) {
		var typeSig = (method.MethodSig.Params[0] as GenericInstSig).GenericArguments[0];
		Console.WriteLine(typeSig.ElementType);
	}
}

class Demo2<T> {
	public void Test(Action<Task<T>> arg) { }
}

[wtfsck]

If you have time, find the line in dnlib code, get the git blame and check why the line was changed. https://github.com/0xd4d/dnlib/blame/master/src/DotNet/Importer.cs

[CreateAndInject]

This code is from:

dnlib/src/DotNet/ReflectionExtensions.cs

Lines 57 to 58 in aa403ba

	if (a.IsGenericType && !a.IsGenericTypeDefinition)
	return ElementType.GenericInst;

Seems the line wasn't changed.
When I modify it to:

	if (a.IsGenericType)
		return ElementType.GenericInst;

I'll get StackOverflowException somewhere (not above code)

[wtfsck]

A generic instance is not a generic type def so both checks are needed. This does not appear to be a dnlib issue.

[CreateAndInject]

But the type inside Import(Task.ContinueWith) should be GenericInstSig.
I found this issue from DynamicMethodBodyReader, when I restore IL, and view it in dnSpy, dnSpy can't navigate a method, that means dnSpy can't resolve the method.
When I investigate it, I find the reason can create this tiny reprouce.
Maybe should set treatAsGenericInst to true in more cases, I don't know.

[wtfsck]

It's a problem that reflection is lying about it. If you can detect it some other way, let me know. There's another method in the same file that's used by some other code that can be used if you pass in some boolean flag.

[CreateAndInject]

I'm sorry, I made a mistake:

class Demo2<T> {
	public void Test(Action<Task<T>> arg) { } // Task<T> => Demo2<T>
}

So, reflection keep pace with Task.ContinueWith

So, reflection isn't lying?
My reproduce is completely wrong, but DynamicMethodBodyReader really build wrong IL like I said

[CreateAndInject]

@wtfsck Can you view this issue
Seems .NET doesn't lie on my example, but I don't know if .NET lie on that DynamicMethod

[ElektroKill]

Hello!
I managed to find another test case for this problem, this time for fields!

namespace TestFile {
	public sealed class Program {
		public static void Main() {
			var resolver = new AssemblyResolver();
			resolver.DefaultModuleContext = new ModuleContext(resolver);
			var mod = ModuleDefMD.Load(typeof(Program).Assembly.Location, resolver.DefaultModuleContext);

			var type = mod.Find("TestFile.Program", true);
			var method = type.FindMethod("TestMethod");

			var operand = (IField)method.Body.Instructions.FirstOrDefault(x => x.OpCode.Code == Code.Ldsfld).Operand;
			Console.WriteLine("Original: {0}", operand);

			var reflectionMember = (FieldInfo)typeof(Program).Assembly.ManifestModule.ResolveMember(operand.MDToken.ToInt32());

			var importer = new Importer(mod, ImporterOptions.TryToUseDefs, GenericParamContext.Create(method));

			Console.WriteLine("Imported: {0}", importer.Import(reflectionMember));
			Console.ReadKey(true);
		}

		private static void TestMethod() {
			Console.WriteLine(GenericClass<NonGenericClass.NestedGenericStruct<Type>>.Field);
		}
	}

	internal sealed class GenericClass<V> {
		private GenericClass() { }

		public static readonly GenericClass<V> Field = new GenericClass<V>();
	}

	internal static class NonGenericClass {
		public struct NestedGenericStruct<K> { }
	}
}

Compiling and running this code with dnlib 3.5.0 yields the following result:

Original: TestFile.GenericClass`1<TestFile.NonGenericClass/NestedGenericStruct`1<System.Type>> TestFile.GenericClass`1<TestFile.NonGenericClass/NestedGenericStruct`1<System.Type>>::Field
Imported: TestFile.GenericClass`1 TestFile.GenericClass`1<TestFile.NonGenericClass/NestedGenericStruct`1<System.Type>>::Field

It is worth mentioning that doing the same using dnlib 3.4.0 yield the expected result:

Original: TestFile.GenericClass`1<TestFile.NonGenericClass/NestedGenericStruct`1<System.Type>> TestFile.GenericClass`1<TestFile.NonGenericClass/NestedGenericStruct`1<System.Type>>::Field
Imported: TestFile.GenericClass`1<TestFile.NonGenericClass/NestedGenericStruct`1<System.Type>> TestFile.GenericClass`1<TestFile.NonGenericClass/NestedGenericStruct`1<System.Type>>::Field

I think something got broken when this PR #452 by @wwh1004 was merged which modified how fields are imported.

[CreateAndInject]

Seems NonGenericClass and NestedGenericStruct are not required, can simply reproduce like:

private static void TestMethod()
{
	Console.WriteLine(GenericClass<string>.Field);
}

[wtfsck]

PRs welcome. I don't have time.

[wwh1004]

In old commit 4ebebc1#diff-3bc1d65c9144274297f0e9a1b2e194898aa3aa9e441d2956ab303ab4522dc554L488 code is

var fieldSig = new FieldSig(ImportAsTypeSig(origField.FieldType));

So I use "origField" instead of "fieldInfo". But after running more tests I find "fieldInfo" is true.
Also I find more problems about generic like nested case of "MustTreatTypeAsGenericInstType", wrong result of "FieldComparer.Equals", inconsistent result of GetHashCode(SRM.FieldInfo) and GetHashCode(DNN.IField), ContainsGenericParameter for generic MemberRef always return true.
I will fix them these days.