[Java.Interop] .GetTypeSignature() supports unsigned types (#1312)

Context: dotnet/android#9747
Context: dotnet/android#9812
Context: 71afce5
Context: dotnet/android@aa5e597
Context: dotnet/android@f800c1a

In the ongoing epic to get MAUI running atop NativeAOT, we hit our
longstanding NativeAOT nemesis: a P/Invoke:

	E AndroidRuntime: net.dot.jni.internal.JavaProxyThrowable: System.InvalidProgramException: InvalidProgram_Specific, IntPtr Android.Runtime.JNIEnv.monodroid_typemap_managed_to_java(System.Type, Byte*)
	E AndroidRuntime:    at Internal.Runtime.TypeLoaderExceptionHelper.CreateInvalidProgramException(ExceptionStringID, String) + 0x4c
	E AndroidRuntime:    at Android.Runtime.JNIEnv.monodroid_typemap_managed_to_java(Type, Byte*) + 0x18
	E AndroidRuntime:    at Android.Runtime.JNIEnv.TypemapManagedToJava(Type) + 0x104
	E AndroidRuntime:    at Android.Runtime.JNIEnv.GetJniName(Type) + 0x1c
	E AndroidRuntime:    at Android.Runtime.JNIEnv.FindClass(Type) + 0x38
	E AndroidRuntime:    at Android.Runtime.JNIEnv.NewArray(IJavaObject[]) + 0x28
	E AndroidRuntime:    at Android.Runtime.JNIEnv.NewArray[T](T[]) + 0x94
	E AndroidRuntime:    at Android.Graphics.Drawables.LayerDrawable..ctor(Drawable[] layers) + 0xd4
	E AndroidRuntime:    at Microsoft.Maui.Platform.MauiRippleDrawableExtensions.UpdateMauiRippleDrawableBackground(View, Paint, IButtonStroke, Func`1, Func`1, Action) + 0x2ac

(`JNIEnv.monodroid_typemap_managed_to_java()` is P/Invoke.  Why are
P/Invokes bad?  See dotnet/android@f800c1a6.)

The reasonable fix/workaround: update `JNIEnv.FindClass(Type)` to
instead use `JniRuntime.JniTypeManager.GetTypeSignature(Type)`.
(Also known as "embrace more JniRuntime abstractions!".)

Unfortunately, this straightforward idea hits a minor schism between
the .NET for Android and builtin java-interop world orders:

How should Java `byte` be bound?

For starters, what *is* a [Java `byte`][0]?

> The values of the integral types are integers in the following ranges:
>
>   * For `byte`, from -128 to 127, inclusive

The Java `byte` is *signed*!  Because of that, and because
java-interop originated as a Second System Syndrome rebuild of
Xamarin.Android, *of course* java-interop bound Java `byte` as
`System.SByte`.

.NET for Android, though, bound Java `byte` as `System.Byte`.

This "minor" change meant that lots of unit tests started failing, e.g.
[`NetworkInterfacesTest.DotNetInterfacesShouldEqualJavaInterfaces()`][2]:

	System.ArgumentException : Could not determine Java type corresponding to System.Byte[], System.Private.CoreLib, Version=10.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e. Arg_ParamName_Name, type
	   at Android.Runtime.JNIEnv.FindClass(Type )
	   at Android.Runtime.JNIEnv.AssertCompatibleArrayTypes(IntPtr , Type )
	   at Android.Runtime.JNIEnv._GetArray(IntPtr , Type )
	   at Android.Runtime.JNIEnv.GetArray(IntPtr , JniHandleOwnership , Type )
	   at Java.Net.NetworkInterface.GetHardwareAddress()
	   at System.NetTests.NetworkInterfacesTest.GetInfos(IEnumeration interfaces)
	   at System.NetTests.NetworkInterfacesTest.DotNetInterfacesShouldEqualJavaInterfaces()
	   at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
	   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object , BindingFlags )

Rephrased, `runtime.TypeManager.GetTypeSignature(typeof(byte[]))`
returned a "default" `JniTypeSignature` instance.

It's time to reduce the size of this schism.

Update `JniBuiltinMarshalers.GetBuiltInTypeSignature()` so that
`TypeCode.Byte` is treated as a synonym for `TypeCode.SByte`.
This is in fact all that's needed in order to add support for `byte[]`!

