[generator] Support Buffer.duplicate() binding in API-34 (#1086)

Context: dotnet/android#7796
Context: 22d5687
Context: 5a0e37e

Imagine the following covariant-return-type -using construct, which
is new in API-34 `android.jar`:

	// Java
	public abstract class Buffer  {
	    public abstract Buffer duplicate ();
	}
	public abstract class CharBuffer extends Buffer {
	    public abstract CharBuffer duplicate ();
	}

By default this is bound as:

	// C#
	[Register(…)]
	public abstract partial class Buffer {
	    [Register(…)]
	    public abstract Buffer Duplicate ();
	}
	[Register(…)]
	public abstract partial class CharBuffer : Buffer {
	    [Register(…)]
	    public abstract CharBuffer Duplicate ();
	}

Alas, this results in [error CS0533][0]:

	error CS0533: 'CharBuffer.Duplicate()' hides inherited abstract member 'Buffer.Duplicate()'

C# supports this semantic if we use [`abstract override`][1], similar
to [reabstraction of Default Interface Methods][2] in 22d5687:

	public abstract class CharBuffer : Buffer {
	    public override abstract CharBuffer Duplicate ();
	}

Theoretically, we can tell `generator` how to fix this via the
`//attr[@name='managedOverride']` metadata property (5a0e37e):

	<attr path="/api/package[@name='java.nio']/class[@name='CharBuffer']/method[@name='duplicate']"
	    name="managedOverride">override</attr>

However, `//attr[@name='managedOverride']` was not written to support
updating`abstract` methods.

Fix that oversight so that using `//attr[@name='managedOverride']`
can be used to emit `override abstract`:

	[Register(…)]
	public abstract partial class CharBuffer : Buffer {
	    [Register(…)]
	    public override abstract CharBuffer Duplicate ();
	}

~~ Method Invokers ~~

This gets us halfway there, but there is another issue: method invokers
for both the class and base class methods are *also* emitted into the
`*Invoker` subclass:

	partial class BufferInvoker : CharBuffer {
	    public override sealed unsafe Buffer Duplicate() {…}
	    // so far so good…
	}
	partial class CharBufferInvoker : CharBuffer {
	    [Register ("duplicate", "()Ljava/nio/Buffer;", "")]
	    public override sealed unsafe Buffer     Duplicate() {…}

	    [Register ("duplicate", "()Ljava/nio/CharBuffer;", "")]
	    public override sealed unsafe CharBuffer Duplicate() {…}
	    // uh oh
	}

This results in the following error:

	error CS0111: Type 'CharBufferInvoker' already defines a member called 'Duplicate' with the same parameter types
	error CS0508: 'CharBufferInvoker.Duplicate()': return type must be 'CharBuffer' to match overridden member 'CharBuffer.Duplicate()'

This perhaps(?) could be something `generator` could detect and
prevent, but the feasibility and cost is unknown and expected to be
high.  Since this is a very rare case, we will fix it with metadata,
however we have never had metadata to apply to invokers before.

We have decided to support `//attr[@name='skipInvokerMethods']`
metadata on the `class` that the invoker class would be created for:

	<attr path="/api/package[@name='java.nio']/class[@name='CharBuffer']"
	    name="skipInvokerMethods">java/nio/Buffer.duplicate()Ljava/nio/Buffer;</attr>

The value is a comma, space, and newline-separated list of
"JNI signature-like" values consisting of:

 1. The simplified JNI name of the declaring class that contains the
    the invoker method to skip, e.g. `java/nio/Buffer`
 2. `.`
 3. The Java name of the invoker method to skip, e.g. `duplicate`.
 4. The JNI method signature of the method to skip, e.g.
   `()Ljava/nio/Buffer;`

The above `java/nio/Buffer.duplicate()Ljava/nio/Buffer;` value tells
`generator` to skip generating the `Buffer Buffer.duplicate ()` invoker
method when generating the `CharBufferInvoker` type.  Multiple invokers
to skip can be specified as a comma or space separated list.

[0]: https://learn.microsoft.com/en-us/dotnet/csharp/misc/cs0533
[1]: https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/language-specification/classes#1467-abstract-methods
[2]: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-8.0/default-interface-methods#reabstraction

Author: jpobst

src/Xamarin.SourceWriter/Models/MethodWriter.cs

@@ -88,7 +88,8 @@ public virtual void WriteSignature (CodeWriter writer)
 
 			if (IsOverride)
 				writer.Write ("override ");
-			else if (IsVirtual)
+
+			if (IsVirtual)
 				writer.Write ("virtual ");
 			else if (IsAbstract)
 				writer.Write ("abstract ");

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

@@ -150,6 +150,30 @@ public void ManagedOverrideMethod_Override ()
 			Assert.True (writer.ToString ().Contains ("public override unsafe int DoStuff ()"), $"was: `{writer.ToString ()}`");
 		}
 
