[generator] Mark Kotlin-mangled methods as non-virtual (#536)

Fixes: #534

There are cases where Kotlin generates Java methods that are invalid
Java identifiers:

	// Kotlin
	public open abstract class AbstractUnsigned {
	  public open fun foo(value : UInt) {
	    return 3u
	  }
	}

Results in Java bytecode:

	public abstract class example.AbstractUnsigned {
	  public abstract void foo-WZ4Q5Ns(int);
	  public example.AbstractUnsigned();
	}

`foo-WZ4Q5Ns` is not a valid Java identifier, due to the `-`.

We already change the name to `Foo()` so C# can compile the method,
and JNI allows us to call the method correctly.  However we cannot
allow the user to override this method because we cannot create the
Java override in the JCW.

In this case, mark the method as *non*-`virtual` to prevent the
user from overriding it:

	// C#
	partial class AbstractUnsigned {
	  public void Foo(int value) …}
	}

Author: jpobst

tests/generator-Tests/Unit-Tests/KotlinFixupsTests.cs

@@ -0,0 +1,39 @@
+using System;
+using System.Linq;
+using System.Xml.Linq;
+using Java.Interop.Tools.Generator.Transformation;
+using MonoDroid.Generation;
+using NUnit.Framework;
+
+namespace generatortests
+{
+	[TestFixture]
+	public class KotlinFixupsTests
+	{
+		[Test]
+		public void CreateMethod_EnsureKotlinImplFix ()
+		{
+			var xml = XDocument.Parse ("<package name=\"com.example.test\" jni-name=\"com/example/test\"><class name=\"test\"><method name=\"add-impl\" final=\"false\" /></class></package>");
+			var klass = XmlApiImporter.CreateClass (xml.Root, xml.Root.Element ("class"));
+
+			KotlinFixups.Fixup (new [] { (GenBase)klass }.ToList ());
+
+			Assert.AreEqual ("Add", klass.Methods [0].Name);
+			Assert.IsTrue (klass.Methods [0].IsFinal);
+			Assert.IsFalse (klass.Methods [0].IsVirtual);
+		}
+
+		[Test]
+		public void CreateMethod_EnsureKotlinHashcodeFix ()
+		{
+			var xml = XDocument.Parse ("<package name=\"com.example.test\" jni-name=\"com/example/test\"><class name=\"test\"><method name=\"add-h4F1V8i\" final=\"false\" /></class></package>");
+			var klass = XmlApiImporter.CreateClass (xml.Root, xml.Root.Element ("class"));
+
+			KotlinFixups.Fixup (new [] { (GenBase) klass }.ToList ());
+
+			Assert.AreEqual ("Add", klass.Methods [0].Name);
+			Assert.IsTrue (klass.Methods [0].IsFinal);
+			Assert.IsFalse (klass.Methods [0].IsVirtual);
+		}
+	}
+}

tests/generator-Tests/Unit-Tests/XmlApiImporterTests.cs

@@ -89,24 +89,6 @@ public void CreateMethod_EnsureValidNameHyphen ()
 			Assert.AreEqual ("_3", klass.Methods [0].Name);
 		}
 