Repeat this exercise for all other unsigned types: `ushort`, `uint`,
and `ulong`, as Kotlin unsigned types require it; see also 71afce5
and dotnet/android@aa5e597eba.  This fixes the exception:

	System.InvalidCastException : Unable to cast from '[I' to '[Ljava/lang/Object;'.
	   at Android.Runtime.JNIEnv.AssertCompatibleArrayTypes(IntPtr , Type )
	   at Android.Runtime.JNIEnv._GetArray(IntPtr , Type )
	   at Android.Runtime.JNIEnv.GetArray(IntPtr , JniHandleOwnership , Type )
	   at Foo.UnsignedInstanceMethods.UintArrayInstanceMethod(UInt32[] value)
	   at Xamarin.Android.JcwGenTests.KotlinUnsignedTypesTests.TestUnsignedArrayTypeMembers()
	   at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
	   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object , BindingFlags )

This is *not* all that's necessary to fix all dotnet/android tests.

Update `JniRuntime.JniTypeManager.GetTypeSignature()` and
`.GetTypeSignatures()` so that if the type is an open generic type
a `System.NotSupportedException` is thrown instead of a
`System.ArgumentException`.  This fixes
[`Java.InteropTests.JnienvTest.NewOpenGenericTypeThrows()`][3].

Also, `JniBuiltinMarshalers.cs` has some hand-made changes, rendering
it out of sync with `JniBuiltinMarshalers.tt`.  Update
`JniBuiltinMarshalers.tt` appropriately and regenerate it.

[0]: https://docs.oracle.com/javase/specs/jls/se21/html/jls-4.html#jls-4.2.1
[1]: https://github.com/dotnet/java-interop/blob/f30e420a1638dc013302e85dcf76642c10c26832/Documentation/Motivation.md
[2]: https://github.com/dotnet/android/blob/1b1f1452f6b05707418d6605c06e106e6a2a6381/tests/Mono.Android-Tests/Mono.Android-Tests/System.Net/NetworkInterfaces.cs#L107-L137
[3]: https://github.com/dotnet/android/blob/1b1f1452f6b05707418d6605c06e106e6a2a6381/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs#L107-L116

Author: jonpryor

src/Java.Interop/Java.Interop/JniBuiltinMarshalers.cs

@@ -49,18 +49,22 @@ static bool GetBuiltInTypeSignature (Type type, ref JniTypeSignature signature)
 				case TypeCode.Boolean:
 					signature = GetCachedTypeSignature (ref __BooleanTypeSignature, "Z", arrayRank: 0, keyword: true);
 					return true;
+				case TypeCode.Byte:
 				case TypeCode.SByte:
 					signature = GetCachedTypeSignature (ref __SByteTypeSignature, "B", arrayRank: 0, keyword: true);
 					return true;
 				case TypeCode.Char:
 					signature = GetCachedTypeSignature (ref __CharTypeSignature, "C", arrayRank: 0, keyword: true);
 					return true;
+				case TypeCode.UInt16:
 				case TypeCode.Int16:
 					signature = GetCachedTypeSignature (ref __Int16TypeSignature, "S", arrayRank: 0, keyword: true);
 					return true;
+				case TypeCode.UInt32:
 				case TypeCode.Int32:
 					signature = GetCachedTypeSignature (ref __Int32TypeSignature, "I", arrayRank: 0, keyword: true);
 					return true;
+				case TypeCode.UInt64:
 				case TypeCode.Int64:
 					signature = GetCachedTypeSignature (ref __Int64TypeSignature, "J", arrayRank: 0, keyword: true);
 					return true;
@@ -74,9 +78,6 @@ static bool GetBuiltInTypeSignature (Type type, ref JniTypeSignature signature)
 				case TypeCode.DBNull:
 				case TypeCode.Decimal:
 				case TypeCode.Empty:
-				case TypeCode.UInt16:
-				case TypeCode.UInt32:
-				case TypeCode.UInt64:
 					return false;
 			}
 

src/Java.Interop/Java.Interop/JniBuiltinMarshalers.tt

