Skip to content

Commit e85564c

Browse files
authored
[generator] Import NRT data from managed assemblies (#912)
Fixes: #857 Context: #917 Imagine you have a `Mono.Android.dll` build which has nullable reference types enabled, with a type declaration of: namespace Java.Lang { public partial interface IIterable { Java.Util.IIterator Iterator (); } } Then imagine that you bind a Java library which contains an interface which implements `java.lang.Iterable`, resulting in a binding: namespace My.Example { partial interface IMyIterable : Java.Lang.IIterable { } } `generator` also implements `IMyIterableInvoker`, to support invoking the `IMyIterable` interface. Unfortunately, because `IIterable` comes from IL, `generator` doesn't know about nullability, and thus assumes that *everything* can be nullable: namespace My.Example { partial class IMyIterableInvoker : Java.Lang.Object, IMyIterable { public Java.Util.IIterator? Iterator() {…} } } This results in a CS8766 warning: warning CS8766: Nullability of reference types in return type of 'IIterator? IMyIterableInvoker.Iterator()' doesn't match implicitly implemented member 'IIterator IIterable.Iterator()' (possibly because of nullability attributes). Fix this use-case by updating `Java.Interop.Tools.Cecil.dll` to know enough about the encoding of nullability information within [`NullableAttribute` and C# Nullable Metadata][0] to determine if a parameter type or return type can be null. This allows `generator` to appropriately emit: namespace My.Example { partial class IMyIterableInvoker : Java.Lang.Object, IMyIterable { public Java.Util.IIterator Iterator() {…} } } Note that our import doesn't support "nested nullability"; it only handles nullability of the "outermost type". It cannot differentiate between `java.lang.String[]?` and `java.lang.String[]?` (possibly null array that can have possibly null elements vs. possibly null array that can only have non-null elements). We don't currently consider this to be a problem because most of the Java annotations *also* don't support this form of differentiation. Kotlin *can*; `Array<String?>` differs from `Array<String?>?`. However, that's apparently not handled via Java annotations, but instead via the Kotlin Protobuf data. We don't feel a need to support nested nullability at this time. TODO: All the above said… we think we made a mistake in 01d0684 around the binding of arrays, and other "nested generic-like" types. We currently bind an annotation-less Java method such as `example(Object[])` as `Example(Java.Lang.Object[]?)`, but we should *actually* bind this as `Example(Java.Lang.Object?[]?)`! This is tracked in #917 [0]: https://github.com/dotnet/roslyn/blob/2da789af0f404e15871b6e6e17706185a2707ab4/docs/features/nullable-metadata.md
1 parent c476cc3 commit e85564c

File tree

5 files changed

+181
-4
lines changed

5 files changed

+181
-4
lines changed
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
using System;
2+
using System.Linq;
3+
using Mono.Cecil;
4+
using Mono.Collections.Generic;
5+
6+
namespace Java.Interop.Tools.Cecil
7+
{
8+
// Partial support for determining NRT status of method and field types.
9+
// Reference: https://github.com/dotnet/roslyn/blob/main/docs/features/nullable-metadata.md
10+
// The basics are supported, but advanced annotations like array elements,
11+
// type parameters, and tuples are not supported. Our use case doesn't really need them.
12+
public static class NullableReferenceTypesRocks
13+
{
14+
public static Nullability GetTypeNullability (this FieldDefinition field)
15+
{
16+
if (field.FieldType.FullName == "System.Void")
17+
return Nullability.NotNull;
18+
19+
// Look for explicit annotation on field
20+
var metadata = NullableMetadata.FromAttributeCollection (field.CustomAttributes);
21+
22+
if (metadata != null)
23+
return (Nullability) metadata.Data [0];
24+
25+
// Default nullability status for type
26+
return GetNullableContext (field.DeclaringType.CustomAttributes);
27+
}
28+
29+
public static Nullability GetReturnTypeNullability (this MethodDefinition method)
30+
{
31+
if (method.MethodReturnType.ReturnType.FullName == "System.Void")
32+
return Nullability.NotNull;
33+
34+
// Look for explicit annotation on return type
35+
var metadata = NullableMetadata.FromAttributeCollection (method.MethodReturnType.CustomAttributes);
36+
37+
if (metadata != null)
38+
return (Nullability) metadata.Data [0];
39+
40+
// Default nullability status for method
41+
var nullable = GetNullableContext (method.CustomAttributes);
42+
43+
if (nullable != Nullability.Oblivous)
44+
return nullable;
45+
46+
// Default nullability status for type
47+
return GetNullableContext (method.DeclaringType.CustomAttributes);
48+
}
49+
50+
public static Nullability GetTypeNullability (this ParameterDefinition parameter, MethodDefinition method)
51+
{
52+
if (parameter.ParameterType.FullName == "System.Void")
53+
return Nullability.NotNull;
54+
55+
// Look for explicit annotation on parameter
56+
var metadata = NullableMetadata.FromAttributeCollection (parameter.CustomAttributes);
57+
58+
if (metadata != null)
59+
return (Nullability) metadata.Data [0];
60+
61+
// Default nullability status for method
62+
var nullable = GetNullableContext (method.CustomAttributes);
63+
64+
if (nullable != Nullability.Oblivous)
65+
return nullable;
66+
67+
// Default nullability status for type
68+
return GetNullableContext (method.DeclaringType.CustomAttributes);
69+
}
70+
71+
static Nullability GetNullableContext (Collection<CustomAttribute> attrs)
72+
{
73+
var attribute = attrs.FirstOrDefault (t => t.AttributeType.FullName == "System.Runtime.CompilerServices.NullableContextAttribute");
74+
75+
if (attribute != null)
76+
return (Nullability) (byte) attribute.ConstructorArguments.First ().Value;
77+
78+
return Nullability.Oblivous;
79+
}
80+
}
81+
82+
public enum Nullability
83+
{
84+
Oblivous,
85+
NotNull,
86+
Nullable
87+
}
88+
89+
class NullableMetadata
90+
{
91+
public byte [] Data { get; private set; }
92+
93+
NullableMetadata (byte [] data) => Data = data;
94+
95+
NullableMetadata (byte data) => Data = new [] { data };
96+
97+
public static NullableMetadata? FromAttributeCollection (Collection<CustomAttribute> attrs)
98+
{
99+
if (attrs is null)
100+
return null;
101+
102+
var attribute = attrs.FirstOrDefault (t => t.AttributeType.FullName == "System.Runtime.CompilerServices.NullableAttribute");
103+
104+
if (attribute is null)
105+
return null;
106+
107+
var ctor_arg = attribute.ConstructorArguments.First ();
108+
109+
if (ctor_arg.Value is CustomAttributeArgument [] caa)
110+
ctor_arg = caa [0];
111+
112+
if (ctor_arg.Value is byte b)
113+
return new NullableMetadata (b);
114+
115+
if (ctor_arg.Value is byte [] b2)
116+
return new NullableMetadata (b2);
117+
118+
return null;
119+
}
120+
}
121+
}

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

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,28 @@ public Dictionary<T, List<string>> DoStuff (IEnumerable<KeyValuePair<T, List<Lis
6464
}
6565
}
6666

