Skip to content

Commit

Permalink
[generator] Extend skipInvokerMethods support to interfaces. (#1202)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jpobst authored and jonpryor committed Mar 6, 2024
1 parent bec0326 commit e557044
Show file tree
Hide file tree
Showing 8 changed files with 48 additions and 13 deletions.
26 changes: 26 additions & 0 deletions tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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 ()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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> ();

Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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; }
Expand All @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion tools/generator/SourceWriters/ClassInvokerClass.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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 ();
Expand Down
13 changes: 8 additions & 5 deletions tools/generator/SourceWriters/InterfaceInvokerClass.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Check failure on line 58 in tools/generator/SourceWriters/InterfaceInvokerClass.cs

View check run for this annotation

Azure Pipelines / Java.Interop (Mac - .NET)

tools/generator/SourceWriters/InterfaceInvokerClass.cs#L58

tools/generator/SourceWriters/InterfaceInvokerClass.cs(58,4): Error CS1501: No overload for method 'AddMemberInvokers' takes 5 arguments

Check failure on line 58 in tools/generator/SourceWriters/InterfaceInvokerClass.cs

View check run for this annotation

Azure Pipelines / Java.Interop

tools/generator/SourceWriters/InterfaceInvokerClass.cs#L58

tools/generator/SourceWriters/InterfaceInvokerClass.cs(58,4): Error CS1501: No overload for method 'AddMemberInvokers' takes 5 arguments
}

void AddMemberInvokers (InterfaceGen iface, HashSet<string> 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);

Check failure on line 64 in tools/generator/SourceWriters/InterfaceInvokerClass.cs

View check run for this annotation

Azure Pipelines / Java.Interop (Mac - .NET)

tools/generator/SourceWriters/InterfaceInvokerClass.cs#L64

tools/generator/SourceWriters/InterfaceInvokerClass.cs(64,110): Error CS0103: The name 'skipInvokers' does not exist in the current context

Check failure on line 64 in tools/generator/SourceWriters/InterfaceInvokerClass.cs

View check run for this annotation

Azure Pipelines / Java.Interop

tools/generator/SourceWriters/InterfaceInvokerClass.cs#L64

tools/generator/SourceWriters/InterfaceInvokerClass.cs(64,110): Error CS0103: The name 'skipInvokers' does not exist in the current 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);

Check failure on line 69 in tools/generator/SourceWriters/InterfaceInvokerClass.cs

View check run for this annotation

Azure Pipelines / Java.Interop (Mac - .NET)

tools/generator/SourceWriters/InterfaceInvokerClass.cs#L69

tools/generator/SourceWriters/InterfaceInvokerClass.cs(69,280): Error CS0103: The name 'skipInvokers' does not exist in the current context

Check failure on line 69 in tools/generator/SourceWriters/InterfaceInvokerClass.cs

View check run for this annotation

Azure Pipelines / Java.Interop

tools/generator/SourceWriters/InterfaceInvokerClass.cs#L69

tools/generator/SourceWriters/InterfaceInvokerClass.cs(69,280): Error CS0103: The name 'skipInvokers' does not exist in the current context
AddCharSequenceEnumerators (i);
}
}
Expand All @@ -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))
Expand Down

0 comments on commit e557044

Please sign in to comment.