Skip to content

Commit

Permalink
[generator] Add nullable reference types (NRT) support. (#563)
Browse files Browse the repository at this point in the history
Fixes: #468
Context: dotnet/android#4227

Add [C#8 nullable reference type][0] (NRT) support to `generator` when
given `generator -lang-features=nullable-reference-types`.  This uses a
variety of Java annotations to infer nullable information (18c29b7)
via the `//method/@return-not-null` and `//parameter/@not-null`
attribute values within `api.xml` to "forward" nullability information
to the generated C# binding.

~~ Goals ~~

`generator` should be able to interpret the nullable annotations
provided by an input `.jar` file (via `class-parse`).  It should use 
this information to generate bindings that expose similar nullable
annotations on the produced public C# API.

For example, this Java:

	// Java
	public class Foo {
	    public void bar (@NotNull Object baz, String value) { … }
	}

Should generate this C# API:

	// C# Binding
	public class Foo : Java.Lang.Object {
	    public void Bar (Java.Lang.Object baz, string? value) { … }
	}

Additionally, the generated binding code should not produce any
additional warnings *on its own*.  That is, the internal plumbing code
itself should not create warnings.


~~ Non-Goals ~~

There exists cases in our generated plumbing code that do not play
nicely with the provability of C#8 nullable reference types.
For example, we may generate code like this:

	int Java.Lang.IComparable.CompareTo (Java.Lang.Object o)
	{
	    return CompareTo (global::Java.Interop.JavaObjectExtensions.JavaCast<Android.Util.Half>(o));
	}

Technically `.JavaCast<>()` can return `null`, which cannot be passed
to `.CompareTo (object o)` because it does not accept `null`.  In these
cases we liberally use the [null forgiving operator (`!`)][1] to
suppress warnings.  It may be desirable to change how this code is
structured to be better provably `null`-safe, however this PR does not
attempt to make those modification.  It is assumed that the code is
currently working, so `null` is prevented here via other mechanisms.

No functional changes are made to generated code.

Additionally, there are cases where Java nullable annotations can
create scenarios that will produce warnings in C#, particularly around
inheritance.  For example:

	// Java
	public class Base {
	    public void m (@NotNull Object baz) { … }
	}

	public class Derived extends Base {
	    @OverRide public void m (Object baz) { … }
	}

This would produce a C# warning such as:

	CS8610: Nullability of reference types in type of parameter 'M' doesn't match overridden member.  

`generator` will not attempt to resolve this error, it is an exercise
for the user.  This can be accomplished by fixing the Java code or
using `metadata` to override the `//@not-null` attribute such as:

	<attr path="/api/package[@name='blah']/class[@name='Foo2']/method[@name='Bar' and count(parameter)=1 and parameter[1][@type='object']]/parameter" name="not-null">true</attr>


~~ Unit Test Changes ~~

Several of the unit test "expected output" files changed their property
type from `java.lang.String` to `string`.  This occurred due to a
related refactoring of parameter & return type generation code.  This
change shouldn't be "user visible" because the unit tests don't go
through a "complete" pipeline which would involve ensuring that get-
and set-method pairs have consistent parameter & return types.

[0]: https://docs.microsoft.com/en-us/dotnet/csharp/nullable-references
[1]: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/operators/null-forgiving
  • Loading branch information
jpobst authored Apr 22, 2020
1 parent c19794e commit 01d0684
Show file tree
Hide file tree
Showing 99 changed files with 3,071 additions and 158 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
<AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath>
<LangVersion>8.0</LangVersion>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
</PropertyGroup>

Expand All @@ -23,6 +25,9 @@
<Compile Include="..\Java.Interop.Tools.TypeNameMappings\Java.Interop.Tools.TypeNameMappings\JavaNativeTypeManager.cs">
<Link>JavaNativeTypeManager.cs</Link>
</Compile>
<Compile Include="..\Java.Interop\NullableAttributes.cs">
<Link>NullableAttributes.cs</Link>
</Compile>
</ItemGroup>

<ItemGroup>
Expand Down

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions src/Java.Interop/Java.Interop.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
<DefineConstants>DEBUG;$(DefineConstants)</DefineConstants>
</PropertyGroup>
<ItemGroup>
<Compile Condition=" '$(TargetFramework)' != 'netstandard2.0' " Remove="NullableAttributes.cs" />
<Compile Remove="Java.Interop\JniLocationException.cs" />
</ItemGroup>
<PropertyGroup>
Expand Down
4 changes: 2 additions & 2 deletions src/Java.Interop/Java.Interop/JavaObjectArray.cs
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ public override IList<T> CreateGenericValue (ref JniObjectReference reference, J
});
}

