[generator] [Register] interface alternatives into a dummy package (#1242)

jpobst · web-flow · commit 001a03e4595d · 2024-08-21T17:07:56.000-04:00
Fixes: #802 Context: #798 (comment) Context: a3b7456 C# and Java are (famously) different languages, and have (had?) different opinions about e.g. what could be within an `interface` declaration. Consider `java.lang.reflect.Member`: // Java public interface Member { public static final int PUBLIC = 0; public static final int DECLARED = 1; public Class<?> getDeclaringClass(); public String getName(); public int getModifiers(); public boolean isSynthetic(); } How do we bind this in C#? Before C# 8 introduced default interface members, we would bind it as *two* types: the interface binding, and an "interface alternative": // C# 7 [Register ("java/lang/reflect/Member", "", "Java.Lang.Reflect.IMemberInvoker")] public interface IMember : IJavaPeerable { Java.Lang.Class DeclaringClass {get;} bool IsSynthetic {get;} int Modifiers {get;} string Name {get;} } // interface alternative [Register ("java/lang/reflect/Member", DoNotGenerateAcw=true)] public abstract class Member : Java.Lang.Object { internal Member(); /* prevent subclassing */ public const int Declared = 1; public const int Public = 0; } An "interface alternative" is a class with the same name as the Java class, containing all the interface members which couldn't be placed within the interface declaration: static fields, static methods, …. C# 8 allowed `interface`s to contain non-instance members, and with commit a3b7456 this could instead be bound as: // C# 8 [Register ("java/lang/reflect/Member", "", "Java.Lang.Reflect.IMemberInvoker")] public partial interface IMember : Java.Interop.IJavaPeerable { public const int Declared = 1; public const int Public = 0; Java.Lang.Class DeclaringClass {get;} bool IsSynthetic {get;} int Modifiers {get;} string Name {get;} } // No interface alternative However, while this was *possible*, removing the "interface alternative" would be an API break, and thus was retained for existing types (bd7c60a). There is one issue with the interface alternative, as previously declared: the `[Register]` attribute was present, and declared the same Java name as the interface binding: [Register ("java/lang/reflect/Member", "", "Java.Lang.Reflect.IMemberInvoker")] public partial interface IMember {} [Register ("java/lang/reflect/Member", DoNotGenerateAcw=true)] public partial class Member {} which in turn means that often there isn't a 1:1 mapping between Java types and C# types, but instead a 1:2 mapping, an ambiguity. This in turn can creep into [binding projects][0] and cause the generation of invalid code: public partial class JSONArray : global::Com.Alibaba.Fastjson.JSON , global::Java.IO.ISerializable , global::Java.Lang.ICloneable , global::Java.Util.List , global::Java.Util.IRandomAccess {} `Java.Util.List` is the interface alternative for the `java.util.List` interface type, which is a class. The above thus fails to compile with a CS1721 ("Class 'JSONArray' cannot have multiple base classes: 'JSON' and 'List'"). `JSONArray` should instead have implemented the `Java.Util.IList` *interface*, but didn't because of this ambiguity. Remove this ambiguity by instead registering a "fake" class for interface alternatives, using a `mono/internal/` prefix: // interface declaration is unchanged [Register ("java/lang/reflect/Member", "", "Java.Lang.Reflect.IMemberInvoker")] public partial interface IMember {} // interface alternative registers a non-existent type [Register ("mono/internal/java/lang/reflect/Member", DoNotGenerateAcw=true)] public partial class Member {} *Note*: `[Register]` must still be used, because if we remove `[Register]` entirely, then `jcw-gen` will attempt to create a new Java Callable Wrapper for it, as interface alternatives are still `Java.Lang.Object` subclasses. [0]: dotnet/android#9188 (comment)
diff --git a/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1-NRT/WriteInterface.txt b/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1-NRT/WriteInterface.txt
@@ -1,4 +1,4 @@
-[Register ("java/code/IMyInterface", DoNotGenerateAcw=true)]
+[Register ("mono/internal/java/code/IMyInterface", DoNotGenerateAcw=true)]
 public abstract class MyInterface : Java.Lang.Object {
 	internal MyInterface ()
 	{
@@ -19,7 +19,7 @@ public abstract class MyInterface : Java.Lang.Object {
 
 }
 
-[Register ("java/code/IMyInterface", DoNotGenerateAcw=true)]
+[Register ("mono/internal/java/code/IMyInterface", DoNotGenerateAcw=true)]
 [global::System.Obsolete (@"Use the 'MyInterface' type. This type will be removed in a future release.", error: true)]
 public abstract class MyInterfaceConsts : MyInterface {
 	private MyInterfaceConsts ()
diff --git a/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/ObsoleteInterfaceAlternativeClass.txt b/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/ObsoleteInterfaceAlternativeClass.txt
@@ -1,4 +1,4 @@
-[Register ("com/xamarin/android/Parent", DoNotGenerateAcw=true)]
+[Register ("mono/internal/com/xamarin/android/Parent", DoNotGenerateAcw=true)]
 [global::System.Obsolete (@"Use the 'Com.Xamarin.Android.IParent' type. This class will be removed in a future release.")]
 public abstract class Parent : Java.Lang.Object {
 	internal Parent ()
diff --git a/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteConstSugarInterfaceFields.txt b/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteConstSugarInterfaceFields.txt
@@ -1,4 +1,4 @@
-[Register ("java/code/IMyInterface", DoNotGenerateAcw=true)]
+[Register ("mono/internal/java/code/IMyInterface", DoNotGenerateAcw=true)]
 public abstract class MyInterface : Java.Lang.Object {
 	internal MyInterface ()
 	{
diff --git a/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteInterface.txt b/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteInterface.txt
@@ -1,4 +1,4 @@
-[Register ("java/code/IMyInterface", DoNotGenerateAcw=true)]
+[Register ("mono/internal/java/code/IMyInterface", DoNotGenerateAcw=true)]
 public abstract class MyInterface : Java.Lang.Object {
 	internal MyInterface ()
 	{
@@ -19,7 +19,7 @@ public abstract class MyInterface : Java.Lang.Object {
 
 }
 
-[Register ("java/code/IMyInterface", DoNotGenerateAcw=true)]
+[Register ("mono/internal/java/code/IMyInterface", DoNotGenerateAcw=true)]
 [global::System.Obsolete (@"Use the 'MyInterface' type. This type will be removed in a future release.", error: true)]
 public abstract class MyInterfaceConsts : MyInterface {
 	private MyInterfaceConsts ()
diff --git a/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteInterfaceFieldAsDimProperty.txt b/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteInterfaceFieldAsDimProperty.txt
@@ -1,4 +1,4 @@
-[Register ("com/xamarin/android/MyInterface", DoNotGenerateAcw=true)]
+[Register ("mono/internal/com/xamarin/android/MyInterface", DoNotGenerateAcw=true)]
 public abstract class MyInterface : Java.Lang.Object {
 	internal MyInterface ()
 	{
@@ -32,7 +32,7 @@ public abstract class MyInterface : Java.Lang.Object {
 
 }
 
-[Register ("com/xamarin/android/MyInterface", DoNotGenerateAcw=true)]
+[Register ("mono/internal/com/xamarin/android/MyInterface", DoNotGenerateAcw=true)]
 [global::System.Obsolete (@"Use the 'MyInterface' type. This type will be removed in a future release.", error: true)]
 public abstract class MyInterfaceConsts : MyInterface {
 	private MyInterfaceConsts ()
diff --git a/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteStaticInterfaceMethod.txt b/tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteStaticInterfaceMethod.txt
@@ -1,4 +1,4 @@
-[Register ("java/code/IMyInterface", DoNotGenerateAcw=true)]
+[Register ("mono/internal/java/code/IMyInterface", DoNotGenerateAcw=true)]
 public abstract class MyInterface : Java.Lang.Object {
 	internal MyInterface ()
 	{
@@ -19,7 +19,7 @@ public abstract class MyInterface : Java.Lang.Object {
 
 }
 
-[Register ("java/code/IMyInterface", DoNotGenerateAcw=true)]
+[Register ("mono/internal/java/code/IMyInterface", DoNotGenerateAcw=true)]
 [global::System.Obsolete (@"Use the 'MyInterface' type. This type will be removed in a future release.", error: true)]
 public abstract class MyInterfaceConsts : MyInterface {
 	private MyInterfaceConsts ()
diff --git a/tests/generator-Tests/expected.xaji/TestInterface/Test.ME.ITestInterface.cs b/tests/generator-Tests/expected.xaji/TestInterface/Test.ME.ITestInterface.cs
@@ -5,7 +5,7 @@
 
 namespace Test.ME {
 
-	[Register ("test/me/TestInterface", DoNotGenerateAcw=true)]
+	[Register ("mono/internal/test/me/TestInterface", DoNotGenerateAcw=true)]
 	public abstract class TestInterface : Java.Lang.Object {
 		internal TestInterface ()
 		{
@@ -31,7 +31,7 @@ internal TestInterface ()
 
 	}
 
-	[Register ("test/me/TestInterface", DoNotGenerateAcw=true)]
+	[Register ("mono/internal/test/me/TestInterface", DoNotGenerateAcw=true)]
 	[global::System.Obsolete (@"Use the 'TestInterface' type. This type will be removed in a future release.", error: true)]
 	public abstract class TestInterfaceConsts : TestInterface {
 		private TestInterfaceConsts ()
diff --git a/tools/generator/SourceWriters/InterfaceMemberAlternativeClass.cs b/tools/generator/SourceWriters/InterfaceMemberAlternativeClass.cs
@@ -39,7 +39,8 @@ public InterfaceMemberAlternativeClass (InterfaceGen iface, CodeGenerationOption
 
 			SourceWriterExtensions.AddSupportedOSPlatform (Attributes, iface, opt);
 
-			Attributes.Add (new RegisterAttr (iface.RawJniName, noAcw: true, additionalProperties: iface.AdditionalAttributeString ()) { AcwLast = true });
+			// Place this class in a dummy package that won't conflict the actual interface
+			Attributes.Add (new RegisterAttr ("mono/internal/" + iface.RawJniName, noAcw: true, additionalProperties: iface.AdditionalAttributeString ()) { AcwLast = true });
 
 			if (should_obsolete)
 				SourceWriterExtensions.AddObsolete (Attributes, $"Use the '{iface.FullName}' type. This class will be removed in a future release.", opt);
@@ -179,7 +180,9 @@ public InterfaceConstsForwardClass (InterfaceGen iface, CodeGenerationOptions op
 			IsPublic = true;
 			IsAbstract = true;
 
-			Attributes.Add (new RegisterAttr (iface.RawJniName, noAcw: true, additionalProperties: iface.AdditionalAttributeString ()));
+			// Place this class in a dummy package that won't conflict the actual interface
+			Attributes.Add (new RegisterAttr ("mono/internal/" + iface.RawJniName, noAcw: true, additionalProperties: iface.AdditionalAttributeString ()));
+
 			SourceWriterExtensions.AddObsolete (Attributes, $"Use the '{iface.Name.Substring (1)}' type. This type will be removed in a future release.", opt, isError: true);
 
 			Constructors.Add (new ConstructorWriter {

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-[Register ("java/code/IMyInterface", DoNotGenerateAcw=true)]`
	`1`	`+[Register ("mono/internal/java/code/IMyInterface", DoNotGenerateAcw=true)]`
`2`	`2`	`public abstract class MyInterface : Java.Lang.Object {`
`3`	`3`	`internal MyInterface ()`
`4`	`4`	`{`
`@@ -19,7 +19,7 @@ public abstract class MyInterface : Java.Lang.Object {`
`19`	`19`
`20`	`20`	`}`
`21`	`21`
`22`		`-[Register ("java/code/IMyInterface", DoNotGenerateAcw=true)]`
	`22`	`+[Register ("mono/internal/java/code/IMyInterface", DoNotGenerateAcw=true)]`
`23`	`23`	`[global::System.Obsolete (@"Use the 'MyInterface' type. This type will be removed in a future release.", error: true)]`
`24`	`24`	`public abstract class MyInterfaceConsts : MyInterface {`
`25`	`25`	`private MyInterfaceConsts ()`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-[Register ("com/xamarin/android/Parent", DoNotGenerateAcw=true)]`
	`1`	`+[Register ("mono/internal/com/xamarin/android/Parent", DoNotGenerateAcw=true)]`
`2`	`2`	`[global::System.Obsolete (@"Use the 'Com.Xamarin.Android.IParent' type. This class will be removed in a future release.")]`
`3`	`3`	`public abstract class Parent : Java.Lang.Object {`
`4`	`4`	`internal Parent ()`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-[Register ("com/xamarin/android/MyInterface", DoNotGenerateAcw=true)]`
	`1`	`+[Register ("mono/internal/com/xamarin/android/MyInterface", DoNotGenerateAcw=true)]`
`2`	`2`	`public abstract class MyInterface : Java.Lang.Object {`
`3`	`3`	`internal MyInterface ()`
`4`	`4`	`{`
`@@ -32,7 +32,7 @@ public abstract class MyInterface : Java.Lang.Object {`
`32`	`32`
`33`	`33`	`}`
`34`	`34`
`35`		`-[Register ("com/xamarin/android/MyInterface", DoNotGenerateAcw=true)]`
	`35`	`+[Register ("mono/internal/com/xamarin/android/MyInterface", DoNotGenerateAcw=true)]`
`36`	`36`	`[global::System.Obsolete (@"Use the 'MyInterface' type. This type will be removed in a future release.", error: true)]`
`37`	`37`	`public abstract class MyInterfaceConsts : MyInterface {`
`38`	`38`	`private MyInterfaceConsts ()`
Original file line number	Diff line number	Diff line change
`@@ -5,7 +5,7 @@`
`5`	`5`
`6`	`6`	`namespace Test.ME {`
`7`	`7`
`8`		`- [Register ("test/me/TestInterface", DoNotGenerateAcw=true)]`
	`8`	`+ [Register ("mono/internal/test/me/TestInterface", DoNotGenerateAcw=true)]`
`9`	`9`	`public abstract class TestInterface : Java.Lang.Object {`
`10`	`10`	`internal TestInterface ()`
`11`	`11`	`{`
`@@ -31,7 +31,7 @@ internal TestInterface ()`
`31`	`31`
`32`	`32`	`}`
`33`	`33`
`34`		`- [Register ("test/me/TestInterface", DoNotGenerateAcw=true)]`
	`34`	`+ [Register ("mono/internal/test/me/TestInterface", DoNotGenerateAcw=true)]`
`35`	`35`	`[global::System.Obsolete (@"Use the 'TestInterface' type. This type will be removed in a future release.", error: true)]`
`36`	`36`	`public abstract class TestInterfaceConsts : TestInterface {`
`37`	`37`	`private TestInterfaceConsts ()`