Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[class-parse, generator] Allow users to expose Kotlin internal types/members with metadata. #793

Merged
merged 1 commit into from
Mar 2, 2021
Merged

[class-parse, generator] Allow users to expose Kotlin internal types/members with metadata. #793

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Xamarin.Android.Tools.Bytecode/ClassFile.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections;
using System.Collections.Generic;
using System.Collections.ObjectModel;
Expand Down
48 changes: 34 additions & 14 deletions src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinFixups.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,36 +70,56 @@ static void FixupClassVisibility (ClassFile klass, KotlinClass metadata)
// Interfaces should be set to "package-private"
if (klass.AccessFlags.HasFlag (ClassAccessFlags.Interface)) {
Log.Debug ($"Kotlin: Setting internal interface {klass.ThisClass.Name.Value} to package-private");
klass.AccessFlags = SetPackagePrivate (klass.AccessFlags);
klass.AccessFlags = SetVisibility (klass.AccessFlags, null);

foreach (var ic in klass.InnerClasses) {
Log.Debug ($"Kotlin: Setting nested type {ic.InnerClass.Name.Value} in an internal interface to package-private");
ic.InnerClassAccessFlags = SetPackagePrivate (ic.InnerClassAccessFlags);
ic.InnerClassAccessFlags = SetVisibility (ic.InnerClassAccessFlags, null);
}

return;
}

Log.Debug ($"Kotlin: Hiding internal class {klass.ThisClass.Name.Value}");
klass.AccessFlags = ClassAccessFlags.Private;
klass.AccessFlags = SetVisibility (klass.AccessFlags, ClassAccessFlags.Private);

foreach (var ic in klass.InnerClasses) {
Log.Debug ($"Kotlin: Hiding nested internal type {ic.InnerClass.Name.Value}");
ic.InnerClassAccessFlags = ClassAccessFlags.Private;
ic.InnerClassAccessFlags = SetVisibility (ic.InnerClassAccessFlags, ClassAccessFlags.Private);
}

return;
}
}

static ClassAccessFlags SetPackagePrivate (ClassAccessFlags flags)
// Passing null for 'newVisibility' parameter means 'package-private'
static ClassAccessFlags SetVisibility (ClassAccessFlags existing, ClassAccessFlags? newVisibility)
{
// Package-private is stored as "no visibility flags"
flags = (flags ^ ClassAccessFlags.Public) & flags;
flags = (flags ^ ClassAccessFlags.Protected) & flags;
flags = (flags ^ ClassAccessFlags.Private) & flags;
// First we need to remove any existing visibility flags,
// without modifying other flags like Abstract
existing = (existing ^ ClassAccessFlags.Public) & existing;
existing = (existing ^ ClassAccessFlags.Protected) & existing;
existing = (existing ^ ClassAccessFlags.Private) & existing;

return flags;
// Package-private is stored as "no visibility flags", so only add flag if specified
if (newVisibility.HasValue)
existing |= newVisibility.Value;

return existing;
}

static MethodAccessFlags SetVisibility (MethodAccessFlags existing, MethodAccessFlags newVisibility)
{
// First we need to remove any existing visibility flags,
// without modifying other flags like Abstract
existing = (existing ^ MethodAccessFlags.Public) & existing;
existing = (existing ^ MethodAccessFlags.Protected) & existing;
existing = (existing ^ MethodAccessFlags.Private) & existing;
existing = (existing ^ MethodAccessFlags.Internal) & existing;

existing |= newVisibility;

return existing;
}

static void FixupJavaMethods (Methods methods)
Expand Down Expand Up @@ -132,7 +152,7 @@ static void FixupConstructor (MethodInfo method, KotlinConstructor metadata)
// Hide constructor if it isn't Public/Protected
if (method.IsPubliclyVisible && !metadata.Flags.IsPubliclyVisible ()) {
Log.Debug ($"Kotlin: Hiding internal constructor {method.DeclaringType?.ThisClass.Name.Value} - {metadata.GetSignature ()}");
method.AccessFlags = MethodAccessFlags.Private;
method.AccessFlags = SetVisibility (method.AccessFlags, MethodAccessFlags.Internal);
}
}