public override JniValueMarshalerState CreateGenericObjectReferenceArgumentState (IList<T> value, ParameterAttributes synchronize)
public override JniValueMarshalerState CreateGenericObjectReferenceArgumentState ([MaybeNull]IList<T> value, ParameterAttributes synchronize)
{
return JavaArray<T>.CreateArgumentState (value, synchronize, (list, copy) => {
var a = copy
Expand All @@ -169,7 +169,7 @@ public override JniValueMarshalerState CreateGenericObjectReferenceArgumentState
});
}

public override void DestroyGenericArgumentState (IList<T> value, ref JniValueMarshalerState state, ParameterAttributes synchronize)
public override void DestroyGenericArgumentState ([AllowNull]IList<T> value, ref JniValueMarshalerState state, ParameterAttributes synchronize)
{
JavaArray<T>.DestroyArgumentState<JavaObjectArray<T>> (value, ref state, synchronize);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
Expand Down Expand Up @@ -203,7 +204,7 @@ public override JniValueMarshalerState CreateArgumentState (object? value, Param
throw new NotSupportedException ();
}

public override JniValueMarshalerState CreateGenericArgumentState (IntPtr value, ParameterAttributes synchronize)
public override JniValueMarshalerState CreateGenericArgumentState ([MaybeNull]IntPtr value, ParameterAttributes synchronize)
{
throw new NotSupportedException ();
}
Expand All @@ -213,7 +214,7 @@ public override JniValueMarshalerState CreateObjectReferenceArgumentState (objec
throw new NotSupportedException ();
}

public override JniValueMarshalerState CreateGenericObjectReferenceArgumentState (IntPtr value, ParameterAttributes synchronize)
public override JniValueMarshalerState CreateGenericObjectReferenceArgumentState ([MaybeNull]IntPtr value, ParameterAttributes synchronize)
{
throw new NotSupportedException ();
}
Expand Down
2 changes: 1 addition & 1 deletion src/Java.Interop/Java.Interop/JniRuntime.JniTypeManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ static Type GetUnderlyingType (Type type, out int rank)
}

// `type` will NOT be an array type.
protected virtual string GetSimpleReference (Type type)
protected virtual string? GetSimpleReference (Type type)
{
return GetSimpleReferences (type).FirstOrDefault ();
}
Expand Down
28 changes: 16 additions & 12 deletions src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ public virtual void DisposePeerUnlessReferenced (IJavaPeerable value)
DisposePeer (h, value);
}

public abstract IJavaPeerable PeekPeer (JniObjectReference reference);
public abstract IJavaPeerable? PeekPeer (JniObjectReference reference);

public object? PeekValue (JniObjectReference reference)
{
Expand Down Expand Up @@ -261,7 +261,7 @@ static Type GetPeerType (Type type)
return type;
}

