[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(varmetinmetadata.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(varjava_methodinc.Methods)
+if(FindKotlinFunctionMetadata(metadata,java_method)isKotlinFunctionfunction_metadata)
+FixupFunction(java_method,function_metadata,class_metadata);
 }
 
-
 if(metadata.Properties!=null){
 foreach(varpropinmetadata.Properties){
 vargetter=FindJavaPropertyGetter(metadata,prop,c);
@@ -334,40 +336,40 @@ static void FixupField (FieldInfo? field, KotlinProperty metadata)
 returnnull;
 }
 
-staticMethodInfo?FindJavaMethod(KotlinFilekotlinFile,KotlinFunctionfunction,ClassFileklass)
+staticKotlinFunction?FindKotlinFunctionMetadata(KotlinFile?kotlinFile,MethodInfojavaMethod)
 {
-varpossible_methods=klass.Methods.Where(method =>method.Name==function.JvmName).ToArray();
-varsignature=function.JvmSignature;
+if(kotlinFile?.Functionsisnull)
+returnnull;
+
+varjava_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)isMethodInfom)
-returnm;
+// The method name absolutely has to match
+varpossible_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)isKotlinFunctionkf)
+returnkf;
 
 // 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)isKotlinFunctionkf2)
+returnkf2;
 
-if(possible_methods.SingleOrDefault(method =>method.Descriptor==signature)isMethodInfom2)
-returnm2;
+// 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)
+varjava_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(varmethodinpossible_methods.Where(method =>method.GetFilteredParameters().Length==function.ValueParameters?.Count)){
+foreach(varfunctioninpossible_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;
 
-returnmethod;
+returnfunction;
 }
 
-// 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()})'");
-
 returnnull;
 }
 

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

@@ -374,5 +374,27 @@ public void MatchParametersWithStaticThis ()
 Assert.AreEqual(1,start);
 Assert.AreEqual(2,end);
 }
+
+[Test]
+publicvoidMatchMetadataToMultipleMethodsWithSameMangledName()
+{
+varklass=LoadClassFile("UByteArray.class");
+varmeta=GetClassMetadataForClass(klass);
+
+// There is only one Kotlin metadata for Java method "contains-7apg3OU"
+varkotlin_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)
+varjava_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);
+}
 }
 }