[jcw-gen] Do not register static methods on an interface.

[jpobst]

Context: dotnet/android#5251

When an Java interface contains a static method in API-30+, we use C# 8.0's new support for these methods to place them directly on the interface. When a C# class implements the interface, the Java Callable Wrapper we generate will register every method on the interface during startup. However, static methods are not virtual and cannot be overridden, so they do not have a connector and do not need to be registered.

This results in a case where the register command fails because no connector is specified (nothing after the final colon).

__md_methods = "n_comparing:(Ljava/util/function/Function;)Ljava/util/Comparator;:\n" +
...

To fix this, when adding methods from implemented interfaces we need to skip static interface members.

A test was added in dotnet/android#5262.

• Failure without this change: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=4201499&view=ms.vss-test-web.build-test-results-tab.
• Success with this change: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=4201951&view=results

[jonpryor]

Could you please update tests/Java.Interop.Tools.JavaCallableWrappers-Tests/Java.Interop.Tools.JavaCallableWrappers/JavaCallableWrapperGeneratorTests.cs to contain a unit test for this?

[jonpryor]

Just thought of an issue with this, for which we'll need to add a unit test for: it is perfectly valid to put [Export] on static methods. Thus, as you feared, this "exclude static methods" check is not strict enough. Perhaps it would be enough to simply exclude static methods which lack [Export]?

diff --git a/tests/Java.Interop.Tools.JavaCallableWrappers-Tests/Java.Interop.Tools.JavaCallableWrappers/JavaCallableWrapperGeneratorTests.cs b/tests/Java.Interop.Tools.JavaCallableWrappers-Tests/Java.Interop.Tools.JavaCallableWrappers/JavaCallableWrapperGeneratorTests.cs
index 186409b4..fe6b4081 100644
--- a/tests/Java.Interop.Tools.JavaCallableWrappers-Tests/Java.Interop.Tools.JavaCallableWrappers/JavaCallableWrapperGeneratorTests.cs
+++ b/tests/Java.Interop.Tools.JavaCallableWrappers-Tests/Java.Interop.Tools.JavaCallableWrappers/JavaCallableWrapperGeneratorTests.cs
@@ -189,6 +189,7 @@ public class ExportsMembers
 		__md_methods = 
 			""n_GetInstance:()Lcrc64197ae30a36756915/ExportsMembers;:__export__\n"" +
 			""n_GetValue:()Ljava/lang/String;:__export__\n"" +
+			""n_staticMethodNotMangled:()V:__export__\n"" +
 			""n_methodNamesNotMangled:()V:__export__\n"" +
 			""n_CompletelyDifferentName:(Ljava/lang/String;I)Ljava/lang/String;:__export__\n"" +
 			""n_methodThatThrows:()V:__export__\n"" +
@@ -218,6 +219,14 @@ public class ExportsMembers
 	private native java.lang.String n_GetValue ();
 
 
+	public static void staticMethodNotMangled ()
+	{
+		n_staticMethodNotMangled ();
+	}
+
+	private static native void n_staticMethodNotMangled ();
+
+
 	public void methodNamesNotMangled ()
 	{
 		n_methodNamesNotMangled ();
diff --git a/tests/Java.Interop.Tools.JavaCallableWrappers-Tests/Java.Interop.Tools.JavaCallableWrappers/SupportDeclarations.cs b/tests/Java.Interop.Tools.JavaCallableWrappers-Tests/Java.Interop.Tools.JavaCallableWrappers/SupportDeclarations.cs
index 4d8c228b..de99b77e 100644
--- a/tests/Java.Interop.Tools.JavaCallableWrappers-Tests/Java.Interop.Tools.JavaCallableWrappers/SupportDeclarations.cs
+++ b/tests/Java.Interop.Tools.JavaCallableWrappers-Tests/Java.Interop.Tools.JavaCallableWrappers/SupportDeclarations.cs
@@ -274,6 +274,11 @@ namespace Xamarin.Android.ToolsTests {
 			return "value";
 		}
 
+		[Export]
+		public static void staticMethodNotMangled ()
+		{
+		}
+
 		[Export]
 		public void methodNamesNotMangled ()
 		{

[jpobst]

Note this is only excluding static methods that are on an interface that a class implements. It would not exclude a static method on a class. Should we allow [Export] on static interface methods?

[jpobst]

Added this test case to PR even though it isn't directly affected by this, as it never hurts to have additional tests!

[jpobst]

We cannot add a test to JavaCallableWrapperGeneratorTests.cs for this, as it requires DIM, which is not supported by net472. We would have to move this test assembly to netcoreapp3.1, and then it would no longer run on Mono.