dotnet / java-interop

Java.Interop provides open-source bindings of Java's Java Native Interface (JNI) for use with .NET managed languages such as C#
Other
189 stars 48 forks source link

[generator] Unable to inherit reference interface with generic method parameters #699

Open jpobst opened 3 years ago

jpobst commented 3 years ago

Context: https://github.com/xamarin/xamarin-android/issues/5027

Given this snippet:

package com.blah;

import android.database.Cursor;

public interface MyCursor extends Cursor { }

Compiling against API-29 succeeds, while API-30 fails with:

Error CS1026: ) expected
Error CS1056: Unexpected character '`'
Error CS1002: ; expected
Error CS1002: ; expected
Error CS1513: } expected

This is due to the interface invoker method for android.database.Cursor.setNotficationUris, added in API-30, which generates this code:

var uris = (global::System.Collections.Generic.IList`1)global::Java.Lang.Object.GetObject<global::System.Collections.Generic.IList`1> (native_uris, JniHandleOwnership.DoNotTransfer);

Using a type with an arity is not legal C# (IList`1). Instead we need to generate the full closed generic type like this:

global::System.Collections.Generic.IList<global::Android.Net.Uri>

When this is fixed, this PR can be reverted: https://github.com/xamarin/xamarin-android/pull/5124.

awattar commented 3 years ago

Hi. Any ETA for the potential fix?

awattar commented 3 years ago

@jpobst ?

jpobst commented 3 years ago

Nothing has changed since https://github.com/xamarin/xamarin-android/issues/5027#issuecomment-681066086.

awattar commented 3 years ago

Thanks. I was hoping it will get a new schedule since being moved to the new issue. No chances to be included in 8.8? 8.8 is still in the preview and this "quick fix" will make it more compatible with API-30 and will also unblock us.

jpobst commented 3 years ago

Unfortunately not. 8.8 is pretty locked down now and this isn't a quick-fix. I don't even know how to fix it yet.

awattar commented 3 years ago

@jpobst We have tested fix provided in #5124 and the original issue is gone as the setNotificationUris from Cursor interface was removed. Now it complains about the getter form AbstractCursor NotificationUris property:

obj/Release/generated/src/com.our.package.Database.Sqlite.AbstractWindowedCursor.cs(608,25,608,54): error CS0534: 'AbstractWindowedCursorInvoker' does not implement inherited abstract member 'AbstractCursor.NotificationUris.get'
obj/Release/generated/src/com.our.package.Database.Sqlite.AbstractCursor.cs(1621,25,1621,46): error CS0534: 'AbstractCursorInvoker' does not implement inherited abstract member 'AbstractCursor.NotificationUris.get'
obj/Release/generated/src/com.our.package.Database.Sqlite.MatrixCursor.cs(9,23,9,35): error CS0534: 'MatrixCursor' does not implement inherited abstract member 'AbstractCursor.NotificationUris.get'
obj/Release/generated/src/com.our.package.Database.Sqlite.MergeCursor.cs(9,23,9,34): error CS0534: 'MergeCursor' does not implement inherited abstract member 'AbstractCursor.NotificationUris.get'
obj/Release/generated/src/com.our.package.Database.Sqlite.SQLiteCursor.cs(9,23,9,35): error CS0534: 'SQLiteCursor' does not implement inherited abstract member 'AbstractCursor.NotificationUris.get'

so it was overridden in the partial class on the bindings level just copying NotificationUris from AbstractCursor for each of the above classes:

        internal partial class AbstractCursorInvoker
    {
        public override global::System.Collections.Generic.IList<global::Android.Net.Uri> NotificationUris
        {
            // Metadata.xml XPath method reference: path="/api/package[@name='com.our.package.database.sqlite']/class[@name='AbstractCursor']/method[@name='getNotificationUris' and count(parameter)=0]"
            [Register("getNotificationUris", "()Ljava/util/List;", "GetGetNotificationUrisHandler")]
            get;
        }
    }

    internal partial class AbstractWindowedCursorInvoker
    {
        public override global::System.Collections.Generic.IList<global::Android.Net.Uri> NotificationUris
        {
            // Metadata.xml XPath method reference: path="/api/package[@name='com.our.package.database.sqlite']/class[@name='AbstractCursor']/method[@name='getNotificationUris' and count(parameter)=0]"
            [Register("getNotificationUris", "()Ljava/util/List;", "GetGetNotificationUrisHandler")]
            get;
        }
    }

    public partial class MergeCursor
    {
        public override global::System.Collections.Generic.IList<global::Android.Net.Uri> NotificationUris
        {
            // Metadata.xml XPath method reference: path="/api/package[@name='com.our.package.database.sqlite']/class[@name='AbstractCursor']/method[@name='getNotificationUris' and count(parameter)=0]"
            [Register("getNotificationUris", "()Ljava/util/List;", "GetGetNotificationUrisHandler")]
            get;
        }
    }

    public partial class MatrixCursor
    {
        public override global::System.Collections.Generic.IList<global::Android.Net.Uri> NotificationUris
        {
            // Metadata.xml XPath method reference: path="/api/package[@name='com.our.package.database.sqlite']/class[@name='AbstractCursor']/method[@name='getNotificationUris' and count(parameter)=0]"
            [Register("getNotificationUris", "()Ljava/util/List;", "GetGetNotificationUrisHandler")]
            get;
        }
    }

    public partial class SQLiteCursor
    {
        public override global::System.Collections.Generic.IList<global::Android.Net.Uri> NotificationUris
        {
            // Metadata.xml XPath method reference: path="/api/package[@name='com.our.package.database.sqlite']/class[@name='AbstractCursor']/method[@name='getNotificationUris' and count(parameter)=0]"
            [Register("getNotificationUris", "()Ljava/util/List;", "GetGetNotificationUrisHandler")]
            get;
        }
    }

and now it complains about the generated code:

obj/Release/generated/src/com.our.package.Database.Sqlite.AbstractCursor.cs(1609,62,1609,85): error CS1503: Argument 1: cannot convert from 'System.Collections.Generic.IList<Android.Net.Uri>' to 'System.Collections.IList?'
obj/Release/generated/src/com.our.package.Database.Sqlite.ICrossProcessCursor.cs(411,62,411,85): error CS1503: Argument 1: cannot convert from 'System.Collections.Generic.IList<Android.Net.Uri>' to 'System.Collections.IList?'
obj/Release/generated/src/com.our.package.Database.Sqlite.ICrossProcessCursor.cs(420,12,420,188): error CS0266: Cannot implicitly convert type 'System.Collections.IList' to 'System.Collections.Generic.IList<Android.Net.Uri>'. An explicit conversion exists (are you missing a cast?)

In com.our.package.Database.Sqlite.AbstractCursor.cs:

        static IntPtr n_GetNotificationUris (IntPtr jnienv, IntPtr native__this)
        {
            var __this = global::Java.Lang.Object.GetObject<global::Com.Good.GD.Database.Sqlite.AbstractCursor> (jnienv, native__this, JniHandleOwnership.DoNotTransfer);
            HERE -> return global::Android.Runtime.JavaList.ToLocalJniHandle (__this.NotificationUris);
        }

In com.our.package.Database.Sqlite.ICrossProcessCursor.cs:

        static IntPtr n_GetNotificationUris (IntPtr jnienv, IntPtr native__this)
        {
            var __this = global::Java.Lang.Object.GetObject<global::Com.Good.GD.Database.Sqlite.ICrossProcessCursor> (jnienv, native__this, JniHandleOwnership.DoNotTransfer);
            HERE -> return global::Android.Runtime.JavaList.ToLocalJniHandle (__this.NotificationUris);
        }
#pragma warning restore 0169

        IntPtr id_getNotificationUris;
        public unsafe global::System.Collections.Generic.IList<global::Android.Net.Uri> NotificationUris {
            get {
                if (id_getNotificationUris == IntPtr.Zero)
                    id_getNotificationUris = JNIEnv.GetMethodID (class_ref, "getNotificationUris", "()Ljava/util/List;");
                HERE -> return global::Android.Runtime.JavaList.FromJniHandle (JNIEnv.CallObjectMethod (((global::Java.Lang.Object) this).Handle, id_getNotificationUris), JniHandleOwnership.TransferLocalRef);
            }
        }

So it is still missing the cast as you have mentioned in https://github.com/xamarin/xamarin-android/issues/5027#issuecomment-680218525 as it tries to put global::System.Collections.Generic.IList<global::Android.Net.Uri> from NotificationUris to global::Android.Runtime.JavaList.ToLocalJniHandle that requires System.Collections.IList and vice versa.

jpobst commented 3 years ago

Removed Cursor.getNotificationUris as well: https://github.com/xamarin/xamarin-android/pull/5161.

jpobst commented 3 years ago

Notes:

This happens when the interface is in a different assembly from the implementing interface/class, due to how types are imported via Cecil differently than from api.xml.

public interface MyInterface {
  void doThing (List<Object> p0);
}

Gets generated as:

[Register("doThing", "(Ljava/util/List;)V", "GetDoThing_Ljava_util_List_Handler:IMyInterfaceInvoker")]
void DoThing (IList<Object> p0);

Note that the managed parameter contains a generic: IList<Object>, but the Java signature in the RegisterAttribute does not: (Ljava/util/List;)V. When the interface is implemented in a separate assembly, generator resolves the Java signature from Cecil to build the managed representation, which ends up binding as:

public class MyClass : IMyInterface {
  void DoThing (IList`1 p0) { ... }
}

That is, we convert java/util/List to the C# type System.Collection.Generic.IList'1.

There are several issues with this:

One possible solution is to look at the C# type IList<Object> and use [Register] attributes to convert back to the Java type of java.util.List<java.lang.Object>.

This should fix the general case of this, like the original ICursor issue. There are some additional fixes needed for correctly marshalling IList, as we need to use JavaList<T>.ToLocalJniHandle and JavaList<T>.FromJniHandle instead of the current non-generic types we use today.

jpobst commented 3 years ago

Here's some code that "implements" using [Register] to convert the C# type back to Java:

static string TypeReferenceToJavaType (TypeReference type)
{
    var retval = GetRegisteredJavaName (type);

    if (retval != null && type is GenericInstanceType generic) {
        var parameters = generic.GenericArguments.Select (ga => GetRegisteredJavaName (ga.Resolve ())).ToArray ();

        if (parameters.WhereNotNull ().Any ())
            retval += $"<{string.Join (", ", parameters.WhereNotNull ())}>";
    }

    return retval;
}

static string GetRegisteredJavaName (TypeReference type)
{
    var td = type.Resolve ();

    if (GetSpecialCase (td) is string s)
        return s;

    var reg_att = GetRegisterAttribute (td.CustomAttributes);

    if (reg_att != null)
        return ((string)reg_att.ConstructorArguments [0].Value).Replace ('/', '.').Replace ('$', '.');

    return null;
}

static string GetSpecialCase (TypeDefinition type)
{
    switch (type.FullName) {
        case "System.Collections.Generic.IList`1":
            return "java.util.List";
        case "System.Collections.Generic.IDictionary`2":
            return "java.util.Map";
        case "System.Collections.Generic.ICollection`1":
            return "java.util.Collection";
        case "System.String":
            return "java.lang.String";
        default:
            return null;
    }
}

static CustomAttribute GetRegisterAttribute (this Collection<CustomAttribute> attributes) =>
    attributes.FirstOrDefault (a => a.AttributeType.FullNameCorrected () == "Android.Runtime.RegisterAttribute");

The issue this uncovers is that we have mapped multiple Java types to C# collection types:

Given an IList'1, we cannot tell which Java type we need to convert to. For the "top-level" generic, like IList<T> we could look at the [Register] signature of (Ljava/util/List;)V and see that we need java.util.List. However for a nested generic like IList<IList<T>> we cannot determine the needed Java type for the type argument IList.

We will likely have to have something generated that provides enough information for us to reverse a managed generic type into a Java generic type.