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

Interface methods that another interface provides a default implementation for shouldn't be made into class abstract methods #1264

Open
jpobst opened this issue Oct 11, 2024 · 0 comments
Labels
bug Component does not function as intended generator Issues binding a Java library (generator, class-parse, etc.)

Comments

@jpobst
Copy link
Contributor

jpobst commented Oct 11, 2024

Context: kotlin.collections.AbstractList

Imagine we have an interface with a method, and another interface that implements that interface and provides a default implementation:

package java.util;

public interface SequencedCollection {
  SequencedCollection reverse ();
}

public interface List implements SequencedCollection {
  default SequencedCollection reversed() { ... }
}

If we have a class that implements List and does not provide its own implementation of reversed, when generator loops through all implemented interfaces, it will make an abstract Reverse method because it cannot detect that another interface provides a compatible default method.

// java
public abstract class AbstractList implements SequencedCollection { ... }

// csharp
public abstract class AbstractList : ISequencedCollection {
  public abstract ISequencedCollection Reversed ();
}

This causes issues for any bound Java class that inherits AbstractList, as they are expected to provide a Reversed method, when they don't provide one in the Java implementation.

AndroidX.Paging.ItemSnapshotList.cs(21,30): error CS0534: 'ItemSnapshotList' does not implement inherited abstract 
member 'AbstractList.Reversed()'

This issue is made worse by the fact it cannot be fixed with any existing metadata.

@jpobst jpobst added bug Component does not function as intended generator Issues binding a Java library (generator, class-parse, etc.) labels Oct 11, 2024
jonpryor pushed a commit that referenced this issue Oct 17, 2024
Context: #1264
Context: 73ebad2
Context: a7e09b7
Context: dotnet/android@00256b6

Consider [`java.util.List<E>`][0], which implements
[`java.util.SequencedCollection<E>`][1] and provides an interface
default method to implement `SequencedCollection<E>.reversed()`:

	// Java
	package java.util;
	public /* partial */ interface SequencedCollection<E> implements Collection<E> {
	  SequencedCollection<E> reverse ();
	}
	public /* partial */ interface List<E> implements SequencedCollection<E> {
	  default List<E> reversed() { … }
	}

We then *bind* the above, along with some manual fixups in
dotnet/android@00256b69:

	// C# bindings
	public partial interface ISequencedCollection {
	  [Register ("reversed", "()Ljava/util/SequencedCollection;", "GetReversedHandler:Java.Util.ISequencedCollectionInvoker, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null", ApiSince = 35)]
	  ISequencedCollection? Reversed ();
	}

	public partial interface IList : ISequencedCollection {
	  [Register ("reversed", "()Ljava/util/List;", "GetReversedHandler:Java.Util.IList, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null", ApiSince = 35)]
	  unsafe Java.Util.ISequencedCollection Java.Util.ISequencedCollection.Reversed () => …
	}

Next, we try to bind [`androidx.paging:paging-common-android`][2].
[`androidx.paging.ItemSnapshotList`][3] inherits from
[`kotlin.collections.AbstractList`][4]:

	// Kotlin
	package kotlin.collections;
	public abstract /* partial */ class AbstractList<out E> protected constructor() :
	    kotlin.collections.AbstractCollection<E>(), java.util.List<E> {
	}

In a scenario that rhymes with 73ebad2, `generator` with
`Mono.Android.dll` (bindings for `Java.Util.IList`) cannot determine
that the `Kotlin.Collections.AbstractList` type has a `Reversed()`
method (via `Java.Util.IList`), and so re-declares it:

	// C#
	public partial class AbstractList : Kotlin.Collections.AbstractCollection, Java.Util.IList {
	  public abstract ISequencedCollection Reversed ();
	}

We fear that a complete fix for this may be complicated, as we would
have to determine if each interface method has a compatible default
interface method on any other interface in scope.

Introduce new `skipInterfaceMethods` metadata, with identical
semantics to `skipInvokerMethods` (73ebad2), to allow specifying
`interface`-derived methods which should be *skipped* when generating
bindings:

	<attr
	    path="/api/package[@name='kotlin.collections']/class[@name='AbstractList']"
	    name="skipInterfaceMethods">
	  java/util/SequencedCollection.reversed()Ljava/util/SequencedCollection;
	</attr>

The above fragment would prevent `Reversed()` from being declared in
the binding for `Kotlin.Collections.AbstractList`.

[0]: https://developer.android.com/reference/java/util/List
[1]: https://developer.android.com/reference/java/util/SequencedCollection
[2]: https://maven.google.com/web/index.html?q=paging#androidx.paging:paging-common-android
[3]: https://developer.android.com/reference/androidx/paging/ItemSnapshotList
[4]: https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.collections/-abstract-list/
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 generator Issues binding a Java library (generator, class-parse, etc.)
Projects
None yet
Development

No branches or pull requests

1 participant