bgen: can't convert System.Collections.Generic.IList<string> <-> System.Collections.IList?

LeadAssimilator commented 11 months ago

VS 17.7.4 / X 17.7.0.216 / XA 13.2.1.2

While trying to generate legacy xamarin android bindings targeting android 13 with class-parse and XAJavaInterop1 for two different aars, Core and Tracker CS1503 and CS0266 are occurring in the generated IJobApiInvoker class for Tracker which depends on Core - Core builds ok.

error CS1503: Argument 1: cannot convert from 'System.Collections.Generic.IList<string>' to 'System.Collections.IList?'
error CS0266: Cannot implicitly convert type 'System.Collections.IList' to 'System.Collections.Generic.IList<string>'. An explicit conversion exists (are you missing a cast?)

Core api.xml snippet:

<interface abstract="true" deprecated="not deprecated" final="false" name="JobApi" static="false" visibility="public" jni-signature="Lcom/kochava/core/job/job/internal/JobApi;">
  <method abstract="true" deprecated="not deprecated" final="false" name="getDependencies" jni-signature="()Ljava/util/List;" bridge="false" native="false" return="java.util.List&lt;java.lang.String&gt;" jni-return="Ljava/util/List&lt;Ljava/lang/String;&gt;;" static="false" synchronized="false" synthetic="false" visibility="public" return-not-null="true" />
</interface>

Tracker api.xml snippet:

<interface abstract="true" deprecated="not deprecated" final="false" name="JobApi" static="false" visibility="public" jni-signature="Lcom/kochava/tracker/job/internal/JobApi;">
  <implements name="com.kochava.core.job.job.internal.JobApi" name-generic-aware="com.kochava.core.job.job.internal.JobApi&lt;com.kochava.tracker.job.internal.JobParams&gt;" jni-type="Lcom/kochava/core/job/job/internal/JobApi&lt;Lcom/kochava/tracker/job/internal/JobParams;&gt;;" />
</interface>

Core bgen'd code snippet that compiles:

// Metadata.xml XPath interface reference: path="/api/package[@name='com.kochava.core.job.job.internal']/interface[@name='JobApi']"
[Register ("com/kochava/core/job/job/internal/JobApi", "", "Com.Kochava.Core.Job.Job.Internal.IJobApiInvoker")]
[global::Java.Interop.JavaTypeParameters (new string [] {"JobHostParametersType"})]
public partial interface IJobApi : IJavaObject, IJavaPeerable {
    global::System.Collections.Generic.IList<string> Dependencies {
        // Metadata.xml XPath method reference: path="/api/package[@name='com.kochava.core.job.job.internal']/interface[@name='JobApi']/method[@name='getDependencies' and count(parameter)=0]"
        [Register ("getDependencies", "()Ljava/util/List;", "GetGetDependenciesHandler:Com.Kochava.Core.Job.Job.Internal.IJobApiInvoker, KochavaCoreBindings.Droid")]
        get; 
    }
}

[global::Android.Runtime.Register ("com/kochava/core/job/job/internal/JobApi", DoNotGenerateAcw=true)]
internal partial class IJobApiInvoker : global::Java.Lang.Object, IJobApi {

    static IntPtr n_GetDependencies (IntPtr jnienv, IntPtr native__this)
    {
        var __this = global::Java.Lang.Object.GetObject<global::Com.Kochava.Core.Job.Job.Internal.IJobApi> (jnienv, native__this, JniHandleOwnership.DoNotTransfer);
        return global::Android.Runtime.JavaList<string>.ToLocalJniHandle (__this.Dependencies);
    }
#pragma warning restore 0169

    IntPtr id_getDependencies;
    public unsafe global::System.Collections.Generic.IList<string> Dependencies {
        get {
            if (id_getDependencies == IntPtr.Zero)
                id_getDependencies = JNIEnv.GetMethodID (class_ref, "getDependencies", "()Ljava/util/List;");
            return global::Android.Runtime.JavaList<string>.FromJniHandle (JNIEnv.CallObjectMethod (((global::Java.Lang.Object) this).Handle, id_getDependencies), JniHandleOwnership.TransferLocalRef);
        }
    }
}

