From 24f398cce3e10b63931be1016a477bcd42c0eede Mon Sep 17 00:00:00 2001 From: Jonathan Pobst Date: Mon, 30 Oct 2023 07:59:27 -1000 Subject: [PATCH] [Xamarin.Android.Tools.Bytecode] Fix for finding getters/setters with Kotlin unsigned types. --- .../Kotlin/KotlinFixups.cs | 12 ++++++---- .../Kotlin/KotlinUtilities.cs | 19 +++++++++++----- .../KotlinFixupsTests.cs | 21 ++++++++++++++++++ .../kotlin/NameShadowing.class | Bin 1936 -> 2624 bytes .../kotlin/NameShadowing.kt | 4 ++++ 5 files changed, 46 insertions(+), 10 deletions(-) diff --git a/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinFixups.cs b/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinFixups.cs index 94f1435ad..56ded5131 100644 --- a/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinFixups.cs +++ b/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinFixups.cs @@ -389,10 +389,12 @@ static void FixupField (FieldInfo? field, KotlinProperty metadata) return null; // Public/protected getters look like "getFoo" + // Public/protected getters with unsigned types look like "getFoo-abcdefg" // Internal getters look like "getFoo$main" + // Internal getters with unsigned types look like "getFoo-WZ4Q5Ns$main" var possible_methods = property.IsInternalVisibility ? - klass.Methods.Where (method => method.Name.StartsWith ($"get{property.Name.Capitalize ()}$", StringComparison.Ordinal)) : - klass.Methods.Where (method => method.Name.Equals ($"get{property.Name.Capitalize ()}", StringComparison.Ordinal)); + klass.Methods.Where (method => method.GetMethodNameWithoutUnsignedSuffix ().StartsWith ($"get{property.Name.Capitalize ()}$", StringComparison.Ordinal)) : + klass.Methods.Where (method => method.GetMethodNameWithoutUnsignedSuffix ().Equals ($"get{property.Name.Capitalize ()}", StringComparison.Ordinal)); possible_methods = possible_methods.Where (method => method.GetParameters ().Length == 0 && @@ -409,10 +411,12 @@ static void FixupField (FieldInfo? field, KotlinProperty metadata) return null; // Public/protected setters look like "setFoo" + // Public/protected setters with unsigned types look like "setFoo-abcdefg" // Internal setters look like "setFoo$main" + // Internal setters with unsigned types look like "setFoo-WZ4Q5Ns$main" var possible_methods = property.IsInternalVisibility ? - klass.Methods.Where (method => method.Name.StartsWith ($"set{property.Name.Capitalize ()}$", StringComparison.Ordinal)) : - klass.Methods.Where (method => method.Name.Equals ($"set{property.Name.Capitalize ()}", StringComparison.Ordinal)); + klass.Methods.Where (method => method.GetMethodNameWithoutUnsignedSuffix ().StartsWith ($"set{property.Name.Capitalize ()}$", StringComparison.Ordinal)) : + klass.Methods.Where (method => method.GetMethodNameWithoutUnsignedSuffix ().Equals ($"set{property.Name.Capitalize ()}", StringComparison.Ordinal)); possible_methods = possible_methods.Where (method => property.ReturnType != null && diff --git a/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinUtilities.cs b/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinUtilities.cs index b9312c40e..bf68a8c8e 100644 --- a/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinUtilities.cs +++ b/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinUtilities.cs @@ -76,16 +76,23 @@ public static ParameterInfo[] GetFilteredParameters (this MethodInfo method) return method.GetParameters ().Where (p => p.Type.BinaryName != "Lkotlin/jvm/internal/DefaultConstructorMarker;" && !p.Name.StartsWith ("$", StringComparison.Ordinal)).ToArray (); } - public static string GetMethodNameWithoutSuffix (this MethodInfo method) + public static string GetMethodNameWithoutUnsignedSuffix (this MethodInfo method) + => GetMethodNameWithoutUnsignedSuffix (method.Name); + + public static string GetMethodNameWithoutUnsignedSuffix (string name) { - // Kotlin will rename some of its constructs to hide them from the Java runtime - // These take the form of thing like: - // - add-impl + // Kotlin will add a type hash suffix to the end of the method name that use unsigned types // - add-H3FcsT8 // We strip them for trying to match up the metadata to the MethodInfo - var index = method.Name.IndexOfAny (new [] { '-', '$' }); + // Additionally, generated setters for unsigned types have multiple suffixes, + // we only want to remove the unsigned suffix. + // - getFoo-pVg5ArA$main + var dollar_index = name.IndexOf ('$'); + var dollar_suffix = dollar_index >= 0 ? name.Substring (dollar_index) : string.Empty; + + var index = name.IndexOf ('-'); - return index >= 0 ? method.Name.Substring (0, index) : method.Name; + return index >= 0 ? name.Substring (0, index) + dollar_suffix : name; } public static bool IsDefaultConstructorMarker (this MethodInfo method) diff --git a/tests/Xamarin.Android.Tools.Bytecode-Tests/KotlinFixupsTests.cs b/tests/Xamarin.Android.Tools.Bytecode-Tests/KotlinFixupsTests.cs index 1108e081a..292ea5f2a 100644 --- a/tests/Xamarin.Android.Tools.Bytecode-Tests/KotlinFixupsTests.cs +++ b/tests/Xamarin.Android.Tools.Bytecode-Tests/KotlinFixupsTests.cs @@ -347,6 +347,17 @@ public void HandleKotlinNameShadowing () Assert.False (klass.Methods.Single (m => m.Name == "getItype2$main").AccessFlags.HasFlag (MethodAccessFlags.Public)); Assert.True (klass.Methods.Single (m => m.Name == "getItype2").AccessFlags.HasFlag (MethodAccessFlags.Public)); Assert.True (klass.Methods.Single (m => m.Name == "setItype2").AccessFlags.HasFlag (MethodAccessFlags.Public)); + + // Internal property with unsigned type + // Generated getter/setter should not be public + Assert.False (klass.Methods.Single (m => m.Name == "getUnsignedInternalProperty-pVg5ArA$main").AccessFlags.HasFlag (MethodAccessFlags.Public)); + Assert.False (klass.Methods.Single (m => m.Name == "setUnsignedInternalProperty-WZ4Q5Ns$main").AccessFlags.HasFlag (MethodAccessFlags.Public)); + + // Public property with unsigned type + // We want to check that KotlinType/KotlinReturnType are filled it as it proves our FindJavaProperty[Getter|Setter] functions are matching + // (We aren't changing the visibility of the getter/setter, so we can't just check the access flags) + Assert.AreEqual ("uint", klass.Methods.Single (m => m.Name == "getUnsignedPublicProperty-pVg5ArA").KotlinReturnType); + Assert.AreEqual ("uint", klass.Methods.Single (m => m.Name == "setUnsignedPublicProperty-WZ4Q5Ns").GetParameters () [0].KotlinType); } [Test] @@ -418,5 +429,15 @@ public void MatchMetadataToMultipleMethodsWithSameMangledName () 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); } + + [Test] + public void GetMethodNameWithoutUnsignedSuffix () + { + // Just a few quick tests to ensure the various cases are covered + Assert.AreEqual ("setFoo", KotlinUtilities.GetMethodNameWithoutUnsignedSuffix ("setFoo")); + Assert.AreEqual ("setFoo", KotlinUtilities.GetMethodNameWithoutUnsignedSuffix ("setFoo-7apg3OU")); + Assert.AreEqual ("setFoo$main", KotlinUtilities.GetMethodNameWithoutUnsignedSuffix ("setFoo-7apg3OU$main")); + Assert.AreEqual ("setFoo$main", KotlinUtilities.GetMethodNameWithoutUnsignedSuffix ("setFoo$main")); + } } } diff --git a/tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin/NameShadowing.class b/tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin/NameShadowing.class index 989e48d095c56e792129b72c04ff12df99c6fdd6..023c36e0609f47e7f3df7b02c7ba95e19305b599 100644 GIT binary patch literal 2624 zcma)8T~8ZF6uq;y*XzX}iveTIcLI%ZLNE^WBcVwNXds z%0plJ3;NhT_oY&!R(+@ted~{^dS+*B%xW!1h`Dp`oO{llnY|BoKa@`N}cz-GElyDWV>5JKS?>6GtMY}Xt~c`pIRatCP@XkG>IkNT`E?W zY9R2j6>LO+D@mG@=y@#qXe_3Hs|vuviD^3k+md=wg5LuI;9f+01A*lSI5DB&tw=_H zn(jm^KBekWqJU+|d7Dw9i^06Zk7|mDKkswE%cr%&_086%&ss!UU+6dAu6@0}!+q#{ z59`$%>2NZd@1YA|d@@GE+s93}VLK~dTCQ0)T@%-(oV<$Qn`2@~h=K6M93%d{Exa;j z^;O1xI&UxNN?y?w1{OO-a#5_>7QXk-vF1p7aKvhy|uhJOWI z2F_^&TEm`J7FvtOs09De7d+;HbB2ELM#iF_-ZK=-nK1Xlk zYK<$cdHWb|L3FEGNBd>AZO7US>BH8Wcf!U~;EnE10P=DGyYaLjRZGo8B zMK{|gR?X%%Ae)ZUbWPW8I-FMVv_$c-Xp|K%4Q7hyde5T8VY8xnJs*AP`RFCj$D`v} zyewWE9ZV`V^9bG!c(5}XE&V}SNqtL6F=WJ${mskz6k`qiO zAO0x3VXTk&+$*qxJ68IglD}f2kRTJ%2=*k$y2-iFqT-d%1sEa@tGGVH@HsQd(e)-*! zr$Uog2Nyp*p!iCtekNG_T&Vt?uKtx9%mi;xX7|6rwa_B6!6N2Ei&zgYqUSA6{EsYn;|Ol{l3-O>mm!bkpNm JfHK+*~V!~>G35?LG;SN4_jX%Y; z8xxI*3qx4?qm19387elTxS8|%zH`n!_uTpM^V@d-mvKj+_|$EAOPlU``;8xLAZ^=GRuy|plpjDSAx2YyIKW_o7XLJqo#Eb;=mwf1fh3W%nK0?Yz5923Zf zo4yn%E&P%HRzzj^e zfg2X8F?xi-1kGTPxTXS3wp>3T*pE|>$KaDh5PTSspQq!(U7l3ch$3;uM70mRjU=TW zrDOwV~&1dP>MlWqNHwQDtn!VHY@Ju_CM%!J=UE}Rh!LHmJ;bCvM-V4 zMCKxCJN=Xc4c#dw_VFHju*V)cY%khzy%+oMAF2+_y$)SbYP#$LSJlx~_80pd<+6#Y zW{tdKOk|CMV=fLj$C9v0V#Hy~F_ToLCv14qvpc)mY%mF%otlh;e?q~Kvd!4IQ zTJQt!>27P)+j;J;Zqrjt1{g$`vOP`Q?LC_A} z&~FD4b6ock};vHzQ=UAgq&R4Gp*Gvtl%g%meh%UrI97DOILi?K7;8RN^G65S@p}xh~mBC6{~2 zg;a8XDtWM%Tudbop^~(HCF+!S_xs#&%XUN@2zqx2I!-4;nEbychEkiD>NfG$ zOQ;+%gfpr2XL^S)l3Ks^f8^0r^0}j%ANyzX8sDCJB-e49QgWxx^O;;&L8gg|P1Kv1 z#T->Mnz)3^D-aS_aCHS030vZtgf97$SBfJsE^$JlBrzcs;H<