@@ -18,14 +18,14 @@ using Java.Interop.Expressions;
 namespace Java.Interop {
 <#
 	var types = new[]{
-		new { Name = "Boolean",     Type = "Boolean",   JniCallType = "Boolean",    JniType = "Z",  GetValue = "booleanValue" },
-		new { Name = "Byte",        Type = "SByte",     JniCallType = "Byte",       JniType = "B",  GetValue = "byteValue" },
-		new { Name = "Character",   Type = "Char",      JniCallType = "Char",       JniType = "C",  GetValue = "charValue" },
-		new { Name = "Short",       Type = "Int16",     JniCallType = "Short",      JniType = "S",  GetValue = "shortValue" },
-		new { Name = "Integer",     Type = "Int32",     JniCallType = "Int",        JniType = "I",  GetValue = "intValue" },
-		new { Name = "Long",        Type = "Int64",     JniCallType = "Long",       JniType = "J",  GetValue = "longValue" },
-		new { Name = "Float",       Type = "Single",    JniCallType = "Float",      JniType = "F",  GetValue = "floatValue" },
-		new { Name = "Double",      Type = "Double",    JniCallType = "Double",     JniType = "D",  GetValue = "doubleValue" },
+		new { Name = "Boolean",     Type = "Boolean",   UnsignedType = "",          JniCallType = "Boolean",    JniType = "Z",  GetValue = "booleanValue" },
+		new { Name = "Byte",        Type = "SByte",     UnsignedType = "Byte",	    JniCallType = "Byte",       JniType = "B",  GetValue = "byteValue" },
+		new { Name = "Character",   Type = "Char",      UnsignedType = "",          JniCallType = "Char",       JniType = "C",  GetValue = "charValue" },
+		new { Name = "Short",       Type = "Int16",     UnsignedType = "UInt16",    JniCallType = "Short",      JniType = "S",  GetValue = "shortValue" },
+		new { Name = "Integer",     Type = "Int32",     UnsignedType = "UInt32",    JniCallType = "Int",        JniType = "I",  GetValue = "intValue" },
+		new { Name = "Long",        Type = "Int64",     UnsignedType = "UInt64",    JniCallType = "Long",       JniType = "J",  GetValue = "longValue" },
+		new { Name = "Float",       Type = "Single",    UnsignedType = "",          JniCallType = "Float",      JniType = "F",  GetValue = "floatValue" },
+		new { Name = "Double",      Type = "Double",    UnsignedType = "",          JniCallType = "Double",     JniType = "D",  GetValue = "doubleValue" },
 	};
 #>
 
@@ -57,6 +57,11 @@ namespace Java.Interop {
 					return true;
 <#
 	foreach (var type in types) {
+		if (!string.IsNullOrEmpty (type.UnsignedType)) {
+#>
+				case TypeCode.<#= type.UnsignedType #>:
+<#
+		}
 #>
 				case TypeCode.<#= type.Type #>:
 					signature = GetCachedTypeSignature (ref __<#= type.Type #>TypeSignature, "<#= type.JniType #>", arrayRank: 0, keyword: true);
@@ -68,9 +73,6 @@ namespace Java.Interop {
 				case TypeCode.DBNull:
 				case TypeCode.Decimal:
 				case TypeCode.Empty:
-				case TypeCode.UInt16:
-				case TypeCode.UInt32:
-				case TypeCode.UInt64:
 					return false;
 			}
 
@@ -185,7 +187,7 @@ namespace Java.Interop {
 		public override object? CreateValue (
 				ref JniObjectReference reference,
 				JniObjectReferenceOptions options,
-				[DynamicallyAccessedMembers (ConstructorsAndInterfaces)]
+				[DynamicallyAccessedMembers (Constructors)]
 				Type? targetType)
 		{
 			if (!reference.IsValid)
@@ -196,7 +198,7 @@ namespace Java.Interop {
 		public override <#= type.Type #> CreateGenericValue (
 				ref JniObjectReference reference,
 				JniObjectReferenceOptions options,
-				[DynamicallyAccessedMembers (ConstructorsAndInterfaces)]
+				[DynamicallyAccessedMembers (Constructors)]
 				Type? targetType)
 		{
 			if (!reference.IsValid)
@@ -230,6 +232,7 @@ namespace Java.Interop {
 			state   = new JniValueMarshalerState ();
 		}
 
+		[RequiresDynamicCode (ExpressionRequiresUnreferencedCode)]
 		[RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)]
 		public override Expression CreateParameterToManagedExpression (JniValueMarshalerContext context, ParameterExpression sourceValue, ParameterAttributes synchronize, Type? targetType)
 		{
@@ -242,6 +245,7 @@ namespace Java.Interop {
 			return sourceValue;
 		}
 
+		[RequiresDynamicCode (ExpressionRequiresUnreferencedCode)]
 		[RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)]
 		public override Expression CreateReturnValueFromManagedExpression (JniValueMarshalerContext context, ParameterExpression sourceValue)
 		{
@@ -256,7 +260,7 @@ namespace Java.Interop {
 		public override <#= type.Type #>? CreateGenericValue (
 				ref JniObjectReference reference,
 				JniObjectReferenceOptions options,
-				[DynamicallyAccessedMembers (ConstructorsAndInterfaces)]
+				[DynamicallyAccessedMembers (Constructors)]
 				Type? targetType)
 		{
 			if (!reference.IsValid)

src/Java.Interop/Java.Interop/JniRuntime.JniTypeManager.cs

@@ -146,7 +146,7 @@ public JniTypeSignature GetTypeSignature (Type type)
 				if (type == null)
  					throw new ArgumentNullException (nameof (type));
 				if (type.ContainsGenericParameters)
-					throw new ArgumentException ($"'{type}' contains a generic type definition. This is not supported.", nameof (type));
+					throw new NotSupportedException ($"'{type}' contains a generic type definition. This is not supported.");
 
 				type = GetUnderlyingType (type, out int rank);
 
@@ -184,7 +184,7 @@ public IEnumerable<JniTypeSignature> GetTypeSignatures (Type type)
 				if (type == null)
 					yield break;
 				if (type.ContainsGenericParameters)
-					throw new ArgumentException ($"'{type}' contains a generic type definition. This is not supported.", nameof (type));
+					throw new NotSupportedException ($"'{type}' contains a generic type definition. This is not supported.");
 
 				type = GetUnderlyingType (type, out int rank);
 

tests/Java.Interop-Tests/Java.Interop/JniTypeManagerTests.cs

@@ -18,7 +18,7 @@ public void GetTypeSignature_Type ()
 			Assert.Throws<ArgumentException>(() => JniRuntime.CurrentRuntime.TypeManager.GetTypeSignature (typeof (int[,])));
 			Assert.Throws<ArgumentException>(() => JniRuntime.CurrentRuntime.TypeManager.GetTypeSignature (typeof (int[,][])));
 			Assert.Throws<ArgumentException>(() => JniRuntime.CurrentRuntime.TypeManager.GetTypeSignature (typeof (int[][,])));
-			Assert.Throws<ArgumentException>(() => JniRuntime.CurrentRuntime.TypeManager.GetTypeSignature (typeof (Action<>)));
+			Assert.Throws<NotSupportedException>(() => JniRuntime.CurrentRuntime.TypeManager.GetTypeSignature (typeof (Action<>)));
 			Assert.AreEqual (null, JniRuntime.CurrentRuntime.TypeManager.GetTypeSignature (typeof (JniRuntimeTest)).SimpleReference);
 
 			AssertGetJniTypeInfoForType (typeof (string),   "java/lang/String",   false,  0);
@@ -44,6 +44,12 @@ public void GetTypeSignature_Type ()
 			AssertGetJniTypeInfoForType (typeof (int[][]),      "[[I",  true,   2);
 			AssertGetJniTypeInfoForType (typeof (int[][][]),    "[[[I", true,   3);
 
+			// We map unsigned types to their signed counterparts
+			AssertGetJniTypeInfoForType (typeof (byte[]),       "[B",   true,   1);
+			AssertGetJniTypeInfoForType (typeof (ushort[]),     "[S",   true,   1);
+			AssertGetJniTypeInfoForType (typeof (uint[]),       "[I",   true,   1);
+			AssertGetJniTypeInfoForType (typeof (ulong[]),      "[J",   true,   1);
+
 			AssertGetJniTypeInfoForType (typeof (JavaSByteArray),       "[B",   true,   1);
 			AssertGetJniTypeInfoForType (typeof (JavaInt16Array),       "[S",   true,   1);
 			AssertGetJniTypeInfoForType (typeof (JavaInt32Array),       "[I",   true,   1);
@@ -89,14 +95,16 @@ public void GetTypeSignature_Type ()
 		static void AssertGetJniTypeInfoForType (Type type, string jniType, bool isKeyword, int arrayRank)
 		{
 			var info = JniRuntime.CurrentRuntime.TypeManager.GetTypeSignature (type);
+			Assert.IsTrue (info.IsValid, $"info.IsValid for `{type}`");
 
 			// `GetTypeSignature() and `GetTypeSignatures()` should be "in sync"; verify that!
 			var info2   = JniRuntime.CurrentRuntime.TypeManager.GetTypeSignatures (type).FirstOrDefault ();
+			Assert.IsTrue (info2.IsValid, $"info2.IsValid for `{type}`");
 
 			Assert.AreEqual (jniType,   info.Name,          $"info.Name for `{type}`");
-			Assert.AreEqual (jniType,   info2.Name,         $"info.Name for `{type}`");
+			Assert.AreEqual (jniType,   info2.Name,         $"info2.Name for `{type}`");
 			Assert.AreEqual (arrayRank, info.ArrayRank,     $"info.ArrayRank for `{type}`");
-			Assert.AreEqual (arrayRank, info2.ArrayRank,    $"info.ArrayRank for `{type}`");
+			Assert.AreEqual (arrayRank, info2.ArrayRank,    $"info2.ArrayRank for `{type}`");
 		}
 
 		[Test]