[Xamarin.Android.Tools.Bytecode] Only hide getters/setters for intern…

…al properties.
dotnet · Oct 17, 2023 · 4931251 · 4931251
1 parent 3c83179
commit 4931251
Show file tree

Hide file tree

Showing 6 changed files with 96 additions and 19 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
diff --git 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) { }
 }