67+
namespace NullableTestTypes
68+
{
69+
#nullable enable
70+
public class NullableClass
71+
{
72+
[Register ("<init>", "(Ljava/lang/String;Ljava/lang/String;)", "")]
73+
public NullableClass (string notnull, string? nullable)
74+
{
75+
}
76+
77+
public string? null_field;
78+
public string not_null_field = string.Empty;
79+
80+
[Register ("nullable_return_method", "(Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;", "")]
81+
public string? NullableReturnMethod (string notnull, string? nullable) => null;
82+
83+
[Register ("not_null_return_method", "(Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;", "")]
84+
public string NotNullReturnMethod (string notnull, string? nullable) => string.Empty;
85+
}
86+
}
87+
#nullable disable
88+
6789
namespace generatortests
6890
{
6991
[TestFixture]
@@ -250,5 +272,32 @@ public void StripArity ()
250272
Assert.AreEqual ("System.Collections.Generic.Dictionary<T,System.Collections.Generic.List<System.String>>", @class.Methods [0].ReturnType);
251273
Assert.AreEqual ("System.Collections.Generic.Dictionary<T,System.Collections.Generic.List<System.String>>", @class.Methods [0].ManagedReturn);
252274
}
275+
276+
[Test]
277+
public void TypeNullability ()
278+
{
279+
var type = module.GetType ("NullableTestTypes.NullableClass");
280+
var gen = CecilApiImporter.CreateClass (module.GetType ("NullableTestTypes.NullableClass"), options);
281+
282+
var not_null_field = CecilApiImporter.CreateField (type.Fields.First (f => f.Name == "not_null_field"));
283+
Assert.AreEqual (true, not_null_field.NotNull);
284+
285+
var null_field = CecilApiImporter.CreateField (type.Fields.First (f => f.Name == "null_field"));
286+
Assert.AreEqual (false, null_field.NotNull);
287+
288+
var null_method = CecilApiImporter.CreateMethod (gen, type.Methods.First (f => f.Name == "NullableReturnMethod"));
289+
Assert.AreEqual (false, null_method.ReturnNotNull);
290+
Assert.AreEqual (true, null_method.Parameters.First (f => f.Name == "notnull").NotNull);
291+
Assert.AreEqual (false, null_method.Parameters.First (f => f.Name == "nullable").NotNull);
292+
293+
var not_null_method = CecilApiImporter.CreateMethod (gen, type.Methods.First (f => f.Name == "NotNullReturnMethod"));
294+
Assert.AreEqual (true, not_null_method.ReturnNotNull);
295+
Assert.AreEqual (true, not_null_method.Parameters.First (f => f.Name == "notnull").NotNull);
296+
Assert.AreEqual (false, not_null_method.Parameters.First (f => f.Name == "nullable").NotNull);
297+
298+
var ctor = CecilApiImporter.CreateCtor (gen, type.Methods.First (f => f.Name == ".ctor"));
299+
Assert.AreEqual (true, ctor.Parameters.First (f => f.Name == "notnull").NotNull);
300+
Assert.AreEqual (false, ctor.Parameters.First (f => f.Name == "nullable").NotNull);
301+
}
253302
}
254303
}

