[Xamarin.Android.Tools.Bytecode] Improve Kotlin metadata matching.

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

@@ -358,9 +358,9 @@ public class KotlinType
  };
  }
 
- public string GetSignature ()
+ public string GetSignature (bool convertUnsignedToPrimitive = true)
  {
- return KotlinUtilities.ConvertKotlinTypeSignature (this);
+ return KotlinUtilities.ConvertKotlinTypeSignature (this, null, convertUnsignedToPrimitive);
  }
 	}
 

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

@@ -175,10 +175,12 @@ static void FixupFunction (MethodInfo? method, KotlinFunction metadata, KotlinCl
  return;
  }
 
- var java_parameters = method.GetFilteredParameters ();
+(var start, var end) = CreateParameterMap (method, metadata, kotlinClass);
 
- for (var i = 0; i < java_parameters.Length; i++) {
- var java_p = java_parameters [i];
+ var java_parameters = method.GetParameters ();
+
+ for (var i = 0; i < end - start; i++) {
+ var java_p = java_parameters [start + i];
  var kotlin_p = metadata.ValueParameters == null ? null : metadata.ValueParameters [i];
  if (kotlin_p == null || kotlin_p.Type == null || kotlin_p.Name == null)
  continue;
@@ -197,6 +199,46 @@ static void FixupFunction (MethodInfo? method, KotlinFunction metadata, KotlinCl
  method.KotlinReturnType = GetKotlinType (method.ReturnType.TypeSignature, metadata.ReturnType?.ClassName);
  }
 
+ public static (int start, int end) CreateParameterMap (MethodInfo method, KotlinFunction function, KotlinClass? kotlinClass)
+ {
+ var parameters = method.GetParameters ();
+ var start = 0;
+ var end = parameters.Length;
+
+ // If the parameter counts are the same, that's good enough (because we know signatures matched)
+ if (IsValidParameterMap (method, function, start, end))
+ return (start, end);
+
+ // Remove the "hidden" receiver type parameter from the start of the parameter list
+ if (function.ReceiverType != null)
+ start++;
+
+ if (IsValidParameterMap (method, function, start, end))
+ return (start, end);
+
+ var last_p = parameters.Last ();
+
+ // Remove the "hidden" coroutine continuation type parameter from the end of the parameter list
+ // We try to restrict it to compiler generated paramteres because a user might have actually used it as a parameter
+ if (last_p.Type.BinaryName == "Lkotlin/coroutines/Continuation;" && (last_p.IsUnnamedParameter () || last_p.IsCompilerNamed ()))
+ end--;
+
+ if (IsValidParameterMap (method, function, start, end))
+ return (start, end);
+
+ // Remove the "hidden" "this" type parameter for a static method from the start of the parameter list
+ // Note we do this last because sometimes it isn't there.
+ if (method.AccessFlags.HasFlag (MethodAccessFlags.Static))
+ start++;
+
+ if (IsValidParameterMap (method, function, start, end))
+ return (start, end);
+
+ return (0, 0);
+ }
+
+ static bool IsValidParameterMap (MethodInfo method, KotlinFunction function, int start, int end) => function.ValueParameters?.Count == end - start;
+
  static void FixupExtensionMethod (MethodInfo method)
  {
  // Kotlin "extension" methods give the first parameter an ugly name
@@ -271,10 +313,23 @@ static void FixupField (FieldInfo? field, KotlinProperty metadata)
 
  static MethodInfo? FindJavaMethod (KotlinFile kotlinFile, KotlinFunction function, ClassFile klass)
  {
- var possible_methods = klass.Methods.Where (method => method.Name == function.JvmName &&
- method.GetFilteredParameters ().Length == function.ValueParameters?.Count);
+ var possible_methods = klass.Methods.Where (method => method.Name == function.JvmName).ToArray ();
+ var signature = function.JvmSignature;
+
+ // 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;
+
+ // Sometimes JvmSignature is null (or unhelpful), so we're going to construct one ourselves and see if they match
+ signature = function.ConstructJvmSignature ();
 
- foreach (var method in possible_methods) {
+ if (possible_methods.SingleOrDefault (method => method.Descriptor == signature) is MethodInfo m2)
+ return m2;
+
+ // 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)) {
  if (function.ReturnType == null)
  continue;
  if (!TypesMatch (method.ReturnType, function.ReturnType, kotlinFile))
@@ -286,6 +341,10 @@ static void FixupField (FieldInfo? field, KotlinProperty metadata)
  return method;
  }
 
+ // 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;
  }
 

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

@@ -8,7 +8,7 @@ namespace Xamarin.Android.Tools.Bytecode
 {
 	public static class KotlinUtilities
 	{
- public static string ConvertKotlinTypeSignature (KotlinType? type, KotlinFile? metadata = null)
+ public static string ConvertKotlinTypeSignature (KotlinType? type, KotlinFile? metadata = null, bool convertUnsignedToPrimitive = true)
  {
  if (type is null)
  return string.Empty;
@@ -27,15 +27,15 @@ public static string ConvertKotlinTypeSignature (KotlinType? type, KotlinFile? m
  return "Ljava/lang/Object;";
  }
 
- var result = ConvertKotlinClassToJava (class_name);
+ var result = ConvertKotlinClassToJava (class_name, convertUnsignedToPrimitive);
 
  if (result == "[")
- result += ConvertKotlinTypeSignature (type.Arguments?.FirstOrDefault ()?.Type);
+ result += ConvertKotlinTypeSignature (type.Arguments?.FirstOrDefault ()?.Type, null, convertUnsignedToPrimitive);
 
  return result;
  }
 
- public static string ConvertKotlinClassToJava (string? className)
+ public static string ConvertKotlinClassToJava (string? className, bool convertUnsignedToPrimitive = true)
  {
  if (className == null || string.IsNullOrWhiteSpace (className))
  return string.Empty;
@@ -45,6 +45,9 @@ public static string ConvertKotlinClassToJava (string? className)
  if (type_map.TryGetValue (className.TrimEnd (';'), out var result))
  return result;
 
+ if (convertUnsignedToPrimitive && unsigned_type_map.TryGetValue (className.TrimEnd (';'), out var result2))
+ return result2;
+
  return "L" + className;
  }
 
@@ -92,6 +95,14 @@ public static bool IsDefaultConstructorMarker (this MethodInfo method)
  parameters [parameters.Length - 1].Type.TypeSignature == "Lkotlin/jvm/internal/DefaultConstructorMarker;";
  }
 
+ // Sometimes the Kotlin provided JvmSignature is null (or unhelpful), so we need to construct one ourselves
+ public static string ConstructJvmSignature (this KotlinFunction function)
+ {
+ // The receiver type (if specified) is a "hidden" parameter passed at the beginning
+ // of the Java parameter list, so we include it so the Signature/Descriptors match.
+ return $"({function.ReceiverType?.GetSignature (false)}{string.Concat (function.ValueParameters?.Select (p => p.Type?.GetSignature (false)) ?? Enumerable.Empty<string> ())}){function.ReturnType?.GetSignature (false)}";
+ }
+
  internal static List<TResult>? ToList<TSource, TResult> (this IEnumerable<TSource>? self, JvmNameResolver resolver, Func<TSource, JvmNameResolver, TResult?> creator)
  where TResult: class
  {
@@ -114,37 +125,42 @@ public static bool IsDefaultConstructorMarker (this MethodInfo method)
 
  public static bool IsUnnamedParameter (this ParameterInfo parameter) => parameter.Name.Length > 1 && parameter.Name.StartsWith ("p", StringComparison.Ordinal) && int.TryParse (parameter.Name.Substring (1), out var _);
 
+ public static bool IsCompilerNamed (this ParameterInfo parameter) => parameter.Name.Length > 0 && parameter.Name.StartsWith ("$", StringComparison.Ordinal);
+
  public static bool IsUnnamedParameter (this KotlinValueParameter parameter) => parameter.Name?.Length > 1 &&
  parameter.Name.StartsWith ("p", StringComparison.Ordinal) &&
  int.TryParse (parameter.Name.Substring (1), out var _);
 
+ static Dictionary<string, string> unsigned_type_map = new Dictionary<string, string> {
+ { "kotlin/UInt", "I" },
+ { "kotlin/ULong", "J" },
+ { "kotlin/UShort", "S" },
+ { "kotlin/UByte", "B" },
+ { "kotlin/UIntArray", "[I" },
+ { "kotlin/ULongArray", "[J" },
+ { "kotlin/UShortArray", "[S" },
+ { "kotlin/UByteArray", "[B" },
+ };
+
  static Dictionary<string, string> type_map = new Dictionary<string, string> {
  { "kotlin/Int", "I" },
- { "kotlin/UInt", "I" },
  { "kotlin/Double", "D" },
  { "kotlin/Char", "C" },
  { "kotlin/Long", "J" },
- { "kotlin/ULong", "J" },
  { "kotlin/Float", "F" },
  { "kotlin/Short", "S" },
- { "kotlin/UShort", "S" },
  { "kotlin/Byte", "B" },
- { "kotlin/UByte", "B" },
  { "kotlin/Boolean", "Z" },
  { "kotlin/Unit", "V" },
 
  { "kotlin/Array", "[" },
  { "kotlin/IntArray", "[I" },
- { "kotlin/UIntArray", "[I" },
  { "kotlin/DoubleArray", "[D" },
  { "kotlin/CharArray", "[C" },
  { "kotlin/LongArray", "[J" },
- { "kotlin/ULongArray", "[J" },
  { "kotlin/FloatArray", "[F" },
  { "kotlin/ShortArray", "[S" },
- { "kotlin/UShortArray", "[S" },
  { "kotlin/ByteArray", "[B" },
- { "kotlin/UByteArray", "[B" },
  { "kotlin/BooleanArray", "[Z" },
 
  { "kotlin/Any", "Ljava/lang/Object;" },

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

@@ -105,6 +105,46 @@ protected static void AssertXmlDeclaration (string[] classResources, string xmlR
  Assert.AreEqual (expected, actual.ToString ());
  }
 
+ protected static KotlinMetadata GetMetadataForClass (ClassFile klass)
+ {
+ var attr = klass.Attributes.OfType<RuntimeVisibleAnnotationsAttribute> ().FirstOrDefault ();
+ var kotlin = attr?.Annotations.FirstOrDefault (a => a.Type == "Lkotlin/Metadata;");
+
+ Assert.NotNull (kotlin);
+
+ var meta = KotlinMetadata.FromAnnotation (kotlin);
+
+ return meta;
+ }
+
+ protected static KotlinClass GetClassMetadataForClass (ClassFile klass)
+ {
+ var meta = GetMetadataForClass (klass);
+ Assert.AreEqual (KotlinMetadataKind.Class, meta.Kind);
+
+ return meta.AsClassMetadata ();
+ }
+
+ protected static KotlinFile GetFileMetadataForClass (ClassFile klass)
+ {
+ var meta = GetMetadataForClass (klass);
+ Assert.AreEqual (KotlinMetadataKind.File, meta.Kind);
+
+ return meta.AsFileMetadata ();
+ }
+
+ protected static KotlinMetadata GetMetadataForClassFile (string file)
+ {
+ var c = LoadClassFile (file);
+ return GetMetadataForClass (c);
+ }
+
+ protected static KotlinClass GetClassMetadata (string file)
+ {
+ var c = LoadClassFile (file);
+ return GetClassMetadataForClass (c);
+ }
+
  static Stream GetResourceStream (string resource)
  {
  // Look for resources that end with our name, this allows us to

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

@@ -310,5 +310,53 @@ public void HandleKotlinNameShadowing ()
  Assert.True (klass.Methods.Single (m => m.Name == "hitCount").AccessFlags.HasFlag (MethodAccessFlags.Public));
  Assert.True (klass.Methods.Single (m => m.Name == "setType").AccessFlags.HasFlag (MethodAccessFlags.Public));
  }
+
+ [Test]
+ public void MatchParametersWithReceiver ()
+ {
+ var klass = LoadClassFile ("DeepRecursiveKt.class");
+ var meta = GetFileMetadataForClass (klass);
+
+ var java_method = klass.Methods.Single (m => m.Name == "invoke");
+ var kotlin_function = meta.Functions.Single (m => m.Name == "invoke");
+
+ (var start, var end) = KotlinFixups.CreateParameterMap (java_method, kotlin_function, null);
+
+ // Start is 1 instead of 0 to skip the receiver
+ Assert.AreEqual (1, start);
+ Assert.AreEqual (2, end);
+ }
+
+ [Test]
+ public void MatchParametersWithContinuation ()
+ {
+ var klass = LoadClassFile ("DeepRecursiveScope.class");
+ var meta = GetClassMetadataForClass (klass);
+
+ var java_method = klass.Methods.Single (m => m.Name == "callRecursive" && m.GetParameters ().Count () == 2);
+ var kotlin_function = meta.Functions.Single (m => m.Name == "callRecursive" && m.JvmSignature.Count (c => c == ';') == 3);
+
+ (var start, var end) = KotlinFixups.CreateParameterMap (java_method, kotlin_function, null);
+
+ // End is 1 instead of 2 to skip the trailing continuation
+ Assert.AreEqual (0, start);
+ Assert.AreEqual (1, end);
+ }
+
+ [Test]
+ public void MatchParametersWithStaticThis ()
+ {
+ var klass = LoadClassFile ("UByteArray.class");
+ var meta = GetClassMetadataForClass (klass);
+
+ var java_method = klass.Methods.Single (m => m.Name == "contains-7apg3OU" && m.GetParameters ().Count () == 2);
+ var kotlin_function = meta.Functions.Single (m => m.Name == "contains");
+
+ (var start, var end) = KotlinFixups.CreateParameterMap (java_method, kotlin_function, null);
+
+ // Start is 1 instead of 0 to skip the static 'this'
+ Assert.AreEqual (1, start);
+ Assert.AreEqual (2, end);
+ }
 	}
 }

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

@@ -201,31 +201,6 @@ public void KotlinExtensionMethods ()
  Assert.AreEqual (KotlinClassInheritability.Final, klass_meta.Inheritability);
  Assert.AreEqual (KotlinClassVisibility.Public, klass_meta.Visibility);
  }
-
- private KotlinMetadata GetMetadataForClassFile (string file)
- {
- var c = LoadClassFile (file);
- var attr = c.Attributes.OfType<RuntimeVisibleAnnotationsAttribute> ().FirstOrDefault ();
- var kotlin = attr?.Annotations.FirstOrDefault (a => a.Type == "Lkotlin/Metadata;");
-
- Assert.NotNull (kotlin);
-
- var meta = KotlinMetadata.FromAnnotation (kotlin);
-
- return meta;
- }
-
- private KotlinClass GetClassMetadata (string file)
- {
- var meta = GetMetadataForClassFile (file);
-
- Assert.AreEqual (KotlinMetadataKind.Class, meta.Kind);
-
- Assert.AreEqual ("1.0.3", meta.ByteCodeVersion.ToString ());
- Assert.AreEqual ("1.1.15", meta.MetadataVersion.ToString ());
-
- return meta.AsClassMetadata ();
- }
 	}
 }
 

tests/Xamarin.Android.Tools.Bytecode-Tests/Xamarin.Android.Tools.Bytecode-Tests.csproj

@@ -25,7 +25,7 @@
 
  <ItemGroup>
  <EmbeddedResource Include="Resources\*" />
- <EmbeddedResource Include="kotlin\**\*.class" />
+ <EmbeddedResource Include="kotlin*\**\*.class" />
 
  <EmbeddedResource Include="$(IntermediateOutputPath)classes\com\xamarin\NotNullClass.class" />
  <EmbeddedResource Include="$(IntermediateOutputPath)classes\com\xamarin\IJavaInterface.class" />

tools/generator/generator.slnf

@@ -15,6 +15,7 @@
  "tests\\Xamarin.Android.Tools.Bytecode-Tests\\Xamarin.Android.Tools.Bytecode-Tests.csproj",
  "tests\\Xamarin.SourceWriter-Tests\\Xamarin.SourceWriter-Tests.csproj",
  "tests\\generator-Tests\\generator-Tests.csproj",
+ "tools\\class-parse\\class-parse.csproj",
  "tools\\generator\\generator.csproj"
  ]
  }