[generator] Remove interface alternatives w/ interface-constants (#600)

Context: #509

When using `generator --lang-features=interface-constants`, remove or
obsolete interface alternatives.

Due to our legacy workarounds for the C# shortcoming of not being able
to place constants or members on interfaces, every interface with
those members creates two additional "alternative" classes to provide
users access to the members originally on the interface.

For example, consider the Java interface [`android.os.Parcelable`][0]:

	// Java
	package android.os;
	public interface Parcelable {
	    public static final int CONTENTS_FILE_DESCRIPTOR      = 1;
	    public static final int PARCELABLE_WRITE_RETURN_VALUE = 1;

	    int describeContents();
	    void writeToParcel(Parcel dest, int flags);

	    // ...
	}

The binding in a C#7 world is:

	// C# Binding
	namespace Android.OS {
	    public partial interface IParcelable {
	        int DescribeContents();
	        void WriteToParcel(Parcel dest, [GeneratedEnum] ParcelableWriteFlags flags);
	    }
	    public abstract partial class Parcelable {
	        internal Parcelable () {}

	        public const int ContentsFileDescriptor = (int) 1;
		public const ParcelableWriteFlags ParcelableWriteReturnValue = (ParcelableWriteFlags) 1;
	    }
	    [Obsolete ("…", error: true)]
	    public abstract partial class ParcelableConsts : Parcelable {
	        private ParcelableConsts () {}

	        public const int ContentsFileDescriptor = (int) 1;
		public const ParcelableWriteFlags ParcelableWriteReturnValue = (ParcelableWriteFlags) 1;
	    }
	}

`Parcelable` contains the interface constants which couldn't exist on
`IParcelable`, and `ParcelableConsts` exists for backward
compatibility (as we used `*Consts` types before we introduced the
non-`*Consts` types), and is `[Obsolete(error:true)]`.

In a C#8 world with `generator --lang-features=interface-constants`
we no longer need to preserve these alternative types.

The `*Consts` types have been `[Obsolete]` for over 5 years, but have
only been `[Obsolete(error:true)]` since d16-4, released 2019-Dec-03.
We feel it hasn't been an error long enough to remove it for *all*
API levels, so we will stop emitting it for API-R.

The `Parcelable` and similar alternative types are now redundant when
`--lang-features=interface-constants,default-interface-methods` is
used, as all members of the Java interface can now exist in the C#
binding of that interface.  Here, we will `[Obsolete]` the class and
all members of the class, with a message referring to the new types
and members to use.

This results in an API-R binding of:

	// C# Binding, API-R
	namespace Android.OS {
	    public partial interface IParcelable {
	        int DescribeContents();
	        void WriteToParcel(Parcel dest, [GeneratedEnum] ParcelableWriteFlags flags);

	        public const int ContentsFileDescriptor = (int) 1;
		public const ParcelableWriteFlags ParcelableWriteReturnValue = (ParcelableWriteFlags) 1;
	    }
	    [Obsolete ("Use 'Android.OS.IParcelable'.  This class will be removed in a future release.")]
	    public abstract partial class Parcelable {
	        internal Parcelable () {}

	        [Obsolete ("Use 'Android.OS.IParcelable.ContentsFileDescriptor'.  This class will be removed in a future release.")]
	        public const int ContentsFileDescriptor = (int) 1;

	        [Obsolete ("Use 'Android.OS.IParcelable.ParcelableWriteReturnValue'.  This class will be removed in a future release.")]
		public const ParcelableWriteFlags ParcelableWriteReturnValue = (ParcelableWriteFlags) 1;
	    }
	}

Additionally, fix a bug where fields that are marked `[Obsolete]` that
are being changed into property getters were not having the obsolete
attribute applied.

[0]: https://developer.android.com/reference/android/os/Parcelable

Author: jpobst

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

@@ -0,0 +1,154 @@
+[Register ("com/xamarin/android/Parent", DoNotGenerateAcw=true)]
+[global::System.Obsolete ("Use the 'Com.Xamarin.Android.IParent' type. This class will be removed in a future release.")]
+public abstract class Parent : Java.Lang.Object {
+
+	internal Parent ()
+	{
+	}
+
+	// Metadata.xml XPath field reference: path="/api/package[@name='com.xamarin.android']/interface[@name='Parent']/field[@name='ACCEPT_HANDOVER']"
+	[Register ("ACCEPT_HANDOVER")]
+	[Obsolete ("Use 'Com.Xamarin.Android.IParent.AcceptHandover'. This class will be removed in a future release.")]
+	public const string AcceptHandover = (string) "android.permission.ACCEPT_HANDOVER";
+
+	// Metadata.xml XPath field reference: path="/api/package[@name='com.xamarin.android']/interface[@name='Parent']/field[@name='ALREADY_OBSOLETE']"
+	[Register ("ALREADY_OBSOLETE")]
+	[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")]
+	[Obsolete ("Use 'Com.Xamarin.Android.IParent.ApiName'. This class will be removed in a future release.")]
+	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]"
+	[Obsolete (@"Use 'Com.Xamarin.Android.IParent.Comparing'. This class will be removed in a future release.")]
+	[Register ("comparing", "()I", "")]
+	public static unsafe int Comparing ()
+	{
+		const string __id = "comparing.()I";
+		try {
+			var __rm = _members.StaticMethods.InvokeInt32Method (__id, null);
+			return __rm;
+		} finally {
+		}
+	}
+
+	// Metadata.xml XPath method reference: path="/api/package[@name='com.xamarin.android']/interface[@name='Parent']/method[@name='comparingOld' and count(parameter)=0]"
+	[Obsolete (@"deprecated")]
+	[Register ("comparingOld", "()I", "")]
+	public static unsafe int ComparingOld ()
+	{
+		const string __id = "comparingOld.()I";
+		try {
+			var __rm = _members.StaticMethods.InvokeInt32Method (__id, null);
+			return __rm;
+		} finally {
+		}
+	}
+
+
+	static readonly JniPeerMembers _members = new JniPeerMembers ("com/xamarin/android/Parent", typeof (Parent));
+}
+
+// Metadata.xml XPath interface reference: path="/api/package[@name='com.xamarin.android']/interface[@name='Parent']"
+[Register ("com/xamarin/android/Parent", "", "Com.Xamarin.Android.IParentInvoker")]
+public partial interface IParent : IJavaObject, IJavaPeerable {
+	private static readonly JniPeerMembers _members = new JniPeerMembers ("com/xamarin/android/Parent", typeof (IParent), isInterface: true);
+
+	// Metadata.xml XPath field reference: path="/api/package[@name='com.xamarin.android']/interface[@name='Parent']/field[@name='ACCEPT_HANDOVER']"
+	[Register ("ACCEPT_HANDOVER")]
+	public const string AcceptHandover = (string) "android.permission.ACCEPT_HANDOVER";
+
+	// Metadata.xml XPath field reference: path="/api/package[@name='com.xamarin.android']/interface[@name='Parent']/field[@name='ALREADY_OBSOLETE']"
+	[Register ("ALREADY_OBSOLETE")]
+	[Obsolete ("deprecated")]
+	public const string AlreadyObsolete = (string) "android.permission.ACCEPT_HANDOVER";
+
+	// 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 ()
+	{
+		const string __id = "comparing.()I";
+		try {
+			var __rm = _members.StaticMethods.InvokeInt32Method (__id, null);
+			return __rm;
+		} finally {
+		}
+	}
+
+	// Metadata.xml XPath method reference: path="/api/package[@name='com.xamarin.android']/interface[@name='Parent']/method[@name='comparingOld' and count(parameter)=0]"
+	[Obsolete (@"deprecated")]
+	[Register ("comparingOld", "()I", "")]
+	public static unsafe int ComparingOld ()
+	{
+		const string __id = "comparingOld.()I";
+		try {
+			var __rm = _members.StaticMethods.InvokeInt32Method (__id, null);
+			return __rm;
+		} finally {
+		}
+	}
+
+}
+
+[global::Android.Runtime.Register ("com/xamarin/android/Parent", DoNotGenerateAcw=true)]
+internal partial class IParentInvoker : global::Java.Lang.Object, IParent {
+
+	static readonly JniPeerMembers _members = new JniPeerMembers ("com/xamarin/android/Parent", typeof (IParentInvoker));
+
+	static IntPtr java_class_ref {
+		get { return _members.JniPeerType.PeerReference.Handle; }
+	}
+
+	public override global::Java.Interop.JniPeerMembers JniPeerMembers {
+		get { return _members; }
+	}
+
+	protected override IntPtr ThresholdClass {
+		get { return class_ref; }
+	}
+
+	protected override global::System.Type ThresholdType {
+		get { return _members.ManagedPeerType; }
+	}
+
+	new IntPtr class_ref;
+
+	public static IParent GetObject (IntPtr handle, JniHandleOwnership transfer)
+	{
+		return global::Java.Lang.Object.GetObject<IParent> (handle, transfer);
+	}
+
+	static IntPtr Validate (IntPtr handle)
+	{
+		if (!JNIEnv.IsInstanceOf (handle, java_class_ref))
+			throw new InvalidCastException (string.Format ("Unable to convert instance of type '{0}' to type '{1}'.",
+						JNIEnv.GetClassNameFromInstance (handle), "com.xamarin.android.Parent"));
+		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 IParentInvoker (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);
+	}
+
+}
+

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

@@ -0,0 +1,154 @@
+[Register ("com/xamarin/android/Parent", DoNotGenerateAcw=true)]
+[global::System.Obsolete ("Use the 'Com.Xamarin.Android.IParent' type. This class will be removed in a future release.")]
+public abstract class Parent : Java.Lang.Object {
+
+	internal Parent ()
+	{
+	}
+
+	// Metadata.xml XPath field reference: path="/api/package[@name='com.xamarin.android']/interface[@name='Parent']/field[@name='ACCEPT_HANDOVER']"
+	[Register ("ACCEPT_HANDOVER")]
+	[Obsolete ("Use 'Com.Xamarin.Android.IParent.AcceptHandover'. This class will be removed in a future release.")]
+	public const string AcceptHandover = (string) "android.permission.ACCEPT_HANDOVER";
+
+	// Metadata.xml XPath field reference: path="/api/package[@name='com.xamarin.android']/interface[@name='Parent']/field[@name='ALREADY_OBSOLETE']"
+	[Register ("ALREADY_OBSOLETE")]
+	[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")]
+	[Obsolete ("Use 'Com.Xamarin.Android.IParent.ApiName'. This class will be removed in a future release.")]
+	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]"
+	[Obsolete (@"Use 'Com.Xamarin.Android.IParent.Comparing'. This class will be removed in a future release.")]
+	[Register ("comparing", "()I", "")]
+	public static unsafe int Comparing ()
+	{
+		const string __id = "comparing.()I";
+		try {
+			var __rm = _members.StaticMethods.InvokeInt32Method (__id, null);
+			return __rm;
+		} finally {
+		}
+	}
+
+	// Metadata.xml XPath method reference: path="/api/package[@name='com.xamarin.android']/interface[@name='Parent']/method[@name='comparingOld' and count(parameter)=0]"
+	[Obsolete (@"deprecated")]
+	[Register ("comparingOld", "()I", "")]
+	public static unsafe int ComparingOld ()
+	{
+		const string __id = "comparingOld.()I";
+		try {
+			var __rm = _members.StaticMethods.InvokeInt32Method (__id, null);
+			return __rm;
+		} finally {
+		}
+	}
+
+
+	static readonly JniPeerMembers _members = new XAPeerMembers ("com/xamarin/android/Parent", typeof (Parent));
+}
+
+// Metadata.xml XPath interface reference: path="/api/package[@name='com.xamarin.android']/interface[@name='Parent']"
+[Register ("com/xamarin/android/Parent", "", "Com.Xamarin.Android.IParentInvoker")]
+public partial interface IParent : IJavaObject, IJavaPeerable {
+	private static readonly JniPeerMembers _members = new XAPeerMembers ("com/xamarin/android/Parent", typeof (IParent), isInterface: true);
+
+	// Metadata.xml XPath field reference: path="/api/package[@name='com.xamarin.android']/interface[@name='Parent']/field[@name='ACCEPT_HANDOVER']"
+	[Register ("ACCEPT_HANDOVER")]
+	public const string AcceptHandover = (string) "android.permission.ACCEPT_HANDOVER";
+
+	// Metadata.xml XPath field reference: path="/api/package[@name='com.xamarin.android']/interface[@name='Parent']/field[@name='ALREADY_OBSOLETE']"
+	[Register ("ALREADY_OBSOLETE")]
+	[Obsolete ("deprecated")]
+	public const string AlreadyObsolete = (string) "android.permission.ACCEPT_HANDOVER";
+
+	// 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 ()
+	{
+		const string __id = "comparing.()I";
+		try {
+			var __rm = _members.StaticMethods.InvokeInt32Method (__id, null);
+			return __rm;
+		} finally {
+		}
+	}
+
+	// Metadata.xml XPath method reference: path="/api/package[@name='com.xamarin.android']/interface[@name='Parent']/method[@name='comparingOld' and count(parameter)=0]"
+	[Obsolete (@"deprecated")]
+	[Register ("comparingOld", "()I", "")]
+	public static unsafe int ComparingOld ()
+	{
+		const string __id = "comparingOld.()I";
+		try {
+			var __rm = _members.StaticMethods.InvokeInt32Method (__id, null);
+			return __rm;
+		} finally {
+		}
+	}
+
+}
+
+[global::Android.Runtime.Register ("com/xamarin/android/Parent", DoNotGenerateAcw=true)]
+internal partial class IParentInvoker : global::Java.Lang.Object, IParent {
+
+	static readonly JniPeerMembers _members = new XAPeerMembers ("com/xamarin/android/Parent", typeof (IParentInvoker));
+
+	static IntPtr java_class_ref {
+		get { return _members.JniPeerType.PeerReference.Handle; }
+	}
+
+	public override global::Java.Interop.JniPeerMembers JniPeerMembers {
+		get { return _members; }
+	}
+
+	protected override IntPtr ThresholdClass {
+		get { return class_ref; }
+	}
+
+	protected override global::System.Type ThresholdType {
+		get { return _members.ManagedPeerType; }
+	}
+
+	new IntPtr class_ref;
+
+	public static IParent GetObject (IntPtr handle, JniHandleOwnership transfer)
+	{
+		return global::Java.Lang.Object.GetObject<IParent> (handle, transfer);
+	}
+
+	static IntPtr Validate (IntPtr handle)
+	{
+		if (!JNIEnv.IsInstanceOf (handle, java_class_ref))
+			throw new InvalidCastException (string.Format ("Unable to convert instance of type '{0}' to type '{1}'.",
+						JNIEnv.GetClassNameFromInstance (handle), "com.xamarin.android.Parent"));
+		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 IParentInvoker (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);
+	}
+
+}
+

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

@@ -303,5 +303,65 @@ public void WriteNestedInterfaceClass ()
 
 			Assert.AreEqual (GetTargetedExpected (nameof (WriteNestedInterfaceClass)), writer.ToString ().NormalizeLineEndings ());
 		}