Tracker bgen'd code snippet that fails compile:

// Metadata.xml XPath interface reference: path="/api/package[@name='com.kochava.tracker.job.internal']/interface[@name='JobApi']"
[Register ("com/kochava/tracker/job/internal/JobApi", "", "Com.Kochava.Tracker.Job.Internal.IJobApiInvoker")]
public partial interface IJobApi : global::Com.Kochava.Core.Job.Job.Internal.IJobApi {
}

[global::Android.Runtime.Register ("com/kochava/tracker/job/internal/JobApi", DoNotGenerateAcw=true)]
internal partial class IJobApiInvoker : global::Java.Lang.Object, IJobApi {

    static IntPtr n_GetDependencies (IntPtr jnienv, IntPtr native__this)
    {
        var __this = global::Java.Lang.Object.GetObject<global::Com.Kochava.Tracker.Job.Internal.IJobApi> (jnienv, native__this, JniHandleOwnership.DoNotTransfer);

//error CS1503: Argument 1: cannot convert from 'System.Collections.Generic.IList<string>' to 'System.Collections.IList?'
        return global::Android.Runtime.JavaList.ToLocalJniHandle (__this.Dependencies);
    }
#pragma warning restore 0169

    IntPtr id_getDependencies;
    public unsafe global::System.Collections.Generic.IList<global::System.String> Dependencies {
        get {
            if (id_getDependencies == IntPtr.Zero)
                id_getDependencies = JNIEnv.GetMethodID (class_ref, "getDependencies", "()Ljava/util/List;");

//error CS0266: Cannot implicitly convert type 'System.Collections.IList' to 'System.Collections.Generic.IList<string>'. An explicit conversion exists (are you missing a cast?)
            return global::Android.Runtime.JavaList.FromJniHandle (JNIEnv.CallObjectMethod (((global::Java.Lang.Object) this).Handle, id_getDependencies), JniHandleOwnership.TransferLocalRef);
        }
    }
}

LeadAssimilator commented 11 months ago

While the method return types are correct in the derived interface's invoker in the second assembly, the conversion code bizarrely loses the fact that its dealing with generics and uses JavaList instead of JavaList<string>.

jpobst commented 11 months ago

There is limited support for Java generics, and I think one of the limitations is indeed that the info is lost when using separate binding assemblies.

You will likely need to use "metadata" to "fix" it to the correct type:

https://learn.microsoft.com/en-us/xamarin/android/platform/binding-java-library/customizing-bindings/java-bindings-metadata#managedtype

LeadAssimilator commented 10 months ago

Perhaps I'm missing something, but how would this be fixable with metadata? As stated in the snippets above, the return types are correct, but the inner code/body that is generated is not. There is no way to workaround this that I can tell, outside of editing the gen'd files. Unless you mean to drop the usage of generics in the super assembly, so the derived doesn't gen the wrong thing, but that makes usage harder.

And the fact that the return types are correct in the first place seems to indicate this is more of a bug, no? Without understanding the code, I would expect the prototype and body to always match. If the proto gen decides to use generics, then shouldn't the body gen be aware of it? And even if it isn't, shouldn't the body gen have looked up the types and made the same decisions as were made in the super assembly - i.e. to choose generics?

I could perhaps understand a case where the proto and body match, but end up differing in the derived vs super, leading to a does not implement class/interface error. But this case where proto with generics and body with generics match in the super assembly, yet somehow mismatch within the derived assembly where proto has generics and matches the super assembly but body doesn't and lacks generics, is rather bizarre.

jpobst commented 10 months ago

Can you provide the aars that demonstrate this issue?

Another possible workaround is to use metadata to remove JobApi.getDependencies (), and then copy/paste the generated code you have above into an Additions .cs file and manually fix it to have the correct implementation.

LeadAssimilator commented 10 months ago

Here are the aars: https://github.com/Kochava/Android-KochavaCore-Releases/releases/download/v3.0.0/KochavaCore.aar https://github.com/Kochava/Android-KochavaTracker-Releases/releases/download/v5.1.0/KochavaTracker.aar

jpobst commented 10 months ago

