diff --git a/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinClassMetadata.cs b/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinClassMetadata.cs index d983df5c4..de5a5513e 100644 --- a/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinClassMetadata.cs +++ b/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinClassMetadata.cs @@ -318,6 +318,10 @@ public class KotlinProperty : KotlinMethodBase } public override string? ToString () => Name; + + public bool IsInternalVisibility => (Flags & KotlinPropertyFlags.VisibilityMask) == KotlinPropertyFlags.Internal; + + public bool IsPrivateVisibility => (Flags & KotlinPropertyFlags.VisibilityMask) == KotlinPropertyFlags.Private; } public class KotlinType @@ -688,6 +692,8 @@ public enum KotlinPropertyFlags PrivateToThis = 0b00_00_100_0, Local = 0b00_00_101_0, + VisibilityMask = 0b00_00_111_0, + Final = 0b00_00_000_0, Open = 0b00_01_000_0, Abstract = 0b00_10_000_0, @@ -698,15 +704,15 @@ public enum KotlinPropertyFlags Delegation = 0b10_00_000_0, Synthesized = 0b11_00_000_0, - IsVar = 0b_000000001_000_00_000_0, - HasGetter = 0b_000000010_000_00_000_0, - HasSetter = 0b_000000100_000_00_000_0, - IsConst = 0b_000001000_000_00_000_0, - IsLateInit = 0b_000010000_000_00_000_0, - HasConstant = 0b_000100000_000_00_000_0, - IsExternalProperty = 0b_001000000_000_00_000_0, - IsDelegated = 0b_010000000_000_00_000_0, - IsExpectProperty = 0b_100000000_000_00_000_0 + IsVar = 0b_000000001_00_00_000_0, + HasGetter = 0b_000000010_00_00_000_0, + HasSetter = 0b_000000100_00_00_000_0, + IsConst = 0b_000001000_00_00_000_0, + IsLateInit = 0b_000010000_00_00_000_0, + HasConstant = 0b_000100000_00_00_000_0, + IsExternalProperty = 0b_001000000_00_00_000_0, + IsDelegated = 0b_010000000_00_00_000_0, + IsExpectProperty = 0b_100000000_00_00_000_0 } [Flags] diff --git a/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinFixups.cs b/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinFixups.cs index 0ad357d84..94f1435ad 100644 --- a/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinFixups.cs +++ b/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinFixups.cs @@ -272,8 +272,8 @@ static void FixupProperty (MethodInfo? getter, MethodInfo? setter, KotlinPropert if (getter is null && setter is null) return; - // Hide property if it isn't Public/Protected - if (!metadata.Flags.IsPubliclyVisible ()) { + // Hide getters/setters if property is Internal + if (metadata.IsInternalVisibility) { if (getter?.IsPubliclyVisible == true) { Log.Debug ($"Kotlin: Hiding internal getter method {getter.DeclaringType?.ThisClass.Name.Value} - {getter.Name}"); @@ -384,7 +384,17 @@ static void FixupField (FieldInfo? field, KotlinProperty metadata) static MethodInfo? FindJavaPropertyGetter (KotlinFile kotlinClass, KotlinProperty property, ClassFile klass) { - var possible_methods = klass.Methods.Where (method => string.Compare (method.GetMethodNameWithoutSuffix (), $"get{property.Name}", StringComparison.OrdinalIgnoreCase) == 0 && + // Private properties do not have getters + if (property.IsPrivateVisibility) + return null; + + // Public/protected getters look like "getFoo" + // Internal getters look like "getFoo$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)); + + possible_methods = possible_methods.Where (method => method.GetParameters ().Length == 0 && property.ReturnType != null && TypesMatch (method.ReturnType, property.ReturnType, kotlinClass)); @@ -394,11 +404,21 @@ static void FixupField (FieldInfo? field, KotlinProperty metadata) static MethodInfo? FindJavaPropertySetter (KotlinFile kotlinClass, KotlinProperty property, ClassFile klass) { - var possible_methods = klass.Methods.Where (method => string.Compare (method.GetMethodNameWithoutSuffix (), $"set{property.Name}", StringComparison.OrdinalIgnoreCase) == 0 && - property.ReturnType != null && - method.GetParameters ().Length == 1 && - method.ReturnType.BinaryName == "V" && - TypesMatch (method.GetParameters () [0].Type, property.ReturnType, kotlinClass)); + // Private properties do not have setters + if (property.IsPrivateVisibility) + return null; + + // Public/protected setters look like "setFoo" + // Internal setters look like "setFoo$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)); + + possible_methods = possible_methods.Where (method => + property.ReturnType != null && + method.GetParameters ().Length == 1 && + method.ReturnType.BinaryName == "V" && + TypesMatch (method.GetParameters () [0].Type, property.ReturnType, kotlinClass)); return possible_methods.FirstOrDefault (); } diff --git a/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinUtilities.cs b/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinUtilities.cs index f177db989..b9312c40e 100644 --- a/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinUtilities.cs +++ b/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinUtilities.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Text; using System.Threading.Tasks; @@ -8,6 +9,18 @@ namespace Xamarin.Android.Tools.Bytecode { public static class KotlinUtilities { + [return: NotNullIfNotNull (nameof (value))] + public static string? Capitalize (this string? value) + { + if (string.IsNullOrWhiteSpace (value)) + return value; + + if (value.Length < 1) + return value; + + return char.ToUpperInvariant (value [0]) + value.Substring (1); + } + public static string ConvertKotlinTypeSignature (KotlinType? type, KotlinFile? metadata = null, bool convertUnsignedToPrimitive = true) { if (type is null) diff --git a/tests/Xamarin.Android.Tools.Bytecode-Tests/KotlinFixupsTests.cs b/tests/Xamarin.Android.Tools.Bytecode-Tests/KotlinFixupsTests.cs index a393c6782..1108e081a 100644 --- a/tests/Xamarin.Android.Tools.Bytecode-Tests/KotlinFixupsTests.cs +++ b/tests/Xamarin.Android.Tools.Bytecode-Tests/KotlinFixupsTests.cs @@ -324,7 +324,29 @@ public void HandleKotlinNameShadowing () Assert.True (klass.Methods.Single (m => m.Name == "count").AccessFlags.HasFlag (MethodAccessFlags.Public)); Assert.True (klass.Methods.Single (m => m.Name == "hitCount").AccessFlags.HasFlag (MethodAccessFlags.Public)); + + // Private property and explicit getter/setter + // There is no generated getter/setter, and explicit ones should be public + Assert.True (klass.Methods.Single (m => m.Name == "getType").AccessFlags.HasFlag (MethodAccessFlags.Public)); Assert.True (klass.Methods.Single (m => m.Name == "setType").AccessFlags.HasFlag (MethodAccessFlags.Public)); + + // Private immutable property and explicit getter/setter + // There is no generated getter/setter, and explicit ones should be public + Assert.True (klass.Methods.Single (m => m.Name == "getType2").AccessFlags.HasFlag (MethodAccessFlags.Public)); + Assert.True (klass.Methods.Single (m => m.Name == "setType2").AccessFlags.HasFlag (MethodAccessFlags.Public)); + + // Internal property and explicit getter/setter + // Generated getter/setter should not be public, and explicit ones should be public + Assert.False (klass.Methods.Single (m => m.Name == "getItype$main").AccessFlags.HasFlag (MethodAccessFlags.Public)); + Assert.False (klass.Methods.Single (m => m.Name == "setItype$main").AccessFlags.HasFlag (MethodAccessFlags.Public)); + Assert.True (klass.Methods.Single (m => m.Name == "getItype").AccessFlags.HasFlag (MethodAccessFlags.Public)); + Assert.True (klass.Methods.Single (m => m.Name == "setItype").AccessFlags.HasFlag (MethodAccessFlags.Public)); + + // Internal immutable property and explicit getter/setter + // Generated getter should not be public, and explicit ones should be public + 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)); } [Test] diff --git a/tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin/NameShadowing.class b/tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin/NameShadowing.class index 9b8440864..989e48d09 100644 Binary files a/tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin/NameShadowing.class and b/tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin/NameShadowing.class differ diff --git a/tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin/NameShadowing.kt b/tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin/NameShadowing.kt index 6c05ca9b4..5c00497fc 100644 --- a/tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin/NameShadowing.kt +++ b/tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin/NameShadowing.kt @@ -8,7 +8,23 @@ public class NameShadowing { private var hitCount = 0 fun hitCount(): Int = hitCount - // Property and setter + // Private property and explicit getter/setter private var type = 0 - fun setType(type: Int) = { println (type); } + fun getType(): Int = type + fun setType(type: Int) { } + + // Private immutable property and explicit getter/setter + private val type2 = 0 + fun getType2(): Int = type2 + fun setType2(type: Int) { } + + // Internal property and explicit getter/setter + internal var itype = 0 + fun getItype(): Int = itype + fun setItype(type: Int) { } + + // Internal immutable property and explicit getter/setter + internal val itype2 = 0 + fun getItype2(): Int = itype2 + fun setItype2(type: Int) { } } \ No newline at end of file