Enable NRT for Java.Interop.dll (#554)

C# 8 introduced [Nullable Reference Types][0], which allows reuse of
the `nullable_type` grammar in which a `?` suffix on a type indicates
that the value may be null  -- which previously could only be placed
on value types -- so that it can be placed onto reference types.

As part of this C# change, within an `enabled` [nullable context][1],
values of reference types such as `string` are *not null* by default.
`string?` would need to be used to indicate that the value may be
`null`.

This change presents a potentially significant *semantic* change,
particularly if `-warnaserror` is used, as source code which
previously allowed `null` values can now elicit C# *warnings* if code
flow analysis/etc. indicates that a non-nullable value may be null.

Enable Nullable Reference Type support for `Java.Interop.dll`:

  * Set `$(Nullable)`=enable, so that nullable reference types are
    enabled project wide.

  * Perform the *minimal* review of the current API to indicate that
    certain parameters or return types may accept or return `null`
    values.

  * Use [additional custom attributes][2] to further "tighten" the
    semantics we're trying to describe.  In particular,
    *type parameters* are generally *non-null*, so if a method
    (1) has a return value which is a type parameter, and (2) the
    value returned may be null, then the method should be annotated
    with a [`MaybeNullAttribute`][3] to indicate that the value could
    be null even though the type appears to forbid it.

  * Add a `NullableAttributes.cs` file so that we can use the
    additional custom attributes, as these types may not be present
    in the BCL that we're building against.
    `INTERNAL_NULLABLE_ATTRIBUTES` is added to `$(DefineConstants)`
    so that these types are `internal` to `Java.Interop.dll`.

  * Update `JniRuntime` to move away from `ConcurrentDictionary` to
    `lock` + `Dictionary`, because `ConcurrentDictionary.TryUpdate()`
    didn't support passing `null` as a new value.  To do so, we'd have
    to move to `ConcurrentDictionary<IntPtr, JniRuntime?>`, which would
    alter the type of `Runtime.Values`, and change other things.

  * Update `JniPeerMembers` so that we no longer `null` out various
    members in `JniPeerMembers.Dispose()` and we instead call
    `InstanceFields.Clear()`.  This was easier than making everything
    nullable, and the `Dispose()` codepath really shouldn't ever be
    invoked anyway....

After all this, there are two remaining nullable-reference type
warnings produced by `Java.Interop.csproj`:

	src\Java.Interop\Java.Interop\JniRuntime.JniValueManager.cs(399,13): Warning CS8601: Possible null reference assignment.
	src\Java.Interop\Java.Interop\JniRuntime.JniValueManager.cs(476,13): Warning CS8601: Possible null reference assignment.

We currently do not know why this CS8601 is being emitted, and will
need to investigate further.

Additionally:

  * Bump to .NET Core 3.1.x which supports nullable reference types.

  * Install a newer `boots` tool which supports .Net Core 3.1.

[0]: https://docs.microsoft.com/en-us/dotnet/csharp/nullable-references
[1]: https://docs.microsoft.com/en-us/dotnet/csharp/nullable-references#nullable-contexts
[2]: https://docs.microsoft.com/en-us/dotnet/csharp/nullable-attributes#specify-post-conditions-maybenull-and-notnull
[3]: https://docs.microsoft.com/en-us/dotnet/api/system.diagnostics.codeanalysis.maybenullattribute?view=netcore-3.1

Author: jpobst

build-tools/automation/azure-pipelines.yaml

@@ -6,7 +6,7 @@ trigger:
 
 # Global variables
 variables:
-  DotNetCoreVersion: 2.1.701
+  DotNetCoreVersion: 3.1.x
   HostedMac: Hosted Mac Internal
   HostedWinVS2019: Hosted Windows 2019 with VS2019
 
@@ -80,7 +80,7 @@ jobs:
       version: $(DotNetCoreVersion)
 
   - script: |
-      dotnet tool install --global boots --version 1.0.0.291   
+      dotnet tool install --global boots  
       boots https://download.mono-project.com/archive/6.4.0/macos-10-universal/MonoFramework-MDK-6.4.0.198.macos10.xamarin.universal.pkg
     displayName: Install Mono 6.4
     

src/Java.Interop/GlobalSuppressions.cs

@@ -72,6 +72,10 @@
 
 [assembly: SuppressMessage ("Performance", "CA1822:Mark members as static", Justification = "<Pending>", Scope = "member", Target = "~M:Java.Interop.JniRuntime.JniMarshalMemberBuilder.IsDirectMethod(System.Reflection.ParameterInfo[])~System.Boolean")]
 [assembly: SuppressMessage ("Performance", "CA1822:Mark members as static", Justification = "<Pending>", Scope = "member", Target = "~M:Java.Interop.JniRuntime.JniValueManager.GetJniIdentityHashCode(Java.Interop.JniObjectReference)~System.Int32")]
+[assembly: SuppressMessage ("Performance", "CA1822:Mark members as static", Justification = "<Pending>", Scope = "member", Target = "~P:Java.Interop.JniFieldInfo.Name")]
+[assembly: SuppressMessage ("Performance", "CA1822:Mark members as static", Justification = "<Pending>", Scope = "member", Target = "~P:Java.Interop.JniFieldInfo.Signature")]
+[assembly: SuppressMessage ("Performance", "CA1822:Mark members as static", Justification = "<Pending>", Scope = "member", Target = "~P:Java.Interop.JniMethodInfo.Name")]
+[assembly: SuppressMessage ("Performance", "CA1822:Mark members as static", Justification = "<Pending>", Scope = "member", Target = "~P:Java.Interop.JniMethodInfo.Signature")]
 
 [assembly: SuppressMessage ("Performance", "CA1823:Avoid unused private fields", Justification = "Used for native interop", Scope = "type", Target = "~T:Java.Interop.JavaException")]
 [assembly: SuppressMessage ("Performance", "CA1823:Avoid unused private fields", Justification = "Used for native interop", Scope = "type", Target = "~T:Java.Interop.JavaObject")]
@@ -85,3 +89,5 @@
 
 [assembly: SuppressMessage ("Usage", "CA2208:Instantiate argument exceptions correctly", Justification = "<Pending>", Scope = "member", Target = "~M:Java.Interop.JniEnvironment.Exceptions.Throw(Java.Interop.JniObjectReference)")]
 [assembly: SuppressMessage ("Usage", "CA2208:Instantiate argument exceptions correctly", Justification = "<Pending>", Scope = "member", Target = "~M:Java.Interop.JniEnvironment.Exceptions.ThrowNew(Java.Interop.JniObjectReference,System.String)")]
+
+[assembly: SuppressMessage ("Usage", "CA2213:Disposable fields should be disposed", Justification = "<Pending>", Scope = "member", Target = "~F:Java.Interop.JniRuntime.valueManager")]

src/Java.Interop/Java.Interop-MonoAndroid.csproj

@@ -15,18 +15,20 @@
     <SignAssembly>true</SignAssembly>
     <AssemblyOriginatorKeyFile>..\..\product.snk</AssemblyOriginatorKeyFile>
     <BaseIntermediateOutputPath>obj-MonoAndroid\</BaseIntermediateOutputPath>
+    <LangVersion>8.0</LangVersion>
   </PropertyGroup>
   <Import Project="..\..\Configuration.props" />
   <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
     <DebugSymbols>true</DebugSymbols>
     <DebugType>full</DebugType>
     <Optimize>false</Optimize>
     <OutputPath>..\..\bin\Debug</OutputPath>
-    <DefineConstants>DEBUG;INTEROP;FEATURE_JNIENVIRONMENT_JI_PINVOKES;FEATURE_JNIOBJECTREFERENCE_INTPTRS</DefineConstants>
+    <DefineConstants>DEBUG;INTEROP;FEATURE_JNIENVIRONMENT_JI_PINVOKES;FEATURE_JNIOBJECTREFERENCE_INTPTRS;INTERNAL_NULLABLE_ATTRIBUTES</DefineConstants>
     <ErrorReport>prompt</ErrorReport>
     <WarningLevel>4</WarningLevel>
     <ConsolePause>false</ConsolePause>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <Nullable>enable</Nullable>
     <DocumentationFile>..\..\bin\Debug\Java.Interop.xml</DocumentationFile>
     <JNIEnvGenPath>..\..\bin\BuildDebug</JNIEnvGenPath>
   </PropertyGroup>
@@ -37,8 +39,9 @@
     <ErrorReport>prompt</ErrorReport>
     <WarningLevel>4</WarningLevel>
     <ConsolePause>false</ConsolePause>
-    <DefineConstants>INTEROP;FEATURE_JNIENVIRONMENT_JI_PINVOKES;FEATURE_JNIOBJECTREFERENCE_INTPTRS</DefineConstants>
+    <DefineConstants>INTEROP;FEATURE_JNIENVIRONMENT_JI_PINVOKES;FEATURE_JNIOBJECTREFERENCE_INTPTRS;INTERNAL_NULLABLE_ATTRIBUTES</DefineConstants>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <Nullable>enable</Nullable>
     <DocumentationFile>..\..\bin\Release\Java.Interop.xml</DocumentationFile>
     <JNIEnvGenPath>..\..\bin\BuildRelease</JNIEnvGenPath>
   </PropertyGroup>

src/Java.Interop/Java.Interop.csproj

@@ -2,30 +2,27 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
     <TargetFramework>netstandard2.0</TargetFramework>
-    <ProjectGuid>{94BD81F7-B06F-4295-9636-F8A3B6BDC762}</ProjectGuid>
     <NoWarn>1591</NoWarn>
     <SignAssembly>true</SignAssembly>
     <AssemblyOriginatorKeyFile>..\..\product.snk</AssemblyOriginatorKeyFile>
-    <DefineConstants>INTEROP;FEATURE_JNIENVIRONMENT_JI_PINVOKES;FEATURE_JNIOBJECTREFERENCE_INTPTRS</DefineConstants>
+    <DefineConstants>INTEROP;FEATURE_JNIENVIRONMENT_JI_PINVOKES;FEATURE_JNIOBJECTREFERENCE_INTPTRS;INTERNAL_NULLABLE_ATTRIBUTES</DefineConstants>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
     <GenerateAssemblyInfo>false</GenerateAssemblyInfo>
   </PropertyGroup>
   <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
     <OutputPath>..\..\bin\Debug</OutputPath>
     <DocumentationFile>..\..\bin\Debug\Java.Interop.xml</DocumentationFile>
     <JNIEnvGenPath>..\..\bin\BuildDebug</JNIEnvGenPath>
-    <DefineConstants>DEBUG;$(DefineConstants)</DefineConstants>
-  </PropertyGroup>
-  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Gendarme|AnyCPU' ">
-    <OutputPath>..\..\bin\GendarmeDebug</OutputPath>
-    <DefineConstants>DEBUG;$(DefineConstants)</DefineConstants>
-    <DocumentationFile>..\..\bin\Debug\Java.Interop.xml</DocumentationFile>
-    <JNIEnvGenPath>..\..\bin\BuildDebug</JNIEnvGenPath>
+    <DefineConstants>$(DefineConstants);DEBUG;NETSTANDARD;NETSTANDARD2_0</DefineConstants>
+    <LangVersion>8.0</LangVersion>
+    <Nullable>enable</Nullable>
   </PropertyGroup>
   <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
     <OutputPath>..\..\bin\Release</OutputPath>
     <DocumentationFile>..\..\bin\Release\Java.Interop.xml</DocumentationFile>
     <JNIEnvGenPath>..\..\bin\BuildRelease</JNIEnvGenPath>
+    <LangVersion>8.0</LangVersion>
+    <Nullable>enable</Nullable>
   </PropertyGroup>
   <ItemGroup>
     <Compile Remove="Java.Interop\JniLocationException.cs" />
@@ -64,9 +61,32 @@
       <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
     </PackageReference>
     <ProjectReference Include="..\..\build-tools\jnienv-gen\jnienv-gen.csproj">
-      <Project>{6410DA0F-5E14-4FC0-9AEE-F4C542C96C7A}</Project>
-      <Name>jnienv-gen</Name>
       <ReferenceOutputAssembly>False</ReferenceOutputAssembly>
     </ProjectReference>
   </ItemGroup>
+  <ItemGroup>
+    <Compile Condition=" '$(EnableDefaultCompileItems)' == 'true' " Update="Java.Interop\JavaPrimitiveArrays.cs">
+      <DependentUpon>JavaPrimitiveArrays.tt</DependentUpon>
+    </Compile>
+    <Compile Condition=" '$(EnableDefaultCompileItems)' == 'true' " Update="Java.Interop\JniBuiltinMarshalers.cs">
+      <DependentUpon>JniBuiltinMarshalers.tt</DependentUpon>
+    </Compile>
+    <Compile Condition=" '$(EnableDefaultCompileItems)' == 'true' " Update="Java.Interop\JniPeerMembers.JniFields.cs">
+      <DependentUpon>JniPeerMembers.JniFields.tt</DependentUpon>
+    </Compile>
+    <Compile Condition=" '$(EnableDefaultCompileItems)' == 'true' " Update="Java.Interop\JniPeerMembers.JniInstanceMethods_Invoke.cs">
+      <DependentUpon>JniPeerMembers.JniInstanceMethods_Invoke.tt</DependentUpon>
+    </Compile>
+  </ItemGroup>
+  <ProjectExtensions>
+    <MonoDevelop>
+      <Properties>
+        <Policies>
+          <VersionControlPolicy>
+            <CommitMessageStyle Indent="&#x9;" LineAlign="0" IncludeDirectoryPaths="True" />
+          </VersionControlPolicy>
+        </Policies>
+      </Properties>
+    </MonoDevelop>
+  </ProjectExtensions>
 </Project>

src/Java.Interop/Java.Interop/IJavaPeerable.cs

@@ -1,3 +1,5 @@
+#nullable enable
+
 using System;
 
 namespace Java.Interop

src/Java.Interop/Java.Interop/JavaArray.cs

@@ -1,7 +1,10 @@
+#nullable enable
+
 using System;
 using System.Collections;
 using System.Collections.Generic;
 using System.Diagnostics;
+using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using System.Reflection;
 
@@ -25,6 +28,7 @@ public int Length {
 			get {return JniEnvironment.Arrays.GetArrayLength (PeerReference);}
 		}
 
+		[MaybeNull]
 		public abstract T this [int index] {
 			get;
 			set;
@@ -56,7 +60,9 @@ public virtual IEnumerator<T> GetEnumerator ()
 		{
 			int len = Length;
 			for (int i = 0; i < len; ++i)
+#pragma warning disable CS8603 // Possible null reference return.
 				yield return this [i];
+#pragma warning restore CS8603 // Possible null reference return.
 		}
 
 		internal static void CheckArrayCopy (int sourceIndex, int sourceLength, int destinationIndex, int destinationLength, int length)
@@ -98,7 +104,7 @@ internal static IList<T> ToList (IEnumerable<T> value)
 			return value.ToList ();
 		}
 
-		internal IList<T> ToTargetType (Type targetType, bool dispose)
+		internal IList<T> ToTargetType (Type? targetType, bool dispose)
 		{
 			if (TargetTypeIsCurrentType (targetType))
 				return this;
@@ -113,26 +119,27 @@ internal IList<T> ToTargetType (Type targetType, bool dispose)
 			throw CreateMarshalNotSupportedException (GetType (), targetType);
 		}
 
-		internal virtual bool TargetTypeIsCurrentType (Type targetType)
+		internal virtual bool TargetTypeIsCurrentType ([NotNullWhen (false)]Type? targetType)
 		{
 			return targetType == null || targetType == typeof (JavaArray<T>);
 		}
 
-		internal static Exception CreateMarshalNotSupportedException (Type sourceType, Type targetType)
+		internal static Exception CreateMarshalNotSupportedException (Type sourceType, Type? targetType)
 		{
-			throw new NotSupportedException (
-					string.Format ("Do not know how to marshal a '{0}' into a '{1}'.",
-						sourceType.FullName, targetType.FullName));
+			return new NotSupportedException (
+					string.Format ("Do not know how to marshal a `{0}`{1}.",
+						sourceType.FullName,
+						targetType != null ? $" into a `{targetType.FullName}`" : ""));
 		}
 
-		internal static IList<T> CreateValue<TArray> (ref JniObjectReference reference, JniObjectReferenceOptions transfer, Type targetType, ArrayCreator<TArray> creator)
+		internal static IList<T> CreateValue<TArray> (ref JniObjectReference reference, JniObjectReferenceOptions transfer, Type? targetType, ArrayCreator<TArray> creator)
 			where TArray : JavaArray<T>
 		{
 			return creator (ref reference, transfer)
 				.ToTargetType (targetType, dispose: true);
 		}
 
-		internal    static  JniValueMarshalerState  CreateArgumentState<TArray> (IList<T> value, ParameterAttributes synchronize, Func<IList<T>, bool, TArray> creator)
+		internal    static  JniValueMarshalerState  CreateArgumentState<TArray> (IList<T>? value, ParameterAttributes synchronize, Func<IList<T>, bool, TArray> creator)
 			where TArray : JavaArray<T>
 		{
 			if (value == null)
@@ -149,10 +156,10 @@ internal    static  JniValueMarshalerState  CreateArgumentState<TArray> (IList<T
 			return new JniValueMarshalerState (a);
 		}
 
-		internal static void DestroyArgumentState<TArray> (IList<T> value, ref JniValueMarshalerState state, ParameterAttributes synchronize)
+		internal static void DestroyArgumentState<TArray> (IList<T>? value, ref JniValueMarshalerState state, ParameterAttributes synchronize)
 			where TArray : JavaArray<T>
 		{
-			var source = (TArray) state.PeerableValue;
+			var source = (TArray?) state.PeerableValue;
 			if (source == null)
 				return;
 
@@ -187,7 +194,9 @@ internal virtual void CopyToList (IList<T> list, int index)
 		{
 			int len = Length;
 			for (int i = 0; i < len; i++) {
+#pragma warning disable CS8601 // Possible null reference assignment.
 				list [index + i] = this [i];
+#pragma warning restore CS8601 // Possible null reference assignment.
 			}
 		}
 
@@ -217,9 +226,11 @@ bool IList.IsFixedSize {
 			}
 		}
 
-		object IList.this [int index] {
+		object? IList.this [int index] {
 			get {return this [index];}
+#pragma warning disable 8601
 			set {this [index] = (T) value;}
+#pragma warning restore 8601
 		}
 
 		void ICollection.CopyTo (Array array, int index)

src/Java.Interop/Java.Interop/JavaException.cs

@@ -1,3 +1,5 @@
+#nullable enable
+
 using System;
 
 namespace Java.Interop
@@ -8,7 +10,7 @@ unsafe public class JavaException : Exception, IJavaPeerable
 		internal    const   string          JniTypeName = "java/lang/Throwable";
 		readonly    static  JniPeerMembers  _members    = new JniPeerMembers (JniTypeName, typeof (JavaException));
 
-		public string                   JavaStackTrace { get; private set; }
+		public string?                  JavaStackTrace { get; private set; }
 		public int                      JniIdentityHashCode { get; private set; }
 		public JniManagedPeerStates     JniManagedPeerState { get; private set; }
 
@@ -177,7 +179,7 @@ public override unsafe int GetHashCode ()
 			return _members.InstanceMethods.InvokeVirtualInt32Method ("hashCode.()I", this, null);
 		}
 
-		static string GetMessage (ref JniObjectReference reference, JniObjectReferenceOptions transfer)
+		static string? GetMessage (ref JniObjectReference reference, JniObjectReferenceOptions transfer)
 		{
 			if (transfer == JniObjectReferenceOptions.None)
 				return null;
@@ -187,7 +189,7 @@ static string GetMessage (ref JniObjectReference reference, JniObjectReferenceOp
 			return JniEnvironment.Strings.ToString (ref s, JniObjectReferenceOptions.CopyAndDispose);
 		}
 
-		static Exception GetCause (ref JniObjectReference reference, JniObjectReferenceOptions transfer)
+		static Exception? GetCause (ref JniObjectReference reference, JniObjectReferenceOptions transfer)
 		{
 			if (transfer == JniObjectReferenceOptions.None)
 				return null;
@@ -197,7 +199,7 @@ static Exception GetCause (ref JniObjectReference reference, JniObjectReferenceO
 			return JniEnvironment.Runtime.GetExceptionForThrowable (ref e, JniObjectReferenceOptions.CopyAndDispose);
 		}
 
-		unsafe string GetJavaStack (JniObjectReference handle)
+		unsafe string? GetJavaStack (JniObjectReference handle)
 		{
 			using (var StringWriter_class   = new JniType ("java/io/StringWriter"))
 			using (var PrintWriter_class    = new JniType ("java/io/PrintWriter")) {

src/Java.Interop/Java.Interop/JavaObject.cs

@@ -1,3 +1,5 @@
+#nullable enable
+
 using System;
 
 namespace Java.Interop
@@ -125,7 +127,7 @@ public override unsafe int GetHashCode ()
 			return _members.InstanceMethods.InvokeVirtualInt32Method ("hashCode.()I", this, null);
 		}
 
-		public override unsafe string ToString ()
+		public override unsafe string? ToString ()
 		{
 			var lref = _members.InstanceMethods.InvokeVirtualObjectMethod (
 					"toString.()Ljava/lang/String;",