Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[generator] Allow 'managedOverride' metadata to support 'abstract' methods. #1086

Merged
merged 4 commits into from
Mar 7, 2023
Merged

[generator] Allow 'managedOverride' metadata to support 'abstract' methods. #1086

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c694313
[generator] Allow 'managedOverride' metadata to support 'abstract' me…
jpobst Feb 28, 2023
1dd91b5
[generator] Add support for 'skipInvokerMethods' metadata.
jpobst Mar 2, 2023
9fbfbef
Allow space as a separator for 'skipInvokerMethods'.
jpobst Mar 2, 2023
dc7a765
Allow newline delimited attribute.
jpobst Mar 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/Xamarin.SourceWriter/Models/MethodWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 ");
Expand Down
54 changes: 54 additions & 0 deletions tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 ()
{
Expand Down Expand Up @@ -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 ()
{
Expand Down
4 changes: 4 additions & 0 deletions tools/generator/Java.Interop.Tools.Generator.Importers/XmlApiImporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 [] { ',', ' ' }))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should use StringSplitOptions.RemoveEmptyEntries: https://learn.microsoft.com/en-us/dotnet/api/system.string.split?view=net-7.0#system-string-split(system-char()-system-stringsplitoptions)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should also add \n and \r in there as well.

klass.SkippedInvokerMethods.Add (m);

FillApiSince (klass, pkg, elem);
SetLineInfo (klass, elem, options);

Expand Down
3 changes: 3 additions & 0 deletions tools/generator/Java.Interop.Tools.Generator.ObjectModel/ClassGen.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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> ();

Expand Down Expand Up @@ -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) {
Expand Down
3 changes: 3 additions & 0 deletions tools/generator/SourceWriters/BoundMethodAbstractDeclaration.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
15 changes: 9 additions & 6 deletions tools/generator/SourceWriters/ClassInvokerClass.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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))
Expand Down