From e557044f81917b61031f2ee887700ac43a526475 Mon Sep 17 00:00:00 2001 From: Jonathan Pobst Date: Wed, 6 Mar 2024 08:20:09 -1000 Subject: [PATCH 1/2] [generator] Extend `skipInvokerMethods` support to interfaces. (#1202) Context: 73ebad2496a0d6df251fd24e216248da70e3fea9 Context: https://github.com/xamarin/AndroidX/pull/779 Commit 73ebad24 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 xamarin/AndroidX#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. --- .../Unit-Tests/CodeGeneratorTests.cs | 26 +++++++++++++++++++ .../XmlApiImporter.cs | 8 +++--- .../ClassGen.cs | 3 --- .../GenBase.cs | 2 ++ .../GenBaseSupport.cs | 5 ++++ .../Method.cs | 2 ++ .../SourceWriters/ClassInvokerClass.cs | 2 +- .../SourceWriters/InterfaceInvokerClass.cs | 13 ++++++---- 8 files changed, 48 insertions(+), 13 deletions(-) diff --git a/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs b/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs index 612ff56d7..1a46ad1c6 100644 --- 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 = @" + + + + + + + + + "; + + 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 index cd248a514..fc2b8415a 100644 --- 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 index b70aa42a3..f2629d0fc 100644 --- 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 skipped_invoker_methods; public List Ctors { get; private set; } = new List (); @@ -356,8 +355,6 @@ public override void ResetValidation () base.ResetValidation (); } - public HashSet SkippedInvokerMethods => skipped_invoker_methods ??= new HashSet (); - public override string ToNative (CodeGenerationOptions opt, string varname, Dictionary 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 index c387b68e0..54e1e54b7 100644 --- 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 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 index a3a1f530a..7aa011dad 100644 --- 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 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 SkippedInvokerMethods => skipped_invoker_methods ??= new HashSet (); + 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 index cd5bced9d..4d711708f 100644 --- 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 index cf0466fdd..41e983ea7 100644 --- a/tools/generator/SourceWriters/ClassInvokerClass.cs +++ b/tools/generator/SourceWriters/ClassInvokerClass.cs @@ -103,7 +103,7 @@ void AddPropertyInvokers (ClassGen klass, IEnumerable properties, Hash void AddMethodInvokers (ClassGen klass, IEnumerable methods, HashSet members, HashSet 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 index c0e39419e..8015ea05a 100644 --- 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 (), opt, context); + AddMemberInvokers (iface, new HashSet (), iface.SkippedInvokerMethods, opt, context); } void AddMemberInvokers (InterfaceGen iface, HashSet members, 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 properties, Properties.Add (new InterfaceInvokerProperty (iface, prop, opt, context)); } } - - void AddMethodInvokers (InterfaceGen iface, IEnumerable methods, HashSet members, CodeGenerationOptions opt, CodeGeneratorContext context) + + void AddMethodInvokers (InterfaceGen iface, IEnumerable methods, HashSet members, HashSet skipInvokers, CodeGenerationOptions opt, CodeGeneratorContext context) { foreach (var m in methods) { + if (skipInvokers.Contains (m.GetSkipInvokerSignature ())) + continue; + var sig = m.GetSignature (); if (members.Contains (sig)) From 66661082f20a9355ff054540b439765953d404c0 Mon Sep 17 00:00:00 2001 From: Jonathan Pobst Date: Wed, 6 Mar 2024 11:49:55 -1000 Subject: [PATCH 2/2] Fix merge issue. --- tools/generator/SourceWriters/InterfaceInvokerClass.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/generator/SourceWriters/InterfaceInvokerClass.cs b/tools/generator/SourceWriters/InterfaceInvokerClass.cs index 8015ea05a..57b2bf406 100644 --- a/tools/generator/SourceWriters/InterfaceInvokerClass.cs +++ b/tools/generator/SourceWriters/InterfaceInvokerClass.cs @@ -58,7 +58,7 @@ public InterfaceInvokerClass (InterfaceGen iface, CodeGenerationOptions opt, Cod AddMemberInvokers (iface, new HashSet (), iface.SkippedInvokerMethods, opt, context); } - void AddMemberInvokers (InterfaceGen iface, HashSet members, CodeGenerationOptions opt, CodeGeneratorContext context) + void AddMemberInvokers (InterfaceGen iface, HashSet members, HashSet 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, skipInvokers, opt, context);