Expand All @@ -144,7 +164,7 @@ static void FixupFunction (MethodInfo method, KotlinFunction metadata, KotlinCla
// Hide function if it isn't Public/Protected
if (!metadata.Flags.IsPubliclyVisible ()) {
Log.Debug ($"Kotlin: Hiding internal method {method.DeclaringType?.ThisClass.Name.Value} - {metadata.GetSignature ()}");
method.AccessFlags = MethodAccessFlags.Private;
method.AccessFlags = SetVisibility (method.AccessFlags, MethodAccessFlags.Internal);
return;
}

Expand Down Expand Up @@ -190,12 +210,12 @@ static void FixupProperty (MethodInfo getter, MethodInfo setter, KotlinProperty

if (getter?.IsPubliclyVisible == true) {
Log.Debug ($"Kotlin: Hiding internal getter method {getter.DeclaringType?.ThisClass.Name.Value} - {getter.Name}");
getter.AccessFlags = MethodAccessFlags.Private;
getter.AccessFlags = SetVisibility (getter.AccessFlags, MethodAccessFlags.Internal);
}

if (setter?.IsPubliclyVisible == true) {
Log.Debug ($"Kotlin: Hiding internal setter method {setter.DeclaringType?.ThisClass.Name.Value} - {setter.Name}");
setter.AccessFlags = MethodAccessFlags.Private;
setter.AccessFlags = SetVisibility (setter.AccessFlags, MethodAccessFlags.Internal);
}

return;
Expand Down
3 changes: 3 additions & 0 deletions src/Xamarin.Android.Tools.Bytecode/Methods.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,9 @@ public enum MethodAccessFlags {
Abstract = 0x0400,
Strict = 0x0800,
Synthetic = 0x1000,

// This is not a real Java MethodAccessFlags, it is used to denote Kotlin "internal" access.
Internal = 0x10000000,
}

[Flags]
Expand Down
13 changes: 7 additions & 6 deletions src/Xamarin.Android.Tools.Bytecode/XmlClassDeclarationBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Globalization;
Expand Down Expand Up @@ -28,9 +28,6 @@ public XmlClassDeclarationBuilder (ClassFile classFile)

public XElement ToXElement ()
{
var visibility = GetClassVisibility (this.classFile.AccessFlags);
if (visibility == "private")
return null;
return new XElement (GetElementName (),
new XAttribute ("abstract", (classFile.AccessFlags & ClassAccessFlags.Abstract) != 0),
new XAttribute ("deprecated", GetDeprecatedValue (classFile.Attributes)),
Expand Down Expand Up @@ -319,7 +316,7 @@ IEnumerable<XElement> GetImplementedInterfaces ()
IEnumerable<XElement> GetConstructors ()
{
return classFile.Methods.Where (m => m.Name == "<init>"
&& (GetMethodVisibility(m.AccessFlags) == "public" || GetMethodVisibility(m.AccessFlags) == "protected"))
&& (GetMethodVisibility(m.AccessFlags) == "public" || GetMethodVisibility(m.AccessFlags) == "protected" || GetMethodVisibility (m.AccessFlags) == "kotlin-internal"))
.OrderBy (m => m.Name + m.Descriptor, StringComparer.OrdinalIgnoreCase)
.Select (c => GetMethod ("constructor", GetThisClassName (), c, null));
}
Expand Down Expand Up @@ -374,6 +371,8 @@ static XAttribute GetSynchronized (MethodInfo method)

static string GetVisibility (MethodAccessFlags accessFlags)
{
if (accessFlags.HasFlag (MethodAccessFlags.Internal))
return "kotlin-internal";
if ((accessFlags & MethodAccessFlags.Public) != 0)
return "public";
if ((accessFlags & MethodAccessFlags.Protected) != 0)
Expand Down Expand Up @@ -605,6 +604,8 @@ static string GetVisibility (FieldAccessFlags accessFlags)

static string GetMethodVisibility (MethodAccessFlags accessFlags)
{
if (accessFlags.HasFlag (MethodAccessFlags.Internal))
return "kotlin-internal";
if ((accessFlags & MethodAccessFlags.Public) != 0)
return "public";
if ((accessFlags & MethodAccessFlags.Protected) != 0)
Expand All @@ -628,7 +629,7 @@ static string GetClassVisibility (ClassAccessFlags accessFlags)
IEnumerable<XElement> GetMethods ()
{
return classFile.Methods.Where (m => !m.Name.StartsWith ("<", StringComparison.OrdinalIgnoreCase)
&& (GetMethodVisibility(m.AccessFlags) == "public" || GetMethodVisibility(m.AccessFlags) == "protected"))
&& (GetMethodVisibility(m.AccessFlags) == "public" || GetMethodVisibility(m.AccessFlags) == "protected" || GetMethodVisibility (m.AccessFlags) == "kotlin-internal"))
.OrderBy (m => m.Name + m.Descriptor, StringComparer.OrdinalIgnoreCase)
.Select (m => GetMethod ("method", m.Name, m,
returns: m.ReturnType.TypeSignature));
Expand Down
12 changes: 12 additions & 0 deletions tests/Xamarin.Android.Tools.Bytecode-Tests/KotlinFixupsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ public void HideInternalClass ()

Assert.False (klass.AccessFlags.HasFlag (ClassAccessFlags.Public));
Assert.False (inner_class.InnerClassAccessFlags.HasFlag (ClassAccessFlags.Public));

var output = new XmlClassDeclarationBuilder (klass).ToXElement ().ToString ();
Assert.True (output.Contains ("visibility=\"private\""));
}

[Test]
Expand Down Expand Up @@ -58,6 +61,9 @@ public void HideInternalConstructor ()
KotlinFixups.Fixup (new [] { klass });

Assert.False (ctor.AccessFlags.HasFlag (MethodAccessFlags.Public));

var output = new XmlClassDeclarationBuilder (klass).ToXElement ().ToString ();
Assert.True (output.Contains ("visibility=\"kotlin-internal\""));
}

[Test]
Expand Down Expand Up @@ -124,6 +130,9 @@ public void HideInternalMethod ()
KotlinFixups.Fixup (new [] { klass });

Assert.False (method.AccessFlags.HasFlag (MethodAccessFlags.Public));

var output = new XmlClassDeclarationBuilder (klass).ToXElement ().ToString ();
Assert.True (output.Contains ("visibility=\"kotlin-internal\""));
}

[Test]
Expand Down Expand Up @@ -154,6 +163,9 @@ public void HideInternalProperty ()

Assert.False (getter.AccessFlags.HasFlag (MethodAccessFlags.Public));
Assert.False (setter.AccessFlags.HasFlag (MethodAccessFlags.Public));

var output = new XmlClassDeclarationBuilder (klass).ToXElement ().ToString ();
Assert.True (output.Contains ("visibility=\"kotlin-internal\""));
}

[Test]
Expand Down
22 changes: 22 additions & 0 deletions tests/generator-Tests/Unit-Tests/XmlApiImporterTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Xml.Linq;
using MonoDroid.Generation;
Expand Down Expand Up @@ -244,5 +245,26 @@ public void PreserveSourceLineInfo ()
Assert.AreEqual (2, method.LinePosition);
Assert.AreEqual ("obj/Debug/api.xml", method.SourceFile);
}

