Skip to content

Commit 5e09a7b

Browse files
jonathanpeppersjonathanpeppers
committed
[generator] fixup access modifiers on base classes
Fixes #168 A "package private" abstract class such as: ``` abstract class PackageClass { public abstract void foo(); } ``` Causes invalid C# to be generated, if Java extends it: ``` public class PublicClass extends PackageClass { @OverRide public void foo(); } ``` C# will be missing the `PackageClass` type because it is non-public. To fix this: - My general idea is to make `PackageClass` in the above example not get emitted at all in C# - Add a `FixupAccessModifiers` method to `GenBase`, 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 - Added tests for this scenario
1 parent 9b59cd9 commit 5e09a7b

File tree

12 files changed

+464
-1
lines changed

12 files changed

+464
-1
lines changed

‎tools/generator/ClassGen.cs‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ public ManagedClassGen (TypeDefinition t)
5959

6060
public override string BaseType {
6161
get { return nominal_base_type != null ? nominal_base_type.FullNameCorrected () : null; }
62+
set { throw new NotImplementedException (); }
6263
}
6364

6465
public override bool IsAbstract {
@@ -121,6 +122,7 @@ public override bool IsFinal {
121122

122123
public override string BaseType {
123124
get { return base_type; }
125+
set { base_type = value; }
124126
}
125127
}
126128

@@ -164,7 +166,7 @@ public IList<Ctor> Ctors {
164166
get { return ctors; }
165167
}
166168

167-
public abstract string BaseType { get; }
169+
public abstract string BaseType { get; set; }
168170

169171
public bool ContainsCtor (string jni_sig)
170172
{
@@ -236,6 +238,20 @@ protected override bool OnValidate (CodeGenerationOptions opt, GenericParameterD
236238

237239
return true;
238240
}
241+
242+
public override void FixupAccessModifiers ()
243+
{
244+
while (!IsAnnotation && !string.IsNullOrEmpty (BaseType)) {
245+
var baseClass = SymbolTable.Lookup (BaseType) as ClassGen;
246+
if (baseClass != null && RawVisibility == "public" && baseClass.RawVisibility != "public") {
247+
BaseType = baseClass.BaseType;
248+
} else {
249+
break;
250+
}
251+
}
252+
253+
base.FixupAccessModifiers ();
254+
}
239255

240256
public override void FixupExplicitImplementation ()
241257
{

‎tools/generator/CodeGenerator.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,8 @@ static void Validate (List<GenBase> gens, CodeGenerationOptions opt)
400400
removed.Clear ();
401401
foreach (GenBase gen in gens)
402402
gen.ResetValidation ();
403+
foreach (GenBase gen in gens)
404+
gen.FixupAccessModifiers ();
403405
foreach (GenBase gen in gens)
404406
if ((opt.IgnoreNonPublicType &&
405407
(gen.RawVisibility != "public" && gen.RawVisibility != "internal"))

‎tools/generator/GenBase.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -727,6 +727,12 @@ public void StripNonBindables ()
727727
n.StripNonBindables ();
728728
}
729729

730+
public virtual void FixupAccessModifiers ()
731+
{
732+
foreach (var nt in NestedTypes)
733+
nt.FixupAccessModifiers ();
734+
}
735+
730736
public void FillProperties ()
731737
{
732738
if (property_filled)

‎tools/generator/Tests/AccessModifiers.cs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
using System;
2+
using NUnit.Framework;
3+
4+
namespace generatortests
5+
{
6+
[TestFixture]
7+
public class AccessModifiers : BaseGeneratorTest
8+
{
9+
[Test]
10+
public void GeneratedOK ()
11+
{
12+
RunAllTargets (
13+
outputRelativePath: "AccessModifiers",
14+
apiDescriptionFile: "expected/AccessModifiers/AccessModifiers.xml",
15+
expectedRelativePath: "AccessModifiers",
16+
additionalSupportPaths: null);
17+
}
18+
}
19+
}
20+

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
3+
<PropertyGroup>
4+
<DefineConstants>$(DefineConstants);ANDROID_1;ANDROID_2;ANDROID_3;ANDROID_4</DefineConstants>
5+
</PropertyGroup>
6+
<!-- Classes -->
7+
<ItemGroup>
8+
<Compile Include="$(MSBuildThisFileDirectory)\Java.Interop.__TypeRegistrations.cs" />
9+
<Compile Include="$(MSBuildThisFileDirectory)\Java.Lang.Object.cs" />
10+
<Compile Include="$(MSBuildThisFileDirectory)\Xamarin.Test.ExtendPublicClass.cs" />
11+
<Compile Include="$(MSBuildThisFileDirectory)\Xamarin.Test.PublicClass.cs" />
12+
<Compile Include="$(MSBuildThisFileDirectory)\__NamespaceMapping__.cs" />
13+
</ItemGroup>
14+
<!-- Enums -->
15+
<ItemGroup />
16+
</Project>

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

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

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

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

‎tools/generator/Tests/expected.targets‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,14 @@
11
<Project DefaultTargets="Build" ToolsVersion="4.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
22
<ItemGroup>
3+
<Content Include='expected\AccessModifiers\AccessModifiers.xml'>
4+
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
5+
</Content>
6+
<Content Include='expected\AccessModifiers\Xamarin.Test.ExtendPublicClass.cs'>
7+
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
8+
</Content>
9+
<Content Include='expected\AccessModifiers\Xamarin.Test.PublicClass.cs'>
10+
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
11+
</Content>
312
<Content Include='expected\Adapters\Adapters.xml'>
413
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
514
</Content>
@@ -124,6 +133,15 @@
124133
<Content Include='expected\java.util.List\Xamarin.Test.SomeObject.cs'>
125134
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
126135
</Content>
136+
<Content Include='expected.ji\AccessModifiers\Mono.Android.projitems'>
137+
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
138+
</Content>
139+
<Content Include='expected.ji\AccessModifiers\Xamarin.Test.ExtendPublicClass.cs'>
140+
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
141+
</Content>
142+
<Content Include='expected.ji\AccessModifiers\Xamarin.Test.PublicClass.cs'>
143+
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
144+
</Content>
127145
<Content Include='expected.ji\Adapters\enumlist'>
128146
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
129147
</Content>

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
<?xml version="1.0" encoding="UTF-8" ?>
2+
<api>
3+
<package name="java.lang">
4+
<class abstract="false" deprecated="not deprecated" final="false" name="Object" static="false" visibility="public">
5+
</class>
6+
</package>
7+
<package name="xamarin.test">
8+
<!--
9+
abstract class PackageClass {
10+
public abstract void foo();
11+
}
12+
-->
13+
<class abstract="true" deprecated="not deprecated" extends="java.lang.Object" extends-generic-aware="java.lang.Object" final="false" name="PackageClass" static="false" visibility="">
14+
<method abstract="true" deprecated="not deprecated" final="false" name="foo" native="false" return="void" static="false" synchronized="false" visibility="public">
15+
</method>
16+
</class>
17+
<!--
18+
public class PublicClass extends PackageClass {
19+
@Override
20+
public void foo();
21+
}
22+
-->
23+
<class abstract="false" deprecated="not deprecated" extends="xamarin.test.PackageClass" extends-generic-aware="xamarin.test.PackageClass" final="false" name="PublicClass" static="false" visibility="public">
24+
<constructor deprecated="not deprecated" final="false" name="PublicClass" static="false" visibility="public" />
25+
<method abstract="false" deprecated="not deprecated" final="false" name="foo" native="false" return="void" static="false" synchronized="false" visibility="public">
26+
</method>
27+
</class>
28+
<!--
29+
abstract class ExtendPackageClass extends PackageClass {
30+
}
31+
-->
32+
<class abstract="true" deprecated="not deprecated" extends="xamarin.test.PackageClass" extends-generic-aware="xamarin.test.PackageClass" final="false" name="ExtendPackageClass" static="false" visibility="">
33+
</class>
34+
<!--
35+
public class ExtendPublicClass extends ExtendPackageClass {
36+
@Override
37+
public void foo();
38+
}
39+
-->
40+
<class abstract="false" deprecated="not deprecated" extends="xamarin.test.ExtendPackageClass" extends-generic-aware="xamarin.test.ExtendPackageClass" final="false" name="ExtendPublicClass" static="false" visibility="public">
41+
<constructor deprecated="not deprecated" final="false" name="ExtendPublicClass" static="false" visibility="public" />
42+
<method abstract="false" deprecated="not deprecated" final="false" name="foo" native="false" return="void" static="false" synchronized="false" visibility="public">
43+
</method>
44+
</class>
45+
</package>
46+
</api>

0 commit comments

Comments
 (0)