[generator] Add support for @explicitInterface metadata (dotnet#1006)

jpobst · web-flow · commit fadbb82c3b8a · 2022-07-10T06:54:35.000-04:00
Fixes: dotnet#1005 Context: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-8.0/default-interface-methods#reabstraction Sometimes Java interfaces will "re-declare" methods from implemented interfaces: // Java interface /* java.lang. */ AutoCloseable { void close(); } interface /* java.io. */ Closeable { void close(); } Historically, this has been bound in C# by "ignoring" that the method was re-declared: // C# partial interface /* Java.Lang. */ IAutoCloseable { void Close(); } partial interface /* Java.IO. */ ICloseable : IAutoCloseable { void Close(); } This would result in a [CS0108 warning][0]: warning CS0108: 'ICloseable.Close()' hides inherited member 'IAutoCloseable.Close()'. Use the new keyword if hiding was intended. *Note*: in .NET for Android, this *particular* scenario doesn't happen, as [`metadata` is used to remove `AutoCloseable`][1]. This scenario happens for other methods, e.g.: warning CS0108: 'IChannel.Close()' hides inherited member 'ICloseable.Close()'. Consequently, the .NET for Android build ignores CS0108 warnings. This in turn means that e.g. `IChannel.Close()` is implicitly declared as `new`. With the introduction of [Default Interface Members in C# 8][2], it is possible to ["re-abstract"][3] a method declared in an implemented interface. However, in order to re-abstract a member, the member must be explicitly qualified: partial interface ICloseable : IAutoCloseable { abstract void IAutoCloseable.Close (); } The declaring type isn't something we have enough information to figure out automatically. Add support for `explicitInterface` metadata which allows the declaring interface to be explicitly specified. <attr path="/api/package[@name='java.io']/interface[@name='Closeable']/method[@name='close']" name="explicitInterface">IAutoCloseable</attr> Note this is not really a concept that `generator` "understands" it will simply output whatever is specified in this attribute. Thus the value is the *C#* interface name, not the *Java* interface name. For properties, placing `explicitInterface` on either the `getFoo` or `setFoo` methods will work. The getter value takes precedence if both are specified. This is implemented for both `interface` and `class` members, should there be other use-cases that can benefit from it. [0]: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/compiler-messages/cs0108 [1]: https://github.com/xamarin/xamarin-android/blob/59352f833846c563768f18dd72133be101b0ddc6/src/Mono.Android/metadata#L1393 [2]: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-8.0/default-interface-methods [3]: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-8.0/default-interface-methods#reabstraction
diff --git a/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs b/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs
@@ -352,6 +352,156 @@ public void InheritedInterfaceAsClass ()
 
 			Assert.Pass ("WriteType did not NRE");
 		}
