From 3f31610a9ef205e107cbfa401bf01cbfbadb23a2 Mon Sep 17 00:00:00 2001 From: Jonathan Pobst Date: Tue, 5 Jul 2022 11:43:33 -0500 Subject: [PATCH 1/3] [Byecode] Hide `internal` Kotlin fields marked with `@JvmField`. --- .../ClassFile.cs | 2 +- src/Xamarin.Android.Tools.Bytecode/Fields.cs | 15 +++++++++--- .../Kotlin/KotlinFixups.cs | 23 ++++++++++++++++++ .../XmlClassDeclarationBuilder.cs | 2 ++ .../KotlinFixupsTests.cs | 16 ++++++++++++ .../kotlin/InternalField.class | Bin 0 -> 690 bytes .../kotlin/InternalField.kt | 4 +++ 7 files changed, 57 insertions(+), 5 deletions(-) create mode 100644 tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin/InternalField.class create mode 100644 tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin/InternalField.kt diff --git a/src/Xamarin.Android.Tools.Bytecode/ClassFile.cs b/src/Xamarin.Android.Tools.Bytecode/ClassFile.cs index d49431f90..830581baf 100644 --- a/src/Xamarin.Android.Tools.Bytecode/ClassFile.cs +++ b/src/Xamarin.Android.Tools.Bytecode/ClassFile.cs @@ -43,7 +43,7 @@ public ClassFile (Stream stream) thisClass = stream.ReadNetworkUInt16 (); superClass = stream.ReadNetworkUInt16 (); Interfaces = new Interfaces (ConstantPool, stream); - Fields = new Fields (ConstantPool, stream); + Fields = new Fields (ConstantPool, this, stream); Methods = new Methods (ConstantPool, this, stream); Attributes = new AttributeCollection (ConstantPool, stream); diff --git a/src/Xamarin.Android.Tools.Bytecode/Fields.cs b/src/Xamarin.Android.Tools.Bytecode/Fields.cs index ac13c5b00..c3cd1ba9d 100644 --- a/src/Xamarin.Android.Tools.Bytecode/Fields.cs +++ b/src/Xamarin.Android.Tools.Bytecode/Fields.cs @@ -10,7 +10,7 @@ public sealed class Fields : Collection { public ConstantPool ConstantPool {get; private set;} - public Fields (ConstantPool constantPool, Stream stream) + public Fields (ConstantPool constantPool, ClassFile declaringClass, Stream stream) { if (constantPool == null) throw new ArgumentNullException ("constantPool"); @@ -20,7 +20,7 @@ public Fields (ConstantPool constantPool, Stream stream) ConstantPool = constantPool; var count = stream.ReadNetworkUInt16 (); for (int i = 0; i < count; ++i) { - Add (new FieldInfo (constantPool, stream)); + Add (new FieldInfo (constantPool, declaringClass, stream)); } } } @@ -31,13 +31,15 @@ public sealed class FieldInfo { ushort descriptorIndex; public ConstantPool ConstantPool {get; private set;} - public FieldAccessFlags AccessFlags {get; private set;} + public ClassFile DeclaringType {get; private set;} + public FieldAccessFlags AccessFlags {get; set;} public AttributeCollection Attributes {get; private set;} public string? KotlinType {get; set;} - public FieldInfo (ConstantPool constantPool, Stream stream) + public FieldInfo (ConstantPool constantPool, ClassFile declaringType, Stream stream) { ConstantPool = constantPool; + DeclaringType = declaringType; AccessFlags = (FieldAccessFlags) stream.ReadNetworkUInt16 (); nameIndex = stream.ReadNetworkUInt16 (); descriptorIndex = stream.ReadNetworkUInt16 (); @@ -61,6 +63,8 @@ public string Descriptor { var signature = Attributes.Get (); return signature != null ? signature.Value : null; } + + public bool IsPubliclyVisible => AccessFlags.HasFlag (FieldAccessFlags.Public) || AccessFlags.HasFlag (FieldAccessFlags.Protected); } [Flags] @@ -74,5 +78,8 @@ public enum FieldAccessFlags { Transient = 0x0080, Synthetic = 0x1000, Enum = 0x4000, + + // This is not a real Java FieldAccessFlags, it is used to denote Kotlin "internal" access. + Internal = 0x10000000, } } diff --git a/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinFixups.cs b/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinFixups.cs index b4b8940de..e6d603da8 100644 --- a/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinFixups.cs +++ b/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinFixups.cs @@ -129,6 +129,20 @@ static MethodAccessFlags SetVisibility (MethodAccessFlags existing, MethodAccess return existing; } + static FieldAccessFlags SetVisibility (FieldAccessFlags existing, FieldAccessFlags newVisibility) + { + // First we need to remove any existing visibility flags, + // without modifying other flags like Abstract + existing = (existing ^ FieldAccessFlags.Public) & existing; + existing = (existing ^ FieldAccessFlags.Protected) & existing; + existing = (existing ^ FieldAccessFlags.Private) & existing; + existing = (existing ^ FieldAccessFlags.Internal) & existing; + + existing |= newVisibility; + + return existing; + } + static void FixupJavaMethods (Methods methods) { // We do the following method level fixups here because we can operate on all methods, @@ -294,6 +308,15 @@ static void FixupField (FieldInfo? field, KotlinProperty metadata) if (field is null) return; + // Hide field if it isn't Public/Protected + if (!metadata.Flags.IsPubliclyVisible ()) { + + if (field.IsPubliclyVisible) { + Log.Debug ($"Kotlin: Hiding internal field {field.DeclaringType?.ThisClass.Name.Value} - {field.Name}"); + field.AccessFlags = SetVisibility (field.AccessFlags, FieldAccessFlags.Internal); + } + } + // Handle erasure of Kotlin unsigned types field.KotlinType = GetKotlinType (field.Descriptor, metadata.ReturnType?.ClassName); } diff --git a/src/Xamarin.Android.Tools.Bytecode/XmlClassDeclarationBuilder.cs b/src/Xamarin.Android.Tools.Bytecode/XmlClassDeclarationBuilder.cs index 008137bae..6fa05991b 100644 --- a/src/Xamarin.Android.Tools.Bytecode/XmlClassDeclarationBuilder.cs +++ b/src/Xamarin.Android.Tools.Bytecode/XmlClassDeclarationBuilder.cs @@ -593,6 +593,8 @@ static string EscapeLiteral (string value) static string GetVisibility (FieldAccessFlags accessFlags) { + if (accessFlags.HasFlag (FieldAccessFlags.Internal)) + return "kotlin-internal"; if ((accessFlags & FieldAccessFlags.Public) != 0) return "public"; if ((accessFlags & FieldAccessFlags.Protected) != 0) diff --git a/tests/Xamarin.Android.Tools.Bytecode-Tests/KotlinFixupsTests.cs b/tests/Xamarin.Android.Tools.Bytecode-Tests/KotlinFixupsTests.cs index a4a272220..4263e342d 100644 --- a/tests/Xamarin.Android.Tools.Bytecode-Tests/KotlinFixupsTests.cs +++ b/tests/Xamarin.Android.Tools.Bytecode-Tests/KotlinFixupsTests.cs @@ -135,6 +135,22 @@ public void HideInternalMethod () Assert.True (output.Contains ("visibility=\"kotlin-internal\"")); } + [Test] + public void HideInternalField () + { + var klass = LoadClassFile ("InternalField.class"); + var field = klass.Fields.First (m => m.Name == "city"); + + Assert.True (field.AccessFlags.HasFlag (FieldAccessFlags.Public)); + + KotlinFixups.Fixup (new [] { klass }); + + Assert.False (field.AccessFlags.HasFlag (FieldAccessFlags.Public)); + + var output = new XmlClassDeclarationBuilder (klass).ToXElement ().ToString (); + Assert.True (output.Contains ("visibility=\"kotlin-internal\"")); + } + [Test] public void ParameterName () { diff --git a/tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin/InternalField.class b/tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin/InternalField.class new file mode 100644 index 0000000000000000000000000000000000000000..718a175cf85df4ff737057e00620f71c81fe26f9 GIT binary patch literal 690 zcmZWlTWb?R6#iy%?KW+;(Q4Xiz0_+T+%``ZMbJngvPNhn2zi=hhd9Y*26kr`e2ag@ zf8c`%K1k@JKT15aw3H$Y=bZ1%cW&RGzrTJ1=-@HKT2EV~9*{Z=S5%Q?1d(o z1XF28iBLKXgx1Cis|>|`WBXYgyG}2aR`wM`>B-YU1FL9wC}XXG3aTsMXwo9~vq~A7 z;g!v)cH?65duc@^te_m9=OsFoIYYI4R6saLH?bL|qCq`;nu&#-S)C`rD{cJvx$wxsPr@0x6cpwtx@OjcZHCZy2$I3Cb z&KnQ@Yl}ltt?VSgh7*y{T|yjn+#~N%v{HD-U`}wd_bGEkKXeyX_kUu2zg7PM|2sjB zy@Jlk(#oXd1BzS`QE5brJjcVr-oYdCUHZF5qpuH9>S42ZH*oU;wR_mYHf6Ch1WR!X Rw}+^tXr Date: Wed, 6 Jul 2022 21:04:22 -0400 Subject: [PATCH 2/3] Formatting --- src/Xamarin.Android.Tools.Bytecode/Fields.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Xamarin.Android.Tools.Bytecode/Fields.cs b/src/Xamarin.Android.Tools.Bytecode/Fields.cs index c3cd1ba9d..7b68b520e 100644 --- a/src/Xamarin.Android.Tools.Bytecode/Fields.cs +++ b/src/Xamarin.Android.Tools.Bytecode/Fields.cs @@ -80,6 +80,6 @@ public enum FieldAccessFlags { Enum = 0x4000, // This is not a real Java FieldAccessFlags, it is used to denote Kotlin "internal" access. - Internal = 0x10000000, + Internal. = 0x10000000, } } From 15b5029c9e6a53a3e826509f7cc7217776a8deff Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Wed, 6 Jul 2022 21:11:05 -0400 Subject: [PATCH 3/3] Argh --- src/Xamarin.Android.Tools.Bytecode/Fields.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Xamarin.Android.Tools.Bytecode/Fields.cs b/src/Xamarin.Android.Tools.Bytecode/Fields.cs index 7b68b520e..0a0ddfd27 100644 --- a/src/Xamarin.Android.Tools.Bytecode/Fields.cs +++ b/src/Xamarin.Android.Tools.Bytecode/Fields.cs @@ -80,6 +80,6 @@ public enum FieldAccessFlags { Enum = 0x4000, // This is not a real Java FieldAccessFlags, it is used to denote Kotlin "internal" access. - Internal. = 0x10000000, + Internal = 0x10000000, } }