Skip to content

Commit b7312e8

Browse files
jpobstjpobst
committed
[generator] Be smarter about when members with same names need "new".
1 parent 3226a4b commit b7312e8

File tree

5 files changed

+148
-8
lines changed

5 files changed

+148
-8
lines changed

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

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Linq;
4+
using System.Text;
5+
using System.Threading.Tasks;
6+
using MonoDroid.Generation;
7+
using NUnit.Framework;
8+
9+
namespace generatortests
10+
{
11+
[TestFixture]
12+
public class GenBaseTests
13+
{
14+
CodeGenerationOptions options = new CodeGenerationOptions ();
15+
16+
[Test]
17+
public void PropertyRequiresNew ()
18+
{
19+
var c = SupportTypeBuilder.CreateClassWithProperty ("MyClass", "java.myClass", "Handle", "int", options);
20+
21+
Assert.True (c.RequiresNew (c.Properties.First ()));
22+
23+
c.Properties.First ().Name = "Handle2";
24+
25+
Assert.False (c.RequiresNew (c.Properties.First ()));
26+
}
27+
28+
[Test]
29+
public void ToStringRequiresNew () => TestParameterlessMethods ("ToString");
30+
31+
[Test]
32+
public void GetTypeRequiresNew () => TestParameterlessMethods ("GetType");
33+
34+
[Test]
35+
public void GetHashCodeRequiresNew () => TestParameterlessMethods ("GetHashCode");
36+
37+
[Test]
38+
public void StaticEqualsCodeRequiresNew () => TestStaticMethodsWithTwoParameters ("Equals");
39+
40+
[Test]
41+
public void ReferenceEqualsEqualsCodeRequiresNew () => TestStaticMethodsWithTwoParameters ("ReferenceEquals");
42+
43+
[Test]
44+
public void TestEqualsMethodsWithOneParameter ()
45+
{
46+
var c = SupportTypeBuilder.CreateClass ("java.myClass", options);
47+
var m = SupportTypeBuilder.CreateMethod (c, "Equals", options, "void", false, false);
48+
49+
// No because 0 parameters
50+
Assert.False (c.RequiresNew (m.Name, m));
51+
52+
var p0 = new Parameter ("p0", "object", "object", false);
53+
m.Parameters.Add (p0);
54+
55+
// Yes
56+
Assert.True (c.RequiresNew (m.Name, m));
57+
58+
// No because static
59+
m.IsStatic = true;
60+
Assert.False (c.RequiresNew (m.Name, m));
61+
m.IsStatic = false;
62+
63+
// No because parameter is wrong type
64+
var p1 = new Parameter ("p1", "string", "string", false);
65+
m = SupportTypeBuilder.CreateMethod (c, "Equals", options, "void", true, false, p1);
66+
67+
Assert.False (c.RequiresNew (m.Name, m));
68+
}
69+
70+
void TestParameterlessMethods (string name)
71+
{
72+
var c = SupportTypeBuilder.CreateClass ("java.myClass", options);
73+
var m = SupportTypeBuilder.CreateMethod (c, name, options);
74+
75+
// Yes
76+
Assert.True (c.RequiresNew (m.Name, m));
77+
78+
// No because static
79+
m.IsStatic = true;
80+
Assert.False (c.RequiresNew (m.Name, m));
81+
m.IsStatic = false;
82+
83+
// No because > 0 parameters
84+
m.Parameters.Add (new Parameter ("value", "int", "int", false));
85+
86+
Assert.False (c.RequiresNew (m.Name, m));
87+
}
88+
89+
void TestStaticMethodsWithTwoParameters (string name)
90+
{
91+
var c = SupportTypeBuilder.CreateClass ("java.myClass", options);
92+
93+
var p0 = new Parameter ("p0", "object", "object", false);
94+
var p1 = new Parameter ("p1", "object", "object", false);
95+
var m = SupportTypeBuilder.CreateMethod (c, name, options, "void", true, false, p0, p1);
96+
97+
// Yes
98+
Assert.True (c.RequiresNew (m.Name, m));
99+
100+
// No because static
101+
m.IsStatic = false;
102+
Assert.False (c.RequiresNew (m.Name, m));
103+
m.IsStatic = true;
104+
105+
// No because != 2 parameters
106+
m.Parameters.Add (new Parameter ("value", "int", "int", false));
107+
Assert.False (c.RequiresNew (m.Name, m));
108+
109+
// No because parameter is wrong type
110+
var p2 = new Parameter ("p1", "string", "string", false);
111+
m = SupportTypeBuilder.CreateMethod (c, name, options, "void", true, false, p0, p2);
112+
113+
Assert.False (c.RequiresNew (m.Name, m));
114+
}
115+
}
116+
}