+
+		[Test]
+		public void ExplicitInterfaceMetadata_InterfaceMethod ()
+		{
+			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/Object;' />
+			  </package>
+			  <package name='com.xamarin.android' jni-name='com/xamarin/android'>
+			    <interface abstract='false' deprecated='not deprecated' extends='java.lang.Object' extends-generic-aware='java.lang.Object' jni-extends='Ljava/lang/Object;' final='false' name='MyInterface' static='false' visibility='public' jni-signature='Lcom/xamarin/android/MyInterface;'>
+			      <method abstract='true' deprecated='not deprecated' final='false' name='countAffectedRows' jni-signature='()I' bridge='false' native='false' return='int' jni-return='I' static='false' synchronized='false' synthetic='false' visibility='public' explicitInterface='IRowCounter'></method>
+			    </interface>
+			  </package>
+			</api>";
+
+			var gens = ParseApiDefinition (xml);
+			var iface = gens.Single (g => g.Name == "IMyInterface");
+
+			generator.Context.ContextTypes.Push (iface);
+			generator.WriteType (iface, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
+			generator.Context.ContextTypes.Pop ();
+
+			// Ensure explicit interface was written
+			Assert.True (writer.ToString ().Contains ("int IRowCounter.CountAffectedRows ("), $"was: `{writer}`");
+		}
+
+		[Test]
+		public void ExplicitInterfaceMetadata_InterfaceProperty ()
+		{
+			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/Object;' />
+			  </package>
+			  <package name='com.xamarin.android' jni-name='com/xamarin/android'>
+			    <interface abstract='false' deprecated='not deprecated' extends='java.lang.Object' extends-generic-aware='java.lang.Object' jni-extends='Ljava/lang/Object;' final='false' name='MyInterface' static='false' visibility='public' jni-signature='Lcom/xamarin/android/MyInterface;'>
+			      <method abstract='true' deprecated='not deprecated' final='false' name='getAge' jni-signature='()I' bridge='false' native='false' return='int' jni-return='I' static='false' synchronized='false' synthetic='false' visibility='public' explicitInterface='IHasAge'></method>
+			    </interface>
+			  </package>
+			</api>";
+
+			var gens = ParseApiDefinition (xml);
+			var iface = gens.Single (g => g.Name == "IMyInterface");
+
+			generator.Context.ContextTypes.Push (iface);
+			generator.WriteType (iface, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
+			generator.Context.ContextTypes.Pop ();
+
+			// Ensure explicit interface was written
+			Assert.True (writer.ToString ().Contains ("int IHasAge.Age {"), $"was: `{writer}`");
+		}
+
+		[Test]
+		public void ExplicitInterfaceMetadata_ClassMethod ()
+		{
+			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/Object;' />
+			  </package>
+			  <package name='com.xamarin.android' jni-name='com/xamarin/android'>
+			    <class abstract='false' deprecated='not deprecated' extends='java.lang.Object' extends-generic-aware='java.lang.Object' jni-extends='Ljava/lang/Object;' final='false' name='MyClass' static='false' visibility='public' jni-signature='Lcom/xamarin/android/MyClass;'>
+			      <method abstract='false' deprecated='not deprecated' final='false' name='countAffectedRows' jni-signature='()I' bridge='false' native='false' return='int' jni-return='I' static='false' synchronized='false' synthetic='false' visibility='public' explicitInterface='IRowCounter'></method>
+			    </class>
+			  </package>
+			</api>";
+
+			var gens = ParseApiDefinition (xml);
+			var iface = gens.Single (g => g.Name == "MyClass");
+
+			generator.Context.ContextTypes.Push (iface);
+			generator.WriteType (iface, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
+			generator.Context.ContextTypes.Pop ();
+
+			// Ensure explicit interface was written
+			Assert.True (writer.ToString ().Contains ("int IRowCounter.CountAffectedRows ("), $"was: `{writer}`");
+		}
+
+		[Test]
+		public void ExplicitInterfaceMetadata_ClassProperty ()
+		{
+			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/Object;' />
+			  </package>
+			  <package name='com.xamarin.android' jni-name='com/xamarin/android'>
+			    <class abstract='false' deprecated='not deprecated' extends='java.lang.Object' extends-generic-aware='java.lang.Object' jni-extends='Ljava/lang/Object;' final='false' name='MyClass' static='false' visibility='public' jni-signature='Lcom/xamarin/android/MyClass;'>
+			      <method abstract='false' deprecated='not deprecated' final='false' name='getAge' jni-signature='()I' bridge='false' native='false' return='int' jni-return='I' static='false' synchronized='false' synthetic='false' visibility='public' explicitInterface='IHasAge'></method>
+			    </class>
+			  </package>
+			</api>";
+
+			var gens = ParseApiDefinition (xml);
+			var iface = gens.Single (g => g.Name == "MyClass");
+
+			generator.Context.ContextTypes.Push (iface);
+			generator.WriteType (iface, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
+			generator.Context.ContextTypes.Pop ();
+
+			// Ensure explicit interface was written
+			Assert.True (writer.ToString ().Contains ("int IHasAge.Age {"), $"was: `{writer}`");
+		}
+
+		[Test]
+		public void ExplicitInterfaceMetadata_AbstractClassMethod ()
+		{
+			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/Object;' />
+			  </package>
+			  <package name='com.xamarin.android' jni-name='com/xamarin/android'>
+			    <class abstract='false' deprecated='not deprecated' extends='java.lang.Object' extends-generic-aware='java.lang.Object' jni-extends='Ljava/lang/Object;' final='false' name='MyClass' static='false' visibility='public' jni-signature='Lcom/xamarin/android/MyClass;'>
+			      <method abstract='true' deprecated='not deprecated' final='false' name='countAffectedRows' jni-signature='()I' bridge='false' native='false' return='int' jni-return='I' static='false' synchronized='false' synthetic='false' visibility='public' explicitInterface='IRowCounter'></method>
+			    </class>
+			  </package>
+			</api>";
+
+			var gens = ParseApiDefinition (xml);
+			var iface = gens.Single (g => g.Name == "MyClass");
+
+			generator.Context.ContextTypes.Push (iface);
+			generator.WriteType (iface, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
+			generator.Context.ContextTypes.Pop ();
+
+			// Ensure explicit interface was written
+			Assert.True (writer.ToString ().Contains ("abstract int IRowCounter.CountAffectedRows ("), $"was: `{writer}`");
+		}
+
+		[Test]
+		public void ExplicitInterfaceMetadata_AbstractClassProperty ()
+		{
+			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/Object;' />
+			  </package>
+			  <package name='com.xamarin.android' jni-name='com/xamarin/android'>
+			    <class abstract='false' deprecated='not deprecated' extends='java.lang.Object' extends-generic-aware='java.lang.Object' jni-extends='Ljava/lang/Object;' final='false' name='MyClass' static='false' visibility='public' jni-signature='Lcom/xamarin/android/MyClass;'>
+			      <method abstract='true' deprecated='not deprecated' final='false' name='getAge' jni-signature='()I' bridge='false' native='false' return='int' jni-return='I' static='false' synchronized='false' synthetic='false' visibility='public' explicitInterface='IHasAge'></method>
+			    </class>
+			  </package>
+			</api>";
+
+			var gens = ParseApiDefinition (xml);
+			var iface = gens.Single (g => g.Name == "MyClass");
+
+			generator.Context.ContextTypes.Push (iface);
+			generator.WriteType (iface, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
+			generator.Context.ContextTypes.Pop ();
+
+			// Ensure explicit interface was written
+			Assert.True (writer.ToString ().Contains ("abstract int IHasAge.Age {"), $"was: `{writer}`");
+		}
 	}
 
 	[TestFixture]
diff --git a/tools/generator/Java.Interop.Tools.Generator.Importers/XmlApiImporter.cs b/tools/generator/Java.Interop.Tools.Generator.Importers/XmlApiImporter.cs
@@ -348,6 +348,7 @@ public static Method CreateMethod (GenBase declaringType, XElement elem, CodeGen
 				ArgsType = elem.Attribute ("argsType")?.Value,
 				CustomAttributes = elem.XGetAttribute ("customAttributes"),
 				Deprecated = elem.Deprecated (),
+				ExplicitInterface = elem.XGetAttribute ("explicitInterface"),
 				EventName = elem.Attribute ("eventName")?.Value,
 				GenerateAsyncWrapper = elem.Attribute ("generateAsyncWrapper") != null,
 				GenerateDispatchingSetter = elem.Attribute ("generateDispatchingSetter") != null,
diff --git a/tools/generator/Java.Interop.Tools.Generator.ObjectModel/Method.cs b/tools/generator/Java.Interop.Tools.Generator.ObjectModel/Method.cs
@@ -33,6 +33,7 @@ public Method (GenBase declaringType) : base (declaringType)
 		public bool ReturnNotNull { get; set; }
 		public ReturnValue RetVal { get; set; }
 		public int SourceApiLevel { get; set; }
+		public string ExplicitInterface { get; set; }
 
 		// it used to be private though...
 		internal string AdjustedName => IsReturnCharSequence ? Name + "Formatted" : Name;
diff --git a/tools/generator/Java.Interop.Tools.Generator.ObjectModel/Property.cs b/tools/generator/Java.Interop.Tools.Generator.ObjectModel/Property.cs
@@ -58,5 +58,7 @@ public void AutoDetectEnumifiedOverrideProperties (AncestorDescendantCache cache
 				}
 			}
 		}
+
+		public string ExplicitInterface => Getter?.ExplicitInterface ?? Setter?.ExplicitInterface;
 	}
 }
diff --git a/tools/generator/SourceWriters/BoundAbstractProperty.cs b/tools/generator/SourceWriters/BoundAbstractProperty.cs
@@ -19,6 +19,7 @@ public class BoundAbstractProperty : PropertyWriter
 		public BoundAbstractProperty (GenBase gen, Property property, CodeGenerationOptions opt)
 		{
 			Name = property.AdjustedName;
+			ExplicitInterfaceImplementation = property.ExplicitInterface;
 			PropertyType = new TypeReferenceWriter (opt.GetTypeReferenceName (property.Getter.RetVal));
 
 			SetVisibility (property.Getter.RetVal.IsGeneric ? "protected" : property.Getter.Visibility);
diff --git a/tools/generator/SourceWriters/BoundInterfaceMethodDeclaration.cs b/tools/generator/SourceWriters/BoundInterfaceMethodDeclaration.cs
@@ -22,6 +22,8 @@ public BoundInterfaceMethodDeclaration (Method method, string adapter, CodeGener
 			this.opt = opt;
 
 			Name = method.AdjustedName;
+			ExplicitInterfaceImplementation = method.ExplicitInterface;
+
 			ReturnType = new TypeReferenceWriter (opt.GetTypeReferenceName (method.RetVal));
 			IsDeclaration = true;
 
diff --git a/tools/generator/SourceWriters/BoundInterfacePropertyDeclaration.cs b/tools/generator/SourceWriters/BoundInterfacePropertyDeclaration.cs
@@ -16,6 +16,7 @@ public class BoundInterfacePropertyDeclaration : PropertyWriter
 		public BoundInterfacePropertyDeclaration (GenBase gen, Property property, string adapter, CodeGenerationOptions opt)
 		{
 			Name = property.AdjustedName;
+			ExplicitInterfaceImplementation = property.ExplicitInterface;
 
 			PropertyType = new TypeReferenceWriter (opt.GetTypeReferenceName (property));
 			IsAutoProperty = true;
diff --git a/tools/generator/SourceWriters/BoundMethod.cs b/tools/generator/SourceWriters/BoundMethod.cs
@@ -48,6 +48,10 @@ public BoundMethod (GenBase type, Method method, CodeGenerationOptions opt, bool
 			if (is_explicit)
 				ExplicitInterfaceImplementation = GetDeclaringTypeOfExplicitInterfaceMethod (method.OverriddenInterfaceMethod);
 
+			// Allow user to override our explicit interface logic
+			if (method.ExplicitInterface.HasValue ())
+				ExplicitInterfaceImplementation = method.ExplicitInterface;
+
 			if ((IsVirtual || !IsOverride) && type.RequiresNew (method.AdjustedName, method))
 				IsShadow = true;
 
diff --git a/tools/generator/SourceWriters/BoundMethodAbstractDeclaration.cs b/tools/generator/SourceWriters/BoundMethodAbstractDeclaration.cs
@@ -36,6 +36,7 @@ public BoundMethodAbstractDeclaration (GenBase gen, Method method, CodeGeneratio
 			}
 
 			Name = method.AdjustedName;
+			ExplicitInterfaceImplementation = method.ExplicitInterface;
 
 			IsAbstract = true;
 			IsShadow = impl.RequiresNew (method.Name, method);
diff --git a/tools/generator/SourceWriters/BoundProperty.cs b/tools/generator/SourceWriters/BoundProperty.cs
@@ -19,6 +19,7 @@ public class BoundProperty : PropertyWriter
 		public BoundProperty (GenBase gen, Property property, CodeGenerationOptions opt, bool withCallbacks = true, bool forceOverride = false)
 		{
 			Name = property.AdjustedName;
+			ExplicitInterfaceImplementation = property.ExplicitInterface;
 			PropertyType = new TypeReferenceWriter (opt.GetTypeReferenceName (property.Getter.RetVal));
 
 			SetVisibility (gen is InterfaceGen ? string.Empty : property.Getter.IsAbstract && property.Getter.RetVal.IsGeneric ? "protected" : (property.Setter ?? property.Getter).Visibility);

Original file line number	Diff line number	Diff line change
`@@ -58,5 +58,7 @@ public void AutoDetectEnumifiedOverrideProperties (AncestorDescendantCache cache`
`58`	`58`	`}`
`59`	`59`	`}`
`60`	`60`	`}`
	`61`	`+`
	`62`	`+ public string ExplicitInterface => Getter?.ExplicitInterface ?? Setter?.ExplicitInterface;`
`61`	`63`	`}`
`62`	`64`	`}`
Original file line number	Diff line number	Diff line change
`@@ -19,6 +19,7 @@ public class BoundAbstractProperty : PropertyWriter`
`19`	`19`	`public BoundAbstractProperty (GenBase gen, Property property, CodeGenerationOptions opt)`
`20`	`20`	`{`
`21`	`21`	`Name = property.AdjustedName;`
	`22`	`+ ExplicitInterfaceImplementation = property.ExplicitInterface;`
`22`	`23`	`PropertyType = new TypeReferenceWriter (opt.GetTypeReferenceName (property.Getter.RetVal));`
`23`	`24`
`24`	`25`	`SetVisibility (property.Getter.RetVal.IsGeneric ? "protected" : property.Getter.Visibility);`
Original file line number	Diff line number	Diff line change
`@@ -16,6 +16,7 @@ public class BoundInterfacePropertyDeclaration : PropertyWriter`
`16`	`16`	`public BoundInterfacePropertyDeclaration (GenBase gen, Property property, string adapter, CodeGenerationOptions opt)`
`17`	`17`	`{`
`18`	`18`	`Name = property.AdjustedName;`
	`19`	`+ ExplicitInterfaceImplementation = property.ExplicitInterface;`
`19`	`20`
`20`	`21`	`PropertyType = new TypeReferenceWriter (opt.GetTypeReferenceName (property));`
`21`	`22`	`IsAutoProperty = true;`
Original file line number	Diff line number	Diff line change
`@@ -36,6 +36,7 @@ public BoundMethodAbstractDeclaration (GenBase gen, Method method, CodeGeneratio`
`36`	`36`	`}`
`37`	`37`
`38`	`38`	`Name = method.AdjustedName;`
	`39`	`+ ExplicitInterfaceImplementation = method.ExplicitInterface;`
`39`	`40`
`40`	`41`	`IsAbstract = true;`
`41`	`42`	`IsShadow = impl.RequiresNew (method.Name, method);`
Original file line number	Diff line number	Diff line change
`@@ -19,6 +19,7 @@ public class BoundProperty : PropertyWriter`
`19`	`19`	`public BoundProperty (GenBase gen, Property property, CodeGenerationOptions opt, bool withCallbacks = true, bool forceOverride = false)`
`20`	`20`	`{`
`21`	`21`	`Name = property.AdjustedName;`
	`22`	`+ ExplicitInterfaceImplementation = property.ExplicitInterface;`
`22`	`23`	`PropertyType = new TypeReferenceWriter (opt.GetTypeReferenceName (property.Getter.RetVal));`
`23`	`24`
`24`	`25`	`SetVisibility (gen is InterfaceGen ? string.Empty : property.Getter.IsAbstract && property.Getter.RetVal.IsGeneric ? "protected" : (property.Setter ?? property.Getter).Visibility);`