Skip to content

[generator] Ensure non-constant static interface fields are generated as interface properties. #831

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

Merged
merged 1 commit into from
May 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,18 @@ public partial interface IParent : IJavaObject, IJavaPeerable {
[Obsolete ("deprecated")]
public const string AlreadyObsolete = (string) "android.permission.ACCEPT_HANDOVER";


// Metadata.xml XPath field reference: path="/api/package[@name='com.xamarin.android']/interface[@name='Parent']/field[@name='API_NAME']"
[Register ("API_NAME")]
public static string ApiName {
get {
const string __id = "API_NAME.Ljava/lang/String;";

var __v = _members.StaticFields.GetObjectValue (__id);
return JNIEnv.GetString (__v.Handle, JniHandleOwnership.TransferLocalRef);
}
}

// Metadata.xml XPath method reference: path="/api/package[@name='com.xamarin.android']/interface[@name='Parent']/method[@name='comparing' and count(parameter)=0]"
[Register ("comparing", "()I", "")]
public static unsafe int Comparing ()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
[Register ("com/xamarin/android/MyInterface", DoNotGenerateAcw=true)]
public abstract class MyInterface : Java.Lang.Object {
internal MyInterface ()
{
}

// Metadata.xml XPath field reference: path="/api/package[@name='com.xamarin.android']/interface[@name='MyInterface']/field[@name='EGL_NATIVE_VISUAL_ID']"
[Register ("EGL_NATIVE_VISUAL_ID")]
public const int EglNativeVisualId = (int) 12334;


// Metadata.xml XPath field reference: path="/api/package[@name='com.xamarin.android']/interface[@name='MyInterface']/field[@name='EGL_NO_SURFACE']"
[Register ("EGL_NO_SURFACE")]
public static int EglNoSurface {
get {
const string __id = "EGL_NO_SURFACE.I";

var __v = _members.StaticFields.GetInt32Value (__id);
return __v;
}
set {
const string __id = "EGL_NO_SURFACE.I";

try {
_members.StaticFields.SetValue (__id, value);
} finally {
}
}
}

static readonly JniPeerMembers _members = new JniPeerMembers ("com/xamarin/android/MyInterface", typeof (MyInterface));

}

[Register ("com/xamarin/android/MyInterface", DoNotGenerateAcw=true)]
[global::System.Obsolete ("Use the 'MyInterface' type. This type will be removed in a future release.", error: true)]
public abstract class MyInterfaceConsts : MyInterface {
private MyInterfaceConsts ()
{
}

}

// Metadata.xml XPath interface reference: path="/api/package[@name='com.xamarin.android']/interface[@name='MyInterface']"
[Register ("com/xamarin/android/MyInterface", "", "Com.Xamarin.Android.IMyInterfaceInvoker")]
public partial interface IMyInterface : IJavaObject, IJavaPeerable {
private static readonly JniPeerMembers _members = new JniPeerMembers ("com/xamarin/android/MyInterface", typeof (IMyInterface), isInterface: true);


// Metadata.xml XPath field reference: path="/api/package[@name='com.xamarin.android']/interface[@name='MyInterface']/field[@name='EGL_NO_SURFACE']"
[Register ("EGL_NO_SURFACE")]
public static int EglNoSurface {
get {
const string __id = "EGL_NO_SURFACE.I";

var __v = _members.StaticFields.GetInt32Value (__id);
return __v;
}
set {
const string __id = "EGL_NO_SURFACE.I";

try {
_members.StaticFields.SetValue (__id, value);
} finally {
}
}
}

}

[global::Android.Runtime.Register ("com/xamarin/android/MyInterface", DoNotGenerateAcw=true)]
internal partial class IMyInterfaceInvoker : global::Java.Lang.Object, IMyInterface {
static readonly JniPeerMembers _members = new JniPeerMembers ("com/xamarin/android/MyInterface", typeof (IMyInterfaceInvoker));

static IntPtr java_class_ref {
get { return _members.JniPeerType.PeerReference.Handle; }
}

[global::System.Diagnostics.DebuggerBrowsable (global::System.Diagnostics.DebuggerBrowsableState.Never)]
[global::System.ComponentModel.EditorBrowsable (global::System.ComponentModel.EditorBrowsableState.Never)]
public override global::Java.Interop.JniPeerMembers JniPeerMembers {
get { return _members; }
}

[global::System.Diagnostics.DebuggerBrowsable (global::System.Diagnostics.DebuggerBrowsableState.Never)]
[global::System.ComponentModel.EditorBrowsable (global::System.ComponentModel.EditorBrowsableState.Never)]
protected override IntPtr ThresholdClass {
get { return class_ref; }
}

[global::System.Diagnostics.DebuggerBrowsable (global::System.Diagnostics.DebuggerBrowsableState.Never)]
[global::System.ComponentModel.EditorBrowsable (global::System.ComponentModel.EditorBrowsableState.Never)]
protected override global::System.Type ThresholdType {
get { return _members.ManagedPeerType; }
}

IntPtr class_ref;

public static IMyInterface GetObject (IntPtr handle, JniHandleOwnership transfer)
{
return global::Java.Lang.Object.GetObject<IMyInterface> (handle, transfer);
}

static IntPtr Validate (IntPtr handle)
{
if (!JNIEnv.IsInstanceOf (handle, java_class_ref))
throw new InvalidCastException ($"Unable to convert instance of type '{JNIEnv.GetClassNameFromInstance (handle)}' to type 'com.xamarin.android.MyInterface'.");
return handle;
}

protected override void Dispose (bool disposing)
{
if (this.class_ref != IntPtr.Zero)
JNIEnv.DeleteGlobalRef (this.class_ref);
this.class_ref = IntPtr.Zero;
base.Dispose (disposing);
}

public IMyInterfaceInvoker (IntPtr handle, JniHandleOwnership transfer) : base (Validate (handle), transfer)
{
IntPtr local_ref = JNIEnv.GetObjectClass (((global::Java.Lang.Object) this).Handle);
this.class_ref = JNIEnv.NewGlobalRef (local_ref);
JNIEnv.DeleteLocalRef (local_ref);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,18 @@ public partial interface IParent : IJavaObject, IJavaPeerable {
[Obsolete ("deprecated")]
public const string AlreadyObsolete = (string) "android.permission.ACCEPT_HANDOVER";


// Metadata.xml XPath field reference: path="/api/package[@name='com.xamarin.android']/interface[@name='Parent']/field[@name='API_NAME']"
[Register ("API_NAME")]
public static string ApiName {
get {
const string __id = "API_NAME.Ljava/lang/String;";

var __v = _members.StaticFields.GetObjectValue (__id);
return JNIEnv.GetString (__v.Handle, JniHandleOwnership.TransferLocalRef);
}
}

// Metadata.xml XPath method reference: path="/api/package[@name='com.xamarin.android']/interface[@name='Parent']/method[@name='comparing' and count(parameter)=0]"
[Register ("comparing", "()I", "")]
public static unsafe int Comparing ()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
[Register ("com/xamarin/android/MyInterface", DoNotGenerateAcw=true)]
public abstract class MyInterface : Java.Lang.Object {
internal MyInterface ()
{
}

// Metadata.xml XPath field reference: path="/api/package[@name='com.xamarin.android']/interface[@name='MyInterface']/field[@name='EGL_NATIVE_VISUAL_ID']"
[Register ("EGL_NATIVE_VISUAL_ID")]
public const int EglNativeVisualId = (int) 12334;


// Metadata.xml XPath field reference: path="/api/package[@name='com.xamarin.android']/interface[@name='MyInterface']/field[@name='EGL_NO_SURFACE']"
[Register ("EGL_NO_SURFACE")]
public static int EglNoSurface {
get {
const string __id = "EGL_NO_SURFACE.I";

var __v = _members.StaticFields.GetInt32Value (__id);
return __v;
}
set {
const string __id = "EGL_NO_SURFACE.I";

try {
_members.StaticFields.SetValue (__id, value);
} finally {
}
}
}

static readonly JniPeerMembers _members = new XAPeerMembers ("com/xamarin/android/MyInterface", typeof (MyInterface));

}

[Register ("com/xamarin/android/MyInterface", DoNotGenerateAcw=true)]
[global::System.Obsolete ("Use the 'MyInterface' type. This type will be removed in a future release.", error: true)]
public abstract class MyInterfaceConsts : MyInterface {
private MyInterfaceConsts ()
{
}

}

// Metadata.xml XPath interface reference: path="/api/package[@name='com.xamarin.android']/interface[@name='MyInterface']"
[Register ("com/xamarin/android/MyInterface", "", "Com.Xamarin.Android.IMyInterfaceInvoker")]
public partial interface IMyInterface : IJavaObject, IJavaPeerable {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this interface declaration also contain:

public const int EglNativeVisualId = (int) 12334;

I would expect the IMyInterface type to "mirror"/contain the same members as the MyInterface type, yet IMyInterface.EglNativeVisualId doesn't exist.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interface constants are gated behind -lang-features=interface-constants, which isn't enabled in this test suite.

I went back and forth on this and decided that "static interface fields" falls more under default-interface-methods than interface-constants. DIM is poorly named and really means "default AND static interface members" in this context.

private static readonly JniPeerMembers _members = new XAPeerMembers ("com/xamarin/android/MyInterface", typeof (IMyInterface), isInterface: true);


// Metadata.xml XPath field reference: path="/api/package[@name='com.xamarin.android']/interface[@name='MyInterface']/field[@name='EGL_NO_SURFACE']"
[Register ("EGL_NO_SURFACE")]
public static int EglNoSurface {
get {
const string __id = "EGL_NO_SURFACE.I";

var __v = _members.StaticFields.GetInt32Value (__id);
return __v;
}
set {
const string __id = "EGL_NO_SURFACE.I";

try {
_members.StaticFields.SetValue (__id, value);
} finally {
}
}
}

}

[global::Android.Runtime.Register ("com/xamarin/android/MyInterface", DoNotGenerateAcw=true)]
internal partial class IMyInterfaceInvoker : global::Java.Lang.Object, IMyInterface {
static readonly JniPeerMembers _members = new XAPeerMembers ("com/xamarin/android/MyInterface", typeof (IMyInterfaceInvoker));

static IntPtr java_class_ref {
get { return _members.JniPeerType.PeerReference.Handle; }
}

[global::System.Diagnostics.DebuggerBrowsable (global::System.Diagnostics.DebuggerBrowsableState.Never)]
[global::System.ComponentModel.EditorBrowsable (global::System.ComponentModel.EditorBrowsableState.Never)]
public override global::Java.Interop.JniPeerMembers JniPeerMembers {
get { return _members; }
}

[global::System.Diagnostics.DebuggerBrowsable (global::System.Diagnostics.DebuggerBrowsableState.Never)]
[global::System.ComponentModel.EditorBrowsable (global::System.ComponentModel.EditorBrowsableState.Never)]
protected override IntPtr ThresholdClass {
get { return class_ref; }
}

[global::System.Diagnostics.DebuggerBrowsable (global::System.Diagnostics.DebuggerBrowsableState.Never)]
[global::System.ComponentModel.EditorBrowsable (global::System.ComponentModel.EditorBrowsableState.Never)]
protected override global::System.Type ThresholdType {
get { return _members.ManagedPeerType; }
}

IntPtr class_ref;

public static IMyInterface GetObject (IntPtr handle, JniHandleOwnership transfer)
{
return global::Java.Lang.Object.GetObject<IMyInterface> (handle, transfer);
}

static IntPtr Validate (IntPtr handle)
{
if (!JNIEnv.IsInstanceOf (handle, java_class_ref))
throw new InvalidCastException ($"Unable to convert instance of type '{JNIEnv.GetClassNameFromInstance (handle)}' to type 'com.xamarin.android.MyInterface'.");
return handle;
}

protected override void Dispose (bool disposing)
{
if (this.class_ref != IntPtr.Zero)
JNIEnv.DeleteGlobalRef (this.class_ref);
this.class_ref = IntPtr.Zero;
base.Dispose (disposing);
}

public IMyInterfaceInvoker (IntPtr handle, JniHandleOwnership transfer) : base (Validate (handle), transfer)
{
IntPtr local_ref = JNIEnv.GetObjectClass (((global::Java.Lang.Object) this).Handle);
this.class_ref = JNIEnv.NewGlobalRef (local_ref);
JNIEnv.DeleteLocalRef (local_ref);
}

}
26 changes: 26 additions & 0 deletions tests/generator-Tests/Unit-Tests/DefaultInterfaceMethodsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -462,5 +462,31 @@ public void GenerateProperNestedInterfaceSignatures ()
Assert.True (generated.Contains ("GetOnActivityDestroyed_IHandler:Com.Xamarin.Android.Application/IActivityLifecycleInterface, MyAssembly"));
Assert.False (generated.Contains ("GetOnActivityDestroyed_IHandler:Com.Xamarin.Android.Application.IActivityLifecycleInterface, MyAssembly"));
}

[Test]
public void WriteInterfaceFieldAsDimProperty ()
{
// Ensure we write interface fields that are not constant, and thus must be written as properties
var xml = @"<api>
<package name='com.xamarin.android' jni-name='com/xamarin/android'>
<interface abstract='true' deprecated='not deprecated' final='false' name='MyInterface' static='false' visibility='public' jni-signature='Lcom/xamarin/android/MyInterface;'>
<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>
<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>
</interface>
</package>
</api>";

var gens = ParseApiDefinition (xml);
var iface = gens.OfType<InterfaceGen> ().Single ();

var result = iface.Validate (options, new GenericParameterDefinitionList (), new CodeGeneratorContext ());
Assert.True (result);

generator.WriteType (iface, string.Empty, new GenerationInfo (string.Empty, string.Empty, "MyAssembly"));

var generated = writer.ToString ();

Assert.AreEqual (GetTargetedExpected (nameof (WriteInterfaceFieldAsDimProperty)), writer.ToString ().NormalizeLineEndings ());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -149,14 +149,23 @@ internal string GetEventDelegateName (Method m)
// These are fields that we currently support generating on the interface with DIM
public IEnumerable<Field> GetGeneratableFields (CodeGenerationOptions options)
{
if (!options.SupportInterfaceConstants)
return Enumerable.Empty<Field> ();
var fields = new List<Field> ();

return Fields.Where (f => !f.NeedsProperty && !(f.DeprecatedComment?.Contains ("constant will be removed") == true));
// Constant fields
if (options.SupportInterfaceConstants)
fields.AddRange (Fields.Where (f => !f.NeedsProperty && !(f.DeprecatedComment?.Contains ("constant will be removed") == true)));

// Invoked fields exposed as properties
if (options.SupportDefaultInterfaceMethods)
fields.AddRange (Fields.Where (f => f.NeedsProperty && !(f.DeprecatedComment?.Contains ("constant will be removed") == true)));

return fields;
}

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

public bool HasFieldsAsProperties => Fields.Any (f => f.NeedsProperty);

public bool HasStaticMethods => GetAllMethods ().Any (m => m.IsStatic);

public bool IsConstSugar (CodeGenerationOptions options)
Expand Down
4 changes: 2 additions & 2 deletions tools/generator/SourceWriters/BoundInterface.cs
Original file line number Diff line number Diff line change
Expand Up @@ -153,14 +153,14 @@ void AddInheritedInterfaces (InterfaceGen iface, CodeGenerationOptions opt)

void AddClassHandle (InterfaceGen iface, CodeGenerationOptions opt)
{
if (opt.SupportDefaultInterfaceMethods && (iface.HasDefaultMethods || iface.HasStaticMethods))
if (opt.SupportDefaultInterfaceMethods && (iface.HasDefaultMethods || iface.HasStaticMethods || iface.HasFieldsAsProperties))
Fields.Add (new PeerMembersField (opt, iface.RawJniName, iface.Name, true));
}

void AddFields (InterfaceGen iface, CodeGenerationOptions opt, CodeGeneratorContext context)
{
// Interface fields are only supported with DIM
if (!opt.SupportInterfaceConstants)
if (!opt.SupportInterfaceConstants && !opt.SupportDefaultInterfaceMethods)
return;

var seen = new HashSet<string> ();
Expand Down