From 6608c590767e501a23d72f00e261dcc71099abac Mon Sep 17 00:00:00 2001 From: Mitchell Blowey <5872539+mblowey@users.noreply.github.com> Date: Thu, 23 Apr 2020 12:34:47 -0400 Subject: [PATCH] [generator] Fix generated code that caused CS0169 warnings. (#625) * 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 ce8dc4001bd7b5ab84c42b9b27711755f051cd2d) --- .../JavaInterop1/WriteClass.txt | 2 +- ...iteKotlinUnsignedArrayTypeMethodsClass.txt | 2 +- ...KotlinUnsignedArrayTypePropertiesClass.txt | 2 +- .../WriteKotlinUnsignedTypeMethodsClass.txt | 2 +- ...WriteKotlinUnsignedTypePropertiesClass.txt | 2 +- .../XAJavaInterop1-NRT/WriteClass.txt | 2 +- .../XAJavaInterop1/WriteClass.txt | 2 +- .../XamarinAndroid/WriteClass.txt | 4 +- .../Unit-Tests/CodeGeneratorTests.cs | 46 +++++++++++++++++++ .../Unit-Tests/SupportTypes.cs | 4 +- .../CodeGenerator.cs | 15 +++--- 11 files changed, 63 insertions(+), 20 deletions(-) diff --git a/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteClass.txt b/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteClass.txt index 8c6217d1b..4c7d4d3f8 100644 --- 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 index 47247bb27..6dc50ca2f 100644 --- 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 index d6f7093ca..3f2241de6 100644 --- 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 index b03f98129..6526f804a 100644 --- 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 index 113e4c242..f7fb28ed9 100644 --- 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 index 01ea5c4e6..fd438b72d 100644 --- 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 index 5936c52fc..cb6feb756 100644 --- 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 index 4e9c67151..d0233db96 100644 --- 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 index 2ea239e11..44edc2110 100644 --- 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 index ed9f4cb1d..a4e4b5335 100644 --- 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 index 66d469ac0..0b593bd36 100644 --- 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");