[generator] better support for "package-private" classes (#223)

Fixes: #168

Java allows public classes to inherit from "package-private" classes, in
which case the `public` & `protected` members of the "package-private"
class are visible within the public class:

    // Java
    /* package */ abstract class SpannableStringInternal {
      public void getChars(int start, int end, char[] dest, int off);
      // ...
    }
    public class SpannableString extends SpannableStringInternal {
      // ...
    }

The problem is that `generator` didn't properly support this construct,
and *skips* binding of "package-private" types, resulting in generated
C# code such as:

    // C#
    public partial class SpannableString : SpannableStringInternal {
      // CS0246: The type or namespace name `SpannableStringInternal` could not be found
    }

This could be worked around via Metadata by making the "package-private"
class `public`, but this means that if (when?) the "package-private"
class is *removed*, we have an ABI break.

Support this construct by updating `generator` to "copy" the members
from the "package-private" class into the declaring class:

    // C#
    public partial class SpannableString : Java.Lang.Object {
      public void GetChars(int start, int end, char[] dest, int off);
      // ...
    }

This allows the generated code to compile without metadata fixup.

Specifics in implementing this:

  - Add a `GenBase.FixupAccessModifiers()` method, later this may
    need to be further extended for methods
  - Call `FixupAccessModifiers()` in the "validation" step
  - Added a setter for `BaseType` so it can be modified
  - In `FixupAccessModifiers()`, lookup the base class of the current
    type and check if it is non-`public`.
  - Skip the package-private types, and replace them with that
    class's base type
  - Look for each method on the base type, and copy it to the public
    type if it does not exist
  - Added tests for this scenario

tools/generator/ClassGen.cs

@@ -59,6 +59,7 @@ public ManagedClassGen (TypeDefinition t)
 
 		public override string BaseType {
 			get { return nominal_base_type != null ? nominal_base_type.FullNameCorrected () : null; }
+			set { throw new NotSupportedException (); }
 		}
 
 		public override bool IsAbstract {
@@ -121,6 +122,7 @@ public override bool IsFinal {
 
 		public override string BaseType {
 			get { return base_type; }
+			set { base_type = value; }
 		}
 	}
 
@@ -164,7 +166,7 @@ public IList<Ctor> Ctors {
 			get { return ctors; }
 		}
 
-		public abstract string BaseType { get; }
+		public abstract string BaseType { get; set; }
 
 		public bool ContainsCtor (string jni_sig)
 		{
@@ -236,6 +238,26 @@ protected override bool OnValidate (CodeGenerationOptions opt, GenericParameterD
 
 			return true;
 		}
+
+		public override void FixupAccessModifiers ()
+		{
+			while (!IsAnnotation && !string.IsNullOrEmpty (BaseType)) {
+				var baseClass = SymbolTable.Lookup (BaseType) as ClassGen;
+				if (baseClass != null && RawVisibility == "public" && baseClass.RawVisibility != "public") {
+					//Skip the BaseType and copy over any "missing" methods
+					foreach (var baseMethod in baseClass.Methods) {
+						var method = Methods.FirstOrDefault (m => m.Name == baseMethod.Name && m.Parameters.JavaSignature == baseMethod.Parameters.JavaSignature);
+						if (method == null)
+							Methods.Add (baseMethod);
+					}
+					BaseType = baseClass.BaseType;
+				} else {
+					break;
+				}
+			}
+
+			base.FixupAccessModifiers ();
+		}
 
 		public override void FixupExplicitImplementation ()
 		{

tools/generator/CodeGenerator.cs

@@ -400,6 +400,8 @@ static void Validate (List<GenBase> gens, CodeGenerationOptions opt)
 				removed.Clear ();
 				foreach (GenBase gen in gens)
 					gen.ResetValidation ();
+				foreach (GenBase gen in gens)
+					gen.FixupAccessModifiers ();
 				foreach (GenBase gen in gens)
 					if ((opt.IgnoreNonPublicType &&
 					    (gen.RawVisibility != "public" && gen.RawVisibility != "internal"))

tools/generator/GenBase.cs

@@ -727,6 +727,12 @@ public void StripNonBindables ()
 				n.StripNonBindables ();
 		}
 
+		public virtual void FixupAccessModifiers ()
+		{
+			foreach (var nt in NestedTypes)
+				nt.FixupAccessModifiers ();
+		}
+
 		public void FillProperties ()
 		{
 			if (property_filled)

tools/generator/Tests/AccessModifiers.cs

@@ -0,0 +1,20 @@
+using System;
+using NUnit.Framework;
+
+namespace generatortests
+{
+	[TestFixture]
+	public class AccessModifiers : BaseGeneratorTest
+	{
+		[Test]
+		public void GeneratedOK ()
+		{
+			RunAllTargets (
+					outputRelativePath: "AccessModifiers",
+					apiDescriptionFile: "expected/AccessModifiers/AccessModifiers.xml",
+					expectedRelativePath: "AccessModifiers",
+					additionalSupportPaths: null);
+		}
+	}
+}
+

tools/generator/Tests/expected.ji/AccessModifiers/Mono.Android.projitems

@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <PropertyGroup>
+    <DefineConstants>$(DefineConstants);ANDROID_1;ANDROID_2;ANDROID_3;ANDROID_4</DefineConstants>
+  </PropertyGroup>
+  <!-- Classes -->
+  <ItemGroup>
+    <Compile Include="$(MSBuildThisFileDirectory)\Java.Interop.__TypeRegistrations.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)\Java.Lang.Object.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)\Xamarin.Test.BasePublicClass.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)\Xamarin.Test.ExtendPublicClass.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)\Xamarin.Test.PublicClass.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)\Xamarin.Test.PublicFinalClass.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)\__NamespaceMapping__.cs" />
+  </ItemGroup>
+  <!-- Enums -->
+  <ItemGroup />
+</Project>

tools/generator/Tests/expected.ji/AccessModifiers/Xamarin.Test.BasePublicClass.cs

@@ -0,0 +1,61 @@
+using System;
+using System.Collections.Generic;
+using Android.Runtime;
+using Java.Interop;
+
+namespace Xamarin.Test {
+
+	// Metadata.xml XPath class reference: path="/api/package[@name='xamarin.test']/class[@name='BasePublicClass']"
+	[global::Android.Runtime.Register ("xamarin/test/BasePublicClass", DoNotGenerateAcw=true)]
+	public partial class BasePublicClass : global::Java.Lang.Object {
+
+		internal    new     static  readonly    JniPeerMembers  _members    = new JniPeerMembers ("xamarin/test/BasePublicClass", typeof (BasePublicClass));
+		internal static new IntPtr class_ref {
+			get {
+				return _members.JniPeerType.PeerReference.Handle;
+			}
+		}
+
+		public override global::Java.Interop.JniPeerMembers JniPeerMembers {
+			get { return _members; }
+		}
+
+		protected override IntPtr ThresholdClass {
+			get { return _members.JniPeerType.PeerReference.Handle; }
+		}
+
+		protected override global::System.Type ThresholdType {
+			get { return _members.ManagedPeerType; }
+		}
+
+		protected BasePublicClass (IntPtr javaReference, JniHandleOwnership transfer) : base (javaReference, transfer) {}
+
+		static Delegate cb_baseMethod;
+#pragma warning disable 0169
+		static Delegate GetBaseMethodHandler ()
+		{
+			if (cb_baseMethod == null)
+				cb_baseMethod = JNINativeWrapper.CreateDelegate ((Action<IntPtr, IntPtr>) n_BaseMethod);
+			return cb_baseMethod;
+		}
+
+		static void n_BaseMethod (IntPtr jnienv, IntPtr native__this)
+		{
+			global::Xamarin.Test.BasePublicClass __this = global::Java.Lang.Object.GetObject<global::Xamarin.Test.BasePublicClass> (jnienv, native__this, JniHandleOwnership.DoNotTransfer);
+			__this.BaseMethod ();
+		}
+#pragma warning restore 0169
+
+		// Metadata.xml XPath method reference: path="/api/package[@name='xamarin.test']/class[@name='BasePublicClass']/method[@name='baseMethod' and count(parameter)=0]"
+		[Register ("baseMethod", "()V", "GetBaseMethodHandler")]
+		public virtual unsafe void BaseMethod ()
+		{
+			const string __id = "baseMethod.()V";
+			try {
+				_members.InstanceMethods.InvokeVirtualVoidMethod (__id, this, null);
+			} finally {
+			}
+		}
+
+	}
+}

tools/generator/Tests/expected.ji/AccessModifiers/Xamarin.Test.ExtendPublicClass.cs

@@ -0,0 +1,79 @@
+using System;
+using System.Collections.Generic;
+using Android.Runtime;
+using Java.Interop;
+
+namespace Xamarin.Test {
+
+	// Metadata.xml XPath class reference: path="/api/package[@name='xamarin.test']/class[@name='ExtendPublicClass']"
+	[global::Android.Runtime.Register ("xamarin/test/ExtendPublicClass", DoNotGenerateAcw=true)]
+	public partial class ExtendPublicClass : global::Java.Lang.Object {
+
+		internal    new     static  readonly    JniPeerMembers  _members    = new JniPeerMembers ("xamarin/test/ExtendPublicClass", typeof (ExtendPublicClass));
+		internal static new IntPtr class_ref {
+			get {
+				return _members.JniPeerType.PeerReference.Handle;
+			}
+		}
+
+		public override global::Java.Interop.JniPeerMembers JniPeerMembers {
+			get { return _members; }
+		}
+
+		protected override IntPtr ThresholdClass {
+			get { return _members.JniPeerType.PeerReference.Handle; }
+		}
+
+		protected override global::System.Type ThresholdType {
+			get { return _members.ManagedPeerType; }
+		}
+
+		protected ExtendPublicClass (IntPtr javaReference, JniHandleOwnership transfer) : base (javaReference, transfer) {}
+
+		// Metadata.xml XPath constructor reference: path="/api/package[@name='xamarin.test']/class[@name='ExtendPublicClass']/constructor[@name='ExtendPublicClass' and count(parameter)=0]"
+		[Register (".ctor", "()V", "")]
+		public unsafe ExtendPublicClass ()
+			: base (IntPtr.Zero, JniHandleOwnership.DoNotTransfer)
+		{
+			const string __id = "()V";
+
+			if (((global::Java.Lang.Object) this).Handle != IntPtr.Zero)
+				return;
+
+			try {
+				var __r = _members.InstanceMethods.StartCreateInstance (__id, ((object) this).GetType (), null);
+				SetHandle (__r.Handle, JniHandleOwnership.TransferLocalRef);
+				_members.InstanceMethods.FinishCreateInstance (__id, this, null);
+			} finally {
+			}
+		}
+
+		static Delegate cb_foo;
+#pragma warning disable 0169
+		static Delegate GetFooHandler ()
+		{
+			if (cb_foo == null)
+				cb_foo = JNINativeWrapper.CreateDelegate ((Action<IntPtr, IntPtr>) n_Foo);
+			return cb_foo;
+		}
+
+		static void n_Foo (IntPtr jnienv, IntPtr native__this)
+		{
+			global::Xamarin.Test.ExtendPublicClass __this = global::Java.Lang.Object.GetObject<global::Xamarin.Test.ExtendPublicClass> (jnienv, native__this, JniHandleOwnership.DoNotTransfer);
+			__this.Foo ();
+		}
+#pragma warning restore 0169
+
+		// Metadata.xml XPath method reference: path="/api/package[@name='xamarin.test']/class[@name='ExtendPublicClass']/method[@name='foo' and count(parameter)=0]"
+		[Register ("foo", "()V", "GetFooHandler")]
+		public virtual unsafe void Foo ()
+		{
+			const string __id = "foo.()V";
+			try {
+				_members.InstanceMethods.InvokeVirtualVoidMethod (__id, this, null);
+			} finally {
+			}
+		}
+
+	}
+}

tools/generator/Tests/expected.ji/AccessModifiers/Xamarin.Test.PublicClass.cs

@@ -0,0 +1,79 @@
+using System;
+using System.Collections.Generic;
+using Android.Runtime;
+using Java.Interop;
+
+namespace Xamarin.Test {
+
+	// Metadata.xml XPath class reference: path="/api/package[@name='xamarin.test']/class[@name='PublicClass']"
+	[global::Android.Runtime.Register ("xamarin/test/PublicClass", DoNotGenerateAcw=true)]
+	public partial class PublicClass : global::Java.Lang.Object {
+
+		internal    new     static  readonly    JniPeerMembers  _members    = new JniPeerMembers ("xamarin/test/PublicClass", typeof (PublicClass));
+		internal static new IntPtr class_ref {
+			get {
+				return _members.JniPeerType.PeerReference.Handle;
+			}
+		}
+
+		public override global::Java.Interop.JniPeerMembers JniPeerMembers {
+			get { return _members; }
+		}
+
+		protected override IntPtr ThresholdClass {
+			get { return _members.JniPeerType.PeerReference.Handle; }
+		}
+
+		protected override global::System.Type ThresholdType {
+			get { return _members.ManagedPeerType; }
+		}
+
+		protected PublicClass (IntPtr javaReference, JniHandleOwnership transfer) : base (javaReference, transfer) {}
+
+		// Metadata.xml XPath constructor reference: path="/api/package[@name='xamarin.test']/class[@name='PublicClass']/constructor[@name='PublicClass' and count(parameter)=0]"
+		[Register (".ctor", "()V", "")]
+		public unsafe PublicClass ()
+			: base (IntPtr.Zero, JniHandleOwnership.DoNotTransfer)
+		{
+			const string __id = "()V";
+
+			if (((global::Java.Lang.Object) this).Handle != IntPtr.Zero)
+				return;
+
+			try {
+				var __r = _members.InstanceMethods.StartCreateInstance (__id, ((object) this).GetType (), null);
+				SetHandle (__r.Handle, JniHandleOwnership.TransferLocalRef);
+				_members.InstanceMethods.FinishCreateInstance (__id, this, null);
+			} finally {
+			}
+		}
+
+		static Delegate cb_foo;
+#pragma warning disable 0169
+		static Delegate GetFooHandler ()
+		{
+			if (cb_foo == null)
+				cb_foo = JNINativeWrapper.CreateDelegate ((Action<IntPtr, IntPtr>) n_Foo);
+			return cb_foo;
+		}
+
+		static void n_Foo (IntPtr jnienv, IntPtr native__this)
+		{
+			global::Xamarin.Test.PublicClass __this = global::Java.Lang.Object.GetObject<global::Xamarin.Test.PublicClass> (jnienv, native__this, JniHandleOwnership.DoNotTransfer);
+			__this.Foo ();
+		}
+#pragma warning restore 0169
+
+		// Metadata.xml XPath method reference: path="/api/package[@name='xamarin.test']/class[@name='PublicClass']/method[@name='foo' and count(parameter)=0]"
+		[Register ("foo", "()V", "GetFooHandler")]
+		public virtual unsafe void Foo ()
+		{
+			const string __id = "foo.()V";
+			try {
+				_members.InstanceMethods.InvokeVirtualVoidMethod (__id, this, null);
+			} finally {
+			}
+		}
+
+	}
+}