[generator] Fix generated code that caused CS0169 warnings. (#625)

mblowey · jpobst · commit 6608c590767e · 2020-04-23T11:42:50.000-05:00
* Modified the generator tool so that it no longer adds a 'new' modifier to internal fields (like class_ref) when the class's base is in an external assembly. - This prevents a CS0109 warnings during binding generation. - Updated expected generator output files for existing tests. - Added two new tests that verify the expected output when the base class is in an external assembly and when the base class is in the same assembly as the derived class. (cherry picked from commit ce8dc40)
diff --git a/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteClass.txt b/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteClass.txt
@@ -3,7 +3,7 @@
 public partial class MyClass  {
 
 	static readonly JniPeerMembers _members = new JniPeerMembers ("java/code/MyClass", typeof (MyClass));
-	internal static new IntPtr class_ref {
+	internal static IntPtr class_ref {
 		get {
 			return _members.JniPeerType.PeerReference.Handle;
 		}
diff --git a/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteKotlinUnsignedArrayTypeMethodsClass.txt b/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteKotlinUnsignedArrayTypeMethodsClass.txt
@@ -3,7 +3,7 @@
 public partial class MyClass  {
 
 	static readonly JniPeerMembers _members = new JniPeerMembers ("java/code/MyClass", typeof (MyClass));
-	internal static new IntPtr class_ref {
+	internal static IntPtr class_ref {
 		get {
 			return _members.JniPeerType.PeerReference.Handle;
 		}
diff --git a/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteKotlinUnsignedArrayTypePropertiesClass.txt b/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteKotlinUnsignedArrayTypePropertiesClass.txt
@@ -3,7 +3,7 @@
 public partial class MyClass  {
 
 	static readonly JniPeerMembers _members = new JniPeerMembers ("java/code/MyClass", typeof (MyClass));
-	internal static new IntPtr class_ref {
+	internal static IntPtr class_ref {
 		get {
 			return _members.JniPeerType.PeerReference.Handle;
 		}
diff --git a/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteKotlinUnsignedTypeMethodsClass.txt b/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteKotlinUnsignedTypeMethodsClass.txt
@@ -3,7 +3,7 @@
 public partial class MyClass  {
 
 	static readonly JniPeerMembers _members = new JniPeerMembers ("java/code/MyClass", typeof (MyClass));
-	internal static new IntPtr class_ref {
+	internal static IntPtr class_ref {
 		get {
 			return _members.JniPeerType.PeerReference.Handle;
 		}
diff --git a/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteKotlinUnsignedTypePropertiesClass.txt b/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteKotlinUnsignedTypePropertiesClass.txt
@@ -3,7 +3,7 @@
 public partial class MyClass  {
 
 	static readonly JniPeerMembers _members = new JniPeerMembers ("java/code/MyClass", typeof (MyClass));
-	internal static new IntPtr class_ref {
+	internal static IntPtr class_ref {
 		get {
 			return _members.JniPeerType.PeerReference.Handle;
 		}
diff --git a/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1-NRT/WriteClass.txt b/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1-NRT/WriteClass.txt
@@ -3,7 +3,7 @@
 public partial class MyClass  {
 
 	static readonly JniPeerMembers _members = new XAPeerMembers ("java/code/MyClass", typeof (MyClass));
-	internal static new IntPtr class_ref {
+	internal static IntPtr class_ref {
 		get {
 			return _members.JniPeerType.PeerReference.Handle;
 		}
diff --git a/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteClass.txt b/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteClass.txt
@@ -3,7 +3,7 @@
 public partial class MyClass  {
 
 	static readonly JniPeerMembers _members = new XAPeerMembers ("java/code/MyClass", typeof (MyClass));
-	internal static new IntPtr class_ref {
+	internal static IntPtr class_ref {
 		get {
 			return _members.JniPeerType.PeerReference.Handle;
 		}
diff --git a/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XamarinAndroid/WriteClass.txt b/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XamarinAndroid/WriteClass.txt
@@ -2,8 +2,8 @@
 [global::Android.Runtime.Register ("java/code/MyClass", DoNotGenerateAcw=true)]
 public partial class MyClass  {
 
-	internal static new IntPtr java_class_handle;
-	internal static new IntPtr class_ref {
+	internal static IntPtr java_class_handle;
+	internal static IntPtr class_ref {
 		get {
 			return JNIEnv.FindClass ("java/code/MyClass", ref java_class_handle);
 		}
diff --git a/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs b/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs
@@ -1016,5 +1016,51 @@ public void WritePropertyExplicitInterfaceParameterName ()
 			var result = writer.ToString ().NormalizeLineEndings ();
 			Assert.False (result.Contains ("p0"));
 		}
+
+		[Test]
+		public void WriteClassExternalBase ()
+		{
+			// Tests the case where a class inherits from a class that is not in the same assembly.
+			// Specifically, the internal class_ref field does NOT need the new modifier.
+			//  - This prevents a CS0109 warning from being generated.
+
+			options.SymbolTable.AddType (new TestClass (null, "Java.Lang.Object"));
+
+			var @class = SupportTypeBuilder.CreateClass ("java.code.MyClass", options, "Java.Lang.Object");
+			@class.Validate (options, new GenericParameterDefinitionList (), generator.Context);
+
+			generator.Context.ContextTypes.Push (@class);
+			generator.WriteClass (@class, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
+			generator.Context.ContextTypes.Pop ();
+
+			var result = writer.ToString ().NormalizeLineEndings ();
+			Assert.True (result.Contains ("internal static IntPtr class_ref"));
+			Assert.False (result.Contains ("internal static new IntPtr class_ref"));
+		}
+
+		[Test]
+		public void WriteClassInternalBase ()
+		{
+			// Tests the case where a class inherits from Java.Lang.Object and is in the same assembly.
+			// Specifically, the internal class_ref field does need the new modifier.
+			// - This prevents a CS0108 warning from being generated.
+
+			options.SymbolTable.AddType (new TestClass (null, "Java.Lang.Object"));
+
+			var @class = SupportTypeBuilder.CreateClass ("java.code.MyClass", options, "Java.Lang.Object");
+			@class.Validate (options, new GenericParameterDefinitionList (), generator.Context);
+
+			// FromXml is set to true when a class is set to true when the api.xml contains an entry for the class. 
+			// Therefore, if a class's base has FromXml set to true, the class and its base will be in the same C# assembly. 
+			@class.BaseGen.FromXml = true;  
+
+			generator.Context.ContextTypes.Push (@class);
+			generator.WriteClass (@class, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
+			generator.Context.ContextTypes.Pop ();
+
+			var result = writer.ToString ().NormalizeLineEndings ();
+			Assert.True (result.Contains ("internal static new IntPtr class_ref"));
+			Assert.False (result.Contains ("internal static IntPtr class_ref"));
+		}
 	}
 }
diff --git a/tests/generator-Tests/Unit-Tests/SupportTypes.cs b/tests/generator-Tests/Unit-Tests/SupportTypes.cs
@@ -215,9 +215,9 @@ public TestInterface (string argsType, string javaName) : base (new TestBaseSupp
 
 	static class SupportTypeBuilder
 	{
-		public static TestClass CreateClass (string className, CodeGenerationOptions options)
+		public static TestClass CreateClass (string className, CodeGenerationOptions options, string baseClass = "Object")
 		{
-			var @class = new TestClass ("Object", className);
+			var @class = new TestClass (baseClass, className);
 
 			var ctor_name = className.Contains ('.') ? className.Substring (className.LastIndexOf ('.')) : className;
 			@class.Ctors.Add (CreateConstructor (@class, ctor_name, options));
diff --git a/tools/generator/Java.Interop.Tools.Generator.CodeGeneration/CodeGenerator.cs b/tools/generator/Java.Interop.Tools.Generator.CodeGeneration/CodeGenerator.cs
@@ -126,15 +126,12 @@ public void WriteClass (ClassGen @class, string indent, GenerationInfo gen_info)
 				writer.WriteLine ();
 			}
 
-			bool requireNew = @class.InheritsObject;
-			if (!requireNew) {
-				for (var bg = @class.BaseGen; bg != null && bg is ClassGen classGen && classGen.FromXml; bg = bg.BaseGen) {
-					if (bg.InheritsObject) {
-						requireNew = true;
-						break;
-					}
-				}
-			}
+			// @class.InheritsObject is true unless @class refers to java.lang.Object or java.lang.Throwable. (see ClassGen constructor)
+			// If @class's base class is defined in the same api.xml file, then it requires the new keyword to overshadow the internal
+			// members of its baseclass since the two classes will be defined in the same assembly. If the base class is not from the
+			// same api.xml file, the new keyword is not needed because the internal access modifier already prevents it from being seen.
+			bool baseFromSameAssembly = @class?.BaseGen?.FromXml ?? false;
+			bool requireNew = @class.InheritsObject && baseFromSameAssembly;
 			WriteClassHandle (@class, indent, requireNew);
 
 			WriteClassConstructors (@class, indent + "\t");

Original file line number	Diff line number	Diff line change
`@@ -3,7 +3,7 @@`
`3`	`3`	`public partial class MyClass {`
`4`	`4`
`5`	`5`	`static readonly JniPeerMembers _members = new JniPeerMembers ("java/code/MyClass", typeof (MyClass));`
`6`		`- internal static new IntPtr class_ref {`
	`6`	`+ internal static IntPtr class_ref {`
`7`	`7`	`get {`
`8`	`8`	`return _members.JniPeerType.PeerReference.Handle;`
`9`	`9`	`}`
Original file line number	Diff line number	Diff line change
`@@ -215,9 +215,9 @@ public TestInterface (string argsType, string javaName) : base (new TestBaseSupp`
`215`	`215`
`216`	`216`	`static class SupportTypeBuilder`
`217`	`217`	`{`
`218`		`- public static TestClass CreateClass (string className, CodeGenerationOptions options)`
	`218`	`+ public static TestClass CreateClass (string className, CodeGenerationOptions options, string baseClass = "Object")`
`219`	`219`	`{`
`220`		`- var @class = new TestClass ("Object", className);`
	`220`	`+ var @class = new TestClass (baseClass, className);`
`221`	`221`
`222`	`222`	`var ctor_name = className.Contains ('.') ? className.Substring (className.LastIndexOf ('.')) : className;`
`223`	`223`	`@class.Ctors.Add (CreateConstructor (@class, ctor_name, options));`