Skip to content

Commit

Permalink
[Xamarin.Android.Tools.Bytecode] Kotlin unsigned internal props (#1156)
Browse files Browse the repository at this point in the history
Context: 6bd7ae4

In commit 6bd7ae4, we updated the logic that matches a Kotlin public
property getter/setter to a generated Java getter/setter that follows
this pattern:

	// Kotlin
	public var type = 0

	// Java
	private int type = 0;

	public int getType () { ... }
	public void setType (int p0) { ... }

However, this caused unit tests in xamarin/xamarin-android to fail
that when using Kotlin unsigned types:

	KotlinUnsignedTypesTests.cs: 
	error CS0200: Property or indexer 'UnsignedInstanceMethods.UnsignedInstanceProperty' cannot be assigned to -- it is read only

This is because properties that use Kotlin unsigned types append a
`-<type-hash>` suffix to their getter/setter names:

	// Kotlin
	public var type: UInt = 0u

	// Java
	private int type = 0;

	public int getType-pVg5ArA () { ... }
	public void setType-WZ4Q5Ns (int p0) { ... }

Update our Kotlin logic to handle this case.
  • Loading branch information
jpobst authored Nov 3, 2023
1 parent 1adb796 commit 38c8a82
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 10 deletions.
12 changes: 8 additions & 4 deletions src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinFixups.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand All @@ -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 &&
Expand Down
19 changes: 13 additions & 6 deletions src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinUtilities.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
21 changes: 21 additions & 0 deletions tests/Xamarin.Android.Tools.Bytecode-Tests/KotlinFixupsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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"));
}
}
}
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,8 @@ public class NameShadowing {
internal val itype2 = 0
fun getItype2(): Int = itype2
fun setItype2(type: Int) { }

// Unsigned types properties
internal var unsignedInternalProperty: UInt = 3u
var unsignedPublicProperty: UInt = 3u
}

0 comments on commit 38c8a82

Please sign in to comment.