[generator] Add support for const fields on interfaces (#439)

Add support for Java-side interface constants to be optionally bound
using C# 8.0's new default interface member support.  This is
feature-flagged behind the `--lang-features=interface-constants`
option (off by default):

	generator --lang-features=interface-constants ...

Here is an example of what is generated from `Mono.Android.dll`:

	// Metadata.xml XPath interface reference: path="/api/package[@name='android.os']/interface[@name='Parcelable']"
	[Register ("android/os/Parcelable", "", "Android.OS.IParcelableInvoker", ApiSince = 1)]
	public partial interface IParcelable : IJavaObject {
	
	+	// Metadata.xml XPath field reference: path="/api/package[@name='android.os']/interface[@name='Parcelable']/field[@name='CONTENTS_FILE_DESCRIPTOR']"
	+	[Register ("CONTENTS_FILE_DESCRIPTOR")]
	+	public const int ContentsFileDescriptor = (int) 1;
	
		// Metadata.xml XPath method reference: path="/api/package[@name='android.os']/interface[@name='Parcelable']/method[@name='describeContents' and count(parameter)=0]"
		[Register ("describeContents", "()I", "GetDescribeContentsHandler:Android.OS.IParcelableInvoker, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null")]
		int DescribeContents ();
	
		// Metadata.xml XPath method reference: path="/api/package[@name='android.os']/interface[@name='Parcelable']/method[@name='writeToParcel' and count(parameter)=2 and parameter[1][@type='android.os.Parcel'] and parameter[2][@type='int']]"
		[Register ("writeToParcel", "(Landroid/os/Parcel;I)V", "GetWriteToParcel_Landroid_os_Parcel_IHandler:Android.OS.IParcelableInvoker, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null")]
		void WriteToParcel (Android.OS.Parcel dest, [global::Android.Runtime.GeneratedEnum] Android.OS.ParcelableWriteFlags flags);
	}

There are several Android interfaces defined that only contain
constants.  Without Default-Interface-Methods, we have not been
generating these interfaces because there was nothing we could
generate for them.  (These are referred to as `IsConstSugar`).  Now
we generate them, but they do not have `[Register]` on the interface
and do not inherit from `IJavaObject` since they are not used for
Java interop, e.g. [`android.icu.lang.UProperty.NameChoice][0]:

[0]: https://developer.android.com/reference/android/icu/lang/UProperty.NameChoice

	// Metadata.xml XPath interface reference: path="/api/package[@name='android.icu.lang']/interface[@name='UProperty.NameChoice']"
	public partial interface IUPropertyNameChoice {
	
		// Metadata.xml XPath field reference: path="/api/package[@name='android.icu.lang']/interface[@name='UProperty.NameChoice']/field[@name='LONG']"
		[Register ("LONG")]
		public const int Long = (int) 1;
	
		// Metadata.xml XPath field reference: path="/api/package[@name='android.icu.lang']/interface[@name='UProperty.NameChoice']/field[@name='SHORT']"
		[Register ("SHORT")]
		public const int Short = (int) 0;
	}

Here is the diff of this change run on `Mono.Android.dll`:
https://gist.github.com/jpobst/7aa0bb1a01975a56038cb7ab12ecdb1c

tools/generator/CodeGenerationOptions.cs

@@ -70,6 +70,7 @@ internal CodeGenerator CreateCodeGenerator (TextWriter writer)
 		public bool UseShortFileNames { get; set; }
 		public IList<GenBase> Gens {get;set;}
 		public int ProductVersion { get; set; }
+		public bool SupportInterfaceConstants { get; set; }
 
 		bool? buildingCoreAssembly;
 		public bool BuildingCoreAssembly {

tools/generator/CodeGenerator.cs

@@ -63,7 +63,8 @@ static void Run (CodeGeneratorOptions options, DirectoryAssemblyResolver resolve
 				UseGlobal             = options.GlobalTypeNames,
 				IgnoreNonPublicType   = true,
 				UseShortFileNames     = options.UseShortFileNames,
-				ProductVersion        = options.ProductVersion
+				ProductVersion        = options.ProductVersion,
+				SupportInterfaceConstants = options.SupportInterfaceConstants,
 			};
 
 			// Load reference libraries

tools/generator/CodeGeneratorOptions.cs

@@ -39,6 +39,7 @@ public CodeGeneratorOptions ()
 		public string               MappingReportFile { get; set; }
 		public bool                 OnlyRunApiXmlAdjuster { get; set; }
 		public string               ApiXmlAdjusterOutput { get; set; }
+		public bool                 SupportInterfaceConstants { get; set; }
 
 		public static CodeGeneratorOptions Parse (string[] args)
 		{
@@ -85,6 +86,9 @@ public static CodeGeneratorOptions Parse (string[] args)
 				{ "sdk-platform|api-level=",
 					"SDK Platform {VERSION}/API level.",
 					v => opts.ApiLevel = v },
+				{ "lang-features=",
+					"For internal use. (Flags: interface-constants)",
+					v => opts.SupportInterfaceConstants = v?.Contains ("interface-constants") == true },
 				{ "preserve-enums",
 					"For internal use.",
 					v => opts.PreserveEnums = v != null },

tools/generator/Java.Interop.Tools.Generator.CodeGeneration/CodeGenerator.cs

@@ -441,61 +441,26 @@ internal virtual void WriteField (Field field, string indent, GenBase type)
 		public void WriteInterface (InterfaceGen @interface, string indent, GenerationInfo gen_info)
 		{
 			opt.ContextTypes.Push (@interface);
+
 			// interfaces don't nest, so generate as siblings
 			foreach (GenBase nest in @interface.NestedTypes) {
 				WriteType (nest, indent, gen_info);
 				writer.WriteLine ();
 			}
 
-			var staticMethods = @interface.Methods.Where (m => m.IsStatic);
-			if (@interface.Fields.Any () || staticMethods.Any ()) {
-				string name = @interface.HasManagedName
-					? @interface.Name.Substring (1) + "Consts"
-					: @interface.Name.Substring (1);
-				writer.WriteLine ("{0}[Register (\"{1}\"{2}, DoNotGenerateAcw=true)]", indent, @interface.RawJniName, @interface.AdditionalAttributeString ());
-				writer.WriteLine ("{0}public abstract class {1} : Java.Lang.Object {{", indent, name);
-				writer.WriteLine ();
-				writer.WriteLine ("{0}\tinternal {1} ()", indent, name);
-				writer.WriteLine ("{0}\t{{", indent);
-				writer.WriteLine ("{0}\t}}", indent);
-
-				var seen = new HashSet<string> ();
-				bool needsClassRef = WriteFields (@interface.Fields, indent + "\t", @interface, seen) || staticMethods.Any ();
-				foreach (var iface in @interface.GetAllImplementedInterfaces ().OfType<InterfaceGen> ()) {
-					writer.WriteLine ();
-					writer.WriteLine ("{0}\t// The following are fields from: {1}", indent, iface.JavaName);
-					bool v = WriteFields (iface.Fields, indent + "\t", iface, seen);
-					needsClassRef = needsClassRef || v;
-				}
-
-				foreach (var m in @interface.Methods.Where (m => m.IsStatic))
-					WriteMethod (m, indent + "\t", @interface, true);
-
-				if (needsClassRef) {
-					writer.WriteLine ();
-					WriteClassHandle (@interface, indent + "\t", name);
-				}
+			WriteInterfaceImplementedMembersAlternative (@interface, indent);
 
-				writer.WriteLine ("{0}}}", indent, @interface.Name);
-				writer.WriteLine ();
+			// If this interface is just fields and we can't generate any of them
+			// then we don't need to write the interface
+			if (@interface.IsConstSugar && @interface.GetGeneratableFields (opt).Count () == 0)
+				return;
 
-				if (!@interface.HasManagedName) {
-					writer.WriteLine ("{0}[Register (\"{1}\"{2}, DoNotGenerateAcw=true)]", indent, @interface.RawJniName, @interface.AdditionalAttributeString ());
-					writer.WriteLine ("{0}[global::System.Obsolete (\"Use the '{1}' type. This type will be removed in a future release.\")]", indent, name);
-					writer.WriteLine ("{0}public abstract class {1}Consts : {1} {{", indent, name);
-					writer.WriteLine ();
-					writer.WriteLine ("{0}\tprivate {1}Consts ()", indent, name);
-					writer.WriteLine ("{0}\t{{", indent);
-					writer.WriteLine ("{0}\t}}", indent);
-					writer.WriteLine ("{0}}}", indent);
-					writer.WriteLine ();
-				}
-			}
+			WriteInterfaceDeclaration (@interface, indent);
 
+			// If this interface is just constant fields we don't need to write all the invoker bits
 			if (@interface.IsConstSugar)
 				return;
 
-			WriteInterfaceDeclaration (@interface, indent);
 			if (!@interface.AssemblyQualifiedName.Contains ('/'))
 				WriteInterfaceExtensionsDeclaration (@interface, indent, null);
 			WriteInterfaceInvoker (@interface, indent);
@@ -547,11 +512,15 @@ public void WriteInterfaceDeclaration (InterfaceGen @interface, string indent)
 
 			if (@interface.IsDeprecated)
 				writer.WriteLine ("{0}[ObsoleteAttribute (@\"{1}\")]", indent, @interface.DeprecatedComment);
-			writer.WriteLine ("{0}[Register (\"{1}\", \"\", \"{2}\"{3})]", indent, @interface.RawJniName, @interface.Namespace + "." + @interface.FullName.Substring (@interface.Namespace.Length + 1).Replace ('.', '/') + "Invoker", @interface.AdditionalAttributeString ());
+
+			if (!@interface.IsConstSugar)
+				writer.WriteLine ("{0}[Register (\"{1}\", \"\", \"{2}\"{3})]", indent, @interface.RawJniName, @interface.Namespace + "." + @interface.FullName.Substring (@interface.Namespace.Length + 1).Replace ('.', '/') + "Invoker", @interface.AdditionalAttributeString ());
+
 			if (@interface.TypeParameters != null && @interface.TypeParameters.Any ())
 				writer.WriteLine ("{0}{1}", indent, @interface.TypeParameters.ToGeneratedAttributeString ());
-			writer.WriteLine ("{0}{1} partial interface {2} : {3} {{", indent, @interface.Visibility, @interface.Name,
-				@interface.Interfaces.Count == 0 || sb.Length == 0 ? "IJavaObject" : sb.ToString ());
+			writer.WriteLine ("{0}{1} partial interface {2}{3} {{", indent, @interface.Visibility, @interface.Name,
+				@interface.IsConstSugar ? string.Empty : @interface.Interfaces.Count == 0 || sb.Length == 0 ? " : IJavaObject" : " : " + sb.ToString ());
+			WriteInterfaceFields (@interface, indent + "\t");
 			writer.WriteLine ();
 			WriteInterfaceProperties (@interface, indent + "\t");
 			WriteInterfaceMethods (@interface, indent + "\t");
@@ -709,6 +678,74 @@ public void WriteInterfaceExtensionsDeclaration (InterfaceGen @interface, string
 			writer.WriteLine ();
 		}
 
+		public void WriteInterfaceFields (InterfaceGen iface, string indent)
+		{
+			// Interface fields are only supported with DIM
+			if (!opt.SupportInterfaceConstants)
+				return;
+
+			var seen = new HashSet<string> ();
+			var fields = iface.GetGeneratableFields (opt).ToList ();
+
+			WriteFields (fields, indent, iface, seen);
+		}
+
+		public void WriteInterfaceImplementedMembersAlternative (InterfaceGen @interface, string indent)
+		{
+			// Historically .NET has not allowed interface implemented fields or constants, so we
+			// initially worked around that by moving them to an abstract class, generally
+			// IMyInterface -> MyInterfaceConsts
+			// This was later expanded to accomodate static interface methods, creating a more appropriately named class
+			// IMyInterface -> MyInterface
+			// In this case the XXXConsts class is [Obsolete]'d and simply inherits from the newer class
+			// in order to maintain backward compatibility.
+			var staticMethods = @interface.Methods.Where (m => m.IsStatic);
+
+			if (@interface.Fields.Any () || staticMethods.Any ()) {
+				string name = @interface.HasManagedName
+					? @interface.Name.Substring (1) + "Consts"
+					: @interface.Name.Substring (1);
+				writer.WriteLine ("{0}[Register (\"{1}\"{2}, DoNotGenerateAcw=true)]", indent, @interface.RawJniName, @interface.AdditionalAttributeString ());
+				writer.WriteLine ("{0}public abstract class {1} : Java.Lang.Object {{", indent, name);
+				writer.WriteLine ();
+				writer.WriteLine ("{0}\tinternal {1} ()", indent, name);
+				writer.WriteLine ("{0}\t{{", indent);
+				writer.WriteLine ("{0}\t}}", indent);
+
+				var seen = new HashSet<string> ();
+				bool needsClassRef = WriteFields (@interface.Fields, indent + "\t", @interface, seen) || staticMethods.Any ();
+				foreach (var iface in @interface.GetAllImplementedInterfaces ().OfType<InterfaceGen> ()) {
+					writer.WriteLine ();
+					writer.WriteLine ("{0}\t// The following are fields from: {1}", indent, iface.JavaName);
+					bool v = WriteFields (iface.Fields, indent + "\t", iface, seen);
+					needsClassRef = needsClassRef || v;
+				}
+
+				foreach (var m in @interface.Methods.Where (m => m.IsStatic))
+					WriteMethod (m, indent + "\t", @interface, true);
+
+				if (needsClassRef) {
+					writer.WriteLine ();
+					WriteClassHandle (@interface, indent + "\t", name);
+				}
+
+				writer.WriteLine ("{0}}}", indent, @interface.Name);
+				writer.WriteLine ();
+
+				if (!@interface.HasManagedName) {
+					writer.WriteLine ("{0}[Register (\"{1}\"{2}, DoNotGenerateAcw=true)]", indent, @interface.RawJniName, @interface.AdditionalAttributeString ());
+					writer.WriteLine ("{0}[global::System.Obsolete (\"Use the '{1}' type. This type will be removed in a future release.\")]", indent, name);
+					writer.WriteLine ("{0}public abstract class {1}Consts : {1} {{", indent, name);
+					writer.WriteLine ();
+					writer.WriteLine ("{0}\tprivate {1}Consts ()", indent, name);
+					writer.WriteLine ("{0}\t{{", indent);
+					writer.WriteLine ("{0}\t}}", indent);
+					writer.WriteLine ("{0}}}", indent);
+					writer.WriteLine ();
+				}
+			}
+		}
+
 		public void WriteInterfaceInvoker (InterfaceGen @interface, string indent)
 		{
 			writer.WriteLine ("{0}[global::Android.Runtime.Register (\"{1}\", DoNotGenerateAcw=true{2})]", indent, @interface.RawJniName, @interface.AdditionalAttributeString ());

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

@@ -1,4 +1,5 @@
 using System;
+using System.Collections;
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
@@ -27,6 +28,15 @@ protected InterfaceGen (GenBaseSupport support)
 
 		public bool HasManagedName => hasManagedName;
 
+		// 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> ();
+
+			return Fields.Where (f => !f.NeedsProperty && !(f.DeprecatedComment?.Contains ("constant will be removed") == true));
+		}
+
 		public bool IsConstSugar {
 			get { 
 				if (Methods.Count > 0 || Properties.Count > 0)

tools/generator/Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteConstSugarInterfaceFields.txt

@@ -0,0 +1,18 @@
+// Metadata.xml XPath interface reference: path="/api/package[@name='java.code']/interface[@name='IMyInterface']"
+public partial interface IMyInterface {
+
+	// Metadata.xml XPath field reference: path="/api/package[@name='java.code']/interface[@name='IMyInterface']/field[@name='MyConstantField']"
+	[Register ("MyConstantField")]
+	public const int MyConstantField = (int) 7;
+
+	// Metadata.xml XPath field reference: path="/api/package[@name='java.code']/interface[@name='IMyInterface']/field[@name='MyConstantStringField']"
+	[Register ("MyConstantStringField")]
+	public const string MyConstantStringField = (string) "hello";
+
+	// Metadata.xml XPath field reference: path="/api/package[@name='java.code']/interface[@name='IMyInterface']/field[@name='MyDeprecatedField']"
+	[Register ("MyDeprecatedField")]
+	[Obsolete ("")]
+	public const int MyDeprecatedField = (int) 7;
+
+}
+

tools/generator/Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteInterfaceFields.txt

@@ -0,0 +1,14 @@
+// Metadata.xml XPath interface reference: path="/api/package[@name='java.code']/interface[@name='IMyInterface']"
+[Register ("java/code/IMyInterface", "", "java.code.IMyInterfaceInvoker")]
+public partial interface IMyInterface : IJavaObject {
+
+	// Metadata.xml XPath field reference: path="/api/package[@name='java.code']/interface[@name='IMyInterface']/field[@name='MyConstantField']"
+	[Register ("MyConstantField")]
+	public const int MyConstantField = (int) 7;
+
+	// Metadata.xml XPath method reference: path="/api/package[@name='java.code']/interface[@name='IMyInterface']/method[@name='DoSomething' and count(parameter)=0]"
+	[Register ("DoSomething", "()V", "GetDoSomethingHandler:java.code.IMyInterfaceInvoker, ")]
+	void DoSomething ();
+
+}
+

tools/generator/Tests/Unit-Tests/CodeGeneratorExpectedResults/XamarinAndroid/WriteConstSugarInterfaceFields.txt

@@ -0,0 +1,18 @@
+// Metadata.xml XPath interface reference: path="/api/package[@name='java.code']/interface[@name='IMyInterface']"
+public partial interface IMyInterface {
+
+	// Metadata.xml XPath field reference: path="/api/package[@name='java.code']/interface[@name='IMyInterface']/field[@name='MyConstantField']"
+	[Register ("MyConstantField")]
+	public const int MyConstantField = (int) 7;
+
+	// Metadata.xml XPath field reference: path="/api/package[@name='java.code']/interface[@name='IMyInterface']/field[@name='MyConstantStringField']"
+	[Register ("MyConstantStringField")]
+	public const string MyConstantStringField = (string) "hello";
+
+	// Metadata.xml XPath field reference: path="/api/package[@name='java.code']/interface[@name='IMyInterface']/field[@name='MyDeprecatedField']"
+	[Register ("MyDeprecatedField")]
+	[Obsolete ("")]
+	public const int MyDeprecatedField = (int) 7;
+
+}
+

tools/generator/Tests/Unit-Tests/CodeGeneratorExpectedResults/XamarinAndroid/WriteInterfaceFields.txt

@@ -0,0 +1,14 @@
+// Metadata.xml XPath interface reference: path="/api/package[@name='java.code']/interface[@name='IMyInterface']"
+[Register ("java/code/IMyInterface", "", "java.code.IMyInterfaceInvoker")]
+public partial interface IMyInterface : IJavaObject {
+
+	// Metadata.xml XPath field reference: path="/api/package[@name='java.code']/interface[@name='IMyInterface']/field[@name='MyConstantField']"
+	[Register ("MyConstantField")]
+	public const int MyConstantField = (int) 7;
+
+	// Metadata.xml XPath method reference: path="/api/package[@name='java.code']/interface[@name='IMyInterface']/method[@name='DoSomething' and count(parameter)=0]"
+	[Register ("DoSomething", "()V", "GetDoSomethingHandler:java.code.IMyInterfaceInvoker, ")]
+	void DoSomething ();
+
+}
+

tools/generator/Tests/Unit-Tests/CodeGeneratorTestBase.cs

@@ -0,0 +1,59 @@
+using System;
+using System.IO;
+using System.Reflection;
+using System.Text;
+using MonoDroid.Generation;
+using NUnit.Framework;
+
+namespace generatortests.Unit_Tests
+{
+	abstract class CodeGeneratorTestBase
+	{
+		protected CodeGenerator generator;
+		protected StringBuilder builder;
+		protected StringWriter writer;
+		protected CodeGenerationOptions options;
+
+		[SetUp]
+		public void SetUp ()
+		{
+			builder = new StringBuilder ();
+			writer = new StringWriter (builder);
+			options = CreateOptions ();
+
+			generator = options.CreateCodeGenerator (writer);
+		}
+
+		[TearDown]
+		public void TearDown ()
+		{
+			writer.Dispose ();
+		}
+
+		protected virtual CodeGenerationOptions CreateOptions ()
+		{
+			return new CodeGenerationOptions {
+				CodeGenerationTarget = Target,
+			};
+		}
+
+		protected abstract Xamarin.Android.Binder.CodeGenerationTarget Target { get; }
+
+		// Get the test results from "Common" for tests with the same results regardless of Target
+		protected string GetExpected (string testName)
+		{
+			var root = Path.GetDirectoryName (Assembly.GetExecutingAssembly ().Location);
+
+			return File.ReadAllText (Path.Combine (root, "Unit-Tests", "CodeGeneratorExpectedResults", "Common", $"{testName}.txt")).NormalizeLineEndings ();
+		}
+
+		// Get the test results from "JavaInterop1" or "XamarinAndroid" for tests with the different results per Target
+		protected string GetTargetedExpected (string testName)
+		{
+			var target = Target.ToString ();
+			var root = Path.GetDirectoryName (Assembly.GetExecutingAssembly ().Location);
+
+			return File.ReadAllText (Path.Combine (root, "Unit-Tests", "CodeGeneratorExpectedResults", target, $"{testName}.txt")).NormalizeLineEndings ();
+		}
+	}
+}