Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't use TabLayout.IOnTabSelectedListener2 #590

Open
PureWeen opened this issue Mar 3, 2020 · 19 comments
Open

Can't use TabLayout.IOnTabSelectedListener2 #590

PureWeen opened this issue Mar 3, 2020 · 19 comments
Assignees
Labels
bug Component does not function as intended callable-wrappers Issues with Java Callable Wrappers

Comments

@PureWeen
Copy link
Member

PureWeen commented Mar 3, 2020

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

@jonpryor
Copy link
Member

jonpryor commented Mar 3, 2020

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 jonpryor transferred this issue from xamarin/AndroidX Mar 3, 2020
@jonpryor
Copy link
Member

jonpryor commented Mar 3, 2020

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
Copy link
Member

jonpryor commented Mar 3, 2020

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.

@jpobst jpobst added callable-wrappers Issues with Java Callable Wrappers bug Component does not function as intended labels Apr 6, 2020
@jonpryor
Copy link
Member

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 jonpryor assigned jpobst and unassigned moljac Jun 17, 2020
moljac added a commit to xamarin/AndroidX that referenced this issue Jun 19, 2020
binding a new Java class which has the appropriate inheritance hierarchy
Addedd class TabLayoutAppCompatActivity
dotnet/java-interop#590
@moljac moljac self-assigned this Jun 19, 2020
jonpryor added a commit to jonpryor/xamarin-android that referenced this issue Jun 19, 2020
Context: dotnet/java-interop#590

Without the Java.Interop bump, fails with:

  obj/Debug/android/src/crc64eadbb4d7d7a577cd/JI590.java(4,8): javac error JAVAC0000:  error: JI590 is not abstract and does not override abstract method invoke(MyRunnable) in InvokeRunnable [/Volumes/Xamarin-Work/xamarin-android/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/Xamarin.Android.JcwGen-Tests.csproj]
obj/Debug/android/src/crc64eadbb4d7d7a577cd/JI590.java(4,8): javac error JAVAC0000: public class JI590 [/Volumes/Xamarin-Work/xamarin-android/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/Xamarin.Android.JcwGen-Tests.csproj]
obj/Debug/android/src/crc64eadbb4d7d7a577cd/JI590.java(4,8): javac error JAVAC0000:  [/Volumes/Xamarin-Work/xamarin-android/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/Xamarin.Android.JcwGen-Tests.csproj]
  obj/Debug/android/src/crc64eadbb4d7d7a577cd/JI590.java(29,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 [/Volumes/Xamarin-Work/xamarin-android/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/Xamarin.Android.JcwGen-Tests.csproj]
obj/Debug/android/src/crc64eadbb4d7d7a577cd/JI590.java(29,14): javac error JAVAC0000: 	public void invoke (java.lang.Runnable p0) [/Volumes/Xamarin-Work/xamarin-android/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/Xamarin.Android.JcwGen-Tests.csproj]
obj/Debug/android/src/crc64eadbb4d7d7a577cd/JI590.java(29,14): javac error JAVAC0000:  [/Volumes/Xamarin-Work/xamarin-android/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/Xamarin.Android.JcwGen-Tests.csproj]
@jonpryor
Copy link
Member

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
Copy link
Member

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.

@jpobst
Copy link
Contributor

jpobst commented Oct 28, 2020

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) { ... }

@jonpryor
Copy link
Member

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.

@Pantheas
Copy link

Pantheas commented Dec 1, 2020

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?

@JesperNJessen
Copy link

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.

@JeroenBer
Copy link

Having same issue.

@KrzysztofFryzlewicz
Copy link

Is there anything happening with this issue? Still exists.

@swirek
Copy link

swirek commented May 6, 2021

Any update here?

@jpobst
Copy link
Contributor

jpobst commented May 6, 2021

There is no update on this issue.

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

@KrzysztofFryzlewicz
Copy link

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

@jpobst
Copy link
Contributor

jpobst commented May 7, 2021

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

@KrzysztofFryzlewicz
Copy link

It should, but it blocks the archiving process

@jpobst
Copy link
Contributor

jpobst commented May 7, 2021

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>.

@KrzysztofFryzlewicz
Copy link

Good point, I rest my case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Component does not function as intended callable-wrappers Issues with Java Callable Wrappers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants