From 5b54d6a9a7d83d1c97d762967193d06fa30ba2bb Mon Sep 17 00:00:00 2001 From: Gustavo Varo Date: Wed, 14 Aug 2019 20:59:57 -0400 Subject: [PATCH] [generator] better support for package-private interfaces --- .../CodeGenerator.cs | 4 +- .../GenBase.cs | 6 +- .../InterfaceGen.cs | 24 ++++ .../AccessModifiers/Mono.Android.projitems | 2 + .../AccessModifiers/AccessModifiers.xml | 30 +++++ .../Xamarin.Test.IExtendedInterface.cs | 114 ++++++++++++++++++ .../AccessModifiers/Xamarin.Test.TestClass.cs | 90 ++++++++++++++ tools/generator/Tests/generator-Tests.csproj | 6 + 8 files changed, 272 insertions(+), 4 deletions(-) create mode 100644 tools/generator/Tests/expected/AccessModifiers/Xamarin.Test.IExtendedInterface.cs create mode 100644 tools/generator/Tests/expected/AccessModifiers/Xamarin.Test.TestClass.cs diff --git a/tools/generator/Java.Interop.Tools.Generator.CodeGeneration/CodeGenerator.cs b/tools/generator/Java.Interop.Tools.Generator.CodeGeneration/CodeGenerator.cs index 78a109c50..2417e7f16 100644 --- a/tools/generator/Java.Interop.Tools.Generator.CodeGeneration/CodeGenerator.cs +++ b/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 (", "); diff --git a/tools/generator/Java.Interop.Tools.Generator.ObjectModel/GenBase.cs b/tools/generator/Java.Interop.Tools.Generator.ObjectModel/GenBase.cs index 1628785ba..674b9b176 100644 --- a/tools/generator/Java.Interop.Tools.Generator.ObjectModel/GenBase.cs +++ b/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 ImplementedInterfaces => implemented_interfaces; + public bool IsValid { get; set; } = true; public string JavaName => $"{PackageName}.{JavaSimpleName}"; diff --git a/tools/generator/Java.Interop.Tools.Generator.ObjectModel/InterfaceGen.cs b/tools/generator/Java.Interop.Tools.Generator.ObjectModel/InterfaceGen.cs index e0bbba10b..510b1f3de 100644 --- a/tools/generator/Java.Interop.Tools.Generator.ObjectModel/InterfaceGen.cs +++ b/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))) { diff --git a/tools/generator/Tests/expected.ji/AccessModifiers/Mono.Android.projitems b/tools/generator/Tests/expected.ji/AccessModifiers/Mono.Android.projitems index 461424327..8202bce47 100644 --- a/tools/generator/Tests/expected.ji/AccessModifiers/Mono.Android.projitems +++ b/tools/generator/Tests/expected.ji/AccessModifiers/Mono.Android.projitems @@ -9,8 +9,10 @@ + + diff --git a/tools/generator/Tests/expected/AccessModifiers/AccessModifiers.xml b/tools/generator/Tests/expected/AccessModifiers/AccessModifiers.xml index 9adec7944..2da352ab3 100644 --- a/tools/generator/Tests/expected/AccessModifiers/AccessModifiers.xml +++ b/tools/generator/Tests/expected/AccessModifiers/AccessModifiers.xml @@ -78,5 +78,35 @@ + + + + + + + > + > + + + + > + + + diff --git a/tools/generator/Tests/expected/AccessModifiers/Xamarin.Test.IExtendedInterface.cs b/tools/generator/Tests/expected/AccessModifiers/Xamarin.Test.IExtendedInterface.cs new file mode 100644 index 000000000..c289f307b --- /dev/null +++ b/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 (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) n_ExtendedMethod); + return cb_extendedMethod; + } + + static void n_ExtendedMethod (IntPtr jnienv, IntPtr native__this) + { + global::Xamarin.Test.IExtendedInterface __this = global::Java.Lang.Object.GetObject (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) n_BaseMethod); + return cb_baseMethod; + } + + static void n_BaseMethod (IntPtr jnienv, IntPtr native__this) + { + global::Xamarin.Test.IExtendedInterface __this = global::Java.Lang.Object.GetObject (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); + } + + } + +} diff --git a/tools/generator/Tests/expected/AccessModifiers/Xamarin.Test.TestClass.cs b/tools/generator/Tests/expected/AccessModifiers/Xamarin.Test.TestClass.cs new file mode 100644 index 000000000..b1759154e --- /dev/null +++ b/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, "", "()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) n_BaseMethod); + return cb_baseMethod; + } + + static void n_BaseMethod (IntPtr jnienv, IntPtr native__this) + { + global::Xamarin.Test.TestClass __this = global::Java.Lang.Object.GetObject (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 { + } + } + + } +} diff --git a/tools/generator/Tests/generator-Tests.csproj b/tools/generator/Tests/generator-Tests.csproj index b95a0f447..453aa5e05 100644 --- a/tools/generator/Tests/generator-Tests.csproj +++ b/tools/generator/Tests/generator-Tests.csproj @@ -617,6 +617,12 @@ PreserveNewest + + PreserveNewest + + + PreserveNewest +