tests/generator-Tests/generator-Tests.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
<TargetFrameworks>net472;net6.0</TargetFrameworks>
55
<IsPackable>false</IsPackable>
66
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
7+
<LangVersion>8.0</LangVersion>
78
</PropertyGroup>
89

910
<PropertyGroup>

tools/generator/Extensions/ManagedExtensions.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
using Java.Interop.Tools.TypeNameMappings;
1+
using Java.Interop.Tools.Cecil;
2+
using Java.Interop.Tools.TypeNameMappings;
23
using Mono.Cecil;
34
using System.Collections.Generic;
45
using System.Linq;
@@ -42,7 +43,9 @@ public static IEnumerable<Parameter> GetParameters (this MethodDefinition m, Cus
4243
// custom enum types and cannot simply use JNI signature here.
4344
var rawtype = e?.Current.Type;
4445
var type = p.ParameterType.FullName == "System.IO.Stream" && e != null ? e.Current.Type : null;
45-
yield return CecilApiImporter.CreateParameter (p, type, rawtype);
46+
var isNotNull = p.GetTypeNullability (m) == Nullability.NotNull;
47+
48+
yield return CecilApiImporter.CreateParameter (p, type, rawtype, isNotNull);
4649
}
4750
}
4851

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System;
22
using System.Linq;
3+
using Java.Interop.Tools.Cecil;
34
using Java.Interop.Tools.TypeNameMappings;
45
using Mono.Cecil;
56
using Mono.Collections.Generic;
@@ -101,6 +102,7 @@ public static Field CreateField (FieldDefinition f)
101102
IsStatic = f.IsStatic,
102103
JavaName = reg_attr != null ? ((string) reg_attr.ConstructorArguments [0].Value).Replace ('/', '.') : f.Name,
103104
Name = f.Name,
105+
NotNull = f.GetTypeNullability () == Nullability.NotNull,
104106
TypeName = f.FieldType.FullNameCorrected ().StripArity (),
105107
Value = f.Constant == null ? null : f.FieldType.FullName == "System.String" ? '"' + f.Constant.ToString () + '"' : f.Constant.ToString (),
106108
Visibility = f.IsPublic ? "public" : f.IsFamilyOrAssembly ? "protected internal" : f.IsFamily ? "protected" : f.IsAssembly ? "internal" : "private"
@@ -198,6 +200,7 @@ public static Method CreateMethod (GenBase declaringType, MethodDefinition m)
198200
JavaName = reg_attr != null ? ((string) reg_attr.ConstructorArguments [0].Value) : m.Name,
199201
ManagedReturn = m.ReturnType.FullNameCorrected ().StripArity ().FilterPrimitive (),
200202
Return = m.ReturnType.FullNameCorrected ().StripArity ().FilterPrimitive (),
203+
ReturnNotNull = m.GetReturnTypeNullability () == Nullability.NotNull,
201204
Visibility = m.Visibility ()
202205
};
203206

@@ -221,12 +224,12 @@ public static Method CreateMethod (GenBase declaringType, MethodDefinition m)
221224
return method;
222225
}
223226

224-
public static Parameter CreateParameter (ParameterDefinition p, string jnitype, string rawtype)
227+
public static Parameter CreateParameter (ParameterDefinition p, string jnitype, string rawtype, bool isNotNull)
225228
{
226229
// FIXME: safe to use CLR type name? assuming yes as we often use it in metadatamap.
227230
// FIXME: IsSender?
228231
var isEnumType = GetGeneratedEnumAttribute (p.CustomAttributes) != null;
229-
return new Parameter (TypeNameUtilities.MangleName (p.Name), jnitype ?? p.ParameterType.FullNameCorrected ().StripArity (), null, isEnumType, rawtype);
232+
return new Parameter (TypeNameUtilities.MangleName (p.Name), jnitype ?? p.ParameterType.FullNameCorrected ().StripArity (), null, isEnumType, rawtype, isNotNull);
230233
}
231234

232235
public static Parameter CreateParameter (string managedType, string javaType)

0 commit comments

Comments
 (0)