java.interop icon indicating copy to clipboard operation
java.interop copied to clipboard

Can't use TabLayout.IOnTabSelectedListener2

Open PureWeen opened this issue 4 years ago • 19 comments

Xamarin.Android Version (eg: 6.0):

Tried with 9.0 and 10.0

Operating System & Version (eg: Mac OSX 10.11):

Mac OSX 10.15.3

Support Libraries Version (eg: 23.3.0):

Android X only

Describe your Issue:

If you implement the following interface

TabLayout.IOnTabSelectedListener2

You'll get the following exception

/Users/shane/Projects/AxTabError/obj/Debug/android/src/crc64f10a0ade51b9bb29/MainActivity.java(8,8): Error JAVAC0000:  error: BaseOnTabSelectedListener cannot be inherited with different arguments: <com.google.android.material.tabs.TabLayout.Tab> and <>
public class MainActivity
 (JAVAC0000) (AxTabError) javac

I first noticed this error when updating to Google.Android.Material 1.1.0-rc2 because on that version TabLayout.IOnTabSelectedListener is marked obsolete so I implemented TabLayout.IOnTabSelectedListener2

But this same exception happens on 1.0.0-preview02

Steps to Reproduce (with link to sample solution if possible):

AxTabError.zip

PureWeen avatar Mar 03 '20 02:03 PureWeen

The shortened version the Java code that is being compiled is:

interface GenericIface<T extends Runnable> {
    public void m (T value);
}

interface ExtendedRunnable extends Runnable {
}

interface SpecificIface extends GenericIface<ExtendedRunnable> {
}

class NonGenericExtends implements SpecificIface, GenericIface {
    public void m (ExtendedRunnable value) {
    }
}

though in the app's specific case it's:

