Skip to content

Commit 5a0e37e

Browse files
jpobstjpobst
authored
[generator] Support 'managedOverride' metadata (dotnet#701)
Fixes: dotnet#681 There are scenarios where `generator` does not correctly find overridden methods, often due to generic parameters. When these issues happen, the fix is to `<remove-node/>` the offending method and add a fully hand-bound version of it as an `Addition`. This is a very manual fix to simply add `override` to a method. While we would like to eventually fix all these scenarios to "do the right thing", that will be a complex fix, and perhaps we will never be able to handle all cases. For now, we can at least alleviate the pain by adding a `metadata` attribute that allows the user to override our detection, avoiding the hand-binding. This new attribute is called `managedOverride` and allows the user to specify the values `virtual` or `override`, forcing us to use the requested modifier instead of `generator`'s usual logic. <attr path="//method[@name='example']" name="managedOverride">override</attr> <attr path="//method[@name='example']" name="managedOverride">virtual</attr>
1 parent f6c12ba commit 5a0e37e

File tree

5 files changed

+116
-0
lines changed

5 files changed

+116
-0
lines changed

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

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,102 @@ public void WriteKotlinUnsignedArrayTypePropertiesClass ()
9999

100100
Assert.AreEqual (GetTargetedExpected (nameof (WriteKotlinUnsignedArrayTypePropertiesClass)), writer.ToString ().NormalizeLineEndings ());
101101
}
102+
103+
[Test]
104+
public void ManagedOverrideMethod_Virtual ()
105+
{
106+
var xml = @"<api>
107+
<package name='java.lang' jni-name='java/lang'>
108+
<class abstract='false' deprecated='not deprecated' final='false' name='Object' static='false' visibility='public' jni-signature='Ljava/lang/Object;' />
109+
</package>
110+
<package name='com.xamarin.android' jni-name='com/xamarin/android'>
111+
<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;'>
112+
<method abstract='false' 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='virtual'></method>
113+
</class>
114+
</package>
115+
</api>";
116+
117+
var gens = ParseApiDefinition (xml);
118+
var klass = gens.Single (g => g.Name == "MyClass");
119+
120+
generator.Context.ContextTypes.Push (klass);
121+
generator.WriteType (klass, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
122+
generator.Context.ContextTypes.Pop ();
123+
124+
Assert.True (writer.ToString ().Contains ("public virtual unsafe int DoStuff ()"));
125+
}
126+
127+
[Test]
128+
public void ManagedOverrideMethod_Override ()
129+
{
130+
var xml = @"<api>
131+
<package name='java.lang' jni-name='java/lang'>
132+
<class abstract='false' deprecated='not deprecated' final='false' name='Object' static='false' visibility='public' jni-signature='Ljava/lang/Object;' />
133+
</package>
134+
<package name='com.xamarin.android' jni-name='com/xamarin/android'>
135+
<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;'>
136+
<method abstract='false' 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>
137+
</class>
138+
</package>
139+
</api>";
140+
141+
var gens = ParseApiDefinition (xml);
142+
var klass = gens.Single (g => g.Name == "MyClass");
143+
144+
generator.Context.ContextTypes.Push (klass);
145+
generator.WriteType (klass, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
146+
generator.Context.ContextTypes.Pop ();
147+
148+
Assert.True (writer.ToString ().Contains ("public override unsafe int DoStuff ()"));
149+
}
150+
151+
[Test]
152+
public void ManagedOverrideProperty_Virtual ()
153+
{
154+
var xml = @"<api>
155+
<package name='java.lang' jni-name='java/lang'>
156+
<class abstract='false' deprecated='not deprecated' final='false' name='Object' static='false' visibility='public' jni-signature='Ljava/lang/Object;' />
157+
</package>
158+
<package name='com.xamarin.android' jni-name='com/xamarin/android'>
159+
<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;'>
160+
<method abstract='false' deprecated='not deprecated' final='true' name='getName' jni-signature='()I' bridge='false' native='false' return='int' jni-return='I' static='false' synchronized='false' synthetic='false' visibility='public' managedOverride='virtual'></method>
161+
</class>
162+
</package>
163+
</api>";
164+
165+
var gens = ParseApiDefinition (xml);
166+
var klass = gens.Single (g => g.Name == "MyClass");
167+
168+
generator.Context.ContextTypes.Push (klass);
169+
generator.WriteType (klass, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
170+
generator.Context.ContextTypes.Pop ();
171+
172+
Assert.True (writer.ToString ().Contains ("public virtual unsafe int Name {"));
173+
}
174+
175+
[Test]
176+
public void ManagedOverrideProperty_Override ()
177+
{
178+
var xml = @"<api>
179+
<package name='java.lang' jni-name='java/lang'>
180+
<class abstract='false' deprecated='not deprecated' final='false' name='Object' static='false' visibility='public' jni-signature='Ljava/lang/Object;' />
181+
</package>
182+
<package name='com.xamarin.android' jni-name='com/xamarin/android'>
183+
<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;'>
184+
<method abstract='false' deprecated='not deprecated' final='true' name='getName' jni-signature='()I' bridge='false' native='false' return='int' jni-return='I' static='false' synchronized='false' synthetic='false' visibility='public' managedOverride='override'></method>
185+
</class>
186+
</package>
187+
</api>";
188+
189+
var gens = ParseApiDefinition (xml);
190+
var klass = gens.Single (g => g.Name == "MyClass");
191+
192+
generator.Context.ContextTypes.Push (klass);
193+
generator.WriteType (klass, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
194+
generator.Context.ContextTypes.Pop ();
195+
196+
Assert.True (writer.ToString ().Contains ("public override unsafe int Name {"));
197+
}
102198
}
103199

104200
[TestFixture]

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,7 @@ public static Method CreateMethod (GenBase declaringType, XElement elem)
256256
IsReturnEnumified = elem.Attribute ("enumReturn") != null,
257257
IsStatic = elem.XGetAttribute ("static") == "true",
258258
JavaName = elem.XGetAttribute ("name"),
259+
ManagedOverride = elem.XGetAttribute ("managedOverride"),
259260
ManagedReturn = elem.XGetAttribute ("managedReturn"),
260261
PropertyNameOverride = elem.XGetAttribute ("propertyName"),
261262
Return = elem.XGetAttribute ("return"),

‎tools/generator/Java.Interop.Tools.Generator.ObjectModel/Method.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ public Method (GenBase declaringType) : base (declaringType)
2626
public bool IsStatic { get; set; }
2727
public bool IsVirtual { get; set; }
2828
public string JavaName { get; set; }
29+
public string ManagedOverride { get; set; }
2930
public string ManagedReturn { get; set; }
3031
public string PropertyNameOverride { get; set; }
3132
public string Return { get; set; }

‎tools/generator/SourceWriters/BoundMethod.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,15 @@ public BoundMethod (GenBase type, Method method, CodeGenerationOptions opt, bool
4444
if ((IsVirtual || !IsOverride) && type.RequiresNew (method.AdjustedName, method))
4545
IsShadow = true;
4646

47+
// Allow user to override our virtual/override logic
48+
if (method.ManagedOverride?.ToLowerInvariant () == "virtual") {
49+
IsVirtual = true;
50+
IsOverride = false;
51+
} else if (method.ManagedOverride?.ToLowerInvariant () == "override") {
52+
IsVirtual = false;
53+
IsOverride = true;
54+
}
55+
4756
ReturnType = new TypeReferenceWriter (opt.GetTypeReferenceName (method.RetVal));
4857

4958
if (method.DeclaringType.IsGeneratable)

‎tools/generator/SourceWriters/BoundProperty.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,15 @@ public BoundProperty (GenBase gen, Property property, CodeGenerationOptions opt,
5555
}
5656
}
5757

58+
// Allow user to override our virtual/override logic
59+
if (!forceOverride && (property.Getter ?? property.Setter).ManagedOverride?.ToLowerInvariant () == "virtual") {
60+
IsVirtual = true;
61+
IsOverride = false;
62+
} else if (!forceOverride && (property.Getter ?? property.Setter).ManagedOverride?.ToLowerInvariant () == "override") {
63+
IsVirtual = false;
64+
IsOverride = true;
65+
}
66+
5867
// Unlike [Register], [Obsolete] cannot be put on property accessors, so we can apply them only under limited condition...
5968
if (property.Getter.Deprecated != null && (property.Setter == null || property.Setter.Deprecated != null))
6069
Attributes.Add (new ObsoleteAttr (property.Getter.Deprecated.Replace ("\"", "\"\"").Trim () + (property.Setter != null && property.Setter.Deprecated != property.Getter.Deprecated ? " " + property.Setter.Deprecated.Replace ("\"", "\"\"").Trim () : null)));

0 commit comments

Comments
 (0)