public virtual IJavaPeerable CreatePeer (ref JniObjectReference reference, JniObjectReferenceOptions transfer, Type? targetType)
public virtual IJavaPeerable? CreatePeer (ref JniObjectReference reference, JniObjectReferenceOptions transfer, Type? targetType)
{
if (disposed)
throw new ObjectDisposedException (GetType ().Name);
Expand Down Expand Up @@ -396,7 +396,9 @@ public T CreateValue<T> (ref JniObjectReference reference, JniObjectReferenceOpt
targetType = targetType ?? typeof (T);

if (typeof (IJavaPeerable).IsAssignableFrom (targetType)) {
#pragma warning disable CS8601 // Possible null reference assignment.
return (T) JavaPeerableValueMarshaler.Instance.CreateGenericValue (ref reference, options, targetType);
#pragma warning restore CS8601 // Possible null reference assignment.
}

var marshaler = GetValueMarshaler<T> ();
Expand Down Expand Up @@ -473,7 +475,9 @@ public T GetValue<T> (ref JniObjectReference reference, JniObjectReferenceOption
}

if (typeof (IJavaPeerable).IsAssignableFrom (targetType)) {
#pragma warning disable CS8601 // Possible null reference assignment.
return (T) JavaPeerableValueMarshaler.Instance.CreateGenericValue (ref reference, options, targetType);
#pragma warning restore CS8601 // Possible null reference assignment.
}

var marshaler = GetValueMarshaler<T> ();
Expand Down Expand Up @@ -607,12 +611,12 @@ public override void DestroyArgumentState (object? value, ref JniValueMarshalerS
}
}

sealed class JavaPeerableValueMarshaler : JniValueMarshaler<IJavaPeerable> {
sealed class JavaPeerableValueMarshaler : JniValueMarshaler<IJavaPeerable?> {

internal static JavaPeerableValueMarshaler Instance = new JavaPeerableValueMarshaler ();

[return: MaybeNull]
public override IJavaPeerable CreateGenericValue (ref JniObjectReference reference, JniObjectReferenceOptions options, Type? targetType)
public override IJavaPeerable? CreateGenericValue (ref JniObjectReference reference, JniObjectReferenceOptions options, Type? targetType)
{
var jvm = JniEnvironment.Runtime;
var marshaler = jvm.ValueManager.GetValueMarshaler (targetType ?? typeof(IJavaPeerable));
Expand All @@ -621,15 +625,15 @@ public override IJavaPeerable CreateGenericValue (ref JniObjectReference referen
return jvm.ValueManager.CreatePeer (ref reference, options, targetType);
}

public override JniValueMarshalerState CreateGenericObjectReferenceArgumentState (IJavaPeerable value, ParameterAttributes synchronize)
public override JniValueMarshalerState CreateGenericObjectReferenceArgumentState ([MaybeNull]IJavaPeerable? value, ParameterAttributes synchronize)
{
if (value == null || !value.PeerReference.IsValid)
return new JniValueMarshalerState ();
var r = value.PeerReference.NewLocalRef ();
return new JniValueMarshalerState (r);
}

public override void DestroyGenericArgumentState (IJavaPeerable value, ref JniValueMarshalerState state, ParameterAttributes synchronize)
public override void DestroyGenericArgumentState ([MaybeNull]IJavaPeerable? value, ref JniValueMarshalerState state, ParameterAttributes synchronize)
{
var r = state.ReferenceValue;
JniObjectReference.Dispose (ref r);
Expand Down Expand Up @@ -694,12 +698,12 @@ public override T CreateGenericValue (ref JniObjectReference reference, JniObjec
return (T) ValueMarshaler.CreateValue (ref reference, options, targetType ?? typeof (T))!;
}

public override JniValueMarshalerState CreateGenericObjectReferenceArgumentState (T value, ParameterAttributes synchronize)
public override JniValueMarshalerState CreateGenericObjectReferenceArgumentState ([MaybeNull]T value, ParameterAttributes synchronize)
{
return ValueMarshaler.CreateObjectReferenceArgumentState (value, synchronize);
}

public override void DestroyGenericArgumentState (T value, ref JniValueMarshalerState state, ParameterAttributes synchronize)
public override void DestroyGenericArgumentState ([AllowNull]T value, ref JniValueMarshalerState state, ParameterAttributes synchronize)
{
ValueMarshaler.DestroyArgumentState (value, ref state, synchronize);
}
Expand All @@ -720,12 +724,12 @@ public override Expression CreateReturnValueFromManagedExpression (JniValueMarsh
}
}

sealed class ProxyValueMarshaler : JniValueMarshaler<object> {
sealed class ProxyValueMarshaler : JniValueMarshaler<object?> {

internal static ProxyValueMarshaler Instance = new ProxyValueMarshaler ();

[return: MaybeNull]
public override object CreateGenericValue (ref JniObjectReference reference, JniObjectReferenceOptions options, Type? targetType)
public override object? CreateGenericValue (ref JniObjectReference reference, JniObjectReferenceOptions options, Type? targetType)
{
var jvm = JniEnvironment.Runtime;

Expand All @@ -748,7 +752,7 @@ public override object CreateGenericValue (ref JniObjectReference reference, Jni
return jvm.ValueManager.CreatePeer (ref reference, options, targetType);
}

public override JniValueMarshalerState CreateGenericObjectReferenceArgumentState (object value, ParameterAttributes synchronize)
public override JniValueMarshalerState CreateGenericObjectReferenceArgumentState ([MaybeNull]object? value, ParameterAttributes synchronize)
{
if (value == null)
return new JniValueMarshalerState ();
Expand All @@ -765,7 +769,7 @@ public override JniValueMarshalerState CreateGenericObjectReferenceArgumentState
return new JniValueMarshalerState (p!.PeerReference.NewLocalRef ());
}

public override void DestroyGenericArgumentState (object value, ref JniValueMarshalerState state, ParameterAttributes synchronize)
public override void DestroyGenericArgumentState (object? value, ref JniValueMarshalerState state, ParameterAttributes synchronize)
{
var vm = state.Extra as JniValueMarshaler;
if (vm != null) {
Expand Down
3 changes: 2 additions & 1 deletion src/Java.Interop/Java.Interop/JniStringValueMarshaler.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#nullable enable

using System;
using System.Diagnostics.CodeAnalysis;
using System.Linq.Expressions;
using System.Reflection;
using System.Runtime.CompilerServices;
Expand All @@ -18,7 +19,7 @@ sealed class JniStringValueMarshaler : JniValueMarshaler<string?> {
return JniEnvironment.Strings.ToString (ref reference, options, targetType ?? typeof (string));
}

public override JniValueMarshalerState CreateGenericObjectReferenceArgumentState (string? value, ParameterAttributes synchronize)
public override JniValueMarshalerState CreateGenericObjectReferenceArgumentState ([MaybeNull]string? value, ParameterAttributes synchronize)
{
var r = JniEnvironment.Strings.NewString (value);
return new JniValueMarshalerState (r);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ int Count {
[Register ("set_Count", "(I)V", "Getset_Count_IHandler:java.code.IMyInterfaceInvoker, ")] set;
}

java.lang.String Key {
string Key {
// Metadata.xml XPath method reference: path="/api/package[@name='java.code']/interface[@name='IMyInterface']/method[@name='get_Key' and count(parameter)=0]"
[Register ("get_Key", "()Ljava/lang/String;", "Getget_KeyHandler:java.code.IMyInterfaceInvoker, ")] get;
// Metadata.xml XPath method reference: path="/api/package[@name='java.code']/interface[@name='IMyInterface']/method[@name='set_Key' and count(parameter)=1 and parameter[1][@type='java.lang.String']]"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public partial interface IMyInterface : IJavaObject, IJavaPeerable {
[Register ("set_Count", "(I)V", "Getset_Count_IHandler:java.code.IMyInterfaceInvoker, ")] set;
}

java.lang.String Key {
string Key {
// Metadata.xml XPath method reference: path="/api/package[@name='java.code']/interface[@name='IMyInterface']/method[@name='get_Key' and count(parameter)=0]"
[Register ("get_Key", "()Ljava/lang/String;", "Getget_KeyHandler:java.code.IMyInterfaceInvoker, ")] get;
// Metadata.xml XPath method reference: path="/api/package[@name='java.code']/interface[@name='IMyInterface']/method[@name='set_Key' and count(parameter)=1 and parameter[1][@type='java.lang.String']]"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ public partial interface IMyInterface : IJavaObject, IJavaPeerable {
[Register ("set_Count", "(I)V", "Getset_Count_IHandler:java.code.IMyInterfaceInvoker, ")] set;
}

java.lang.String Key {
string Key {
// Metadata.xml XPath method reference: path="/api/package[@name='java.code']/interface[@name='IMyInterface']/method[@name='get_Key' and count(parameter)=0]"
[Register ("get_Key", "()Ljava/lang/String;", "Getget_KeyHandler:java.code.IMyInterfaceInvoker, ")] get;
// Metadata.xml XPath method reference: path="/api/package[@name='java.code']/interface[@name='IMyInterface']/method[@name='set_Key' and count(parameter)=1 and parameter[1][@type='java.lang.String']]"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator ()
{
return GetEnumerator ();
}

public System.Collections.Generic.IEnumerator<char> GetEnumerator ()
{
for (int i = 0; i < Length(); i++)
yield return CharAt (i);
}

Loading

0 comments on commit 01d0684

Please sign in to comment.