[generator] Extend skipInvokerMethods support to interfaces. (#1202) (

#1203) Context: 73ebad2 Context: dotnet/android-libraries#779 Commit 73ebad2 added support for the metadata attribute `//class[@skipInvokerMethods]`, which allowed us to suppress generation of invoker methods for a class. The AndroidX Media3 binding in dotnet/android-libraries#779 hits a similar issue wherein we generate incorrect generics on an invoker type. However, this is an invoker type for an *`interface`*, not a `class`. Extend our `skipInvokerMethods` support so that it can be used as `//interface[@skipInvokerMethods]` as well. Co-authored-by: Jonathan Pobst <[email protected]>
dotnet · Mar 7, 2024 · 3c5cf34 · 3c5cf34
1 parent bec0326
commit 3c5cf34
Show file tree

Hide file tree

Showing 8 changed files with 49 additions and 14 deletions.
diff --git a/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs b/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs
@@ -352,6 +352,32 @@ public void SkipInvokerMethodsMetadata ()
 			Assert.False (writer.ToString ().Contains ("public abstract Com.Xamarin.Android.MyBaseClass DoStuff ();"), $"was: `{writer}`");
 		}
 
+		[Test]
+		public void SkipInterfaceInvokerMethodsMetadata ()
+		{
+			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='true' 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;' skipInvokerMethods='com/xamarin/android/MyInterface.countAffectedRows()I'>
+			      <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'></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 the invoker for 'countAffectedRows' isn't generated
+			Assert.False (writer.ToString ().Contains ("static Delegate cb_countAffectedRows;"), $"was: `{writer}`");
+			Assert.False (writer.ToString ().Contains ("InvokeAbstractInt32Method"), $"was: `{writer}`");
+		}
+
 		[Test]
 		public void CompatVirtualMethod_Class ()
 		{

diff --git a/tools/generator/Java.Interop.Tools.Generator.Importers/XmlApiImporter.cs b/tools/generator/Java.Interop.Tools.Generator.Importers/XmlApiImporter.cs
@@ -117,10 +117,6 @@ 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);
 
@@ -264,6 +260,10 @@ public static GenBaseSupport CreateGenBaseSupport (XElement pkg, XElement elem,
 				Visibility = elem.XGetAttribute ("visibility")
 			};
 
