Skip to content

Commit 12e670a

Browse files
jpobstjpobst
authored
[generator] Support binding non-const Java interface fields (#831)
Fixes: #821 Context: 29f9707 Context: #509 Context: dotnet/android#5818 When we started generating static methods and properties on interfaces when running with C#8 default interface members enabled, it looks like we did not include static fields that cannot be treated as constants. Thus, consider the [EGL10][0] interface mentioned in dotnet/android#5818: // Java /* partial */ interface EGL10 { public static final int EGL_ALPHA_FORMAT = 12424; public static final EGLSurface EGL_NO_SURFACE = … } The [`EGL10.EGL_NO_SURFACE`][1] field is not a compile-time constant. In C# parlance, it's `readonly`, *not* `const`. In commit 29f9707, we *ignored* `interface` fields which: 1. Were read+write, or 2. Did not contain compile-time constant values. Thus, we generated: // C# binding partial interface IGL10 { public const int EglAlphaFormat = (int) 12424; } This is an issue because we `[Obsolete]`'d the members in the `EGL10` "proxy class", *and* referred to a non-existent `interface` member: // C# binding; note class, not interface [Obsolete ("Use the 'Javax.Microedition.Khronos.Egl.IEGL10' type. This class will be removed in a future release.")] public abstract partial class EGL10 { [Obsolete ("Use 'Javax.Microedition.Khronos.Egl.IEGL10.EglAlphaFormat'. …")] public const int EglAlphaFormat = 12424; [Obsolete ("Use 'Javax.Microedition.Khronos.Egl.IEGL10.EglNoSurface'. …")] public static Javax.Microedition.Khronos.Egl.EGLSurface? EglNoSurface => … } `EGL10.EglNoSurface` directs us to `IEGL10.EglNoSurface`, while `IEGL10.EglNoSurface` doesn't exist! This is a bad user experience. Fix generator so that when `generator -lang-features=default-interface-methods` is used, non-`const` interface members are also emitted, allowing: // C# binding; fixed partial interface IEGL10 { public static EGLSurface? EglNoSurface => … } Should a Java interface contain a mutable field, those can now be bound as well: // Java interface Example { public static int MUTABLE; } // C# binding partial interface IExample { public static int Mutable { get => … set => … } } [0]: https://developer.android.com/reference/javax/microedition/khronos/egl/EGL10 [1]: https://developer.android.com/reference/javax/microedition/khronos/egl/EGL10#EGL_NO_SURFACE
1 parent c925b78 commit 12e670a

File tree

7 files changed

+316
-5
lines changed

7 files changed

+316
-5
lines changed

‎tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/ObsoleteInterfaceAlternativeClass.txt‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,18 @@ public partial interface IParent : IJavaObject, IJavaPeerable {
7272
[Obsolete ("deprecated")]
7373
public const string AlreadyObsolete = (string) "android.permission.ACCEPT_HANDOVER";
7474

75+
76+
// Metadata.xml XPath field reference: path="/api/package[@name='com.xamarin.android']/interface[@name='Parent']/field[@name='API_NAME']"
77+
[Register ("API_NAME")]
78+
public static string ApiName {
79+
get {
80+
const string __id = "API_NAME.Ljava/lang/String;";
81+
82+
var __v = _members.StaticFields.GetObjectValue (__id);
83+
return JNIEnv.GetString (__v.Handle, JniHandleOwnership.TransferLocalRef);
84+
}
85+
}
86+
7587
// Metadata.xml XPath method reference: path="/api/package[@name='com.xamarin.android']/interface[@name='Parent']/method[@name='comparing' and count(parameter)=0]"
7688
[Register ("comparing", "()I", "")]
7789
public static unsafe int Comparing ()

‎tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteInterfaceFieldAsDimProperty.txt‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
[Register ("com/xamarin/android/MyInterface", DoNotGenerateAcw=true)]
2+
public abstract class MyInterface : Java.Lang.Object {
3+
internal MyInterface ()
4+
{
5+
}
6+
7+
// Metadata.xml XPath field reference: path="/api/package[@name='com.xamarin.android']/interface[@name='MyInterface']/field[@name='EGL_NATIVE_VISUAL_ID']"
8+
[Register ("EGL_NATIVE_VISUAL_ID")]
9+
public const int EglNativeVisualId = (int) 12334;
10+
11+
12+
// Metadata.xml XPath field reference: path="/api/package[@name='com.xamarin.android']/interface[@name='MyInterface']/field[@name='EGL_NO_SURFACE']"
13+
[Register ("EGL_NO_SURFACE")]
14+
public static int EglNoSurface {
15+
get {
16+
const string __id = "EGL_NO_SURFACE.I";
17+
18+
var __v = _members.StaticFields.GetInt32Value (__id);
19+
return __v;
20+
}
21+
set {
22+
const string __id = "EGL_NO_SURFACE.I";
23+
24+
try {
25+
_members.StaticFields.SetValue (__id, value);
26+
} finally {
27+
}
28+
}
29+
}
30+
31+
static readonly JniPeerMembers _members = new JniPeerMembers ("com/xamarin/android/MyInterface", typeof (MyInterface));
32+
33+
}
34+
35+
[Register ("com/xamarin/android/MyInterface", DoNotGenerateAcw=true)]
36+
[global::System.Obsolete ("Use the 'MyInterface' type. This type will be removed in a future release.", error: true)]
37+
public abstract class MyInterfaceConsts : MyInterface {
38+
private MyInterfaceConsts ()
39+
{
40+
}
41+
42+
}
43+
44+
// Metadata.xml XPath interface reference: path="/api/package[@name='com.xamarin.android']/interface[@name='MyInterface']"
45+
[Register ("com/xamarin/android/MyInterface", "", "Com.Xamarin.Android.IMyInterfaceInvoker")]
46+
public partial interface IMyInterface : IJavaObject, IJavaPeerable {
47+
private static readonly JniPeerMembers _members = new JniPeerMembers ("com/xamarin/android/MyInterface", typeof (IMyInterface), isInterface: true);
48+
49+
50+
// Metadata.xml XPath field reference: path="/api/package[@name='com.xamarin.android']/interface[@name='MyInterface']/field[@name='EGL_NO_SURFACE']"
51+
[Register ("EGL_NO_SURFACE")]
52+
public static int EglNoSurface {
53+
get {
54+
const string __id = "EGL_NO_SURFACE.I";
55+
56+
var __v = _members.StaticFields.GetInt32Value (__id);
57+
return __v;
58+
}
59+
set {
60+
const string __id = "EGL_NO_SURFACE.I";
61+
62+
try {
63+
_members.StaticFields.SetValue (__id, value);
64+
} finally {
65+
}
66+
}
67+
}
68+
69+
}
70+
71+
[global::Android.Runtime.Register ("com/xamarin/android/MyInterface", DoNotGenerateAcw=true)]
72+
internal partial class IMyInterfaceInvoker : global::Java.Lang.Object, IMyInterface {
73+
static readonly JniPeerMembers _members = new JniPeerMembers ("com/xamarin/android/MyInterface", typeof (IMyInterfaceInvoker));
74+
75+
static IntPtr java_class_ref {
76+
get { return _members.JniPeerType.PeerReference.Handle; }
77+
}
78+
79+
[global::System.Diagnostics.DebuggerBrowsable (global::System.Diagnostics.DebuggerBrowsableState.Never)]
80+
[global::System.ComponentModel.EditorBrowsable (global::System.ComponentModel.EditorBrowsableState.Never)]
81+
public override global::Java.Interop.JniPeerMembers JniPeerMembers {
82+
get { return _members; }
83+
}
84+
85+
[global::System.Diagnostics.DebuggerBrowsable (global::System.Diagnostics.DebuggerBrowsableState.Never)]
86+
[global::System.ComponentModel.EditorBrowsable (global::System.ComponentModel.EditorBrowsableState.Never)]
87+
protected override IntPtr ThresholdClass {
88+
get { return class_ref; }
89+
}
90+
91+
[global::System.Diagnostics.DebuggerBrowsable (global::System.Diagnostics.DebuggerBrowsableState.Never)]
92+
[global::System.ComponentModel.EditorBrowsable (global::System.ComponentModel.EditorBrowsableState.Never)]
93+
protected override global::System.Type ThresholdType {
94+
get { return _members.ManagedPeerType; }
95+
}
96+
97+
IntPtr class_ref;
98+
99+
public static IMyInterface GetObject (IntPtr handle, JniHandleOwnership transfer)
100+
{
101+
return global::Java.Lang.Object.GetObject<IMyInterface> (handle, transfer);
102+
}
103+
104+
static IntPtr Validate (IntPtr handle)
105+
{
106+
if (!JNIEnv.IsInstanceOf (handle, java_class_ref))
107+
throw new InvalidCastException ($"Unable to convert instance of type '{JNIEnv.GetClassNameFromInstance (handle)}' to type 'com.xamarin.android.MyInterface'.");
108+
return handle;
109+
}
110+
111+
protected override void Dispose (bool disposing)
112+
{
113+
if (this.class_ref != IntPtr.Zero)
114+
JNIEnv.DeleteGlobalRef (this.class_ref);
115+
this.class_ref = IntPtr.Zero;
116+
base.Dispose (disposing);
117+
}
118+
119+
public IMyInterfaceInvoker (IntPtr handle, JniHandleOwnership transfer) : base (Validate (handle), transfer)
120+
{
121+
IntPtr local_ref = JNIEnv.GetObjectClass (((global::Java.Lang.Object) this).Handle);
122+
this.class_ref = JNIEnv.NewGlobalRef (local_ref);
123+
JNIEnv.DeleteLocalRef (local_ref);
124+
}
125+
126+
}

‎tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/ObsoleteInterfaceAlternativeClass.txt‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,18 @@ public partial interface IParent : IJavaObject, IJavaPeerable {
7272
[Obsolete ("deprecated")]
7373
public const string AlreadyObsolete = (string) "android.permission.ACCEPT_HANDOVER";
7474

75+
76+
// Metadata.xml XPath field reference: path="/api/package[@name='com.xamarin.android']/interface[@name='Parent']/field[@name='API_NAME']"
77+
[Register ("API_NAME")]
78+
public static string ApiName {
79+
get {
80+
const string __id = "API_NAME.Ljava/lang/String;";
81+
82+
var __v = _members.StaticFields.GetObjectValue (__id);
83+
return JNIEnv.GetString (__v.Handle, JniHandleOwnership.TransferLocalRef);
84+
}
85+
}
86+
7587
// Metadata.xml XPath method reference: path="/api/package[@name='com.xamarin.android']/interface[@name='Parent']/method[@name='comparing' and count(parameter)=0]"
7688
[Register ("comparing", "()I", "")]
7789
public static unsafe int Comparing ()

‎tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteInterfaceFieldAsDimProperty.txt‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
[Register ("com/xamarin/android/MyInterface", DoNotGenerateAcw=true)]
2+
public abstract class MyInterface : Java.Lang.Object {
3+
internal MyInterface ()
4+
{
5+
}
6+
7+
// Metadata.xml XPath field reference: path="/api/package[@name='com.xamarin.android']/interface[@name='MyInterface']/field[@name='EGL_NATIVE_VISUAL_ID']"
8+
[Register ("EGL_NATIVE_VISUAL_ID")]
9+
public const int EglNativeVisualId = (int) 12334;
10+
11+
12+
// Metadata.xml XPath field reference: path="/api/package[@name='com.xamarin.android']/interface[@name='MyInterface']/field[@name='EGL_NO_SURFACE']"
13+
[Register ("EGL_NO_SURFACE")]
14+
public static int EglNoSurface {
15+
get {
16+
const string __id = "EGL_NO_SURFACE.I";
17+
18+
var __v = _members.StaticFields.GetInt32Value (__id);
19+
return __v;
20+
}
21+
set {
22+
const string __id = "EGL_NO_SURFACE.I";
23+
24+
try {
25+
_members.StaticFields.SetValue (__id, value);
26+
} finally {
27+
}
28+
}
29+
}
30+
31+
static readonly JniPeerMembers _members = new XAPeerMembers ("com/xamarin/android/MyInterface", typeof (MyInterface));
32+
33+
}
34+
35+
[Register ("com/xamarin/android/MyInterface", DoNotGenerateAcw=true)]
36+
[global::System.Obsolete ("Use the 'MyInterface' type. This type will be removed in a future release.", error: true)]
37+
public abstract class MyInterfaceConsts : MyInterface {
38+
private MyInterfaceConsts ()
39+
{
40+
}
41+
42+
}
43+
44+
// Metadata.xml XPath interface reference: path="/api/package[@name='com.xamarin.android']/interface[@name='MyInterface']"
45+
[Register ("com/xamarin/android/MyInterface", "", "Com.Xamarin.Android.IMyInterfaceInvoker")]
46+
public partial interface IMyInterface : IJavaObject, IJavaPeerable {
47+
private static readonly JniPeerMembers _members = new XAPeerMembers ("com/xamarin/android/MyInterface", typeof (IMyInterface), isInterface: true);
48+
49+
50+
// Metadata.xml XPath field reference: path="/api/package[@name='com.xamarin.android']/interface[@name='MyInterface']/field[@name='EGL_NO_SURFACE']"
51+
[Register ("EGL_NO_SURFACE")]
52+
public static int EglNoSurface {
53+
get {
54+
const string __id = "EGL_NO_SURFACE.I";
55+
56+
var __v = _members.StaticFields.GetInt32Value (__id);
57+
return __v;
58+
}
59+
set {
60+
const string __id = "EGL_NO_SURFACE.I";
61+
62+
try {
63+
_members.StaticFields.SetValue (__id, value);
64+
} finally {
65+
}
66+
}
67+
}
68+
69+
}
70+
71+
[global::Android.Runtime.Register ("com/xamarin/android/MyInterface", DoNotGenerateAcw=true)]
72+
internal partial class IMyInterfaceInvoker : global::Java.Lang.Object, IMyInterface {
73+
static readonly JniPeerMembers _members = new XAPeerMembers ("com/xamarin/android/MyInterface", typeof (IMyInterfaceInvoker));
74+
75+
static IntPtr java_class_ref {
76+
get { return _members.JniPeerType.PeerReference.Handle; }
77+
}
78+
79+
[global::System.Diagnostics.DebuggerBrowsable (global::System.Diagnostics.DebuggerBrowsableState.Never)]
80+
[global::System.ComponentModel.EditorBrowsable (global::System.ComponentModel.EditorBrowsableState.Never)]
81+
public override global::Java.Interop.JniPeerMembers JniPeerMembers {
82+
get { return _members; }
83+
}
84+
85+
[global::System.Diagnostics.DebuggerBrowsable (global::System.Diagnostics.DebuggerBrowsableState.Never)]
86+
[global::System.ComponentModel.EditorBrowsable (global::System.ComponentModel.EditorBrowsableState.Never)]
87+
protected override IntPtr ThresholdClass {
88+
get { return class_ref; }
89+
}
90+
91+
[global::System.Diagnostics.DebuggerBrowsable (global::System.Diagnostics.DebuggerBrowsableState.Never)]
92+
[global::System.ComponentModel.EditorBrowsable (global::System.ComponentModel.EditorBrowsableState.Never)]
93+
protected override global::System.Type ThresholdType {
94+
get { return _members.ManagedPeerType; }
95+
}
96+
97+
IntPtr class_ref;
98+
99+
public static IMyInterface GetObject (IntPtr handle, JniHandleOwnership transfer)
100+
{
101+
return global::Java.Lang.Object.GetObject<IMyInterface> (handle, transfer);
102+
}
103+
104+
static IntPtr Validate (IntPtr handle)
105+
{
106+
if (!JNIEnv.IsInstanceOf (handle, java_class_ref))
107+
throw new InvalidCastException ($"Unable to convert instance of type '{JNIEnv.GetClassNameFromInstance (handle)}' to type 'com.xamarin.android.MyInterface'.");
108+
return handle;
109+
}
110+
111+
protected override void Dispose (bool disposing)
112+
{
113+
if (this.class_ref != IntPtr.Zero)
114+
JNIEnv.DeleteGlobalRef (this.class_ref);
115+
this.class_ref = IntPtr.Zero;
116+
base.Dispose (disposing);
117+
}
118+
119+
public IMyInterfaceInvoker (IntPtr handle, JniHandleOwnership transfer) : base (Validate (handle), transfer)
120+
{
121+
IntPtr local_ref = JNIEnv.GetObjectClass (((global::Java.Lang.Object) this).Handle);
122+
this.class_ref = JNIEnv.NewGlobalRef (local_ref);
123+
JNIEnv.DeleteLocalRef (local_ref);
124+
}
125+
126+
}

‎tests/generator-Tests/Unit-Tests/DefaultInterfaceMethodsTests.cs‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -462,5 +462,31 @@ public void GenerateProperNestedInterfaceSignatures ()
462462
Assert.True (generated.Contains ("GetOnActivityDestroyed_IHandler:Com.Xamarin.Android.Application/IActivityLifecycleInterface, MyAssembly"));
463463
Assert.False (generated.Contains ("GetOnActivityDestroyed_IHandler:Com.Xamarin.Android.Application.IActivityLifecycleInterface, MyAssembly"));
464464
}
465+
466+
[Test]
467+
public void WriteInterfaceFieldAsDimProperty ()
468+
{
469+
// Ensure we write interface fields that are not constant, and thus must be written as properties
470+
var xml = @"<api>
471+
<package name='com.xamarin.android' jni-name='com/xamarin/android'>
472+
<interface abstract='true' deprecated='not deprecated' final='false' name='MyInterface' static='false' visibility='public' jni-signature='Lcom/xamarin/android/MyInterface;'>
473+
<field deprecated='not deprecated' final='true' name='EGL_NATIVE_VISUAL_ID' jni-signature='I' static='true' transient='false' type='int' type-generic-aware='int' value='12334' visibility='public' volatile='false'></field>
474+
<field deprecated='not deprecated' final='false' name='EGL_NO_SURFACE' jni-signature='I' static='true' transient='false' type='int' type-generic-aware='int' visibility='public' volatile='false'></field>
475+
</interface>
476+
</package>
477+
</api>";
478+
479+
var gens = ParseApiDefinition (xml);
480+
var iface = gens.OfType<InterfaceGen> ().Single ();
481+
482+
var result = iface.Validate (options, new GenericParameterDefinitionList (), new CodeGeneratorContext ());
483+
Assert.True (result);
484+
485+
generator.WriteType (iface, string.Empty, new GenerationInfo (string.Empty, string.Empty, "MyAssembly"));
486+
487+
var generated = writer.ToString ();
488+
489+
Assert.AreEqual (GetTargetedExpected (nameof (WriteInterfaceFieldAsDimProperty)), writer.ToString ().NormalizeLineEndings ());
490+
}
465491
}
466492
}