Initial research:

Issue can be reproduced when .aars are in separate projects.
Issue doesn't occur when both .aars are in the same project, but this configuration is not supported for classic Xamarin.Android, only .NET 6+.

Possible workaround:

The full type is com.kochava.tracker.job.internal.JobApi. If the type is actually considered internal and not intended for public use, the interface could simply be removed. Or if that API simply isn't needed it could be removed.

I will continue investigating what the issue in generator is, but it does appear like the aforementioned "loses generic information when reading a reference assembly". However we definitely should not be generating code that does not compile, so we should try to fix this issue even without fully supporting generics.

jpobst commented 10 months ago

When both are in the same project, we have access to the base interface method's return type information contained in the .aar (api.xml):

<interface abstract="true" deprecated="not deprecated" final="false" name="JobApi" static="false" visibility="public" jni-signature="Lcom/kochava/core/job/job/internal/JobApi;">
  <method abstract="true" deprecated="not deprecated" final="false" name="getDependencies" jni-signature="()Ljava/util/List;" bridge="false" native="false" return="java.util.List&lt;java.lang.String&gt;" jni-return="Ljava/util/List&lt;Ljava/lang/String;&gt;;" static="false" synchronized="false" synthetic="false" visibility="public" return-not-null="true" />
</interface>

When we are accessing the base interface method's return type as a managed type in a separate assembly, we have access to less information about the Java signature:

[Register("com/kochava/core/job/job/internal/JobApi", "", "Com.Kochava.Core.Job.Job.Internal.IJobApiInvoker")]
[JavaTypeParameters(new string[] { "JobHostParametersType" })]
public interface IJobApi : IJavaObject, IDisposable, IJavaPeerable
{
    IList<string> Dependencies
    {
        [Register("getDependencies", "()Ljava/util/List;", "GetGetDependenciesHandler:Com.Kochava.Core.Job.Job.Internal.IJobApiInvoker, poof2")]
        get;
    }
}

Specifically, we have lost access to the fact that the Java return type signature for getDependencies was a generic type:

api.xml: Ljava/util/List<Ljava/lang/String;>;
.dll: Ljava/util/List;

When we resolve the return type for the generated code we use the Java signature. Because we no longer know that the Java type was generic, we end up with JavaList instead of JavaList<string>.

https://github.com/xamarin/java.interop/blob/38c8a827e78ffe9c80ad2313a9e0e0d4f8215184/tools/generator/Java.Interop.Tools.Generator.ObjectModel/ReturnValue.cs#L146

Unfortunately I cannot think of a small, targeted fix for this issue. The "proper" fix would probably be to extend [Register] to additionally emit a full Java signature with generics info. However, there are 2 issues with this approach:

The entire ecosystem would need to be recompiled to include the new generated info.
Consuming new generic info instead of type erased info would potentially affect every case out there that is currently working without generics.

Elinares-82 commented 6 months ago

Hi, I'm facing the same issue then try to create a Android binding for CleverTap .aar.

The structure of my solution is the following.

I have configured the Metadata