+		[Test]
+		public void ManagedOverrideAbstractMethod_Override ()
+		{
+			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='true' name='DoStuff' jni-signature='()I' bridge='false' native='false' return='int' jni-return='I' static='false' synchronized='false' synthetic='false' visibility='public' managedOverride='override'></method>
+			    </class>
+			  </package>
+			</api>";
+
+			var gens = ParseApiDefinition (xml);
+			var klass = gens.Single (g => g.Name == "MyClass");
+
+			generator.Context.ContextTypes.Push (klass);
+			generator.WriteType (klass, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
+			generator.Context.ContextTypes.Pop ();
+
+			Assert.True (writer.ToString ().Contains ("public override abstract int DoStuff ();"), $"was: `{writer}`");
+		}
+
 		[Test]
 		public void ManagedOverrideMethod_None ()
 		{
@@ -298,6 +322,36 @@ public void ManagedOverrideInterfaceProperty_Reabstract ()
 			Assert.True (writer.ToString ().Contains ("abstract int Name {"), $"was: `{writer}`");
 		}
 
+		[Test]
+		public void SkipInvokerMethodsMetadata ()
+		{
+			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='true' deprecated='not deprecated' extends='java.lang.Object' extends-generic-aware='java.lang.Object' jni-extends='Ljava/lang/Object;' final='false' name='MyBaseClass' static='false' visibility='public' jni-signature='Lcom/xamarin/android/MyBaseClass;'>
+			      <method abstract='true' deprecated='not deprecated' final='false' name='doStuff' jni-signature='()Lcom/xamarin/android/MyBaseClass;' bridge='false' native='false' return='com.xamarin.android.MyBaseClass' jni-return='Lcom/xamarin/android/MyBaseClass;' static='false' synchronized='false' synthetic='false' visibility='public'></method>
+			    </class>
+			    <class abstract='true' deprecated='not deprecated' extends='com.xamarin.android.MyBaseClass' extends-generic-aware='com.xamarin.android.MyBaseClass' jni-extends='Lcom/xamarin/android/MyBaseClass;' final='false' name='MyClass' static='false' visibility='public' jni-signature='Lcom/xamarin/android/MyClass;' skipInvokerMethods='com/xamarin/android/MyBaseClass.doStuff()Lcom/xamarin/android/MyBaseClass;'>
+			      <method abstract='true' deprecated='not deprecated' final='false' name='doStuff' jni-signature='()Lcom/xamarin/android/MyClass;' bridge='false' native='false' return='com.xamarin.android.MyClass' jni-return='Lcom/xamarin/android/MyClass;' static='false' synchronized='false' synthetic='false' visibility='public' managedOverride='override' return-not-null='true'></method>
+			    </class>
+			  </package>
+			</api>";
+
+			var gens = ParseApiDefinition (xml);
+			var klass = gens.Single (g => g.Name == "MyClass");
+
+			generator.Context.ContextTypes.Push (klass);
+			generator.WriteType (klass, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
+			generator.Context.ContextTypes.Pop ();
+
+			// `override abstract` causes both invoker methods to get generated. We use metadata
+			// to suppress the base class's method to prevent a conflict.
+			Assert.True (writer.ToString ().Contains ("public override abstract Com.Xamarin.Android.MyClass DoStuff ();"), $"was: `{writer}`");
+			Assert.False (writer.ToString ().Contains ("public abstract Com.Xamarin.Android.MyBaseClass DoStuff ();"), $"was: `{writer}`");
+		}
+
 		[Test]
 		public void WriteDuplicateInterfaceEventArgs ()
 		{

tools/generator/Java.Interop.Tools.Generator.Importers/XmlApiImporter.cs

@@ -116,6 +116,10 @@ public static ClassGen CreateClass (XElement pkg, XElement elem, CodeGenerationO
 					 !options.SupportNestedInterfaceTypes
 			};
 
+			if (elem.Attribute ("skipInvokerMethods")?.Value is string skip)
+				foreach (var m in skip.Split (new char [] { ',', ' ', '\n', '\r' }, StringSplitOptions.RemoveEmptyEntries))
+					klass.SkippedInvokerMethods.Add (m);
+
 			FillApiSince (klass, pkg, elem);
 			SetLineInfo (klass, elem, options);
 

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

@@ -11,6 +11,7 @@ namespace MonoDroid.Generation
 	public class ClassGen : GenBase
 	{
 		bool fill_explicit_implementation_started;
+		HashSet<string> skipped_invoker_methods;
 
 		public List<Ctor> Ctors { get; private set; } = new List<Ctor> ();
 
@@ -355,6 +356,8 @@ public override void ResetValidation ()
 			base.ResetValidation ();
 		}
 
+		public HashSet<string> SkippedInvokerMethods => skipped_invoker_methods ??= new HashSet<string> ();
+
 		public override string ToNative (CodeGenerationOptions opt, string varname, Dictionary<string, string> mappings = null)
 		{
 			if (opt.CodeGenerationTarget == CodeGenerationTarget.JavaInterop1) {

tools/generator/SourceWriters/BoundMethodAbstractDeclaration.cs

@@ -44,6 +44,9 @@ public BoundMethodAbstractDeclaration (GenBase gen, Method method, CodeGeneratio
 
 			NewFirst = true;
 
+			if (method.ManagedOverride?.ToLowerInvariant () == "override")
+				IsOverride = true;
+
 			if (opt.CodeGenerationTarget != CodeGenerationTarget.JavaInterop1) {
 				method_callback = new MethodCallback (impl, method, opt, null, method.IsReturnCharSequence);
 			}

tools/generator/SourceWriters/ClassInvokerClass.cs

@@ -64,21 +64,21 @@ public ClassInvokerClass (ClassGen klass, CodeGenerationOptions opt)
 				Properties.Add (new ThresholdTypeGetter ());
 			}
 
-			AddMemberInvokers (klass, opt, new HashSet<string> ());
+			AddMemberInvokers (klass, opt, new HashSet<string> (), klass.SkippedInvokerMethods);
 		}
 
-		void AddMemberInvokers (ClassGen klass, CodeGenerationOptions opt, HashSet<string> members)
+		void AddMemberInvokers (ClassGen klass, CodeGenerationOptions opt, HashSet<string> members, HashSet<string> skipInvokers)
 		{
 			AddPropertyInvokers (klass, klass.Properties, members, opt);
-			AddMethodInvokers (klass, klass.Methods, members, null, opt);
+			AddMethodInvokers (klass, klass.Methods, members, skipInvokers, null, opt);
 
 			foreach (var iface in klass.GetAllDerivedInterfaces ()) {
 				AddPropertyInvokers (klass, iface.Properties.Where (p => !klass.ContainsProperty (p.Name, false, false)), members, opt);
-				AddMethodInvokers (klass, iface.Methods.Where (m => (opt.SupportDefaultInterfaceMethods || !m.IsInterfaceDefaultMethod) && !klass.ContainsMethod (m, false, false) && !klass.IsCovariantMethod (m) && !klass.ExplicitlyImplementedInterfaceMethods.Contains (m.GetSignature ())), members, iface, opt);
+				AddMethodInvokers (klass, iface.Methods.Where (m => (opt.SupportDefaultInterfaceMethods || !m.IsInterfaceDefaultMethod) && !klass.ContainsMethod (m, false, false) && !klass.IsCovariantMethod (m) && !klass.ExplicitlyImplementedInterfaceMethods.Contains (m.GetSignature ())), members, skipInvokers, iface, opt);
 			}
 
 			if (klass.BaseGen != null && klass.BaseGen.FullName != "Java.Lang.Object")
-				AddMemberInvokers (klass.BaseGen, opt, members);
+				AddMemberInvokers (klass.BaseGen, opt, members, skipInvokers);
 		}
 
 		void AddPropertyInvokers (ClassGen klass, IEnumerable<Property> properties, HashSet<string> members, CodeGenerationOptions opt)
@@ -100,9 +100,12 @@ void AddPropertyInvokers (ClassGen klass, IEnumerable<Property> properties, Hash
 			}
 		}
 
-		void AddMethodInvokers (ClassGen klass, IEnumerable<Method> methods, HashSet<string> members, InterfaceGen gen, CodeGenerationOptions opt)
+		void AddMethodInvokers (ClassGen klass, IEnumerable<Method> methods, HashSet<string> members, HashSet<string> skipInvokers, InterfaceGen gen, CodeGenerationOptions opt)
 		{
 			foreach (var m in methods) {
+				if (skipInvokers.Contains ($"{m.DeclaringType.RawJniName}.{m.JavaName}{m.JniSignature}"))
+					continue;
+
 				var sig = m.GetSignature ();
 
 				if (members.Contains (sig))