[Test]
public void IgnoreKotlinInternalMembers ()
{
var xml = XDocument.Parse (@"
<api>
<package name='com.example.test' jni-name='com/example/test'>
<class name='Test' visibility='public'>
<constructor name='Test' visibility='kotlin-internal' />
<method name='DoStuff' visibility='kotlin-internal' />
<field name='MyField' visibility='kotlin-internal' />
</class>
</package>
</api>");

var gens = new Parser (opt).Parse (xml, new List<string> (), "0", 0);
var klass = gens.Single ();

Assert.AreEqual (0, klass.Fields.Count);
Assert.AreEqual (0, klass.Methods.Count);
}
}
}
15 changes: 10 additions & 5 deletions tools/generator/Java.Interop.Tools.Generator.Importers/XmlApiImporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,16 @@ public static ClassGen CreateClass (XElement pkg, XElement elem, CodeGenerationO
klass.AddImplementedInterface (iname);
break;
case "method":
klass.AddMethod (CreateMethod (klass, child, options));
if (child.XGetAttribute ("visibility") != "kotlin-internal")
klass.AddMethod (CreateMethod (klass, child, options));
break;
case "constructor":
klass.Ctors.Add (CreateCtor (klass, child, options));
if (child.XGetAttribute ("visibility") != "kotlin-internal")
klass.Ctors.Add (CreateCtor (klass, child, options));
break;
case "field":
klass.AddField (CreateField (klass, child, options));
if (child.XGetAttribute ("visibility") != "kotlin-internal")
klass.AddField (CreateField (klass, child, options));
break;
case "typeParameters":
break; // handled at GenBaseSupport
Expand Down Expand Up @@ -230,10 +233,12 @@ public static InterfaceGen CreateInterface (XElement pkg, XElement elem, CodeGen
iface.AddImplementedInterface (iname);
break;
case "method":
iface.AddMethod (CreateMethod (iface, child, options));
if (child.XGetAttribute ("visibility") != "kotlin-internal")
iface.AddMethod (CreateMethod (iface, child, options));
break;
case "field":
iface.AddField (CreateField (iface, child, options));
if (child.XGetAttribute ("visibility") != "kotlin-internal")
iface.AddField (CreateField (iface, child, options));
break;
case "typeParameters":
break; // handled at GenBaseSupport
Expand Down