+			if (elem.Attribute ("skipInvokerMethods")?.Value is string skip)
+				foreach (var m in skip.Split (new char [] { ',', ' ', '\n', '\r' }, StringSplitOptions.RemoveEmptyEntries))
+					support.SkippedInvokerMethods.Add (m);
+
 			if (support.IsDeprecated) {
 				support.DeprecatedComment = elem.XGetAttribute ("deprecated");
 

diff --git a/tools/generator/Java.Interop.Tools.Generator.ObjectModel/ClassGen.cs b/tools/generator/Java.Interop.Tools.Generator.ObjectModel/ClassGen.cs
@@ -11,7 +11,6 @@ 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> ();
 
@@ -356,8 +355,6 @@ 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) {

diff --git a/tools/generator/Java.Interop.Tools.Generator.ObjectModel/GenBase.cs b/tools/generator/Java.Interop.Tools.Generator.ObjectModel/GenBase.cs
@@ -833,6 +833,8 @@ bool ReturnTypeMatches (Method m, Method mm)
 
 		public bool ShouldGenerateAnnotationAttribute => IsAnnotation;
 
+		public HashSet<string> SkippedInvokerMethods => support.SkippedInvokerMethods;
+
 		public void StripNonBindables (CodeGenerationOptions opt)
 		{
 			// Strip out default interface methods if not desired

diff --git a/tools/generator/Java.Interop.Tools.Generator.ObjectModel/GenBaseSupport.cs b/tools/generator/Java.Interop.Tools.Generator.ObjectModel/GenBaseSupport.cs
@@ -1,9 +1,12 @@
 using System;
+using System.Collections.Generic;
 
 namespace MonoDroid.Generation
 {
 	public class GenBaseSupport
 	{
+		HashSet<string> skipped_invoker_methods;
+
 		public string AnnotatedVisibility { get; set; }
 		public bool IsAcw { get; set; }
 		public bool IsDeprecated { get; set; }
@@ -21,6 +24,8 @@ public class GenBaseSupport
 		public string Visibility { get; set; }
 		public GenericParameterDefinitionList TypeParameters { get; set; }
 
+		public HashSet<string> SkippedInvokerMethods => skipped_invoker_methods ??= new HashSet<string> ();
+
 		public virtual bool OnValidate (CodeGenerationOptions opt)
 		{
 			// See com.google.inject.internal.util package for this case.

diff --git a/tools/generator/Java.Interop.Tools.Generator.ObjectModel/Method.cs b/tools/generator/Java.Interop.Tools.Generator.ObjectModel/Method.cs
@@ -193,6 +193,8 @@ public string GetMetadataXPathReference (GenBase declaringType) =>
 
 		public string GetSignature () => $"n_{JavaName}:{JniSignature}:{ConnectorName}";
 
+		public string GetSkipInvokerSignature () => $"{DeclaringType.RawJniName}.{JavaName}{JniSignature}";
+
 		public bool IsEventHandlerWithHandledProperty => RetVal.JavaName == "boolean" && EventName != "";
 
 		public override bool IsGeneric => base.IsGeneric || RetVal.IsGeneric;

diff --git a/tools/generator/SourceWriters/ClassInvokerClass.cs b/tools/generator/SourceWriters/ClassInvokerClass.cs
@@ -103,7 +103,7 @@ void AddPropertyInvokers (ClassGen klass, IEnumerable<Property> properties, Hash
 		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}"))
+				if (skipInvokers.Contains (m.GetSkipInvokerSignature ()))
 					continue;
 
 				var sig = m.GetSignature ();

diff --git a/tools/generator/SourceWriters/InterfaceInvokerClass.cs b/tools/generator/SourceWriters/InterfaceInvokerClass.cs
@@ -55,18 +55,18 @@ public InterfaceInvokerClass (InterfaceGen iface, CodeGenerationOptions opt, Cod
 
 			Constructors.Add (new InterfaceInvokerConstructor (opt, iface, context));
 
-			AddMemberInvokers (iface, new HashSet<string> (), opt, context);
+			AddMemberInvokers (iface, new HashSet<string> (), iface.SkippedInvokerMethods, opt, context);
 		}
 
-		void AddMemberInvokers (InterfaceGen iface, HashSet<string> members, CodeGenerationOptions opt, CodeGeneratorContext context)
+		void AddMemberInvokers (InterfaceGen iface, HashSet<string> members, HashSet<string> skipInvokers, CodeGenerationOptions opt, CodeGeneratorContext context)
 		{
 			AddPropertyInvokers (iface, iface.Properties.Where (p => !p.Getter.IsStatic && !p.Getter.IsInterfaceDefaultMethod), members, opt, context);
-			AddMethodInvokers (iface, iface.Methods.Where (m => !m.IsStatic && !m.IsInterfaceDefaultMethod), members, opt, context);
+			AddMethodInvokers (iface, iface.Methods.Where (m => !m.IsStatic && !m.IsInterfaceDefaultMethod), members, skipInvokers, opt, context);
 			AddCharSequenceEnumerators (iface);
 
 			foreach (var i in iface.GetAllDerivedInterfaces ()) {
 				AddPropertyInvokers (iface, i.Properties.Where (p => !p.Getter.IsStatic && !p.Getter.IsInterfaceDefaultMethod), members, opt, context);
-				AddMethodInvokers (iface, i.Methods.Where (m => !m.IsStatic && !m.IsInterfaceDefaultMethod && !iface.IsCovariantMethod (m) && !(i.FullName.StartsWith ("Java.Lang.ICharSequence", StringComparison.Ordinal) && m.Name.EndsWith ("Formatted", StringComparison.Ordinal))), members, opt, context);
+				AddMethodInvokers (iface, i.Methods.Where (m => !m.IsStatic && !m.IsInterfaceDefaultMethod && !iface.IsCovariantMethod (m) && !(i.FullName.StartsWith ("Java.Lang.ICharSequence", StringComparison.Ordinal) && m.Name.EndsWith ("Formatted", StringComparison.Ordinal))), members, skipInvokers, opt, context);
 				AddCharSequenceEnumerators (i);
 			}
 		}
@@ -90,10 +90,13 @@ void AddPropertyInvokers (InterfaceGen iface, IEnumerable<Property> properties,
 				Properties.Add (new InterfaceInvokerProperty (iface, prop, opt, context));
 			}
 		}
-
-		void AddMethodInvokers (InterfaceGen iface, IEnumerable<Method> methods, HashSet<string> members, CodeGenerationOptions opt, CodeGeneratorContext context)
+		
+		void AddMethodInvokers (InterfaceGen iface, IEnumerable<Method> methods, HashSet<string> members, HashSet<string> skipInvokers, CodeGenerationOptions opt, CodeGeneratorContext context)
 		{
 			foreach (var m in methods) {
+				if (skipInvokers.Contains (m.GetSkipInvokerSignature ()))
+					continue;
+
 				var sig = m.GetSignature ();
 
 				if (members.Contains (sig))