[Mono.Android] Bind Android 15 DP 2

[jpobst]

Context: https://developer.android.com/about/versions/15
Context: https://android-developers.googleblog.com/2024/03/the-second-developer-preview-of-android-15.html

Android 15 Developer Preview 2 has been released. The Android 15
Developer Preview Program Overview Timeline and updates section
suggests the following timeline:

• Feb/Mar: Developer Previews
• April/May: Unstable Betas
• June/July: Stable Betas
• ???: Final

This will be usable in its preview form to .NET 9 Preview users who explicitly target net9.0-android35.

PublicApiAnalyzers reports 3 categories of API changes (PublicAPI.Shipped.txt):

• NRT attribute changes (? -> !)
• Method parameter name changes, technically a source breaking change, but something we've never tracked or fixed before
• Some methods went from virtual to override due to implementation being provided by base class

[jonpryor]

I don't understand this comment. Do you mean the body of n_Reversed()? If so, should the comment be moved there?

[jpobst]

Moved.

[jonpryor]

This feels like we'll wind up having two Reversed methods in the inheritance tree, a'la:

interface ISequencedCollection {
    ISequencedCollection Reversed();
}
interface List : ISequencedCollection {
    ISequencedCollection Reversed();
}

If correct, shouldn't we instead "reabstract" IList.Reversed()? dotnet/java-interop@22d5687

…and ditto most of the subsequent managedReturn changes?

[jpobst]

This is how it looks in Java:

public interface SequencedCollection<E> extends Collection<E> {
  SequencedCollection<E> reversed();
}

public interface List<E> extends SequencedCollection<E>, Collection<E> {
  default List<E> reversed() {
    throw new RuntimeException("Stub!");
  }
}

This is just a normal "provide a default implementation for an inherited interface method".

I think reabstraction is for the opposite case: when we want to "hide" a default implementation and make it abstract again.

[jonpryor]

Aside: This is a matter of "leaky abstraction I'd rather not make worse". C# permits "explicitly implementing interfaces", which is required for things like IEnumerable.GetEnumerator() vs. Enumerable<T>.GetEnumeartor() (same name! Different semantics! Supported via explicitly implementing interface members).

Java does not have an equivalent to explicit interface implementation. All method names must have the same semantics and "compatible" return types.

This is a "leaky abstraction" that we've long allowed to happen, largely because we had no way to address it other than <remove-node/> (which perhaps we should have done?), which results in things like:

namespace Java.Util;
partial interface ICollection {
    bool IsEmpty {get;}
    bool Add (Object? e);
    // …
}
partial interface IList : ICollection {
    // same methods
    bool IsEmpty {get;}
    bool Add (Object? e);
}

Things are "fine" so long as you don't explicitly implement them. If you do:

class MyList : Java.Lang.Object, Java.Util.IList {
    bool Java.Util.ICollection.IsEmpty { get => true; }
    bool Java.Util.IList.IsEmpty { get => false; }
}

which do you get? There Can Be Only One™.

Answer: IList.IsEmpty, because it's the most derived type, as per the JCW:

		__md_methods = 
			"n_isEmpty:()Z:GetIsEmptyHandler:Java.Util.IListInvoker, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null\n" +

So it "works", but is not necessarily ideal.

Thus, the question: What is ideal, and can we get closer to it?

Given that our current C# approach results in numerous C# warnings that we have to ignore (CS0114), I think that the ideal scenerio would be to "reabstract" the members:

namespace Java.Util;
interface IList : ICollection {
    abstract bool ICollection.Add(Java.Lang.Object? e);
    abstract bool ICollection.IsEmpty {get;}
}

This would allow you to explicitly implement e.g. only ICollection.Add(), and not IList.Add(), as they're now "flattened"/"unified".

We can't do this for existing members (API & ABI break).

We can do this for newly added members, if/when appropriate.

On to default method implementations: C# supports this as well, again via explicit interface implementation:

namespace Java.Util;
interface ICollection {
    // has a body for back-combat reasons
    ISequencedCollection Reversed() => throw new NotImplementedException();
}
interface IList : ICollection {
    IList ICollection.Reversed() => _members.InstanceMethods.InvokeAbstracMethod(…);
}

[jonpryor]

On Discord, @jpobst asks:

i'm not sure if your latest comment is suggesting a change? or just opining? 😁

Er, yes.

Background info, plus opinion on where we should consider going, neither of which is directly relevant for .Reversed(). The comment is thus not actionable.

My bad.

However, now that I have that background thinking in place, it allows me to point to:

• SequencedCollection and/vs
• List

Both declare getFirst()/getLast()/removeFirst()/removeLast()/reversed() methods, suggesting -- as I haven't build this PR locally, I just believe that this will likely be the case -- that the binding with this PR as-is will contain:

namespace Java.Util;
partial interface ISequencedCollection {
    void AddFirst(Object? e);
    void AddLast(Object? e);
    Object? GetFirst();
    Object? GetLast();
    // …
}

partial interface IList : ISequencedCollection {
    void AddFirst(Object? e);  // CS0114 warning
    void AddLast(Object? e);   // CS0114 warning
    Object? GetFirst();        // CS0114 warning
    Object? GetLast();         // CS0114 warning
    // …
}

and I ask: should it be this way?

It's been this way, so it's not "broken" per-se, but it doesn't feel "entirely proper" either.

There are two solutions for making this "more proper":

1. Use <remove-node/> to remove the of identically declared methods from List.
2. Use reabstract metadata to declare the IList methods as abstract and explicltly declare them.

Or (3), update generator to internally handle (1) or (2). :-).
Which becomes all the more important/required given that we don't know how many other interfaces this impacts (though SequencedMap/Map & SequencedSet/Set also hit it, minimum). (For comparison, if I stop ignoring CS0114 in Java.Base, there are 61 warnings emitted that I now feel shouldn't be emitted, and that's a very small subset of java.base.jmod…)

This comment is thus deeply in "the perfect is the enemy of the good" territory, mind you…

[jonpryor]

…so where does that leave me?

If we were doing it all over again -- cough Java.Base cough -- then I'd want the CS0114 warnings to not be emitted by tooling, either by removing identically declared overriding members or by re-abstracting them.

However, we're not doing it all over again, we have no tooling to assist in finding these situations (other than re-enabling CS0114, which would introduce 686 warnings!), so manually fixing these is a non-starter. We could update generator, but I'm not certain that we have the time, and this would need to be an "opt-in" construct, somehow, so that we don't re-abstract or remove previously declared methods (though API-compat is a good backstop for this).

I thus think that we can't/shouldn't attempt to address this right now. :-(

[jonpryor]

Attempted to clarify my thinking here: dotnet/java-interop#1215

[jpobst]

I think my sticking point with understanding this is that the new methods defined in IList are default methods, so I'm not sure that I comprehend marking them abstract with reabstraction.

IE:

namespace Java.Util;
partial interface ISequencedCollection {
    void AddFirst(Object? e);
    void AddLast(Object? e);
    Object? GetFirst();
    Object? GetLast();
    // …
}

partial interface IList : ISequencedCollection {
    void AddFirst(Object? e) { // implementation };  // CS0108 warning
    void AddLast(Object? e); { // implementation };   // CS0108 warning
    Object? GetFirst(); { // implementation };       // CS0108 warning
    Object? GetLast(); { // implementation };         // CS0108 warning
    // …
}

Aside, it's a CS0108 warning instead of CS0114.

CS0108 'IList.AddFirst(Object?)' hides inherited member 'ISequencedCollection.AddFirst(Object?)'. Use the new keyword if hiding was intended.

I think CS0108 is the point you are getting at: we aren't "providing an default implementation of ISequencedCollection.AddFirst", we are declaring a new AddFirst that hides the original one.

However we appear to actually handle this correctly for the specific case where a DIM is providing an implementation for an inherited interface method, as this is what we generate:

partial interface IList : ISequencedCollection {
    void Java.Util.AddFirst(Object? e) { // implementation };
    void Java.Util.AddLast(Object? e); { // implementation };
    Object? Java.Util.GetFirst(); { // implementation };
    Object? Java.Util.GetLast(); { // implementation };
    // …
}

This prevents the CS0108 warning and provides the correct "provide a default implementation" semantics rather than hiding the inherited method.

Looking at other existing methods in IList/ICollection I do see what you are talking about:

public partial interface ICollection
    bool AddAll (System.Collections.ICollection c);
}

public partial interface IList : Java.Util.ICollection {
    bool AddAll (System.Collections.ICollection c);  // CS0108
}

Fixing that (which we do not do today) indeed would be:

abstract bool ICollection.AddAll (System.Collections.ICollection c);

To your point, without a way to detect these cases currently, we do not know if API-35 adds any additional cases that we could manually fix with reabstraction or removal.

[jonpryor]

This might be another case for "reabstraction". Though ABI-wise, i think removing this method should also work.

[jpobst]

In Java, this is:

public interface SequencedCollection<E> extends Collection<E> {
  SequencedCollection<E> reversed();
}

public interface SequencedSet<E> extends SequencedCollection<E>, Set<E> {
  SequencedSet<E> reversed();
}

C# does not support covariant return type changes from implemented interfaces. We could fix the covariant return type, but then the methods are identical, so I just removed it instead.