[generator] Kotlin metadata can apply to multiple Java method (#1009)

Fixes: #984

There exists a scenario with Kotlin metadata where an instance method
and an extension method can have the same *Kotlin* signature:

	// Kotlin
	class UByteArray {
	    fun contains(element: UByte): Boolean
	}

	// Extension method
	operator fun <T> Array<out T>.contains(element: T): Boolean

This results in 2 different Java methods:

	// "Java"
	class UByteArray {
	    public boolean contains-7apg3OU(byte element) {
	        return contains-WZ4Q5Ns(this.storage, element);
	    }
	
	    public static boolean contains-7apg3OU(byte[] arg0, byte element) {
	        Intrinsics.checkNotNullParameter(arg0, "arg0");
	        return ArraysKt.contains(arg0, element);
	    }
	}

Kotlin only generates 1 metadata entry, for JvmSignature=`([BB)Z`:

![image](https://user-images.githubusercontent.com/179295/178528374-a6e9a568-dc68-4b88-af6b-1c51946d7997.png)

Previously (439bd83) we assumed that a piece of Kotlin metadata
would apply to only a single Java method.  In this case, we would
find `contains-7apg3OU.([BB)Z`, then apply it to only the `static`
method, resulting in a binding of:

	// C#; previous binding
	partial class UByteArray {
	    public bool Contains(int element);
	    public static bool Contains(int[] arg0, uint element);
	}

Note: `element` is of type `int` for the instance method, `uint` for
the static method.  They *should* be consistent, and are not.

If we don't exclusively look at `JvmSignature`, and also check
`ValueParameters`, then the Kotlin metadata will match the *instance*
`contains-7apg3OU` method.

We surmise that this is intentional on Kotlin's part, in order to
save bytes by omitting the second metadata entry, and that we should
apply this metadata to both Java methods.

*Note*: the "hash" appended to the Java method name (eg: `7apg3OU`)
is a short hash of the Kotlin method parameter types.

Fix this scenario by "inverting" our lookup logic: instead of
starting with Kotlin metadata and looking for an applicable Java
method, start with the Java method and look for applicable Kotlin
metadata.  This way, each Java method can consume a matching Kotlin
metadata entry, even if another Java method already matched it.

This allows us to emit a binding of:

	// C#; new binding
	partial class UByteArray {
	    public bool Contains(uint element);
	    public static bool Contains(int[] arg0, uint element);
	}

This fixes the 4 `contains` methods for `UIntArray`, `UByteArray`,
`ULongArray`, and `UShortArray`.

Author: jpobst

src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinFixups.cs

@@ -47,11 +47,13 @@ public static void Fixup (IList<ClassFile> classes)
 					FixupJavaMethods (c.Methods);
 
 					if (metadata.Functions != null) {
-						foreach (var met in metadata.Functions)
-							FixupFunction (FindJavaMethod (metadata, met, c), met, class_metadata);
+						// We work from Java methods to Kotlin metadata because they aren't a 1:1 relation
+						// and we need to find the "best" match for each Java method.
+						foreach (var java_method in c.Methods)
+							if (FindKotlinFunctionMetadata (metadata, java_method) is KotlinFunction function_metadata)
+								FixupFunction (java_method, function_metadata, class_metadata);
 					}
 
-
 					if (metadata.Properties != null) {
 						foreach (var prop in metadata.Properties) {
 							var getter = FindJavaPropertyGetter (metadata, prop, c);
@@ -334,40 +336,40 @@ static void FixupField (FieldInfo? field, KotlinProperty metadata)
 			return null;
 		}
 
-		static MethodInfo? FindJavaMethod (KotlinFile kotlinFile, KotlinFunction function, ClassFile klass)
+		static KotlinFunction? FindKotlinFunctionMetadata (KotlinFile? kotlinFile, MethodInfo javaMethod)
 		{
-			var possible_methods = klass.Methods.Where (method => method.Name == function.JvmName).ToArray ();
-			var signature = function.JvmSignature;
+			if (kotlinFile?.Functions is null)
+				return null;
+
+			var java_descriptor = javaMethod.Descriptor;
 
-			// If the Descriptor/Signature match, that means all parameters and return type match
-			if (signature != null && possible_methods.SingleOrDefault (method => method.Descriptor == signature) is MethodInfo m)
-					return m;
+			// The method name absolutely has to match
+			var possible_functions = kotlinFile.Functions.Where (f => f.JvmName == javaMethod.Name).ToArray ();
+
+			// If we have metadata with a Descriptor/JvmSignature match, that means all parameters and return type match
+			if (possible_functions.SingleOrDefault (f => f.JvmSignature != null && f.JvmSignature == java_descriptor) is KotlinFunction kf)
+				return kf;
 
 			// Sometimes JvmSignature is null (or unhelpful), so we're going to construct one ourselves and see if they match
-			signature = function.ConstructJvmSignature ();
+			if (possible_functions.SingleOrDefault (f => f.ConstructJvmSignature () == java_descriptor) is KotlinFunction kf2)
+				return kf2;
 
-			if (possible_methods.SingleOrDefault (method => method.Descriptor == signature) is MethodInfo m2)
-				return m2;
+			// If that didn't work, let's try it the hard way!
+			// This catches cases where Kotlin only wrote one metadata entry for multiple methods with the same mangled JvmName (ex: contains-WZ4Q5Ns)
+			var java_param_count = javaMethod.GetFilteredParameters ().Length;
 
-			// If that didn't work, let's do it the hard way!
-			// I don't know if this catches anything additional, but it was the original code we shipped, so
-			// we'll keep it just in case something in the wild requires it.
-			foreach (var method in possible_methods.Where (method => method.GetFilteredParameters ().Length == function.ValueParameters?.Count)) {
+			foreach (var function in possible_functions.Where (f => f.ValueParameters?.Count == java_param_count)) {
 				if (function.ReturnType == null)
 					continue;
-				if (!TypesMatch (method.ReturnType, function.ReturnType, kotlinFile))
+				if (!TypesMatch (javaMethod.ReturnType, function.ReturnType, kotlinFile))
 					continue;
 
-				if (!ParametersMatch (kotlinFile, method, function.ValueParameters!))
+				if (!ParametersMatch (kotlinFile, javaMethod, function.ValueParameters!))
 					continue;
 
-				return method;
+				return function;
 			}
 
-			// Theoretically this should never be hit, but who knows. At worst, it just means
-			// Kotlin niceties won't be applied to the method.
-			Log.Debug ($"Kotlin: Could not find Java method to match '{function.Name} ({function.ConstructJvmSignature ()})'");
-
 			return null;
 		}
 

tests/Xamarin.Android.Tools.Bytecode-Tests/KotlinFixupsTests.cs

@@ -374,5 +374,27 @@ public void MatchParametersWithStaticThis ()
 			Assert.AreEqual (1, start);
 			Assert.AreEqual (2, end);
 		}
+
+		[Test]
+		public void MatchMetadataToMultipleMethodsWithSameMangledName ()
+		{
+			var klass = LoadClassFile ("UByteArray.class");
+			var meta = GetClassMetadataForClass (klass);
+
+			// There is only one Kotlin metadata for Java method "contains-7apg3OU"
+			var kotlin_function = meta.Functions.Single (m => m.Name == "contains");
+			
+			// However there are 2 Java methods named "contains-7apg3OU"
+			// - public boolean contains-7apg3OU (byte element)
+			// - public static boolean contains-7apg3OU (byte[] arg0, byte element)
+			var java_methods = klass.Methods.Where (m => m.Name == "contains-7apg3OU");
+			Assert.AreEqual (2, java_methods.Count ());
+
+			KotlinFixups.Fixup (new [] { klass });
+
+			// Ensure the fixup "fixed" the parameter "element" type to "ubyte"
+			Assert.AreEqual ("ubyte", java_methods.ElementAt (0).GetParameters ().Single (p => p.Name == "element").KotlinType);
+			Assert.AreEqual ("ubyte", java_methods.ElementAt (1).GetParameters ().Single (p => p.Name == "element").KotlinType);
+		}
 	}
 }