‎tests/generator-Tests/expected.ji/NormalMethods/Xamarin.Test.SomeObject.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ static int n_Handle_Ljava_lang_Object_Ljava_lang_Throwable_ (IntPtr jnienv, IntP
9999

100100
// Metadata.xml XPath method reference: path="/api/package[@name='xamarin.test']/class[@name='SomeObject']/method[@name='handle' and count(parameter)=2 and parameter[1][@type='java.lang.Object'] and parameter[2][@type='java.lang.Throwable']]"
101101
[Register ("handle", "(Ljava/lang/Object;Ljava/lang/Throwable;)I", "GetHandle_Ljava_lang_Object_Ljava_lang_Throwable_Handler")]
102-
public new virtual unsafe int Handle (global::Java.Lang.Object o, global::Java.Lang.Throwable t)
102+
public virtual unsafe int Handle (global::Java.Lang.Object o, global::Java.Lang.Throwable t)
103103
{
104104
const string __id = "handle.(Ljava/lang/Object;Ljava/lang/Throwable;)I";
105105
try {

‎tests/generator-Tests/expected/NormalMethods/Xamarin.Test.SomeObject.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ static int n_Handle_Ljava_lang_Object_Ljava_lang_Throwable_ (IntPtr jnienv, IntP
110110
static IntPtr id_handle_Ljava_lang_Object_Ljava_lang_Throwable_;
111111
// Metadata.xml XPath method reference: path="/api/package[@name='xamarin.test']/class[@name='SomeObject']/method[@name='handle' and count(parameter)=2 and parameter[1][@type='java.lang.Object'] and parameter[2][@type='java.lang.Throwable']]"
112112
[Register ("handle", "(Ljava/lang/Object;Ljava/lang/Throwable;)I", "GetHandle_Ljava_lang_Object_Ljava_lang_Throwable_Handler")]
113-
public new virtual unsafe int Handle (global::Java.Lang.Object o, global::Java.Lang.Throwable t)
113+
public virtual unsafe int Handle (global::Java.Lang.Object o, global::Java.Lang.Throwable t)
114114
{
115115
if (id_handle_Ljava_lang_Object_Ljava_lang_Throwable_ == IntPtr.Zero)
116116
id_handle_Ljava_lang_Object_Ljava_lang_Throwable_ = JNIEnv.GetMethodID (class_ref, "handle", "(Ljava/lang/Object;Ljava/lang/Throwable;)I");

‎tools/generator/Java.Interop.Tools.Generator.CodeGeneration/CodeGenerator.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1119,7 +1119,7 @@ public void WriteMethodAbstractDeclaration (Method method, string indent, Interf
11191119
WriteMethodCustomAttributes (method, indent);
11201120
writer.WriteLine ("{0}{1}{2} abstract {3} {4} ({5});",
11211121
indent,
1122-
impl.RequiresNew (method.Name) ? "new " : "",
1122+
impl.RequiresNew (method.Name, method) ? "new " : "",
11231123
method.Visibility,
11241124
opt.GetOutputName (method.RetVal.FullName),
11251125
name,
@@ -1367,7 +1367,7 @@ public void WriteMethod (Method method, string indent, GenBase type, bool genera
13671367
seal = string.Empty;
13681368
}
13691369

1370-
if ((string.IsNullOrEmpty (virt_ov) || virt_ov == " virtual") && type.RequiresNew (method.AdjustedName)) {
1370+
if ((string.IsNullOrEmpty (virt_ov) || virt_ov == " virtual") && type.RequiresNew (method.AdjustedName, method)) {
13711371
virt_ov = " new" + virt_ov;
13721372
}
13731373
string ret = opt.GetOutputName (method.RetVal.FullName);
@@ -1460,7 +1460,7 @@ public void WriteProperty (Property property, GenBase gen, string indent, bool w
14601460
force_override = true;
14611461

14621462
string decl_name = property.AdjustedName;
1463-
string needNew = gen.RequiresNew (decl_name) ? " new" : "";
1463+
string needNew = gen.RequiresNew (property) ? " new" : "";
14641464
string virtual_override = String.Empty;
14651465
bool is_virtual = property.Getter.IsVirtual && (property.Setter == null || property.Setter.IsVirtual);
14661466
if (with_callbacks && is_virtual) {
@@ -1537,7 +1537,7 @@ public void WritePropertyAbstractDeclaration (Property property, string indent,
15371537
string abstract_name = property.AdjustedName;
15381538
string visibility = property.Getter.RetVal.IsGeneric ? "protected" : property.Getter.Visibility;
15391539
if (!overrides) {
1540-
requiresNew = gen.RequiresNew (abstract_name);
1540+
requiresNew = gen.RequiresNew (property);
15411541
WritePropertyCallbacks (property, indent, gen, abstract_name);
15421542
}
15431543
writer.WriteLine ("{0}{1}{2} abstract{3} {4} {5} {{",

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

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -727,11 +727,35 @@ public string [] PreCallback (CodeGenerationOptions opt, string var_name, bool o
727727

728728
public string RawVisibility => support.Visibility;
729729

730-
public bool RequiresNew (string memberName)
730+
public bool RequiresNew (Property property)
731731
{
732-
if (ObjectRequiresNew.Contains (memberName))
732+
if (string.Compare (property.AdjustedName, "Handle", true) == 0)
733733
return true;
734734

735+
return IsThrowable () && ThrowableRequiresNew.Contains (property.AdjustedName);
736+
}
737+
738+
public bool RequiresNew (string memberName, Method method)
739+
{
740+
switch (memberName.ToLowerInvariant ()) {
741+
case "gethashcode":
742+
case "gettype":
743+
case "tostring":
744+
return !method.IsStatic && method.Parameters.Count == 0;
745+
case "equals":
746+
if (!method.IsStatic && method.Parameters.Count == 1 && method.Parameters.All (p => p.Type == "object"))
747+
return true;
748+
if (method.IsStatic && method.Parameters.Count == 2 && method.Parameters.All (p => p.Type == "object"))
749+
return true;
750+
751+
break;
752+
case "referenceequals":
753+
if (method.IsStatic && method.Parameters.Count == 2 && method.Parameters.All (p => p.Type == "object"))
754+
return true;
755+
756+
break;
757+
}
758+
735759
return IsThrowable () && ThrowableRequiresNew.Contains (memberName);
736760
}
737761

0 commit comments

Comments
 (0)