[Java.Interop] JniValueManager.CreatePeer() tries more types (#1328)

Context: 77a6bf8
Context: dotnet/android#9962

While investigating dotnet/android#9962, we observed "curious"
behavior around `JniRuntime.JniValueManager.CreatePeer()`, simplified
in this unit test fragment:

	JniObjectReference float_ref;
	using (var value = new Java.Lang.Float (1.0f)) {
	    float_ref   = value.PeerReference.NewLocalRef ();
	}

	var peer    = JniEnvironment.Runtime.ValueManager.CreatePeer (
	        ref float_ref,
	        JniObjectReferenceOptions.Copy,
	        targetType: null);
	Assert.IsNotNull (peer, "Could not create Java.Lang.Float peer!");
	Assert.AreEqual (
	        typeof (Java.Lang.Float),
	        peer.GetType (),
	        $"Peer type mismatch: expected Java.Lang.Float, got {peer.GetType ()}");

What *actually* happened was that the type assertion failed:

	Error Message:
	   Peer type mismatch: expected Java.Lang.Float, got Java.Interop.JavaObject
	Expected: <Java.Lang.Float>
	But was:  <Java.Interop.JavaObject>

`float_ref` is a `java.lang.Float`.  Why is `CreatePeer()` creating a
`JavaObject`?

The reason why `CreatePeer()` created a `JavaObject` instance is tied
to the semantics of
`JniRuntime.JniTypeManager.GetType(JniTypeSignature)`, which in turn
is influenced by 77a6bf8, which basically said "wouldn't it be cool
if `float?` could be projected as `java.lang.Float`"?  This would
allow `null` to be represented:

	partial class ExportTest {

	    [JavaCallable ("staticActionNullableFloat", Signature="(Ljava/lang/Float;)V")]
	    public static void StaticActionNullableFloat (float? f)
	    {
	    }
	}

It makes sense!  (Kinda?)

However, because of *when* "builtin" mappings are checked within
`JniRuntime.JniTypeManager.GetType(JniTypeSignature)` -- *very early*
-- this meant that when looking up the `Type` for `java/lang/Float`,
`GetType(JniTypeSignature)` would return `System.Single?` and *not*
`Java.Lang.Float`.  As `System.Single?` does not implement
`IJavaPeerable`, `JniRuntime.JniValueManager.CreatePeerInstance()`
would *skip over* it, and ultimately hit `JavaObject` as a fallback.

Fix this by having `JniRuntime.JniValueManager.CreatePeerInstance()`
call `JniRuntime.JniTypeManager.GetTypes(JniTypeSignature)` instead
of `.GetType(JniTypeSignature)`.  This will allow it to find the
binding for `Java.Lang.Float`, which *ISA* `IJavaPeerable`, allowing
an instance with the appropriate type to be created and returned by
`CreatePeer()`.

Author: simonrozsival

src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs

@@ -354,15 +354,13 @@ static Type GetPeerType ([DynamicallyAccessedMembers (Constructors)] Type type)
 			{
 				var jniTypeName = JniEnvironment.Types.GetJniTypeNameFromClass (klass);
 
-				Type? type = null;
 				while (jniTypeName != null) {
 					JniTypeSignature sig;
 					if (!JniTypeSignature.TryParse (jniTypeName, out sig))
 						return null;
 
-					type    = Runtime.TypeManager.GetType (sig);
-
-					if (type != null && targetType.IsAssignableFrom (type)) {
+					Type? type = GetTypeAssignableTo (sig, targetType);
+					if (type != null) {
 						var peer = TryCreatePeerInstance (ref reference, transfer, type);
 
 						if (peer != null) {
@@ -382,6 +380,18 @@ static Type GetPeerType ([DynamicallyAccessedMembers (Constructors)] Type type)
 				JniObjectReference.Dispose (ref klass, JniObjectReferenceOptions.CopyAndDispose);
 
 				return TryCreatePeerInstance (ref reference, transfer, targetType);
+
+				[UnconditionalSuppressMessage ("Trimming", "IL2073", Justification = "Types returned here should be preserved via other means.")]
+				[return: DynamicallyAccessedMembers (Constructors)]
+				Type? GetTypeAssignableTo (JniTypeSignature sig, Type targetType)
+				{
+					foreach (var t in Runtime.TypeManager.GetTypes (sig)) {
+						if (targetType.IsAssignableFrom (t)) {
+							return t;
+						}
+					}
+					return null;
+				}
 			}
 
 			IJavaPeerable? TryCreatePeerInstance (
@@ -637,7 +647,7 @@ public JniValueMarshaler GetValueMarshaler (Type type)
 						return marshaler.Value;
 				}
 
-				
+
 				var listType    = GetListType (type);
 				if (listType != null) {
 					var elementType = listType.GenericTypeArguments [0];

tests/Java.Base-Tests/Java.Base/JavaVMFixture.cs

@@ -17,6 +17,7 @@ static partial void CreateJavaVM ()
 			var c = new TestJVM (
 					jars:           new[]{ "java.base-tests.jar" },
 					typeMappings:   new Dictionary<string, Type> () {
+						["java/lang/Float"]         = typeof (Java.Lang.Float),
 						["example/MyIntConsumer"]   = typeof (MyIntConsumer),
 						["example/MyRunnable"]      = typeof (MyRunnable),
 						[JavaInvoker.JniTypeName]   = typeof (JavaInvoker),

tests/Java.Base-Tests/Java.Base/JniRuntimeJniValueManagerContractExtras.cs

@@ -0,0 +1,32 @@
+using System;
+
+using Java.Interop;
+
+using NUnit.Framework;
+
+namespace Java.BaseTests;
+
+[TestFixture]
+public class JniRuntimeJniValueManagerContractExtras : JavaVMFixture {
+
+	[Test]
+	public void CreatePeer_FloatIsNotNullableSingle ()
+	{
+		JniObjectReference float_ref;
+#pragma warning disable CS0618
+		using (var value = new Java.Lang.Float (1.0f)) {
+			float_ref   = value.PeerReference.NewLocalRef ();
+		}
+#pragma warning restore CS0618
+		try {
+			var peer    = JniEnvironment.Runtime.ValueManager.CreatePeer (ref float_ref, JniObjectReferenceOptions.Copy, targetType: null);
+			Assert.IsNotNull (peer, "Could not create Java.Lang.Float peer!");
+			Assert.AreEqual (
+					typeof (Java.Lang.Float),
+					peer!.GetType (),
+					$"Peer type mismatch: expected Java.Lang.Float, got {peer.GetType ()}");
+		} finally {
+			JniObjectReference.Dispose (ref float_ref);
+		}
+	}
+}

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

@@ -33,6 +33,8 @@ public void GetTypeSignature_Type ()
 			AssertGetJniTypeInfoForType (typeof (bool),     "Z",    true,   0);
 			AssertGetJniTypeInfoForType (typeof (void),     "V",    true,   0);
 
+			AssertGetJniTypeInfoForType (typeof (float?),      "java/lang/Float",   true,   0);
+
 			AssertGetJniTypeInfoForType (typeof (JavaObject),  "java/lang/Object",  false,  0);
 
 			// Enums are their underlying type
@@ -130,6 +132,7 @@ static void AssertGetJniTypeInfoForType (Type type, string jniType, bool isKeywo
 			Assert.AreEqual (typeof (float),    GetType ("F"));
 			Assert.AreEqual (typeof (double),   GetType ("D"));
 			Assert.AreEqual (typeof (string),   GetType ("java/lang/String"));
+			Assert.AreEqual (typeof (float?),   GetType ("java/lang/Float"));
 			Assert.AreEqual (null,              GetType ("com/example/does/not/exist"));
 			Assert.AreEqual (null,              GetType ("Lcom/example/does/not/exist;"));
 			Assert.AreEqual (null,              GetType ("[Lcom/example/does/not/exist;"));

tests/Java.Interop.Export-Tests/Java.Interop/ExportTest.cs

@@ -108,6 +108,11 @@ public static void StaticActionFloat (float f)
 		{
 		}
 
+		// TODO: [JavaCallable ("staticActionNullableFloat", Signature="(Ljava/lang/Float;)V")]
+		public static void StaticActionNullableFloat (float? f)
+		{
+		}
+
 		[JavaCallable ("staticFuncThisMethodTakesLotsOfParameters", Signature="(ZBCSIJFDLjava/lang/Object;Ljava/lang/String;Ljava/util/ArrayList;Ljava/lang/String;Ljava/lang/Object;DFJ)Z")]
 		public static bool StaticFuncThisMethodTakesLotsOfParameters (
 				bool                a,

tests/Java.Interop.Export-Tests/java/net/dot/jni/test/ExportType.java

@@ -103,6 +103,7 @@ public void testMethods () {
 
 		staticActionInt (1);
 		staticActionFloat (2.0f);
+		// staticActionNullableFloat (3.0f);
 
 		boolean r = staticFuncThisMethodTakesLotsOfParameters (
 				false,
@@ -144,6 +145,9 @@ public void testMethods () {
 	public          void    staticActionFloat (float f) {n_StaticActionFloat (f);}
 	private native  void    n_StaticActionFloat (float f);
 
+	// public          void    staticActionNullableFloat (Float f) {n_StaticActionNullableFloat (f);}
+	// private native  void    n_StaticActionNullableFloat (Float f);
+
 	ArrayList<Object>       managedReferences     = new ArrayList<Object>();
 
 	public void jiAddManagedReference (java.lang.Object obj)