<metadata>
  <!--
  This sample removes the class: android.support.v4.content.AsyncTaskLoader.LoadTask:
  <remove-node path="/api/package[@name='android.support.v4.content']/class[@name='AsyncTaskLoader.LoadTask']" />

  This sample removes the method: android.support.v4.content.CursorLoader.loadInBackground:
  <remove-node path="/api/package[@name='android.support.v4.content']/class[@name='CursorLoader']/method[@name='loadInBackground']" />
  -->

  <attr path="/api/package[@name='com.clevertap.android.sdk']/class[@name='Utils']" name="managedName">UtilsHelper</attr>

  <attr path="/api/package[@name='com.clevertap.android.sdk.inapp']/class['CTInAppWebView']" name="visibility">public</attr>
  <attr path="/api/package[@name='com.clevertap.android.sdk.task']/class[@name='IOExecutor']" name="visibility">public</attr>
  <attr path="/api/package[@name='com.clevertap.android.sdk.task']/class[@name='IOExecutor']/method[@name='invokeAll' and count(parameter)=1 and parameter[1][@type='java.util.Collection&lt;? extends java.util.concurrent.Callable&lt;T&gt;&gt;']]/parameter[1]" name="managedType">System.Collections.ICollection</attr>
  <attr path="/api/package[@name='com.clevertap.android.sdk.task']/class[@name='IOExecutor']/method[@name='invokeAll' and count(parameter)=1 and parameter[1][@type='java.util.Collection&lt;? extends java.util.concurrent.Callable&lt;T&gt;&gt;']]" name="managedReturn">System.Collections.IList</attr>

  <attr path="/api/package[@name='com.clevertap.android.sdk.task']/class[@name='IOExecutor']/method[@name='invokeAll' and count(parameter)=3 and parameter[1][@type='java.util.Collection&lt;? extends java.util.concurrent.Callable&lt;T&gt;&gt;'] and parameter[2][@type='long'] and parameter[3][@type='java.util.concurrent.TimeUnit']]/parameter[1]" name="managedType">System.Collections.ICollection</attr>
  <attr path="/api/package[@name='com.clevertap.android.sdk.task']/class[@name='IOExecutor']/method[@name='invokeAll' and count(parameter)=3 and parameter[1][@type='java.util.Collection&lt;? extends java.util.concurrent.Callable&lt;T&gt;&gt;'] and parameter[2][@type='long'] and parameter[3][@type='java.util.concurrent.TimeUnit']]" name="managedReturn">System.Collections.IList</attr>

  <attr path="/api/package[@name='com.clevertap.android.sdk.task']/class[@name='IOExecutor']/method[@name='invokeAny' and count(parameter)=1 and parameter[1][@type='java.util.Collection&lt;? extends java.util.concurrent.Callable&lt;T&gt;&gt;']]/parameter[1]" name="managedType">System.Collections.ICollection</attr>
  <attr path="/api/package[@name='com.clevertap.android.sdk.task']/class[@name='IOExecutor']/method[@name='invokeAny' and count(parameter)=3 and parameter[1][@type='java.util.Collection&lt;? extends java.util.concurrent.Callable&lt;T&gt;&gt;'] and parameter[2][@type='long'] and parameter[3][@type='java.util.concurrent.TimeUnit']]/parameter[1]" name="managedType">System.Collections.ICollection</attr>

</metadata>

Compiling results:

So, there is class called IOExecutor that implements global::Java.Util.Concurrent.IExecutorService. The return type is the correct one, but the return logic or body is using global::Android.Runtime.JavaList<global::Java.Util.Concurrent.IFuture>.FromJniHandle

I can fix this manually by removing the generic in the four methods, however, is there a way to create the binding with the generated code?

Elinares-82 commented 6 months ago

I'm attaching a project to replicate the issue.