‎tools/generator/Java.Interop.Tools.Generator.ObjectModel/InterfaceGen.cs‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -149,14 +149,23 @@ internal string GetEventDelegateName (Method m)
149149
// These are fields that we currently support generating on the interface with DIM
150150
public IEnumerable<Field> GetGeneratableFields (CodeGenerationOptions options)
151151
{
152-
if (!options.SupportInterfaceConstants)
153-
return Enumerable.Empty<Field> ();
152+
var fields = new List<Field> ();
154153

155-
return Fields.Where (f => !f.NeedsProperty && !(f.DeprecatedComment?.Contains ("constant will be removed") == true));
154+
// Constant fields
155+
if (options.SupportInterfaceConstants)
156+
fields.AddRange (Fields.Where (f => !f.NeedsProperty && !(f.DeprecatedComment?.Contains ("constant will be removed") == true)));
157+
158+
// Invoked fields exposed as properties
159+
if (options.SupportDefaultInterfaceMethods)
160+
fields.AddRange (Fields.Where (f => f.NeedsProperty && !(f.DeprecatedComment?.Contains ("constant will be removed") == true)));
161+
162+
return fields;
156163
}
157164

158165
public bool HasDefaultMethods => GetAllMethods ().Any (m => m.IsInterfaceDefaultMethod);
159166

