[Xamain.Android.Tools.Bytecode] Hide private Kotlin default ctors (#1206

)

Context: f6c12ba

`Kotlin.Stdlib` contains [`kotlin.io.encoding.Base64`][0], a public
class which is not intended to be instantiated by external users.
Thus it only contains a `private` default constructor:

	// Kotlin
	public open /* partial */ class Base64 private constructor(
	    internal val isUrlSafe: Boolean,
	    internal val isMimeScheme: Boolean
	) {
	    // …
	    public companion object Default : Base64(isUrlSafe = false, isMimeScheme = false) {
	        // …
	    }
	}

which compiles into the equivalent Java code:

	// Java
	public /* partial */ class Base64 {
	    private Base64 (boolean isUrlSafe, boolean isMimeScheme) {…}
	    public Base64(boolean isUrlSafe, boolean isMimeScheme, kotlin.jvm.internal.DefaultConstructorMarker $constructor_marker) {…}
	    // ^ synthetic constructor
	}

Previously (f6c12ba) we believed that a synthetic default constructor
would always end in an `int, DefaultConstructorMarker)` parameter pair,
however this one does not.

This synthetic constructor causes us to generate some bizarre
"usable but shouldn't be used" constructors:

	// C#
	partial class Base64 {
	    [Register (".ctor", "(ZZLkotlin/jvm/internal/DefaultConstructorMarker;)V", "")]
	    public unsafe Base64 (bool isUrlSafe, bool isMimeScheme, global::Kotlin.Jvm.Internal.DefaultConstructorMarker? _constructor_marker)
	        : base (IntPtr.Zero, JniHandleOwnership.DoNotTransfer)
	    {
	        // …
	    }
	}

Fix this by extending our Kotlin default constructor marker detection
logic to look for any synthetic constructor whose final parameter is
`DefaultConstructorMarker` to handle this additional case.

With the change, `Kotlin.IO.Encoding.Base64` no longer generates a
`public` constructor.

[0]: https://github.com/JetBrains/kotlin/blob/3fbb7bc92086bdf3bde123a8f774bce25b19ef37/libraries/stdlib/src/kotlin/io/encoding/Base64.kt

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

@@ -97,8 +97,7 @@ public static string GetMethodNameWithoutUnsignedSuffix (string name)
 
 		public static bool IsDefaultConstructorMarker (this MethodInfo method)
 		{
-			// A default constructor is synthetic and always has an int and a
-			// DefaultConstructorMarker as its final 2 parameters.
+			// A default constructor is synthetic and has a DefaultConstructorMarker as its final parameter.
 			if (method.Name != "<init>")
 				return false;
 
@@ -107,12 +106,11 @@ public static bool IsDefaultConstructorMarker (this MethodInfo method)
 
 			var parameters = method.GetParameters ();
 
-			if (parameters.Length < 2)
+			if (parameters.Length < 1)
 				return false;
 
-			// Parameter list ends with `int, DefaultConstructorMarker`.
-			return parameters [parameters.Length - 2].Type.TypeSignature == "I" &&
-				parameters [parameters.Length - 1].Type.TypeSignature == "Lkotlin/jvm/internal/DefaultConstructorMarker;";
+			// Parameter list ends with `DefaultConstructorMarker`.
+			return parameters [parameters.Length - 1].Type.TypeSignature == "Lkotlin/jvm/internal/DefaultConstructorMarker;";
 		}
 
 		// Sometimes the Kotlin provided JvmSignature is null (or unhelpful), so we need to construct one ourselves

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

@@ -92,6 +92,28 @@ public void HideDefaultConstructorMarker ()
 			Assert.False (ctor_3p.AccessFlags.HasFlag (MethodAccessFlags.Public));
 		}
 
+		[Test]
+		public void HidePrivateDefaultConstructorMarker ()
+		{
+			var klass = LoadClassFile ("PrivateDefaultConstructor.class");
+
+			// init (int isFoo)
+			var ctor_1p = klass.Methods.Single (m => m.Name == "<init>" && m.GetParameters ().Length == 1);
+
+			// init (int isFoo, DefaultConstructorMarker p1)
+			var ctor_2p = klass.Methods.Single (m => m.Name == "<init>" && m.GetParameters ().Length == 2);
+
+			Assert.True (ctor_2p.AccessFlags.HasFlag (MethodAccessFlags.Public));
+
+			KotlinFixups.Fixup ([klass]);
+
+			// Assert that the normal constructor is still public
+			Assert.False (ctor_1p.AccessFlags.HasFlag (MethodAccessFlags.Public));
+
+			// Assert that the synthetic "DefaultConstructorMarker" constructor has been marked private
+			Assert.False (ctor_2p.AccessFlags.HasFlag (MethodAccessFlags.Public));
+		}
+
 		[Test]
 		public void HideImplementationMethod ()
 		{

tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin/PrivateDefaultConstructor.kt

@@ -0,0 +1,5 @@
+public open class PrivateDefaultConstructor private constructor (internal val isFoo: Boolean) {
+	init { }
+
+	public companion object Default : PrivateDefaultConstructor (isFoo = false) { }
+}