Skip to content

Commit 855ecfa

Browse files
authored
[generator] Don't generate unexpected NRT types like void? (#856)
Context: #850 (comment) When a user binds a Java library that contains a Java interface which implements another Java interface: // Java; android.jar public interface /* android.view. */ Menu {…} // Java; androidx.core.core.jar public interface /* androidx.core.internal.view. */ SupportMenu implements android.view.Menu {…} then when we create the binding for the "leaf" interface: // C# binding for androidx.core.core namespace AndroidX.Core.Internal.View { public interface ISupportMenu : Android.Views.IMenu {…} internal partial class ISupportMenuInvoker : Java.Lang.Object, ISupportMenu { // … } } The generated `*Invoker` type implements all methods for all implemented interfaces, e.g. methods from both `IMenu` and `ISupportMenu`. When: 1. The base interface (e.g. `IMenu`) comes from a referenced assembly, *and* 2. `$(Nullable)`=Enable when binding the derived interface then we interpret the return types on imported interface methods as *always* being of a nullable type, even for types such as `void`: // C# binding for androidx.core.core with $(Nullable)=Enable partial class ISupportMenuInvoker : Java.Lang.Object, ISupportMenu { public unsafe void? Clear () {…} } This results in errors from the C# compiler: Error CS1519: Invalid token '?' in class, record, struct, or interface member declaration Error CS1520: Method must have a return type Error CS0535: 'ISupportMenuInvoker' does not implement interface member 'IMenu.Clear()' The culprit is twofold: - In `CecilApiImporter`, we set `Method.ManagedReturn` to `System.Void` instead of `void`. - In `CodeGenerationOptions`, we only check for `void` to omit the null operator, not `System.Void`. Note this also happens for all primitive types like `System.Int32`/`int`. This commit fixes both aspects: - Change `Method.ManagedReturn` to contain primitive types instead of fully qualified types. - Update `CodeGenerationOptions.GetNullable()` to correctly handle fully qualified types if one slips through. With this change, all of AndroidX/GooglePlayServices/FaceBook/MLKit can be successfully compiled with `<Nullable>enable</Nullable>`.
1 parent 4a02bc3 commit 855ecfa

File tree

5 files changed

+49
-12
lines changed

5 files changed

+49
-12
lines changed

tests/generator-Tests/Unit-Tests/CodeGenerationOptionsTests.cs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,5 +51,21 @@ public void GetOutputNameUseGlobal ()
5151
Assert.AreEqual ("global::System.Collections.Generic.List<global::System.Collections.Generic.List<string>.Enumerator[]>",
5252
opt.GetOutputName ("System.Collections.Generic.List<System.Collections.Generic.List<string>.Enumerator[]>"));
5353
}
54+
55+
[Test]
56+
public void GetTypeReferenceName_Nullable ()
57+
{
58+
var opt = new CodeGenerationOptions { SupportNullableReferenceTypes = true };
59+
60+
var system_void = new ReturnValue (null, "void", "System.Void", false, false);
61+
system_void.Validate (opt, null, null);
62+
63+
Assert.AreEqual ("void", opt.GetTypeReferenceName (system_void));
64+
65+
var primitive_void = new ReturnValue (null, "void", "void", false, false);
66+
primitive_void.Validate (opt, null, null);
67+
68+
Assert.AreEqual ("void", opt.GetTypeReferenceName (primitive_void));
69+
}
5470
}
5571
}

tests/generator-Tests/Unit-Tests/ManagedTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ public void Method ()
121121

122122
Assert.AreEqual ("public", method.Visibility);
123123
Assert.AreEqual ("void", method.Return);
124-
Assert.AreEqual ("System.Void", method.ReturnType);
124+
Assert.AreEqual ("void", method.ReturnType);
125125
Assert.AreEqual ("Bar", method.Name);
126126
Assert.AreEqual ("bar", method.JavaName);
127127
Assert.AreEqual ("()V", method.JniSignature);
@@ -165,7 +165,7 @@ public void MethodWithParameters ()
165165
Assert.IsTrue (method.Validate (new CodeGenerationOptions (), new GenericParameterDefinitionList (), new CodeGeneratorContext ()), "method.Validate failed!");
166166
Assert.AreEqual ("(ZID)Ljava/lang/String;", method.JniSignature);
167167
Assert.AreEqual ("java.lang.String", method.Return);
168-
Assert.AreEqual ("System.String", method.ManagedReturn);
168+
Assert.AreEqual ("string", method.ManagedReturn);
169169

170170
var parameter = method.Parameters [0];
171171
Assert.AreEqual ("a", parameter.Name);

tools/generator/CodeGenerationOptions.cs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -141,21 +141,23 @@ string GetNullable (string s)
141141
switch (s) {
142142
case "void":
143143
case "int":
144-
//case "int[]":
145144
case "bool":
146-
//case "bool[]":
147145
case "float":
148-
//case "float[]":
149146
case "sbyte":
150-
//case "sbyte[]":
151147
case "long":
152-
//case "long[]":
153148
case "char":
154-
//case "char[]":
155149
case "double":
156-
//case "double[]":
157150
case "short":
158-
//case "short[]":
151+
case "System.Boolean":
152+
case "System.Byte":
153+
case "System.Char":
154+
case "System.Double":
155+
case "System.Int16":
156+
case "System.Int32":
157+
case "System.Int64":
158+
case "System.Single":
159+
case "System.SByte":
160+
case "System.Void":
159161
case "Android.Graphics.Color":
160162
return string.Empty;
161163
}

tools/generator/Extensions/ManagedExtensions.cs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,25 @@ public static string StripArity (this string type)
6565

6666
return type;
6767
}
68+
69+
// Convert a fully qualified type like "System.Void" to the primitive type "void" if applicable
70+
public static string FilterPrimitive (this string type)
71+
{
72+
return type switch {
73+
"System.Boolean" => "bool",
74+
"System.Char" => "char",
75+
"System.Byte" => "byte",
76+
"System.SByte" => "byte",
77+
"System.Int16" => "short",
78+
"System.Int32" => "int",
79+
"System.Int64" => "long",
80+
"System.Single" => "float",
81+
"System.Double" => "double",
82+
"System.Void" => "void",
83+
"System.String" => "string",
84+
_ => type
85+
};
86+
}
6887
}
6988
#endif
7089
}

tools/generator/Java.Interop.Tools.Generator.Importers/CecilApiImporter.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,8 +196,8 @@ public static Method CreateMethod (GenBase declaringType, MethodDefinition m)
196196
IsStatic = m.IsStatic,
197197
IsVirtual = m.IsVirtual,
198198
JavaName = reg_attr != null ? ((string) reg_attr.ConstructorArguments [0].Value) : m.Name,
199-
ManagedReturn = m.ReturnType.FullNameCorrected ().StripArity (),
200-
Return = m.ReturnType.FullNameCorrected ().StripArity (),
199+
ManagedReturn = m.ReturnType.FullNameCorrected ().StripArity ().FilterPrimitive (),
200+
Return = m.ReturnType.FullNameCorrected ().StripArity ().FilterPrimitive (),
201201
Visibility = m.Visibility ()
202202
};
203203

0 commit comments

Comments
 (0)