Support C# generics in base types

[jonpryor]

trafficstars

Consider the BouncyCastle-provided bcprov-ext-jdk15on-161.jar. This Java library contains a type equivalent to:

/* partial */ interface X509AttributeCertificate extends java.security.cert.X509Extension {
}

Which looks fine enough, except that it implements the IX509Extension interface, which has two properties which are generic types:

partial interface IX509Extension {
    ICollection<string> CriticalExtensionOIDs {get;}
    ICollection<string> NonCriticalExtensionOIDs {get;}
}

Unfortunately, generator's support for C# generic methods results in the resulting binding...not being valid C# code:

internal partial class internal class IX509AttributeCertificateInvoker : global::Java.Lang.Object, IX509AttributeCertificate {
	IntPtr id_getCriticalExtensionOIDs;
	public unsafe global::System.Collections.Generic.ICollection`1<global::System.String> CriticalExtensionOIDs {
		get {
			if (id_getCriticalExtensionOIDs == IntPtr.Zero)
				id_getCriticalExtensionOIDs = JNIEnv.GetMethodID (class_ref, "getCriticalExtensionOIDs", "()Ljava/util/Set;");
			return global::Android.Runtime.JavaSet.FromJniHandle (JNIEnv.CallObjectMethod (((global::Java.Lang.Object) this).Handle, id_getCriticalExtensionOIDs), JniHandleOwnership.TransferLocalRef);
		}
	}
}

The string global::System.Collections.Generic.ICollection`1<global::System.String> is not a valid C# type -- the ICollection`1<global::System.String> needs to be ICollection<global::System.String> -- which prevents the binding from compiling. (Along with a host of other issues, all of which should be addressed as well.)

generator needs to be fixed so that this can be properly supported.

[jpobst]

I tried naively removing the arity from the return type, which at least generates valid C# code, but it still doesn't compile due to:

JavaSet.ToLocalJniHandle (__this.CriticalExtensionOIDs)

ToLocalJniHandle expects an ICollection while CriticalExtensionOIDs is an ICollection<string>.

A similar error occurs here:

public unsafe global::System.Collections.Generic.ICollection<global::System.String> CriticalExtensionOIDs {
	get {
		if (id_getCriticalExtensionOIDs == IntPtr.Zero)
			id_getCriticalExtensionOIDs = JNIEnv.GetMethodID (class_ref, "getCriticalExtensionOIDs", "()Ljava/util/Set;");
		return global::Android.Runtime.JavaSet.FromJniHandle (JNIEnv.CallObjectMethod (((global::Java.Lang.Object) this).Handle, id_getCriticalExtensionOIDs), JniHandleOwnership.TransferLocalRef);
	}
}

because JavaSet.FromJniHandle (...) returns an ICollection and we need the property to return an ICollection<string>.

I'm not sure if that means my change is incorrect, or if that is an additional separate issue that needs to be addressed.

[jonpryor]

but it still doesn't compile due to:

JavaSet.ToLocalJniHandle (__this.CriticalExtensionOIDs)

This should use JavaSet<string>.ToLocalJniHandle (__this.CriticalExtensionOIDs).

See also: https://github.com/xamarin/xamarin-android/blob/a3267e34d35a457f11ec1bb4565c350002f8c332/src/Mono.Android/Android.Runtime/JavaSet.cs#L173-L286

[jpobst]

Is this something we would expect generator to figure out, and if so, how?

Or is this something where we would expect metadata to be written to change the base class from JavaSet to JavaSet<T>?

[jonpryor]

Is this something we would expect generator to figure out?

Yes.

and if so, how?

Good question. :-)

Have a more concrete example in mind? There are only ("only") 25 instances of JavaSet.ToLocalJniHandle in API-28:

$ grep -r JavaSet.ToLocalJniH obj/Debug/android-28/mcw
obj/Debug/android-28/mcw/Java.Util.HashMap.cs:			return Android.Runtime.JavaSet.ToLocalJniHandle (__this.EntrySet ());
...

None of the existing declarations seem quite that useful for this discussion, as they all deal with non-generic types, so more "demo code" might be helpful.

That said, within the context of the CriticalExtensionOIDs property declaration, generator can "know" that the return type is ICollection<T> -- it has to, otherwise it wouldn't have emitted that in the first place, right? -- so it seems reasonable for that "generic-ness" information to be obtainable, so that generator could in turn emit JavaSet<T>.ToLocalJniHandle(...).

However, this might also be interpreted as a limitation in our current binding infrastructure. Consider the IX509Extension declaration we currently emit:

// Metadata.xml XPath interface reference: path="/api/package[@name='java.security.cert']/interface[@name='X509Extension']"
[Register ("java/security/cert/X509Extension", "", "Java.Security.Cert.IX509ExtensionInvoker", ApiSince = 1)]
public partial interface IX509Extension : IJavaObject {

  System.Collections.Generic.ICollection<string> CriticalExtensionOIDs {
    // Metadata.xml XPath method reference: path="/api/package[@name='java.security.cert']/interface[@name='X509Extension']/method[@name='getCriticalExtensionOIDs' and count(parameter)=0]"
    [Register ("getCriticalExtensionOIDs", "()Ljava/util/Set;", "GetGetCriticalExtensionOIDsHandler:Java.Security.Cert.IX509ExtensionInvoker, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null")]
    get;
  }
}

Reflection-wise, the only way anything can "know" that getCriticalExtensionOIDs() returns a Set<string> is by "reading between the lines" between the specified JNI signature (()Ljava/util/Set;) and the managed type (ICollection<string>).

We may want to consider adding additional attributes to remove the need for this inference, e.g. by using a JniGenericTypeArguments custom attribute, a'la:

System.Collections.Generic.ICollection<string> CriticalExtensionOIDs {
  [return:JniGenericTypeReference ("java/util/Set", new[]{"java/lang/String"})]
  [Register ("getCriticalExtensionOIDs", "()Ljava/util/Set;", "GetGetCriticalExtensionOIDsHandler:Java.Security.Cert.IX509ExtensionInvoker, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null")]
  get;
}

generator could then put [Register] + [JniGenericTypeArguments] "together" to deduce that the Java-side return type is java/util/Set<java/lang/String>, i.e. java.util.Set<java.lang.String> i.e. Set<String>, and carry on without needing to "interrogate" and parse the managed return types.

[mrrozenbergs]

Is it possible to fix this with metadata.xml?