public class MainActivity
	extends androidx.appcompat.app.AppCompatActivity
	implements
		mono.android.IGCUserPeer,
		com.google.android.material.tabs.TabLayout.OnTabSelectedListener,
		com.google.android.material.tabs.TabLayout.BaseOnTabSelectedListener
{

The problem here is that TabLayout.OnTabSelectedListener extends TabLayout.BaseOnTabSelectedListener<TabLayout.Tab>, and as the Java Callable Wrapper attempts to implement both interfaces, even though TabLayout.OnTabSelectedListener extends TabLayout.BaseOnTabSelectedListener, the result doesn't compile.

We will need to update the Java Callable Wrapper generator to only emit TabLayout.OnTabSelectedListener in this case.

(Related: why is the Java Callable Wrapper generator emitting both interface types in the first place, when the C# code is class MainActivity : AppCompatActivity, TabLayout.IOnTabSelectedListener2?)

jonpryor avatar Mar 03 '20 15:03 jonpryor

A possible workaround is to bypass this bug in the Java Callable Wrapper generator by binding a new Java class which has the appropriate inheritance hierarchy:

package jcw_kludge;

public class MainAppCompatActivity extends androidx.appcompat.app.AppCompatActivity
    implements com.google.android.material.tabs.TabLayout.OnTabSelectedListener
{
    public void onTabSelected(com.google.android.material.tabs.TabLayout.Tab tab) {}
    public void onTabUnselected(com.google.android.material.tabs.TabLayout.Tab tab) {}
    public void onTabReselected(com.google.android.material.tabs.TabLayout.Tab tab) {}
}

Compile jcw_kludge.MainAppCompatActivity into a new .jar file, bind the .jar file, and then the C# class can do:

public class MainActivity : JcwKludge.MainAppCompatActivity {
    // ...
}

This should in turn result in a Java Callable Wrapper for MainActivity which compiles.

jonpryor avatar Mar 03 '20 15:03 jonpryor

Possible Workaround number 2 -- only sketched out here -- doesn't require an intermediate .jar file, but does require the same jcw_kludge.MainAppCompatActivity Java class.

Step 1: Add a MainAppCompatActivity.java to the .csproj, with a Build action of @(AndroidJavaSource).

Step 2: Contents of MainAppCompatActivity.java:

package jcw_kludge;

public class MainAppCompatActivity extends androidx.appcompat.app.AppCompatActivity
    implements com.google.android.material.tabs.TabLayout.OnTabSelectedListener
{
    public void onTabSelected(com.google.android.material.tabs.TabLayout.Tab tab) {}
    public void onTabUnselected(com.google.android.material.tabs.TabLayout.Tab tab) {}
    public void onTabReselected(com.google.android.material.tabs.TabLayout.Tab tab) {}
}

Then in C#-land, "hand-bind" this type

    [Register ("jcw_kludge. MainAppCompatActivity", DoNotGenerateAcw=true)]
    public class AppCompatActivityKludge : AppCompatActivity, TabLayout.IOnTabSelectedListener2 {

        [Register ("onTabReselected", "()V", "")]
        public virtual void OnTabReselected(TabLayout.Tab tab)
        {
            throw new NotImplementedException();
        }

        public virtual void OnTabSelected(TabLayout.Tab tab)
        {
            throw new NotImplementedException();
        }

        public virtual void OnTabUnselected(TabLayout.Tab tab)
        {
            throw new NotImplementedException();
        }
    }

Then the C# MainActivity class can inherit from AppCompatActivityKludge.

Note: This solution is incomplete. In order to fully work, AppCompatActivityKludge must be fully "hand-written", including proper [Register] values so that the methods can be overridden.

Binding the .jar is likely to be easier.

jonpryor avatar Mar 03 '20 16:03 jonpryor

The bug is here:

https://github.com/xamarin/java.interop/blob/56955d9ad3952070de3bb1718375b368437f7427/src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers/JavaCallableWrapperGenerator.cs#L547-L550

The problem is that we iterate over all C# interfaces implemented by the type (e.g. MainActivity), and this iteration returns both TabLayout.IOnTabSelectedListener2 and TabLayout.IOnTabSelectedListener, even though one implements the other.

The fix is to filter out interfaces "already implemented" by another interface found in the iteration.

jonpryor avatar Apr 29 '20 19:04 jonpryor

The bug is here:

That's not the only issue in play. Fix it, and we get:

JI590.java(4,8): javac error JAVAC0000:  error: JI590 is not abstract and does not override abstract method invoke(MyRunnable) in InvokeRunnable
JI590.java(4,8): javac error JAVAC0000: public class JI590
JI590.java(28,14): javac error JAVAC0000:  error: name clash: invoke(Runnable) in JI590 and invoke(MyRunnable) in InvokeRunnable have the same erasure, yet neither overrides the other
JI590.java(28,14): javac error JAVAC0000: 	public void invoke (java.lang.Runnable p0)

in which JI590.java contains:

public class JI590
	extends java.lang.Object
	implements
		mono.android.IGCUserPeer,
		com.xamarin.android.InvokeMyRunnable
{
	/* … */

	public void invoke (java.lang.Runnable p0)
	{
		n_invoke (p0);
	}
}

Yes, now we only have the "most derived" interface type listed, but we also need to emit an invoke(MyRunnable) method, but we're instead emitting an invoke(Runnable) method.

jonpryor avatar Jun 23 '20 18:06 jonpryor

Background: this is the Java code to bind:

package com.xamarin.android;

public interface InvokeRunnable<T extends Runnable> {
    void invoke (T runnable);
}

public final class MyRunnable implements Runnable {
    public void run() {
    }
}

public interface InvokeMyRunnable extends InvokeRunnable<MyRunnable> {
}

This is the C# code we want to be able to write:

class JI590 : Java.Lang.Object, IInvokeMyRunnable {
    // though see https://github.com/xamarin/java.interop/issues/669
    public void Invoke (Java.Lang.Object p0)
    {
    }
}

How do we fix things so that JI590.invoke() has the correct parameter type? To do so, we need to extend generator and JavaCallableWrapper generation:

  1. generator output needs to contain better information regarding type parameters. In particular, the binding for InvokeMyRunnable needs to mention that it is providing type parameters to InvokeRunnable, and what the replacement is. Additionally, the binding for InvokeRunnable needs to have some mechanism of specifying which parameters "come from" type parameters.

  2. Java Callable Wrapper generation needs to take the new generator data into consideration so that it can emit appropriate Java code.

generator changes

Java.Interop.dll should gain the following new custom attributes:

namespace Java.Interop {
    [AttributeUsage (AttributeTargets.Class | AttributeTargets.Interface)]
    public sealed class JavaTypeArgumentAttribute  : Attribute {
        public JavaTypeArgumentsAttribute (string typeParameter, string typeArgument);
        public string TypeParameter {get;}
        public string TypeArgument {get;}
    }

    [AttributeUsage (AttributeTargets.Parameter | AttributeTargets.Property | AttributeTargets.ReturnValue)]
    public sealed class JavaTypeParameterAttribute : Attribute {
        public JavaTypeArgumentAttribute (string typeParameterName");
    }
}

When a Java type is generic, [JavaTypeParameter] should be placed on all parameters or return types (and thus properties) of a member with a [Register] attribute, for which the parameter is a Java generic type parameter. Thus:

	[Register ("com/xamarin/android/InvokeRunnable", "", "Com.Xamarin.Android.IInvokeRunnableInvoker")]
	[global::Java.Interop.JavaTypeParameters (new string [] {"T extends java.lang.Runnable"})]
	public partial interface IInvokeRunnable : IJavaObject, IJavaPeerable {
		[Register ("invoke", "(Ljava/lang/Runnable;)V", "GetInvoke_Ljava_lang_Runnable_Handler:Com.Xamarin.Android.IInvokeRunnableInvoker, Xamarin.Android.McwGen-Tests")]
		void Invoke (
				[JavaTypeParameter ("T")] global::Java.Lang.Object p0);
	}

When a Java type inherits a Java generic type and provides generic type arguments, then generator should emit [JavaTypeArgument]s, providing the type parameter name and value. Thus, IInvokeMyRunnable would be bound as:

	[Register ("com/xamarin/android/InvokeMyRunnable", "", "Com.Xamarin.Android.IInvokeMyRunnableInvoker")]
	[JavaTypeArgument ("T", "Lcom/xamarin/android/MyRunnable;")]
	public partial interface IInvokeMyRunnable : global::Com.Xamarin.Android.IInvokeRunnable {
	}

This adds two crucial bits of information: which parameters/return types "come from" generic type parameters, and thus may need to be "replaced", and what are the replacement values?

Java Callable Wrapper updates

With the addition of [JavaTypeParameter] and [JavaTypeArgument], Java Callable Wrapper generation can walk the method's JNI signature -- (Ljava/lang/Runnable;)V -- and via the [JavaTypeParameter] know that the first argument, Ljava/lang/Runnable;, may need to be replaced with "something else". That "something else" would come from the [JavaTypeArgument] value on IInvokeMyRunnable, which would "replace" the original value. This in turn would eventually result in emitting the Java code:

public class JI590
	extends java.lang.Object
	implements
		mono.android.IGCUserPeer,
		com.xamarin.android.InvokeMyRunnable
{
	/* … */

	public void invoke (com.xamarin.android.MyRunnable p0)
	{
		n_invoke (p0);
	}
}

which should compile.

jonpryor avatar Jun 23 '20 19:06 jonpryor

It's never this easy!

Given:

public final class GenericMethodClass<T extends java.lang.Object> {
    public void NestedGeneric (java.lang.Iterable<T> myObj) { }
}

What should we generate for the [JavaTypeParameter]? This?

public unsafe void NestedGeneric ([global::Java.Interop.JavaTypeParameter ("Java.Lang.IIterable<T>")]Java.Lang.IIterable p0) { ... }

jpobst avatar Oct 28 '20 18:10 jpobst

For that specific example, I don't think it matters: GenericMethodClass is final, and thus can't be inherited, and thus we can't possibly emit a Java Callable Wrapper which subclasses it in the first place.

If we remove final, then GenericMethodClass is more "interesting".

The T extends java.lang.Object is "unchanged," becoming:

[global::Java.Interop.JavaTypeParameters (new string [] {"T extends java.lang.Runnable"})]

This brings us to what I believe is your actual question: what do we do about constructed types? The original example had T as the type parameter, not Constructed<T>.

The answer will be glib, but: we should choose a representation which makes the Java Callable Wrapper update as simple and understandable as possible.

That could mean emitting:

partial class GenericMethodClass {
    public unsafe void NestedGeneric (
        [global::Java.Interop.JavaTypeParameter ("java.lang.Iterable<T>")]Java.Lang.IIterable p0) { ... }
}

(as we usually use Java or JNI names in these attributes, not C# names), or we may want to break it into pieces:

namespace Java.Interop
    [AttributeUsage (AttributeTargets.Parameter | AttributeTargets.Property | AttributeTargets.ReturnValue)]
    public sealed class JavaConstructedTypeAttribute : Attribute {
        public JavaConstructedTypeAttribute (string jniTypeName, string[] jniTypeParameters, string[] jniTypeArguments);
    }
}

allowing:

partial class GenericMethodClass {
    public unsafe void NestedGeneric (
        [global::Java.Interop. JavaConstructedType ("java/lang/Iterable", new[]{"T"}, new []{"java/lang/Object"})]
        Java.Lang.IIterable p0) { ... }
}

My only concern with this is it might make for "custom attribute bloat" and increase the size of the Mono.Android.dll that we ship int he installer, but that's not a huge concern because we should be able to remove these attributes within applications, resulting in no net .apk size increase.

jonpryor avatar Oct 28 '20 18:10 jonpryor

Hi, I am facing the same issue as I'm trying to customize the TabLayoutMediator class which is sealed. I ported the Java source code but am now getting the above error when trying to build my project.

I tried to create a Java class as mentioned in Workaround #2. Unfortunately, my app is crashing when I want to open the page where the TabLayoutMediator is used: Java.Lang.ClassNotFoundException: 'Didn't find class "com.company.tablayout.OnTabSelectedListener" on path:

The Java class and my custom TabLayoutMediator are defined in a Xamarin.Android library because I need to access it from different Xamarin.Android apps.

Am I missing something? Files.zip

@jonpryor @jpobst any idea?

Pantheas avatar Dec 01 '20 13:12 Pantheas

The workaround did not work for me either. At the moment I cannot get an event when the user re-selects the currently selected tab. Having tried for a few hours, I'm going to suggest my team waits for this issue to be resolved.

JesperNJessen avatar Mar 17 '21 13:03 JesperNJessen

Having same issue.

JeroenBer avatar Apr 25 '21 11:04 JeroenBer

Is there anything happening with this issue? Still exists.

KrzysztofFryzlewicz avatar May 05 '21 16:05 KrzysztofFryzlewicz

Any update here?

swirek avatar May 06 '21 08:05 swirek

There is no update on this issue.

The easiest workaround is to continue to use IOnTabSelectedListener instead of IOnTabSelectedListener2.

jpobst avatar May 06 '21 15:05 jpobst

@jpobst Archiving android app with target API 29 will fail since IOnTabSelectedListener is obsolete.

KrzysztofFryzlewicz avatar May 07 '21 06:05 KrzysztofFryzlewicz

I'm not sure I understand why it would fail? Using an obsolete API should just be a warning, not an error?

jpobst avatar May 07 '21 13:05 jpobst

It should, but it blocks the archiving process

KrzysztofFryzlewicz avatar May 07 '21 13:05 KrzysztofFryzlewicz

Do you have some form of treating warnings as errors turned on for your Release configuration? I don't think this should happen by default. For example <TreatWarningsAsErrors>.

jpobst avatar May 07 '21 14:05 jpobst

Good point, I rest my case.

KrzysztofFryzlewicz avatar May 07 '21 14:05 KrzysztofFryzlewicz