Skip to content

Commit 2f2060f

Browse files
gugavarojonpryor
gugavaro
authored and
jonpryor
committed
[generator] better support for package-private interfaces (#471)
Context: 4ec5d4e Java allows public classes/interfaces to inherit from/implement "package-private" interfaces, in which case the `public` and `protected` members of the "package-private" interface are visible within the public class: // Java /* package */ interface BaseInterface { void foo(); // ... } public interface TestInterface extends BaseInterface { // ... } public class TestClass implements TestInterface { // ... } // -or- public class TestClass implements BaseInterface { // ... } The problem is that `generator` didn't properly this construct, and *skips* binding of "package-private" types, resulting in generated C# code such as: // C# public partial interface TestInterface : BaseInterface { // CS0234: The type or namespace name `BaseInterface` does not exist in the namespace } Take a page out of 4ec5d4e -- which added support for this scenario for package-private *classes* used as base classes -- and support this construct by updating `generator` to "copy" the members from the "package-private" interfaces into the declaring interface: // C# public partial class TestClass : Java.Lang.Object { public void Foo() { // ... } This allows the generated code to compile without metadata fixups. Specifics in implementing this: - Add a `InterfaceGen.FixupAccessModifiers()` to fix the private interfaces - Call `FixupAccessModifiers()` in the "validation" step - Add a property to expose the `ImplementedInterfaces` - In `FixupAccessModifiers()`, lookup the base interface of the current type and check if it is non-`public`. - Skip the package-private types, and replace them with that interface'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 the follow scenarios - Private Interface -> Public Interface -> Public Class - Private Interface -> Public Class
1 parent 2abfc1e commit 2f2060f

File tree

8 files changed

+272
-4
lines changed

8 files changed

+272
-4
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public void WriteClass (ClassGen @class, string indent, GenerationInfo gen_info)
5454
foreach (ISymbol isym in @class.Interfaces) {
5555
GenericSymbol gs = isym as GenericSymbol;
5656
InterfaceGen gen = (gs == null ? isym : gs.Gen) as InterfaceGen;
57-
if (gen != null && gen.IsConstSugar)
57+
if (gen != null && (gen.IsConstSugar || gen.RawVisibility != "public"))
5858
continue;
5959
if (sb.Length > 0)
6060
sb.Append (", ");
@@ -506,7 +506,7 @@ public void WriteInterfaceDeclaration (InterfaceGen @interface, string indent)
506506
StringBuilder sb = new StringBuilder ();
507507
foreach (ISymbol isym in @interface.Interfaces) {
508508
InterfaceGen igen = (isym is GenericSymbol ? (isym as GenericSymbol).Gen : isym) as InterfaceGen;
509-
if (igen.IsConstSugar)
509+
if (igen.IsConstSugar || igen.RawVisibility != "public")
510510
continue;
511511
if (sb.Length > 0)
512512
sb.Append (", ");

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ void AddPropertyAccessors ()
9494
string prop_name = m.PropertyName;
9595
if (m.CanSet || prop_name == string.Empty || Name == prop_name || m.Name == "GetHashCode" || HasNestedType (prop_name) || IsInfrastructural (prop_name))
9696
unmatched.Add (m);
97-
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)))
97+
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)))
9898
// this is to filter out those method that was *not* a property
9999
// in the base type for some reason (e.g. name overlap).
100100
// For example, android.graphics.drawable.BitmapDrawable#getConstantState()
@@ -129,7 +129,7 @@ void AddPropertyAccessors ()
129129
continue;
130130
}
131131

132-
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))))
132+
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))))
133133
unmatched.Add (m); // base setter exists, and it was not a property.
134134
else if (prop_hash.ContainsKey (m.PropertyName)) {
135135
Property baseProp = BaseGen?.Properties.FirstOrDefault (p => p.Name == m.PropertyName);
@@ -575,6 +575,8 @@ static bool IsTypeCommensurate (CodeGenerationOptions opt, ISymbol sym)
575575
return false;
576576
}
577577

578+
public IEnumerable<string> ImplementedInterfaces => implemented_interfaces;
579+
578580
public bool IsValid { get; set; } = true;
579581

580582
public string JavaName => $"{PackageName}.{JavaSimpleName}";

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,30 @@ public override string FromNative (CodeGenerationOptions opt, string varname, bo
4343
*/
4444
}
4545

46+
public override void FixupAccessModifiers (CodeGenerationOptions opt)
47+
{
48+
if (!IsAnnotation) {
49+
foreach (var implementedInterface in ImplementedInterfaces) {
50+
if (string.IsNullOrEmpty (implementedInterface)) {
51+
System.Diagnostics.Debug.Assert (false, "BUGBUG - We should never have an empty or null string added on the implemented interface list.");
52+
continue;
53+
}
54+
55+
var baseType = opt.SymbolTable.Lookup (implementedInterface);
56+
if (baseType is InterfaceGen interfaceGen && interfaceGen.RawVisibility != "public") {
57+
// Copy over "private" methods
58+
interfaceGen.Methods.Where (m => !Methods.Contains (m)).ToList ().ForEach (Methods.Add);
59+
60+
} else {
61+
break;
62+
}
63+
}
64+
}
65+
66+
67+
base.FixupAccessModifiers (opt);
68+
}
69+
4670
public override void Generate (CodeGenerationOptions opt, GenerationInfo gen_info)
4771
{
4872
using (var sw = gen_info.OpenStream (opt.GetFileName (FullName))) {

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,10 @@
99
<Compile Include="$(MSBuildThisFileDirectory)Java.Lang.Object.cs" />
1010
<Compile Include="$(MSBuildThisFileDirectory)Xamarin.Test.BasePublicClass.cs" />
1111
<Compile Include="$(MSBuildThisFileDirectory)Xamarin.Test.ExtendPublicClass.cs" />
12+
<Compile Include="$(MSBuildThisFileDirectory)Xamarin.Test.IExtendedInterface.cs" />
1213
<Compile Include="$(MSBuildThisFileDirectory)Xamarin.Test.PublicClass.cs" />
1314
<Compile Include="$(MSBuildThisFileDirectory)Xamarin.Test.PublicFinalClass.cs" />
15+
<Compile Include="$(MSBuildThisFileDirectory)Xamarin.Test.TestClass.cs" />
1416
<Compile Include="$(MSBuildThisFileDirectory)__NamespaceMapping__.cs" />
1517
</ItemGroup>
1618
<!-- Enums -->

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,5 +78,35 @@
7878
<method abstract="false" deprecated="not deprecated" final="false" name="publicMethod" native="false" return="void" static="false" synchronized="false" visibility="public">
7979
</method>
8080
</class>
81+
<!--
82+
interface BaseInterface {
83+
public void baseMethod();
84+
}
85+
-->
86+
<interface abstract="true" deprecated="not deprecated" final="false" name="BaseInterface" static="false" visibility="">
87+
<method abstract="true" deprecated="not deprecated" final="false" name="baseMethod" native="false" return="void" static="false" synchronized="false" synthetic="false" visibility="public" />
88+
</interface>
89+
<!--
90+
public interface ExtendedInterface extends BaseInterface {
91+
public void publicMethod2();
92+
}
93+
-->
94+
<interface abstract="true" deprecated="not deprecated" final="false" name="ExtendedInterface" static="false" visibility="public">
95+
<implements name="xamarin.test.BaseInterface" name-generic-aware="xamarin.test.BaseInterface" />>
96+
<method abstract="true" deprecated="not deprecated" final="false" name="extendedMethod" native="false" return="void" static="false" synchronized="false" synthetic="false" visibility="public" />>
97+
</interface>
98+
<!--
99+
public class TestClass implements BaseInterface {
100+
101+
@Override
102+
public void baseMethod() {
103+
}
104+
}
105+
-->
106+
<class abstract="false" deprecated="not deprecated" extends="java.lang.Object" extends-generic-aware="java.lang.Object" final="false" name="TestClass" static="false" visibility="public">
107+
<implements name="xamarin.test.BaseInterface" name-generic-aware="xamarin.test.BaseInterface" />>
108+
<constructor deprecated="not deprecated" final="false" name="TestClass" jni-signature="()V" bridge="false" static="false" type="xamarin.test.TestClass" synthetic="false" visibility="public" />
109+
<method abstract="false" deprecated="not deprecated" final="false" name="baseMethod" native="false" return="void" static="false" synchronized="false" synthetic="false" visibility="public" />
110+
</class>
81111
</package>
82112
</api>

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

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using Android.Runtime;
4+
5+
namespace Xamarin.Test {
6+
7+
// Metadata.xml XPath interface reference: path="/api/package[@name='xamarin.test']/interface[@name='ExtendedInterface']"
8+
[Register ("xamarin/test/ExtendedInterface", "", "Xamarin.Test.IExtendedInterfaceInvoker")]
9+
public partial interface IExtendedInterface : IJavaObject {
10+
11+
// Metadata.xml XPath method reference: path="/api/package[@name='xamarin.test']/interface[@name='ExtendedInterface']/method[@name='extendedMethod' and count(parameter)=0]"
12+
[Register ("extendedMethod", "()V", "GetExtendedMethodHandler:Xamarin.Test.IExtendedInterfaceInvoker, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null")]
13+
void ExtendedMethod ();
14+
15+
// Metadata.xml XPath method reference: path="/api/package[@name='xamarin.test']/interface[@name='BaseInterface']/method[@name='baseMethod' and count(parameter)=0]"
16+
[Register ("baseMethod", "()V", "GetBaseMethodHandler:Xamarin.Test.IExtendedInterfaceInvoker, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null")]
17+
void BaseMethod ();
18+
19+
}
20+
21+
[global::Android.Runtime.Register ("xamarin/test/ExtendedInterface", DoNotGenerateAcw=true)]
22+
internal partial class IExtendedInterfaceInvoker : global::Java.Lang.Object, IExtendedInterface {
23+
24+
static IntPtr java_class_ref = JNIEnv.FindClass ("xamarin/test/ExtendedInterface");
25+
26+
protected override IntPtr ThresholdClass {
27+
get { return class_ref; }
28+
}
29+
30+
protected override global::System.Type ThresholdType {
31+
get { return typeof (IExtendedInterfaceInvoker); }
32+
}
33+
34+
new IntPtr class_ref;
35+
36+
public static IExtendedInterface GetObject (IntPtr handle, JniHandleOwnership transfer)
37+
{
38+
return global::Java.Lang.Object.GetObject<IExtendedInterface> (handle, transfer);
39+
}
40+
41+
static IntPtr Validate (IntPtr handle)
42+
{
43+
if (!JNIEnv.IsInstanceOf (handle, java_class_ref))
44+
throw new InvalidCastException (string.Format ("Unable to convert instance of type '{0}' to type '{1}'.",
45+
JNIEnv.GetClassNameFromInstance (handle), "xamarin.test.ExtendedInterface"));
46+
return handle;
47+
}
48+
49+
protected override void Dispose (bool disposing)
50+
{
51+
if (this.class_ref != IntPtr.Zero)
52+
JNIEnv.DeleteGlobalRef (this.class_ref);
53+
this.class_ref = IntPtr.Zero;
54+
base.Dispose (disposing);
55+
}
56+
57+
public IExtendedInterfaceInvoker (IntPtr handle, JniHandleOwnership transfer) : base (Validate (handle), transfer)
58+
{
59+
IntPtr local_ref = JNIEnv.GetObjectClass (((global::Java.Lang.Object) this).Handle);
60+
this.class_ref = JNIEnv.NewGlobalRef (local_ref);
61+
JNIEnv.DeleteLocalRef (local_ref);
62+
}
63+
64+
static Delegate cb_extendedMethod;
65+
#pragma warning disable 0169
66+
static Delegate GetExtendedMethodHandler ()
67+
{
68+
if (cb_extendedMethod == null)
69+
cb_extendedMethod = JNINativeWrapper.CreateDelegate ((Action<IntPtr, IntPtr>) n_ExtendedMethod);
70+
return cb_extendedMethod;
71+
}
72+
73+
static void n_ExtendedMethod (IntPtr jnienv, IntPtr native__this)
74+
{
75+
global::Xamarin.Test.IExtendedInterface __this = global::Java.Lang.Object.GetObject<global::Xamarin.Test.IExtendedInterface> (jnienv, native__this, JniHandleOwnership.DoNotTransfer);
76+
__this.ExtendedMethod ();
77+
}
78+
#pragma warning restore 0169
79+
80+
IntPtr id_extendedMethod;
81+
public unsafe void ExtendedMethod ()
82+
{
83+
if (id_extendedMethod == IntPtr.Zero)
84+
id_extendedMethod = JNIEnv.GetMethodID (class_ref, "extendedMethod", "()V");
85+
JNIEnv.CallVoidMethod (((global::Java.Lang.Object) this).Handle, id_extendedMethod);
86+
}
87+
88+
static Delegate cb_baseMethod;
89+
#pragma warning disable 0169
90+
static Delegate GetBaseMethodHandler ()
91+
{
92+
if (cb_baseMethod == null)
93+
cb_baseMethod = JNINativeWrapper.CreateDelegate ((Action<IntPtr, IntPtr>) n_BaseMethod);
94+
return cb_baseMethod;
95+
}
96+
97+
static void n_BaseMethod (IntPtr jnienv, IntPtr native__this)
98+
{
99+
global::Xamarin.Test.IExtendedInterface __this = global::Java.Lang.Object.GetObject<global::Xamarin.Test.IExtendedInterface> (jnienv, native__this, JniHandleOwnership.DoNotTransfer);
100+
__this.BaseMethod ();
101+
}
102+
#pragma warning restore 0169
103+
104+
IntPtr id_baseMethod;
105+
public unsafe void BaseMethod ()
106+
{
107+
if (id_baseMethod == IntPtr.Zero)
108+
id_baseMethod = JNIEnv.GetMethodID (class_ref, "baseMethod", "()V");
109+
JNIEnv.CallVoidMethod (((global::Java.Lang.Object) this).Handle, id_baseMethod);
110+
}
111+
112+
}
113+
114+
}

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

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using Android.Runtime;
4+
5+
namespace Xamarin.Test {
6+
7+
// Metadata.xml XPath class reference: path="/api/package[@name='xamarin.test']/class[@name='TestClass']"
8+
[global::Android.Runtime.Register ("xamarin/test/TestClass", DoNotGenerateAcw=true)]
9+
public partial class TestClass : global::Java.Lang.Object {
10+
11+
internal static new IntPtr java_class_handle;
12+
internal static new IntPtr class_ref {
13+
get {
14+
return JNIEnv.FindClass ("xamarin/test/TestClass", ref java_class_handle);
15+
}
16+
}
17+
18+
protected override IntPtr ThresholdClass {
19+
get { return class_ref; }
20+
}
21+
22+
protected override global::System.Type ThresholdType {
23+
get { return typeof (TestClass); }
24+
}
25+
26+
protected TestClass (IntPtr javaReference, JniHandleOwnership transfer) : base (javaReference, transfer) {}
27+
28+
static IntPtr id_ctor;
29+
// Metadata.xml XPath constructor reference: path="/api/package[@name='xamarin.test']/class[@name='TestClass']/constructor[@name='TestClass' and count(parameter)=0]"
30+
[Register (".ctor", "()V", "")]
31+
public unsafe TestClass ()
32+
: base (IntPtr.Zero, JniHandleOwnership.DoNotTransfer)
33+
{
34+
if (((global::Java.Lang.Object) this).Handle != IntPtr.Zero)
35+
return;
36+
37+
try {
38+
if (((object) this).GetType () != typeof (TestClass)) {
39+
SetHandle (
40+
global::Android.Runtime.JNIEnv.StartCreateInstance (((object) this).GetType (), "()V"),
41+
JniHandleOwnership.TransferLocalRef);
42+
global::Android.Runtime.JNIEnv.FinishCreateInstance (((global::Java.Lang.Object) this).Handle, "()V");
43+
return;
44+
}
45+
46+
if (id_ctor == IntPtr.Zero)
47+
id_ctor = JNIEnv.GetMethodID (class_ref, "<init>", "()V");
48+
SetHandle (
49+
global::Android.Runtime.JNIEnv.StartCreateInstance (class_ref, id_ctor),
50+
JniHandleOwnership.TransferLocalRef);
51+
JNIEnv.FinishCreateInstance (((global::Java.Lang.Object) this).Handle, class_ref, id_ctor);
52+
} finally {
53+
}
54+
}
55+
56+
static Delegate cb_baseMethod;
57+
#pragma warning disable 0169
58+
static Delegate GetBaseMethodHandler ()
59+
{
60+
if (cb_baseMethod == null)
61+
cb_baseMethod = JNINativeWrapper.CreateDelegate ((Action<IntPtr, IntPtr>) n_BaseMethod);
62+
return cb_baseMethod;
63+
}
64+
65+
static void n_BaseMethod (IntPtr jnienv, IntPtr native__this)
66+
{
67+
global::Xamarin.Test.TestClass __this = global::Java.Lang.Object.GetObject<global::Xamarin.Test.TestClass> (jnienv, native__this, JniHandleOwnership.DoNotTransfer);
68+
__this.BaseMethod ();
69+
}
70+
#pragma warning restore 0169
71+
72+
static IntPtr id_baseMethod;
73+
// Metadata.xml XPath method reference: path="/api/package[@name='xamarin.test']/class[@name='TestClass']/method[@name='baseMethod' and count(parameter)=0]"
74+
[Register ("baseMethod", "()V", "GetBaseMethodHandler")]
75+
public virtual unsafe void BaseMethod ()
76+
{
77+
if (id_baseMethod == IntPtr.Zero)
78+
id_baseMethod = JNIEnv.GetMethodID (class_ref, "baseMethod", "()V");
79+
try {
80+
81+
if (((object) this).GetType () == ThresholdType)
82+
JNIEnv.CallVoidMethod (((global::Java.Lang.Object) this).Handle, id_baseMethod);
83+
else
84+
JNIEnv.CallNonvirtualVoidMethod (((global::Java.Lang.Object) this).Handle, ThresholdClass, JNIEnv.GetMethodID (ThresholdClass, "baseMethod", "()V"));
85+
} finally {
86+
}
87+
}
88+
89+
}
90+
}

‎tools/generator/Tests/generator-Tests.csproj‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -617,6 +617,12 @@
617617
<Content Include="Unit-Tests\EnumGeneratorExpectedResults\WriteFlagsEnum.txt">
618618
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
619619
</Content>
620+
<Content Include="expected\AccessModifiers\Xamarin.Test.IExtendedInterface.cs">
621+
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
622+
</Content>
623+
<Content Include="expected\AccessModifiers\Xamarin.Test.TestClass.cs">
624+
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
625+
</Content>
620626
</ItemGroup>
621627
<ItemGroup>
622628
<Content Include="expected\**\*">

0 commit comments

Comments
 (0)