[CleverTap.Bindings.Android.zip](https://github.com/xamarin/java.interop/files/14302683/CleverTap.Bindings.Android.zip)

jpobst commented 6 months ago

If you do not need this API, the easiest fix is simply to remove them:

Remove the IOExecutor class
Or remove the implemented interface of the IOExecutor class (<implements name="java.util.concurrent.ExecutorService" ...) and then remove the four methods

If you do need them, the easiest fix is probably to copy the generated code to an Additions file, fix it up, and remove the generated methods.

You're probably not going to be able to have it correctly generated for you, due to the bug listed in this issue.

Elinares-82 commented 6 months ago

If you do not need this API, the easiest fix is simply to remove them:

Remove the IOExecutor class

Or remove the implemented interface of the IOExecutor class (<implements name="java.util.concurrent.ExecutorService" ...) and then remove the four methods

If you do need them, the easiest fix is probably to copy the generated code to an Additions file, fix it up, and remove the generated methods.

You're probably not going to be able to have it correctly generated for you, due to the bug listed in this issue.

We actually do not know if the correct path is to remove the entire IOExecutor class. This is a binding configuration from CleverTap original XF Binding. When we remove the IOExecutor effectible the library is created, however the main class called CleverTapAPI to initialize the component is not included in the DLL.

In the second approach, how can I remove the interface implementation? is there an example I can use?

And in the last one, I can take the IOExecutor class and fixed it, however, how can I generate the DLL manually?

jpobst commented 6 months ago

In the second approach, how can I remove the interface implementation? is there an example I can use?

<remove-node path="/api/package[@name='com.clevertap.android.sdk.task']/class[@name='IOExecutor']/implements" />

And in the last one, I can take the IOExecutor class and fixed it, however, how can I generate the DLL manually?

You should take the generated code for each method and copy it into a .cs file in Additions in a partial class. Fix the class here.

Use metadata to remove the method from the generated bindings:

<remove-node path="/api/package[@name='com.clevertap.android.sdk.task']/class[@name='IOExecutor']/method[@name='invokeAll' and count(parameter)=1 and parameter[1][@type='java.util.Collection&lt;? extends java.util.concurrent.Callable&lt;T&gt;&gt;']]" />

You do not need to generate the DLL manually. The next time you compile it, it will not generate anything for the specified method and it will include the code you copied to Additions.

Elinares-82 commented 6 months ago

In the second approach, how can I remove the interface implementation? is there an example I can use?
<remove-node path="/api/package[@name='com.clevertap.android.sdk.task']/class[@name='IOExecutor']/implements" />
And in the last one, I can take the IOExecutor class and fixed it, however, how can I generate the DLL manually?

You should take the generated code for each method and copy it into a .cs file in Additions in a partial class. Fix the class here.

Use metadata to remove the method from the generated bindings:
<remove-node path="/api/package[@name='com.clevertap.android.sdk.task']/class[@name='IOExecutor']/method[@name='invokeAll' and count(parameter)=1 and parameter[1][@type='java.util.Collection&lt;? extends java.util.concurrent.Callable&lt;T&gt;&gt;']]" />
You do not need to generate the DLL manually. The next time you compile it, it will not generate anything for the specified method and it will include the code you copied to Additions.

I was able to make it compile and generate the DLL, however several classes are not in there for like CleverTapAPI.

I decompiled and XF version of the DLL and I can see the CleverTapAPI

And de Net8 Version

jpobst commented 6 months ago

The reason why it was removed will generally be listed in obj\Debug\net8.0-android\java-resolution-report.log:

The class '[Class] com.clevertap.android.sdk.CleverTapAPI' was removed because the Java implemented interface type 'com.clevertap.android.sdk.inbox.CTInboxActivity.InboxActivityListener' could not be found.

Why is CTInboxActivity.InboxActivityListener missing?

The class '[Class] com.clevertap.android.sdk.inbox.CTInboxActivity' was removed because the Java base type 'androidx.fragment.app.FragmentActivity' could not be found.

Which means you need to add a NuGet reference to the Xamarin.AndroidX.Fragment package.

There is some information here for troubleshooting binding errors: https://github.com/xamarin/java.interop/wiki/Troubleshooting-Android-Bindings-Issues

Elinares-82 commented 6 months ago

The reason why it was removed will generally be listed in obj\Debug\net8.0-android\java-resolution-report.log:
The class '[Class] com.clevertap.android.sdk.CleverTapAPI' was removed because the Java implemented interface type 'com.clevertap.android.sdk.inbox.CTInboxActivity.InboxActivityListener' could not be found.
Why is CTInboxActivity.InboxActivityListener missing?
The class '[Class] com.clevertap.android.sdk.inbox.CTInboxActivity' was removed because the Java base type 'androidx.fragment.app.FragmentActivity' could not be found.
Which means you need to add a NuGet reference to the Xamarin.AndroidX.Fragment package.

There is some information here for troubleshooting binding errors: https://github.com/xamarin/java.interop/wiki/Troubleshooting-Android-Bindings-Issues

@jpobst thanks so much for your help!,

I'm understanding now how the binding libraries work.

I was able to compile and better assembly, however, is not working as expected, so I'll talk with the CleverTap team as well since I'm getting a lot of errors like

'CleverTapAPI.CoreState.get' is obsolete: 'While this member is 'public', Google considers it internal API and reserves the right to modify or delete it in the future. Use at your own risk.'

I'll let you know the results.

dotnet / java-interop

bgen: can't convert System.Collections.Generic.IList<string> <-> System.Collections.IList? #1147