167+
public bool HasFieldsAsProperties => Fields.Any (f => f.NeedsProperty);
168+
160169
public bool HasStaticMethods => GetAllMethods ().Any (m => m.IsStatic);
161170

162171
public bool IsConstSugar (CodeGenerationOptions options)

‎tools/generator/SourceWriters/BoundInterface.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,14 +153,14 @@ void AddInheritedInterfaces (InterfaceGen iface, CodeGenerationOptions opt)
153153

154154
void AddClassHandle (InterfaceGen iface, CodeGenerationOptions opt)
155155
{
156-
if (opt.SupportDefaultInterfaceMethods && (iface.HasDefaultMethods || iface.HasStaticMethods))
156+
if (opt.SupportDefaultInterfaceMethods && (iface.HasDefaultMethods || iface.HasStaticMethods || iface.HasFieldsAsProperties))
157157
Fields.Add (new PeerMembersField (opt, iface.RawJniName, iface.Name, true));
158158
}
159159

160160
void AddFields (InterfaceGen iface, CodeGenerationOptions opt, CodeGeneratorContext context)
161161
{
162162
// Interface fields are only supported with DIM
163-
if (!opt.SupportInterfaceConstants)
163+
if (!opt.SupportInterfaceConstants && !opt.SupportDefaultInterfaceMethods)
164164
return;
165165

166166
var seen = new HashSet<string> ();

0 commit comments

Comments
 (0)