-		[Test]
-		public void CreateMethod_EnsureKotlinImplFix ()
-		{
-			var xml = XDocument.Parse ("<package name=\"com.example.test\" jni-name=\"com/example/test\"><class name=\"test\"><method name=\"add-impl\" /></class></package>");
-			var klass = XmlApiImporter.CreateClass (xml.Root, xml.Root.Element ("class"));
-
-			Assert.AreEqual ("Add", klass.Methods [0].Name);
-		}
-
-		[Test]
-		public void CreateMethod_EnsureKotlinHashcodeFix ()
-		{
-			var xml = XDocument.Parse ("<package name=\"com.example.test\" jni-name=\"com/example/test\"><class name=\"test\"><method name=\"add-h4F1V8i\" /></class></package>");
-			var klass = XmlApiImporter.CreateClass (xml.Root, xml.Root.Element ("class"));
-
-			Assert.AreEqual ("Add", klass.Methods [0].Name);
-		}
-
 		[Test]
 		public void CreateParameter_EnsureValidName ()
 		{

tools/generator/CodeGenerator.cs

@@ -12,6 +12,7 @@
 using Java.Interop.Tools.Diagnostics;
 using Java.Interop.Tools.TypeNameMappings;
 using MonoDroid.Generation.Utilities;
+using Java.Interop.Tools.Generator.Transformation;
 
 namespace Xamarin.Android.Binder
 {
@@ -149,6 +150,9 @@ static void Run (CodeGeneratorOptions options, DirectoryAssemblyResolver resolve
 					AddTypeToTable (opt, gen);
 			}
 
+			// Apply fixups
+			KotlinFixups.Fixup (gens);
+
 			Validate (gens, opt, new CodeGeneratorContext ());
 
 			if (api_versions_xml != null)

tools/generator/Java.Interop.Tools.Generator.Importers/XmlApiImporter.cs

@@ -245,25 +245,8 @@ public static Method CreateMethod (GenBase declaringType, XElement elem)
 
 			if (elem.Attribute ("managedName") != null)
 				method.Name = elem.XGetAttribute ("managedName");
-			else {
-				var name = method.JavaName;
-
-				// Kotlin generates methods that cannot be referenced in Java,
-				// like `add-impl` and `add-V5j3Lk8`. We mangle them back into
-				// something a user would expect by truncating anything after the hyphen.
-				var index = name.IndexOf ("-impl");
-
-				if (index >= 0)
-					name = name.Substring (0, index);
-
-				index = name.IndexOf ('-');
-
-				// `add-V5j3Lk8` is always a 7 character hashcode
-				if (index >= 0 && name.Length - index == 8)
-					name = name.Substring (0, index);
-
-				method.Name = StringRocks.MemberToPascalCase (name);
-			}
+			else
+				method.Name = StringRocks.MemberToPascalCase (method.JavaName);
 
 			if (method.IsReturnEnumified) {
 				method.ManagedReturn = elem.XGetAttribute ("enumReturn");

tools/generator/Java.Interop.Tools.Generator.ObjectModel/MethodBase.cs

@@ -79,6 +79,24 @@ public string GetSignature (CodeGenerationOptions opt)
 
 		public virtual bool IsGeneric => Parameters.HasGeneric;
 
+		public bool IsKotlinNameMangled {
+			get {
+				// Kotlin generates methods that cannot be referenced in Java,
+				// like `add-impl` and `add-V5j3Lk8`. We will need to fix those later.
+				if (this is Method method) {
+					if (method.JavaName.IndexOf ("-impl") >= 0)
+						return true;
+
+					var index = method.JavaName.IndexOf ('-');
+
+					// `add-V5j3Lk8` is always a 7 character hashcode
+					return index >= 0 && method.JavaName.Length - index == 8;
+				}
+
+				return false;
+			}
+		}
+
 		public virtual bool Matches (MethodBase other)
 		{
 			if (Name != other.Name)

tools/generator/Java.Interop.Tools.Generator.Transformation/KotlinFixups.cs

@@ -0,0 +1,46 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using MonoDroid.Generation;
+using MonoDroid.Utils;
+
+namespace Java.Interop.Tools.Generator.Transformation
+{
+	public static class KotlinFixups
+	{
+		public static void Fixup (List<GenBase> gens)
+		{
+			foreach (var c in gens.OfType<ClassGen> ())
+				FixupClass (c);
+		}
+
+		private static void FixupClass (ClassGen c)
+		{
+			// Kotlin mangles the name of some methods to make them
+			// inaccessible from Java, like `add-impl` and `add-V5j3Lk8`.
+			// We need to generate C# compatible names as well as prevent overriding 
+			// them as we cannot generate JCW for them.
+			var invalid_methods = c.Methods.Where (m => m.IsKotlinNameMangled).ToList ();
+
+			foreach (var method in invalid_methods) {
+
+				// If the method is virtual, mark it as !virtual as it can't be overridden in Java
+				if (!method.IsFinal)
+					method.IsFinal = true;
+
+				if (method.IsVirtual)
+					method.IsVirtual = false;
+
+				// Only run this if it's the default name (ie: not a user's "managedName")
+				if (method.Name == StringRocks.MemberToPascalCase (method.JavaName).Replace ('-', '_')) {
+					// We want to remove the hyphen and anything afterwards to fix mangled names,
+					// but a previous step converted it to an underscore. Remove the final
+					// underscore and anything after it.
+					var index = method.Name.LastIndexOf ('_');
+
+					method.Name = method.Name.Substring (0, index);
+				}
+			}
+		}
+	}
+}

tools/generator/generator.csproj

@@ -104,6 +104,7 @@
     <Compile Include="Java.Interop.Tools.Generator.ObjectModel\NamespaceMapping.cs" />
     <Compile Include="Java.Interop.Tools.Generator.ObjectModel\Parameter.cs" />
     <Compile Include="Java.Interop.Tools.Generator.ObjectModel\ParameterList.cs" />
+    <Compile Include="Java.Interop.Tools.Generator.Transformation\KotlinFixups.cs" />
     <Compile Include="Java.Interop.Tools.Generator.Transformation\Parser.cs" />
     <Compile Include="Utilities\AncestorDescendantCache.cs" />
     <Compile Include="Utilities\ProcessRocks.cs" />