+
+		[Test]
+		public void DontWriteInterfaceConstsClass ()
+		{
+			// If SupportInterfaceConstants is true we no longer write the legacy
+			// XXXXConsts class that has been [Obsolete (iseeror: true)] for a while.
+			options.SupportInterfaceConstants = true;
+
+			var xml = @"<api>
+			  <package name='java.lang' jni-name='java/lang'>
+			    <class abstract='false' deprecated='not deprecated' final='false' name='Object' static='false' visibility='public' jni-signature='Ljava/lang/EmptyOverrideClass;' />
+			  </package>
+			  <package name='com.xamarin.android' jni-name='com/xamarin/android'>
+			    <interface abstract='true' deprecated='not deprecated' final='false' name='Parent' static='false' visibility='public' jni-signature='Lcom/xamarin/android/Parent;'>
+			      <field deprecated='not deprecated' final='true' name='ACCEPT_HANDOVER' jni-signature='Ljava/lang/String;' static='true' transient='false' type='java.lang.String' type-generic-aware='java.lang.String' value='&quot;android.permission.ACCEPT_HANDOVER&quot;' visibility='public' volatile='false'></field>
+			    </interface>
+			  </package>
+			</api>";
+
+			var gens = ParseApiDefinition (xml);
+			var iface = gens.OfType<InterfaceGen> ().Single ();
+
+			iface.Validate (options, new GenericParameterDefinitionList (), new CodeGeneratorContext ());
+
+			generator.WriteInterface (iface, string.Empty, new GenerationInfo (string.Empty, string.Empty, "MyAssembly"));
+
+			Assert.False (writer.ToString ().Contains ("class ParentConsts"));
+		}
+
+		[Test]
+		public void ObsoleteInterfaceAlternativeClass ()
+		{
+			// If SupportInterfaceConstants and SupportDefaultInterfaceMethods is true we want to
+			// [Obsolete] the members of the "interface alternative" class so we can eventually remove it.
+			options.SupportInterfaceConstants = true;
+
+			var xml = @"<api>
+			  <package name='java.lang' jni-name='java/lang'>
+			    <class abstract='false' deprecated='not deprecated' final='false' name='Object' static='false' visibility='public' jni-signature='Ljava/lang/EmptyOverrideClass;' />
+			  </package>
+			  <package name='com.xamarin.android' jni-name='com/xamarin/android'>
+			    <interface abstract='true' deprecated='not deprecated' final='false' name='Parent' static='false' visibility='public' jni-signature='Lcom/xamarin/android/Parent;'>
+			      <field deprecated='not deprecated' final='true' name='ACCEPT_HANDOVER' jni-signature='Ljava/lang/String;' static='true' transient='false' type='java.lang.String' type-generic-aware='java.lang.String' value='&quot;android.permission.ACCEPT_HANDOVER&quot;' visibility='public' volatile='false'></field>
+			      <field deprecated='deprecated' final='true' name='ALREADY_OBSOLETE' jni-signature='Ljava/lang/String;' static='true' transient='false' type='java.lang.String' type-generic-aware='java.lang.String' value='&quot;android.permission.ACCEPT_HANDOVER&quot;' visibility='public' volatile='false'></field>
+			      <field deprecated='not deprecated' final='true' name='API_NAME' jni-signature='Ljava/lang/String;' static='true' transient='false' type='java.lang.String' type-generic-aware='java.lang.String' visibility='public' volatile='false'></field>
+			      <method abstract='false' deprecated='not deprecated' final='false' name='comparing' jni-signature='()I' bridge='false' native='false' return='int' jni-return='I' static='true' synchronized='false' synthetic='false' visibility='public' />
+			      <method abstract='false' deprecated='deprecated' final='false' name='comparingOld' jni-signature='()I' bridge='false' native='false' return='int' jni-return='I' static='true' synchronized='false' synthetic='false' visibility='public' />
+			    </interface>
+			  </package>
+			</api>";
+
+			var gens = ParseApiDefinition (xml);
+			var iface = gens.OfType<InterfaceGen> ().Single ();
+
+			iface.Validate (options, new GenericParameterDefinitionList (), new CodeGeneratorContext ());
+
+			generator.WriteInterface (iface, string.Empty, new GenerationInfo (string.Empty, string.Empty, "MyAssembly"));
+
+			Assert.AreEqual (GetTargetedExpected (nameof (ObsoleteInterfaceAlternativeClass)), writer.ToString ().NormalizeLineEndings ());
+		}
 	}
 }