diff --git a/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinClassMetadata.cs b/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinClassMetadata.cs index d798192f7..d983df5c4 100644 --- a/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinClassMetadata.cs +++ b/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); } } diff --git a/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinFixups.cs b/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinFixups.cs index a763718f4..b4b8940de 100644 --- a/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinFixups.cs +++ b/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; } diff --git a/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinUtilities.cs b/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinUtilities.cs index 8b27073fa..f177db989 100644 --- a/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinUtilities.cs +++ b/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 ())}){function.ReturnType?.GetSignature (false)}"; + } + internal static List? ToList (this IEnumerable? self, JvmNameResolver resolver, Func 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 unsigned_type_map = new Dictionary { + { "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 type_map = new Dictionary { { "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;" }, diff --git a/tests/Xamarin.Android.Tools.Bytecode-Tests/ClassFileFixture.cs b/tests/Xamarin.Android.Tools.Bytecode-Tests/ClassFileFixture.cs index e119fa58f..51795a60c 100644 --- a/tests/Xamarin.Android.Tools.Bytecode-Tests/ClassFileFixture.cs +++ b/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 ().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 diff --git a/tests/Xamarin.Android.Tools.Bytecode-Tests/KotlinFixupsTests.cs b/tests/Xamarin.Android.Tools.Bytecode-Tests/KotlinFixupsTests.cs index 39841abe2..a4a272220 100644 --- a/tests/Xamarin.Android.Tools.Bytecode-Tests/KotlinFixupsTests.cs +++ b/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); + } } } diff --git a/tests/Xamarin.Android.Tools.Bytecode-Tests/KotlinMetadataTests.cs b/tests/Xamarin.Android.Tools.Bytecode-Tests/KotlinMetadataTests.cs index 0475dba2d..cf3e97ff1 100644 --- a/tests/Xamarin.Android.Tools.Bytecode-Tests/KotlinMetadataTests.cs +++ b/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 ().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 (); - } } } diff --git a/tests/Xamarin.Android.Tools.Bytecode-Tests/Xamarin.Android.Tools.Bytecode-Tests.csproj b/tests/Xamarin.Android.Tools.Bytecode-Tests/Xamarin.Android.Tools.Bytecode-Tests.csproj index cb07bdc4b..3f39f0716 100644 --- a/tests/Xamarin.Android.Tools.Bytecode-Tests/Xamarin.Android.Tools.Bytecode-Tests.csproj +++ b/tests/Xamarin.Android.Tools.Bytecode-Tests/Xamarin.Android.Tools.Bytecode-Tests.csproj @@ -25,7 +25,7 @@ - + diff --git a/tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin-ThirdParty/DeepRecursiveKt.class b/tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin-ThirdParty/DeepRecursiveKt.class new file mode 100644 index 000000000..e115f1336 Binary files /dev/null and b/tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin-ThirdParty/DeepRecursiveKt.class differ diff --git a/tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin-ThirdParty/DeepRecursiveScope.class b/tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin-ThirdParty/DeepRecursiveScope.class new file mode 100644 index 000000000..71b527366 Binary files /dev/null and b/tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin-ThirdParty/DeepRecursiveScope.class differ diff --git a/tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin-ThirdParty/UByteArray.class b/tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin-ThirdParty/UByteArray.class new file mode 100644 index 000000000..15ba35755 Binary files /dev/null and b/tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin-ThirdParty/UByteArray.class differ diff --git a/tools/generator/generator.slnf b/tools/generator/generator.slnf index 577b7bf1a..e56de5ee3 100644 --- a/tools/generator/generator.slnf +++ b/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" ] }