[Xamarin.Android.Tools.Bytecode] Only hide getters/setters for internal properties.

Author: jpobst

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

@@ -318,6 +318,9 @@ public class KotlinProperty : KotlinMethodBase
 		}
 
 		public override string? ToString () => Name;
+
+		// Internal has value 0, so we have to check for it by eliminating all other possibilities.
+		public bool IsInternalVisible => !Flags.HasFlag (KotlinPropertyFlags.Public) && !Flags.HasFlag (KotlinPropertyFlags.Protected) && !Flags.HasFlag (KotlinPropertyFlags.Private) && !Flags.HasFlag (KotlinPropertyFlags.Private) && !Flags.HasFlag (KotlinPropertyFlags.Local);
 	}
 
 	public class KotlinType
@@ -698,15 +701,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]

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.IsInternalVisible) {
 
 				if (getter?.IsPubliclyVisible == true) {
 					Log.Debug ($"Kotlin: Hiding internal getter method {getter.DeclaringType?.ThisClass.Name.Value} - {getter.Name}");
@@ -384,7 +384,9 @@ 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 &&
+			var method_name = $"get{property.Name.Capitalize ()}$";
+
+			var possible_methods = klass.Methods.Where (method => method.Name.StartsWith (method_name, StringComparison.Ordinal) &&
 					method.GetParameters ().Length == 0 &&
 					property.ReturnType != null &&
 					TypesMatch (method.ReturnType, property.ReturnType, kotlinClass));
@@ -394,7 +396,9 @@ 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 &&
+			var method_name = $"set{property.Name.Capitalize ()}$";
+
+			var possible_methods = klass.Methods.Where (method => method.Name.StartsWith (method_name, StringComparison.Ordinal) &&
 									      property.ReturnType != null &&
 									      method.GetParameters ().Length == 1 &&
 									      method.ReturnType.BinaryName == "V" &&

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)

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]

tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin/NameShadowing.class

@@ -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) { }
 }