[Byecode] Hide internal Kotlin fields marked with @JvmField (#1004)

Fixes: #954
Context: https://github.com/Kotlin/kotlinx.coroutines/blob/f0874d1c5ce1e5c8f091be2981bea3b80168c14e/kotlinx-coroutines-core/jvm/src/scheduling/Tasks.kt#L71-L75
When binding `org.jetbrains.kotlinx.kotlinx-coroutines-core-jvm`, the
following Kotlin-internal declarations:
@JvmField
internal val NonBlockingContext: TaskContext = TaskContextImpl(TASK_NON_BLOCKING)
@JvmField
internal val BlockingContext: TaskContext = TaskContextImpl(TASK_PROBABLY_BLOCKING)
are not being marked as private in the Java API. This causes binding
errors because their type (`TaskContext`) is a Kotlin-internal
interface which is removed from the Java API.
Internal Kotlin fields that are marked with [`@JvmField`][0] are
exposed as `public` in the compiled Bytecode so they can be consumed
by Java.
// Kotlin
@JvmField
internal val city: String = "London"
// Java
@JvmField
@NotNull
public final String city = "London";
This is a bit of a conundrum because Kotlin `internal` means "can't
be consumed by other libraries" while `@JvmField` means "make this
consumable by other Java libraries". 🤷‍♂️ Let's honor the original Kotlin `internal` visibility to produce
consistent bindings. (The `kotlin-internal` visibility can be
overridden with `generator` metadata if desired.)
We need to examine the encoded Kotlin metadata to determine if the
field should be marked as `kotlin-internal` the same way we do with
methods and properties.
[0]: https://kotlinlang.org/docs/java-to-kotlin-interop.html#instance-fields

Author: jpobst

src/Xamarin.Android.Tools.Bytecode/ClassFile.cs

@@ -43,7 +43,7 @@ public ClassFile (Stream stream)
 thisClass=stream.ReadNetworkUInt16();
 superClass=stream.ReadNetworkUInt16();
 Interfaces=newInterfaces(ConstantPool,stream);
-Fields=newFields(ConstantPool,stream);
+Fields=newFields(ConstantPool,this,stream);
 Methods=newMethods(ConstantPool,this,stream);
 Attributes=newAttributeCollection(ConstantPool,stream);
 

src/Xamarin.Android.Tools.Bytecode/Fields.cs

@@ -10,7 +10,7 @@ public sealed class Fields : Collection<FieldInfo> {
 
 publicConstantPoolConstantPool{get;privateset;}
 
-publicFields(ConstantPoolconstantPool,Streamstream)
+publicFields(ConstantPoolconstantPool,ClassFiledeclaringClass,Streamstream)
 {
 if(constantPool==null)
 thrownewArgumentNullException("constantPool");
@@ -20,7 +20,7 @@ public Fields (ConstantPool constantPool, Stream stream)
 ConstantPool=constantPool;
 varcount=stream.ReadNetworkUInt16();
 for(inti=0;i<count;++i){
-Add(newFieldInfo(constantPool,stream));
+Add(newFieldInfo(constantPool,declaringClass,stream));
 }
 }
 }
@@ -31,13 +31,15 @@ public sealed class FieldInfo {
 ushortdescriptorIndex;
 
 publicConstantPoolConstantPool{get;privateset;}
-publicFieldAccessFlagsAccessFlags{get;privateset;}
+publicClassFileDeclaringType{get;privateset;}
+publicFieldAccessFlagsAccessFlags{get;set;}
 publicAttributeCollectionAttributes{get;privateset;}
 publicstring?KotlinType{get;set;}
 
-publicFieldInfo(ConstantPoolconstantPool,Streamstream)
+publicFieldInfo(ConstantPoolconstantPool,ClassFiledeclaringType,Streamstream)
 {
 ConstantPool=constantPool;
+DeclaringType=declaringType;
 AccessFlags=(FieldAccessFlags)stream.ReadNetworkUInt16();
 nameIndex=stream.ReadNetworkUInt16();
 descriptorIndex=stream.ReadNetworkUInt16();
@@ -61,6 +63,8 @@ public string Descriptor {
 varsignature=Attributes.Get<SignatureAttribute>();
 returnsignature!=null?signature.Value:null;
 }
+
+publicboolIsPubliclyVisible=>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,
 }
 }

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

@@ -129,6 +129,20 @@ static MethodAccessFlags SetVisibility (MethodAccessFlags existing, MethodAccess
 returnexisting;
 }
 
+staticFieldAccessFlagsSetVisibility(FieldAccessFlagsexisting,FieldAccessFlagsnewVisibility)
+{
+// 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;
+
+returnexisting;
+}
+
 staticvoidFixupJavaMethods(Methodsmethods)
 {
 // 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(fieldisnull)
 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);
 }

src/Xamarin.Android.Tools.Bytecode/XmlClassDeclarationBuilder.cs

@@ -593,6 +593,8 @@ static string EscapeLiteral (string value)
 
 staticstringGetVisibility(FieldAccessFlagsaccessFlags)
 {
+if(accessFlags.HasFlag(FieldAccessFlags.Internal))
+return"kotlin-internal";
 if((accessFlags&FieldAccessFlags.Public)!=0)
 return"public";
 if((accessFlags&FieldAccessFlags.Protected)!=0)

tests/Xamarin.Android.Tools.Bytecode-Tests/KotlinFixupsTests.cs

@@ -135,6 +135,22 @@ public void HideInternalMethod ()
 Assert.True(output.Contains("visibility=\"kotlin-internal\""));
 }
 
+[Test]
+publicvoidHideInternalField()
+{
+varklass=LoadClassFile("InternalField.class");
+varfield=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));
+
+varoutput=newXmlClassDeclarationBuilder(klass).ToXElement().ToString();
+Assert.True(output.Contains("visibility=\"kotlin-internal\""));
+}
+
 [Test]
 publicvoidParameterName()
 {

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

@@ -0,0 +1,4 @@
+classInternalField {
+	@JvmField
+internalval city:String="London"
+}