[generator] better support for package-private interfaces

tools/generator/Java.Interop.Tools.Generator.CodeGeneration/CodeGenerator.cs

@@ -54,7 +54,7 @@ public void WriteClass (ClassGen @class, string indent, GenerationInfo gen_info)
 			foreach (ISymbol isym in @class.Interfaces) {
 				GenericSymbol gs = isym as GenericSymbol;
 				InterfaceGen gen = (gs == null ? isym : gs.Gen) as InterfaceGen;
-				if (gen != null && gen.IsConstSugar)
+				if (gen != null && (gen.IsConstSugar || gen.RawVisibility != "public"))
 					continue;
 				if (sb.Length > 0)
 					sb.Append (", ");
@@ -506,7 +506,7 @@ public void WriteInterfaceDeclaration (InterfaceGen @interface, string indent)
 			StringBuilder sb = new StringBuilder ();
 			foreach (ISymbol isym in @interface.Interfaces) {
 				InterfaceGen igen = (isym is GenericSymbol ? (isym as GenericSymbol).Gen : isym) as InterfaceGen;
-				if (igen.IsConstSugar)
+				if (igen.IsConstSugar || igen.RawVisibility != "public")
 					continue;
 				if (sb.Length > 0)
 					sb.Append (", ");

tools/generator/Java.Interop.Tools.Generator.ObjectModel/GenBase.cs

@@ -94,7 +94,7 @@ void AddPropertyAccessors ()
 					string prop_name = m.PropertyName;
 					if (m.CanSet || prop_name == string.Empty || Name == prop_name || m.Name == "GetHashCode" || HasNestedType (prop_name) || IsInfrastructural (prop_name))
 						unmatched.Add (m);
-					else if (BaseGen != null && !BaseGen.prop_hash.ContainsKey (prop_name) && BaseGen.Methods.Any (mm => mm.Name == m.Name && ReturnTypeMatches(m, mm) && ParameterList.Equals (mm.Parameters, m.Parameters)))
+					else if (BaseGen != null && !BaseGen.prop_hash.ContainsKey (prop_name) && BaseGen.Methods.Any (mm => mm.Name == m.Name && ReturnTypeMatches (m, mm) && ParameterList.Equals (mm.Parameters, m.Parameters)))
 						// this is to filter out those method that was *not* a property
 						// in the base type for some reason (e.g. name overlap).
 						// For example, android.graphics.drawable.BitmapDrawable#getConstantState()
@@ -129,7 +129,7 @@ void AddPropertyAccessors ()
 					continue;
 				}
 
-				if (Ancestors ().All (a => !a.prop_hash.ContainsKey (m.PropertyName)) && Ancestors ().Any (a => a.Methods.Any (mm => mm.Name == m.Name && ReturnTypeMatches(m, mm) && ParameterList.Equals (mm.Parameters, m.Parameters))))
+				if (Ancestors ().All (a => !a.prop_hash.ContainsKey (m.PropertyName)) && Ancestors ().Any (a => a.Methods.Any (mm => mm.Name == m.Name && ReturnTypeMatches (m, mm) && ParameterList.Equals (mm.Parameters, m.Parameters))))
 					unmatched.Add (m); // base setter exists, and it was not a property.
 				else if (prop_hash.ContainsKey (m.PropertyName)) {
 					Property baseProp = BaseGen?.Properties.FirstOrDefault (p => p.Name == m.PropertyName);
@@ -575,6 +575,8 @@ static bool IsTypeCommensurate (CodeGenerationOptions opt, ISymbol sym)
 			return false;
 		}
 
+		public IEnumerable<string> ImplementedInterfaces => implemented_interfaces;
+
 		public bool IsValid { get; set; } = true;
 
 		public string JavaName => $"{PackageName}.{JavaSimpleName}";

tools/generator/Java.Interop.Tools.Generator.ObjectModel/InterfaceGen.cs

@@ -43,6 +43,30 @@ public override string FromNative (CodeGenerationOptions opt, string varname, bo
 			*/
 		}
 
+		public override void FixupAccessModifiers (CodeGenerationOptions opt)
+		{
+			if (!IsAnnotation) {
+				foreach (var implementedInterface in ImplementedInterfaces) { 
+					if (string.IsNullOrEmpty (implementedInterface)) {
+						System.Diagnostics.Debug.Assert (false, "BUGBUG - We should never have an empty or null string added on the implemented interface list.");
+						continue;
+					}
+
+					var baseType = opt.SymbolTable.Lookup (implementedInterface);
+					if (baseType is InterfaceGen interfaceGen && interfaceGen.RawVisibility != "public") {
+						// Copy over "private" methods
+						interfaceGen.Methods.Where (m => !Methods.Contains (m)).ToList ().ForEach (Methods.Add);
+
+					} else {
+						break;
+					}
+				}
+			}
+
+
+			base.FixupAccessModifiers (opt);
+		}
+
 		public override void Generate (CodeGenerationOptions opt, GenerationInfo gen_info)
 		{
 			using (var sw = gen_info.OpenStream (opt.GetFileName (FullName))) {

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

@@ -9,8 +9,10 @@
     <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.IExtendedInterface.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Xamarin.Test.PublicClass.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Xamarin.Test.PublicFinalClass.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Xamarin.Test.TestClass.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)__NamespaceMapping__.cs" />
   </ItemGroup>
   <!-- Enums -->

tools/generator/Tests/expected/AccessModifiers/AccessModifiers.xml

@@ -78,5 +78,35 @@
 			<method abstract="false" deprecated="not deprecated" final="false" name="publicMethod" native="false" return="void" static="false" synchronized="false" visibility="public">
 			</method>
 		</class>
+    <!-- 
+      interface BaseInterface {
+        public void baseMethod();
+      }
+    -->
+     <interface abstract="true" deprecated="not deprecated" final="false" name="BaseInterface" static="false" visibility="">
+      <method abstract="true" deprecated="not deprecated" final="false" name="baseMethod" native="false" return="void" static="false" synchronized="false" synthetic="false" visibility="public" />
+    </interface>
+    <!--
+      public interface ExtendedInterface extends BaseInterface {
+        public void publicMethod2();
+      }
+    -->
+    <interface abstract="true" deprecated="not deprecated" final="false" name="ExtendedInterface" static="false" visibility="public">
+      <implements name="xamarin.test.BaseInterface" name-generic-aware="xamarin.test.BaseInterface" />>
+      <method abstract="true" deprecated="not deprecated" final="false" name="extendedMethod" native="false" return="void" static="false" synchronized="false" synthetic="false" visibility="public" />>
+    </interface>
+    <!--
+      public class TestClass implements BaseInterface {
+
+        @Override
+        public void baseMethod() {
+        }
+      }
+    -->
+    <class abstract="false" deprecated="not deprecated" extends="java.lang.Object" extends-generic-aware="java.lang.Object" final="false" name="TestClass" static="false" visibility="public">
+      <implements name="xamarin.test.BaseInterface" name-generic-aware="xamarin.test.BaseInterface" />>
+      <constructor deprecated="not deprecated" final="false" name="TestClass" jni-signature="()V" bridge="false" static="false" type="xamarin.test.TestClass" synthetic="false" visibility="public" />
+      <method abstract="false" deprecated="not deprecated" final="false" name="baseMethod" native="false" return="void" static="false" synchronized="false" synthetic="false" visibility="public" />
+    </class>
 	</package>
 </api>

tools/generator/Tests/expected/AccessModifiers/Xamarin.Test.IExtendedInterface.cs

@@ -0,0 +1,114 @@
+using System;
+using System.Collections.Generic;
+using Android.Runtime;
+
+namespace Xamarin.Test {
+
+	// Metadata.xml XPath interface reference: path="/api/package[@name='xamarin.test']/interface[@name='ExtendedInterface']"
+	[Register ("xamarin/test/ExtendedInterface", "", "Xamarin.Test.IExtendedInterfaceInvoker")]
+	public partial interface IExtendedInterface : IJavaObject {
+
+		// Metadata.xml XPath method reference: path="/api/package[@name='xamarin.test']/interface[@name='ExtendedInterface']/method[@name='extendedMethod' and count(parameter)=0]"
+		[Register ("extendedMethod", "()V", "GetExtendedMethodHandler:Xamarin.Test.IExtendedInterfaceInvoker, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null")]
+		void ExtendedMethod ();
+
+		// Metadata.xml XPath method reference: path="/api/package[@name='xamarin.test']/interface[@name='BaseInterface']/method[@name='baseMethod' and count(parameter)=0]"
+		[Register ("baseMethod", "()V", "GetBaseMethodHandler:Xamarin.Test.IExtendedInterfaceInvoker, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null")]
+		void BaseMethod ();
+
+	}
+
+	[global::Android.Runtime.Register ("xamarin/test/ExtendedInterface", DoNotGenerateAcw=true)]
+	internal partial class IExtendedInterfaceInvoker : global::Java.Lang.Object, IExtendedInterface {
+
+		static IntPtr java_class_ref = JNIEnv.FindClass ("xamarin/test/ExtendedInterface");
+
+		protected override IntPtr ThresholdClass {
+			get { return class_ref; }
+		}
+
+		protected override global::System.Type ThresholdType {
+			get { return typeof (IExtendedInterfaceInvoker); }
+		}
+
+		new IntPtr class_ref;
+
+		public static IExtendedInterface GetObject (IntPtr handle, JniHandleOwnership transfer)
+		{
+			return global::Java.Lang.Object.GetObject<IExtendedInterface> (handle, transfer);
+		}
+
+		static IntPtr Validate (IntPtr handle)
+		{
+			if (!JNIEnv.IsInstanceOf (handle, java_class_ref))
+				throw new InvalidCastException (string.Format ("Unable to convert instance of type '{0}' to type '{1}'.",
+							JNIEnv.GetClassNameFromInstance (handle), "xamarin.test.ExtendedInterface"));
+			return handle;
+		}
+
+		protected override void Dispose (bool disposing)
+		{
+			if (this.class_ref != IntPtr.Zero)
+				JNIEnv.DeleteGlobalRef (this.class_ref);
+			this.class_ref = IntPtr.Zero;
+			base.Dispose (disposing);
+		}
+
+		public IExtendedInterfaceInvoker (IntPtr handle, JniHandleOwnership transfer) : base (Validate (handle), transfer)
+		{
+			IntPtr local_ref = JNIEnv.GetObjectClass (((global::Java.Lang.Object) this).Handle);
+			this.class_ref = JNIEnv.NewGlobalRef (local_ref);
+			JNIEnv.DeleteLocalRef (local_ref);
+		}
+
+		static Delegate cb_extendedMethod;
+#pragma warning disable 0169
+		static Delegate GetExtendedMethodHandler ()
+		{
+			if (cb_extendedMethod == null)
+				cb_extendedMethod = JNINativeWrapper.CreateDelegate ((Action<IntPtr, IntPtr>) n_ExtendedMethod);
+			return cb_extendedMethod;
+		}
+
+		static void n_ExtendedMethod (IntPtr jnienv, IntPtr native__this)
+		{
+			global::Xamarin.Test.IExtendedInterface __this = global::Java.Lang.Object.GetObject<global::Xamarin.Test.IExtendedInterface> (jnienv, native__this, JniHandleOwnership.DoNotTransfer);
+			__this.ExtendedMethod ();
+		}
+#pragma warning restore 0169
+
+		IntPtr id_extendedMethod;
+		public unsafe void ExtendedMethod ()
+		{
+			if (id_extendedMethod == IntPtr.Zero)
+				id_extendedMethod = JNIEnv.GetMethodID (class_ref, "extendedMethod", "()V");
+			JNIEnv.CallVoidMethod (((global::Java.Lang.Object) this).Handle, id_extendedMethod);
+		}
+
+		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.IExtendedInterface __this = global::Java.Lang.Object.GetObject<global::Xamarin.Test.IExtendedInterface> (jnienv, native__this, JniHandleOwnership.DoNotTransfer);
+			__this.BaseMethod ();
+		}
+#pragma warning restore 0169
+
+		IntPtr id_baseMethod;
+		public unsafe void BaseMethod ()
+		{
+			if (id_baseMethod == IntPtr.Zero)
+				id_baseMethod = JNIEnv.GetMethodID (class_ref, "baseMethod", "()V");
+			JNIEnv.CallVoidMethod (((global::Java.Lang.Object) this).Handle, id_baseMethod);
+		}
+
+	}
+
+}

tools/generator/Tests/expected/AccessModifiers/Xamarin.Test.TestClass.cs

@@ -0,0 +1,90 @@
+using System;
+using System.Collections.Generic;
+using Android.Runtime;
+
+namespace Xamarin.Test {
+
+	// Metadata.xml XPath class reference: path="/api/package[@name='xamarin.test']/class[@name='TestClass']"
+	[global::Android.Runtime.Register ("xamarin/test/TestClass", DoNotGenerateAcw=true)]
+	public partial class TestClass : global::Java.Lang.Object {
+
+		internal static new IntPtr java_class_handle;
+		internal static new IntPtr class_ref {
+			get {
+				return JNIEnv.FindClass ("xamarin/test/TestClass", ref java_class_handle);
+			}
+		}
+
+		protected override IntPtr ThresholdClass {
+			get { return class_ref; }
+		}
+
+		protected override global::System.Type ThresholdType {
+			get { return typeof (TestClass); }
+		}
+
+		protected TestClass (IntPtr javaReference, JniHandleOwnership transfer) : base (javaReference, transfer) {}
+
+		static IntPtr id_ctor;
+		// Metadata.xml XPath constructor reference: path="/api/package[@name='xamarin.test']/class[@name='TestClass']/constructor[@name='TestClass' and count(parameter)=0]"
+		[Register (".ctor", "()V", "")]
+		public unsafe TestClass ()
+			: base (IntPtr.Zero, JniHandleOwnership.DoNotTransfer)
+		{
+			if (((global::Java.Lang.Object) this).Handle != IntPtr.Zero)
+				return;
+
+			try {
+				if (((object) this).GetType () != typeof (TestClass)) {
+					SetHandle (
+							global::Android.Runtime.JNIEnv.StartCreateInstance (((object) this).GetType (), "()V"),
+							JniHandleOwnership.TransferLocalRef);
+					global::Android.Runtime.JNIEnv.FinishCreateInstance (((global::Java.Lang.Object) this).Handle, "()V");
+					return;
+				}
+
+				if (id_ctor == IntPtr.Zero)
+					id_ctor = JNIEnv.GetMethodID (class_ref, "<init>", "()V");
+				SetHandle (
+						global::Android.Runtime.JNIEnv.StartCreateInstance (class_ref, id_ctor),
+						JniHandleOwnership.TransferLocalRef);
+				JNIEnv.FinishCreateInstance (((global::Java.Lang.Object) this).Handle, class_ref, id_ctor);
+			} finally {
+			}
+		}
+
+		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.TestClass __this = global::Java.Lang.Object.GetObject<global::Xamarin.Test.TestClass> (jnienv, native__this, JniHandleOwnership.DoNotTransfer);
+			__this.BaseMethod ();
+		}
+#pragma warning restore 0169
+
+		static IntPtr id_baseMethod;
+		// Metadata.xml XPath method reference: path="/api/package[@name='xamarin.test']/class[@name='TestClass']/method[@name='baseMethod' and count(parameter)=0]"
+		[Register ("baseMethod", "()V", "GetBaseMethodHandler")]
+		public virtual unsafe void BaseMethod ()
+		{
+			if (id_baseMethod == IntPtr.Zero)
+				id_baseMethod = JNIEnv.GetMethodID (class_ref, "baseMethod", "()V");
+			try {
+
+				if (((object) this).GetType () == ThresholdType)
+					JNIEnv.CallVoidMethod (((global::Java.Lang.Object) this).Handle, id_baseMethod);
+				else
+					JNIEnv.CallNonvirtualVoidMethod (((global::Java.Lang.Object) this).Handle, ThresholdClass, JNIEnv.GetMethodID (ThresholdClass, "baseMethod", "()V"));
+			} finally {
+			}
+		}
+
+	}
+}

tools/generator/Tests/generator-Tests.csproj

@@ -617,6 +617,12 @@
     <Content Include="Unit-Tests\EnumGeneratorExpectedResults\WriteFlagsEnum.txt">
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
     </Content>
+    <Content Include="expected\AccessModifiers\Xamarin.Test.IExtendedInterface.cs">
+      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
+    </Content>
+    <Content Include="expected\AccessModifiers\Xamarin.Test.TestClass.cs">
+      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
+    </Content>
   </ItemGroup>
   <ItemGroup>
     <Content Include="expected\**\*">