Commit ef092a8
[generator] All bound interfaces should inherit IJavaPeerable (dotnet#455)
Context: dotnet#341
Context: dotnet#341 (comment)
Context: dotnet#25
Java 8 introduced support for [interface default methods][0]:
// Java
public interface Example {
int defaultInterfaceMethod() {
return 42;
}
}
The question is, how should we bind them?
Currently, we *don't* bind interface default methods:
// C#
public interface IExample : IJavaObject {
// No `DefaultInterfaceMethod()` method
}
This means that C# types which implement `IExample` don't need to
implement `Example.defaultInterfaceMethod()`, reducing the work that
needs to be done:
// C#
public class MyExample : Java.Lang.Object, IExample {
// No need to implement Example.defaultInterfaceMethod()!
}
However, if a C# type *does* wish to implement
`Example.defaultInterfaceMethod()`, they *can* do so by using
[`ExportAttribute`][1], but this can be painful, as it requires
ensuring that the Java side and C# sides are consistent, without
compiler assistance:
// C#
partial class MyExample : Java.Lang.Object, IExample {
[Java.Interop.Export ("defaultInterfaceMethod")]
// If the above string is wrong, e.g. via typo, the method isn't overridden!
public int DefaultInterfaceMethod()
{
return 42*2;
}
}
We want to improve support for this scenario by binding Java
interface default methods as [C#8 Default Interface Members][2],
as C#8 Default Interface Methods have similar semantics as Java 8
interface default methods, and means that `[ExportAttribute]` would
no longer be necessary to override them:
// C#8?
partial class MyExample : Java.Lang.Object, IExample {
// Just Works™, and the compiler will complain if there are typos.
public override int DefaultInterfaceMethod()
{
return 42*2;
}
}
However, a question arises: how do we *do* that?
// C#8
public interface IExample : IJavaObject {
[Register ("defaultInterfaceMethod", "()I", ...)]
int DefaultInterfaceMethod()
{
// How do we call `Example.defaultInterfaceMethod()` here?
}
}
The desirable thing would be for `IExample.DefaultInterfaceMethod()`
to have the same implementation as any other method binding, e.g.
when using `generator --codegen-target=XAJavaInterop1`:
// C#8
partial interface IExample {
[Register ("defaultInterfaceMethod", "()I", ...)]
void DefaultInterfaceMethod()
{
const string __id = "defaultInterfaceMethod.()I";
return _members.InstanceMethods.InvokeVirtualInt32Method (__id, this, null);
}
}
The problem is twofold:
1. There is no `_members` field to access here, and
2. Even if there were a `_members` field,
`JniPeerMembers.JniInstanceMethods.InvokeVirtualInt32Method()`
requires an `IJavaPeerable` instance, and `IExample` doesn't
implement `IJavaPeerable`.
(1) is straight forwardly solvable, as C#8 Default Interface Members
also adds support for static members of all sorts, so it should be
possible to add a static `_members` field, if deemed necessary.
(2) is the holdup, and has a simple solution: "Just" have every
bound interface *also* implement `IJavaPeerable`!
- public partial interface IExample : IJavaObject
+ public partial interface IExample : IJavaObject, IJavaPeerable
"But!", someone cries, "What about API compatibility?!"
Isn't adding `IJavaPeerable` to the implements list of an interface a
Breaking Change?
In theory and *most* practice, *Yes*, this *would* be a
Breaking Change, and thus something to be avoided.
*However*, Xamarin.Android is *not* "most" practice.
It has been an XA4212 error since ab3c2b2 / Xamarin.Android 8.0 to
implement an interface that implements `IJavaObject` *without*
inheriting from `Java.Lang.Object` or `Java.Lang.Throwable`:
// elicits XA4212 error
class MyBadClass : IExample {
// Implements IJavaObject.Handle
public IntPtr Handle {
get {return IntPtr.Zero;}
}
}
Meanwhile, both `Java.Lang.Object` and `Java.Lang.Throwable` have
implemented `IJavaPeerable` since Xamarin.Android *6.1*.
*Because* it has been an error to manually implement `IJavaObject`
for nearly two years now, it should be Reasonably Safe™ to update
interfaces to implement *both* `IJavaObject` *and* `IJavaPeerable`.
Doing so should not break any code -- unless they've overridden the
`$(AndroidErrorOnCustomJavaObject)` MSBuild property to False, in
which case they deserve what happens to them. (It's not really
possible to implement `IJavaObject` in a sane manner, and the
"straightforward" approach means that passing instances of e.g.
`MyBadClass` to Java code will result in passing *`null`* to Java,
which almost certainly is NOT what is intended, hence XA4212!)
[0]: https://docs.oracle.com/javase/tutorial/java/IandI/defaultmethods.html
[1]: https://docs.microsoft.com/en-us/xamarin/android/platform/java-integration/working-with-jni#exportattribute-and-exportfieldattribute
[2]: https://github.com/dotnet/csharplang/blob/f7952cdddf85316a4beec493a0ecc14fcb3241c8/proposals/csharp-8.0/default-interface-methods.md1 parent be58159 commit ef092a8
File tree
21 files changed
+86
-19
lines changed- tools/generator
- Java.Interop.Tools.Generator.CodeGeneration
- Tests-Core/expected.ji
- Tests
- Unit-Tests
- CodeGeneratorExpectedResults
- JavaInterop1
- XamarinAndroid
- expected.ji
- Adapters
- GenericArguments
- InterfaceMethodsConflict
- NestedTypes
- TestInterface
- java.lang.Enum
21 files changed
+86
-19
lines changedLines changed: 3 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
22 | 22 | | |
23 | 23 | | |
24 | 24 | | |
| 25 | + | |
| 26 | + | |
25 | 27 | | |
26 | 28 | | |
27 | 29 | | |
| |||
522 | 524 | | |
523 | 525 | | |
524 | 526 | | |
525 | | - | |
| 527 | + | |
526 | 528 | | |
527 | 529 | | |
528 | 530 | | |
| |||
Lines changed: 1 addition & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
23 | 23 | | |
24 | 24 | | |
25 | 25 | | |
| 26 | + | |
26 | 27 | | |
27 | 28 | | |
28 | 29 | | |
| |||
Lines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
10 | | - | |
| 10 | + | |
11 | 11 | | |
12 | 12 | | |
13 | 13 | | |
| |||
Lines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
11 | 11 | | |
12 | 12 | | |
13 | 13 | | |
14 | | - | |
| 14 | + | |
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
| |||
Lines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
30 | 30 | | |
31 | 31 | | |
32 | 32 | | |
33 | | - | |
| 33 | + | |
34 | 34 | | |
35 | 35 | | |
36 | 36 | | |
| |||
Lines changed: 39 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
Lines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | | - | |
| 3 | + | |
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
| |||
Lines changed: 11 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
Lines changed: 11 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
0 commit comments