From a856c2ae7ebf095fc2c8bfd01f270d9d555e6c38 Mon Sep 17 00:00:00 2001 From: Jonathan Pobst Date: Tue, 11 May 2021 08:44:54 -0500 Subject: [PATCH 1/4] [generator] Fix NRE from return type not getting set for abstract methods with generics. --- .../Unit-Tests/CodeGeneratorTests.cs | 55 +++++++++++++++++++ .../BoundMethodAbstractDeclaration.cs | 5 +- 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs b/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs index 496b57400..d09316346 100644 --- a/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs +++ b/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs @@ -1,9 +1,11 @@ using System; using System.Collections.Generic; using System.Linq; +using generator.SourceWriters; using MonoDroid.Generation; using NUnit.Framework; using Xamarin.Android.Binder; +using Xamarin.SourceWriter; namespace generatortests { @@ -1271,5 +1273,58 @@ public void WriteClassInternalBase () Assert.True (result.Contains ("internal static new IntPtr class_ref".NormalizeLineEndings ())); Assert.False (result.Contains ("internal static IntPtr class_ref".NormalizeLineEndings ())); } + + [Test] + public void WriteBoundMethodAbstractDeclarationWithGenericReturn () + { + // Fix a case where the ReturnType of a class method implementing a generic interface method + // that has a generic parameter type return (like T) wasn't getting set, resulting in a NRE. + var gens = ParseApiDefinition (@" + + + + + + + + + + + + + + + + + + + + + + + + + + + + + "); + + var iface = gens.OfType ().Single (c => c.Name == "RangeIterator"); + var method = iface.Methods.Single (); + + var bmad = new BoundMethodAbstractDeclaration (iface, method, options, null); + var source_writer = new CodeWriter (writer); + + bmad.Write (source_writer); + + var expected = @"Java.Lang.Object Com.Example.RangeIterator.Next (int count) + { + throw new NotImplementedException (); + }"; + + // Ignore nullable operator so this test works on all generators ;) + Assert.AreEqual (expected.NormalizeLineEndings (), writer.ToString ().NormalizeLineEndings ().Replace ("?", "")); + } } } diff --git a/tools/generator/SourceWriters/BoundMethodAbstractDeclaration.cs b/tools/generator/SourceWriters/BoundMethodAbstractDeclaration.cs index cf6ab032b..a2fbf0f0e 100644 --- a/tools/generator/SourceWriters/BoundMethodAbstractDeclaration.cs +++ b/tools/generator/SourceWriters/BoundMethodAbstractDeclaration.cs @@ -19,6 +19,9 @@ public BoundMethodAbstractDeclaration (GenBase gen, Method method, CodeGeneratio this.method = method; this.opt = opt; + ReturnType = new TypeReferenceWriter (opt.GetTypeReferenceName (method.RetVal)); + this.AddMethodParameters (method.Parameters, opt); + if (method.RetVal.IsGeneric && gen != null) { Name = method.Name; ExplicitInterfaceImplementation = opt.GetOutputName (gen.FullName); @@ -35,7 +38,6 @@ public BoundMethodAbstractDeclaration (GenBase gen, Method method, CodeGeneratio IsAbstract = true; IsShadow = impl.RequiresNew (method.Name, method); SetVisibility (method.Visibility); - ReturnType = new TypeReferenceWriter (opt.GetTypeReferenceName (method.RetVal)); NewFirst = true; @@ -51,7 +53,6 @@ public BoundMethodAbstractDeclaration (GenBase gen, Method method, CodeGeneratio Attributes.Add (new RegisterAttr (method.JavaName, method.JniSignature, method.ConnectorName, additionalProperties: method.AdditionalAttributeString ())); SourceWriterExtensions.AddMethodCustomAttributes (Attributes, method); - this.AddMethodParameters (method.Parameters, opt); } public override void Write (CodeWriter writer) From a13ab89f2d6fa1016b4f44416870e55418fbd166 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Wed, 12 May 2021 19:23:16 -0400 Subject: [PATCH 2/4] Fix unit test? MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Context: https://github.com/xamarin/java.interop/pull/834#issuecomment-838802709 Context: https://gist.github.com/jonpryor/41a0a920d27e5a761f499da4ade791a8 I was operating under the (deluded?) assumption that it should be possible to use the XML within `WriteBoundMethodAbstractDeclarationWithGenericReturn()` to reproduce the `NullReferenceException`, but I was unable to do so. I thus returned to the [full API declaration][0] to recreate a minimal test case which *would* result in a `NullReferenceException`, and found one. Oddly, the "missing piece" to trigger the NRE was *nested types* (?!): the original PR #834 had `RangeIterator` as a top-level type, and in this situation I wasn't able to get an NRE with main. Once I "reverted" it to a nested `FlowIterator.RangeIterator` type, it broke. I don't know if that's the *only* relevant change, as I changed other pieces as well: * Simpler (no parameters) `next()` method * Fixed the `//interface/@jni-signature` value for `FlowIterator` Regardless, this "newer" XML *does* crash, but also only with `--codegen-target=XAJavaInterop1`. It doesn't NRE w/o `--codegen-target`: $ mono bin/Debug/generator.exe -o yyy ji-834-fixed.xml --codegen-target=XAJavaInterop1 … Unhandled Exception: System.NullReferenceException: Object reference not set to an instance of an object at Xamarin.SourceWriter.MethodWriter.WriteReturnType (Xamarin.SourceWriter.CodeWriter writer) at Xamarin.SourceWriter.MethodWriter.WriteSignature (Xamarin.SourceWriter.CodeWriter writer) at Xamarin.SourceWriter.MethodWriter.Write (Xamarin.SourceWriter.CodeWriter writer) [0]: https://github.com/xamarin/java.interop/pull/834#issuecomment-838954034 --- .../Unit-Tests/CodeGeneratorTests.cs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs b/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs index d09316346..ea12c4387 100644 --- a/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs +++ b/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs @@ -1286,15 +1286,13 @@ public void WriteBoundMethodAbstractDeclarationWithGenericReturn () - + - - - + - + @@ -1303,14 +1301,12 @@ public void WriteBoundMethodAbstractDeclarationWithGenericReturn () - - - + "); - var iface = gens.OfType ().Single (c => c.Name == "RangeIterator"); + var iface = gens.OfType ().Single (c => c.Name == "FlowIterator.RangeIterator"); var method = iface.Methods.Single (); var bmad = new BoundMethodAbstractDeclaration (iface, method, options, null); @@ -1318,7 +1314,7 @@ public void WriteBoundMethodAbstractDeclarationWithGenericReturn () bmad.Write (source_writer); - var expected = @"Java.Lang.Object Com.Example.RangeIterator.Next (int count) + var expected = @"Java.Lang.Object Com.Example.RangeIterator.Next () { throw new NotImplementedException (); }"; From ea4df3c3fd83e4cbe95b1c2cd40e25393c2eac8f Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Wed, 12 May 2021 20:31:34 -0400 Subject: [PATCH 3/4] oops --- tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs b/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs index ea12c4387..20b7680a1 100644 --- a/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs +++ b/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs @@ -1286,7 +1286,7 @@ public void WriteBoundMethodAbstractDeclarationWithGenericReturn () - From 22efe166eef944dcf26d6fe0e8bae1f88ab8513b Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Thu, 13 May 2021 12:54:34 -0400 Subject: [PATCH 4/4] Fixity Fix. --- tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs b/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs index 20b7680a1..4fc9185c8 100644 --- a/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs +++ b/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs @@ -1306,15 +1306,16 @@ public void WriteBoundMethodAbstractDeclarationWithGenericReturn () "); - var iface = gens.OfType ().Single (c => c.Name == "FlowIterator.RangeIterator"); - var method = iface.Methods.Single (); + var declIface = gens.OfType ().Single (c => c.Name == "IFlowIterator"); + var declClass = declIface.NestedTypes.OfType ().Single (c => c.Name == "FlowIteratorRangeIterator"); + var method = declClass.Methods.Single (); - var bmad = new BoundMethodAbstractDeclaration (iface, method, options, null); + var bmad = new BoundMethodAbstractDeclaration (declClass, method, options, null); var source_writer = new CodeWriter (writer); bmad.Write (source_writer); - var expected = @"Java.Lang.Object Com.Example.RangeIterator.Next () + var expected = @"Java.Lang.Object Com.Example.FlowIteratorRangeIterator.Next () { throw new NotImplementedException (); }";