Skip to content

[generator] Add nullable reference types (NRT) support. #563

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Apr 22, 2020
Merged

[generator] Add nullable reference types (NRT) support. #563

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
d87725b
[WIP] [generator] Enable NRT support.
jpobst Feb 4, 2020
8c26a3c
Add a little null forgiveness!
jpobst Feb 6, 2020
3714b13
[J.I.T.JavaCallableWrappers] Enable NRT support.
jpobst Mar 30, 2020
da1ec79
More Java.Interop nullability work.
jpobst Mar 31, 2020
9f7d634
Couple more nullability fixes.
jpobst Apr 2, 2020
c199e1f
Guard some nullability operators.
jpobst Apr 2, 2020
758b0df
Guarding fixes and don't default NRT to on.
jpobst Apr 3, 2020
57df7b7
Various fixes.
jpobst Apr 8, 2020
95224d8
More fixes.
jpobst Apr 8, 2020
ce1b2dc
[TypeNameMappings] Fix NRT warnings caused by an annotated BCL.
jpobst Apr 9, 2020
8871a63
[generator] Fix generated warning caused by annotated BCL.
jpobst Apr 9, 2020
26ddbe9
[generator] Add NRT test suite.
jpobst Apr 10, 2020
7be59de
Merge branch 'master' into generator-nrt
jpobst Apr 21, 2020
745589b
Remove attributes added to generated files.
jpobst Apr 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers.csproj
View file
Open in desktop
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
90 changes: 47 additions & 43 deletions ...terop.Tools.TypeNameMappings/Java.Interop.Tools.TypeNameMappings/JavaNativeTypeManager.cs
View file
Open in desktop

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions src/Java.Interop/Java.Interop.csproj
View file
Open in desktop
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
View file
Open in desktop
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
5 changes: 3 additions & 2 deletions src/Java.Interop/Java.Interop/JniRuntime.JniMarshalMemberBuilder.cs
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion ...nerator-Tests/Unit-Tests/CodeGeneratorExpectedResults/Common/WriteInterfaceProperties.txt
View file
Open in desktop
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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change concerns me: why did it occur? For that matter, how was it java.lang.String in the first place?

Fortunately this doesn't trigger any API breakage in Mono.Android.dll, as per https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=3648625&view=results

I just find this really weird. What change to generator is triggering this? Was it deliberate? What's the rationale?

I don't see this mentioned in the commit message.

Copy link
Contributor Author

@jpobst jpobst Apr 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change is that previously we were writing this type from property.Type, which is Setter != null ? Setter.Parameters [0].Type : Getter.ReturnType.

https://github.com/xamarin/java.interop/blob/master/tools/generator/Java.Interop.Tools.Generator.CodeGeneration/CodeGenerator.cs#L1690

This was refactored and combined with other places that were writing property.Getter.ReturnType, to a new method which always prefers Getter.ReturnType:

https://github.com/xamarin/java.interop/pull/563/files#diff-18357c6193fe262963d21bb1d0bab9feR99-R105

The test is a unit test and does not go through the full pipeline of resolving all the types, thus its Setter.Parameters [0].Type isn't changing from java.lang.String to string, but its Getter.ReturnType is getting resolved to string.

That is, the unit test code is incorrectly this:

  • Setter.Parameters [0].Type = java.lang.String
  • Getter.ReturnType = string

In the real world, Setter.Parameters [0].Type and Getter.ReturnType should always refer to the same symbol, so this should not affect real code.

// 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
2 changes: 1 addition & 1 deletion ...s/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteInterface.txt
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion ...-Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteInterfaceDeclaration.txt
View file
Open in desktop
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
11 changes: 11 additions & 0 deletions ...nit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1-NRT/WriteCharSequenceEnumerator.txt
View file
Open in desktop
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