[Xamarin.Android.Tools.Bytecode] Add @not-null support (#526)

Context: #468

Add support for reading `@NotNull`/`@NonNull` annotations from Java
bytecode and inserting them into `api.xml` as `//*/@not-null` and
`//*/@return-not-null` attribute values: `@not-null` is for
parameters and fields, while `@return-not-null` is emitted for method
return values.

Support for `generator` to consume the annotations will be done later.

For example, given the Java code:

        // Java:
        public class Example {
            @nonnull
            public static final Creator<DirectAction> CREATOR = /* ... */ ;

            public void foo(@nonnull android.os.Handler handler) { /* ... */ }

            @nonnull
            public Context getContext() { /* ... */ }
        }

then `Example.CREATOR` would contain:

        <field 
          deprecated="not deprecated" 
          final="true" 
          name="CREATOR" 
          jni-signature="Landroid/os/Parcelable$Creator;" 
          not-null="true" 
          static="true" 
          transient="false" 
          type="android.os.Parcelable.Creator" 
          type-generic-aware="android.os.Parcelable.Creator&lt;android.app.DirectAction&gt;" 
          visibility="public" 
          volatile="false" />

The `handler` parameter for `Example.foo()` would contain:

        <parameter 
          name="handler" 
          type="android.os.Handler" 
          jni-type="Landroid/os/Handler;" 
          not-null="true" />

And the `Example.getContext()` method would contain:

        <method 
          abstract="false" 
          deprecated="not deprecated" 
          final="true" 
          name="getContext" 
          jni-signature="()Landroid/content/Context;" 
          bridge="false" 
          native="false" 
          return="android.content.Context" 
          jni-return="Landroid/content/Context;" 
          static="false" 
          synchronized="false" 
          synthetic="false" 
          visibility="public" 
          return-not-null="true" />

We look for the following annotations:

  * Android:
      * `android/annotation/NonNull`
      * `androidx/annotation/RecentlyNonNull`

  * Java:
      * `android/support/annotation/NonNull`
      * `edu/umd/cs/findbugs/annotations/NonNull`
      * `javax/annotation/Nonnull`
      * `javax/validation/constraints/NotNull`
      * `lombok/NonNull`
      * `org/eclipse/jdt/annotation/NonNull`
      * `org/jetbrains/annotations/NotNull`

List of annotation types partially taken from:

         https://stackoverflow.com/questions/4963300/which-notnull-java-annotation-should-i-use

Author: jpobst

src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApi.XmlModel.cs

@@ -176,7 +176,8 @@ public JavaField (JavaType parent)
 			: base (parent)
 		{
 		}
-		
+
+		public bool NotNull { get; set; }
 		public bool Transient { get; set; }
 		public string Type { get; set; }
 		public string TypeGeneric { get; set; }
@@ -248,6 +249,7 @@ public JavaMethod (JavaType parent)
 		public bool Abstract { get; set; }
 		public bool Native { get; set; }
 		public string Return { get; set; }
+		public bool ReturnNotNull { get; set; }
 		public bool Synchronized { get; set; }
 
 		// Content of this value is not stable.
@@ -268,6 +270,7 @@ public JavaParameter (JavaMethodBase parent)
 		public string Name { get; set; }
 		public string Type { get; set; }
 		public string JniType { get; set; }
+		public bool NotNull { get; set; }
 
 		// Content of this value is not stable.
 		public override string ToString ()

src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApiXmlGeneratorExtensions.cs

@@ -160,12 +160,13 @@ static void Save (this JavaField field, XmlWriter writer)
 				null,
 				null,
 				null,
-				null);
+				null,
+				field.NotNull);
 		}
 
 		static void Save (this JavaConstructor ctor, XmlWriter writer)
 		{
-			SaveCommon (ctor, writer, "constructor", null, null, null, null, null, ctor.Type ?? ctor.Parent.FullName, null, null, null, ctor.TypeParameters, ctor.Parameters, ctor.Exceptions, ctor.ExtendedBridge, ctor.ExtendedJniReturn, ctor.ExtendedSynthetic);
+			SaveCommon (ctor, writer, "constructor", null, null, null, null, null, ctor.Type ?? ctor.Parent.FullName, null, null, null, ctor.TypeParameters, ctor.Parameters, ctor.Exceptions, ctor.ExtendedBridge, ctor.ExtendedJniReturn, ctor.ExtendedSynthetic, null);
 		}
 
 		static void Save (this JavaMethod method, XmlWriter writer)
@@ -217,7 +218,8 @@ static void Save (this JavaMethod method, XmlWriter writer)
 				method.Exceptions,
 				method.ExtendedBridge,
 				method.ExtendedJniReturn,
-				method.ExtendedSynthetic);
+				method.ExtendedSynthetic,
+				method.ReturnNotNull);
 		}
 
 		static void SaveCommon (this JavaMember m, XmlWriter writer, string elementName,
@@ -227,7 +229,7 @@ static void SaveCommon (this JavaMember m, XmlWriter writer, string elementName,
 					JavaTypeParameters typeParameters,
 					IEnumerable<JavaParameter> parameters,
 					IEnumerable<JavaException> exceptions,
-					bool? extBridge, string jniReturn, bool? extSynthetic)
+					bool? extBridge, string jniReturn, bool? extSynthetic, bool? notNull)
 		{
 			// If any of the parameters contain reference to non-public type, it cannot be generated.
 			if (parameters != null && parameters.Any (p => p.ResolvedType.ReferencedType != null && string.IsNullOrEmpty (p.ResolvedType.ReferencedType.Visibility)))
@@ -248,6 +250,8 @@ static void SaveCommon (this JavaMember m, XmlWriter writer, string elementName,
 				writer.WriteAttributeString ("return", ret);
 			if (jniReturn != null)
 				writer.WriteAttributeString ("jni-return", jniReturn);
+			if (notNull.GetValueOrDefault ())
+				writer.WriteAttributeString (m is JavaField ? "not-null" : "return-not-null", "true");
 			writer.WriteAttributeString ("static", XmlConvert.ToString (m.Static));
 			if (sync != null)
 				writer.WriteAttributeString ("synchronized", sync);
@@ -276,6 +280,9 @@ static void SaveCommon (this JavaMember m, XmlWriter writer, string elementName,
 					if (!string.IsNullOrEmpty (p.JniType)) {
 						writer.WriteAttributeString ("jni-type", p.JniType);
 					}
+					if (p.NotNull == true) {
+						writer.WriteAttributeString ("not-null", "true");
+					}
 					writer.WriteString ("\n        ");
 					writer.WriteFullEndElement ();
 				}

src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApiXmlLoaderExtensions.cs

@@ -215,10 +215,11 @@ public static void Load (this JavaField field, XmlReader reader)
 		{
 			field.LoadMemberAttributes (reader);
 			field.Transient = XmlConvert.ToBoolean (XmlUtil.GetRequiredAttribute (reader, "transient"));
-			field.Volatile = XmlConvert.ToBoolean (XmlUtil.GetRequiredAttribute (reader, "transient"));
+			field.Volatile = XmlConvert.ToBoolean (XmlUtil.GetRequiredAttribute (reader, "volatile"));
 			field.Type = XmlUtil.GetRequiredAttribute (reader, "type");
 			field.TypeGeneric = XmlUtil.GetRequiredAttribute (reader, "type-generic-aware");
 			field.Value = reader.GetAttribute ("value");
+			field.NotNull = reader.GetAttribute ("not-null") == "true";
 
 			reader.Skip ();
 		}
@@ -277,9 +278,10 @@ public static void Load (this JavaMethod method, XmlReader reader)
 			method.Abstract = XmlConvert.ToBoolean (XmlUtil.GetRequiredAttribute (reader, "abstract"));
 			method.Native = XmlConvert.ToBoolean (XmlUtil.GetRequiredAttribute (reader, "native"));
 			method.Return = XmlUtil.GetRequiredAttribute (reader, "return");
+			method.ReturnNotNull = reader.GetAttribute ("return-not-null") == "true";
 			method.Synchronized = XmlConvert.ToBoolean (XmlUtil.GetRequiredAttribute (reader, "synchronized"));
 			XmlUtil.CheckExtraneousAttributes ("method", reader, "deprecated", "final", "name", "static", "visibility", "jni-signature", "jni-return", "synthetic", "bridge",
-				"abstract", "native", "return", "synchronized");
+				"abstract", "native", "return", "synchronized", "return-not-null");
 			method.LoadMethodBase ("method", reader);
 		}
 
@@ -288,6 +290,7 @@ internal static void Load (this JavaParameter p, XmlReader reader)
 			p.Name = XmlUtil.GetRequiredAttribute (reader, "name");
 			p.Type = XmlUtil.GetRequiredAttribute (reader, "type");
 			p.JniType = reader.GetAttribute ("jni-type");
+			p.NotNull = reader.GetAttribute ("not-null") == "true";
 			reader.Skip ();
 		}
 

src/Xamarin.Android.Tools.Bytecode/Annotation.cs

@@ -1,5 +1,6 @@
 using System.Collections.Generic;
 using System.IO;
+using System.Linq;
 using System.Text;
 
 namespace Xamarin.Android.Tools.Bytecode
@@ -48,4 +49,31 @@ void Append (KeyValuePair<string, AnnotationElementValue> value)
 			}
 		}
 	}
+
+	public sealed class ParameterAnnotation
+	{
+		public int ParameterIndex { get; }
+		public IList<Annotation> Annotations { get; } = new List<Annotation> ();
+		public ConstantPool ConstantPool { get; }
+
+		public ParameterAnnotation (ConstantPool constantPool, Stream stream, int index)
+		{
+			ConstantPool = constantPool;
+
+			ParameterIndex = index;
+
+			var ann_count = stream.ReadNetworkUInt16 ();
+
+			for (var i = 0; i < ann_count; ++i) {
+				var a = new Annotation (constantPool, stream);
+				Annotations.Add (a);
+			}
+		}
+
+		public override string ToString ()
+		{
+			var annotations = string.Join (", ", Annotations.Select (v => v.ToString ()));
+			return $"Parameter{ParameterIndex}({annotations})";
+		}
+	}
 }

src/Xamarin.Android.Tools.Bytecode/AttributeInfo.cs

@@ -49,6 +49,7 @@ public class AttributeInfo {
 		public  const   string  StackMapTable           = "StackMapTable";
 		public	const	string	RuntimeVisibleAnnotations	= "RuntimeVisibleAnnotations";
 		public	const	string	RuntimeInvisibleAnnotations	= "RuntimeInvisibleAnnotations";
+		public	const	string	RuntimeInvisibleParameterAnnotations = "RuntimeInvisibleParameterAnnotations";
 
 		ushort          nameIndex;
 
@@ -115,6 +116,7 @@ static AttributeInfo CreateAttribute (string name, ConstantPool constantPool, us
 			case MethodParameters:      return new MethodParametersAttribute (constantPool, nameIndex, stream);
 			case RuntimeVisibleAnnotations:		return new RuntimeVisibleAnnotationsAttribute (constantPool, nameIndex, stream);
 			case RuntimeInvisibleAnnotations:	return new RuntimeInvisibleAnnotationsAttribute (constantPool, nameIndex, stream);
+			case RuntimeInvisibleParameterAnnotations:	return new RuntimeInvisibleParameterAnnotationsAttribute (constantPool, nameIndex, stream);
 			case Signature:             return new SignatureAttribute (constantPool, nameIndex, stream);
 			case SourceFile:            return new SourceFileAttribute (constantPool, nameIndex, stream);
 			case StackMapTable:         return new StackMapTableAttribute (constantPool, nameIndex, stream);
@@ -543,7 +545,32 @@ public RuntimeInvisibleAnnotationsAttribute (ConstantPool constantPool, ushort n
 		public override string ToString ()
 		{
 			var annotations = string.Join (", ", Annotations.Select (v => v.ToString ()));
-			return $"RuntimeVisibleAnnotations({annotations})";
+			return $"RuntimeInvisibleAnnotations({annotations})";
+		}
+	}
+
+
+	// https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.7.19
+	public sealed class RuntimeInvisibleParameterAnnotationsAttribute : AttributeInfo
+	{
+		public IList<ParameterAnnotation> Annotations { get; } = new List<ParameterAnnotation> ();
+
+		public RuntimeInvisibleParameterAnnotationsAttribute (ConstantPool constantPool, ushort nameIndex, Stream stream)
+			: base (constantPool, nameIndex, stream)
+		{
+			var length = stream.ReadNetworkUInt32 ();
+			var param_count = stream.ReadNetworkByte ();
+
+			for (var i = 0; i < param_count; ++i) {
+				var a = new ParameterAnnotation (constantPool, stream, i);
+				Annotations.Add (a);
+			}
+		}
+
+		public override string ToString ()
+		{
+			var annotations = string.Join (", ", Annotations.Select (v => v.ToString ()));
+			return $"RuntimeInvisibleParameterAnnotationsAttribute({annotations})";
 		}
 	}
 

src/Xamarin.Android.Tools.Bytecode/XmlClassDeclarationBuilder.cs

@@ -350,6 +350,7 @@ XElement GetMethod (string element, string name, MethodInfo method, string retur
 				new XAttribute ("bridge",       (method.AccessFlags & MethodAccessFlags.Bridge) != 0),
 				new XAttribute ("synthetic",    (method.AccessFlags & MethodAccessFlags.Synthetic) != 0),
 				new XAttribute ("jni-signature",    method.Descriptor),
+				GetNotNull (method),
 				GetTypeParmeters (msig == null ? null : msig.TypeParameters),
 				GetMethodParameters (method),
 				GetExceptions (method));
@@ -382,6 +383,7 @@ static string GetVisibility (MethodAccessFlags accessFlags)
 
 		IEnumerable<XElement> GetMethodParameters (MethodInfo method)
 		{
+			var annotations = method.Attributes?.OfType<RuntimeInvisibleParameterAnnotationsAttribute> ().FirstOrDefault ()?.Annotations;
 			var varargs     = (method.AccessFlags & MethodAccessFlags.Varargs) != 0;
 			var parameters  = method.GetParameters ();
 			for (int i = 0; i < parameters.Length; ++i) {
@@ -405,7 +407,8 @@ IEnumerable<XElement> GetMethodParameters (MethodInfo method)
 				yield return new XElement ("parameter",
 						new XAttribute ("name", p.Name),
 						new XAttribute ("type",     genericType),
-						new XAttribute ("jni-type", p.Type.TypeSignature));
+						new XAttribute ("jni-type", p.Type.TypeSignature),
+						GetNotNull (annotations, i));
 			}
 		}
 
@@ -421,6 +424,56 @@ IEnumerable<XElement> GetExceptions (MethodInfo method)
 			}
 		}
 
+		static XAttribute GetNotNull (MethodInfo method)
+		{
+			var annotations = method.Attributes?.OfType<RuntimeInvisibleAnnotationsAttribute> ().FirstOrDefault ()?.Annotations;
+
+			if (annotations?.Any (a => IsNotNullAnnotation (a)) == true)
+				return new XAttribute ("return-not-null", "true");
+
+			return null;
+		}
+
+		static XAttribute GetNotNull (IList<ParameterAnnotation> annotations, int parameterIndex)
+		{
+			var ann = annotations?.FirstOrDefault (a => a.ParameterIndex == parameterIndex)?.Annotations;
+
+			if (ann?.Any (a => IsNotNullAnnotation (a)) == true)
+				return new XAttribute ("not-null", "true");
+
+			return null;
+		}
+
+		static XAttribute GetNotNull (FieldInfo field)
+		{
+			var annotations = field.Attributes?.OfType<RuntimeInvisibleAnnotationsAttribute> ().FirstOrDefault ()?.Annotations;
+
+			if (annotations?.Any (a => IsNotNullAnnotation (a)) == true)
+				return new XAttribute ("not-null", "true");
+
+			return null;
+		}
+
+		static bool IsNotNullAnnotation (Annotation annotation)
+		{
+			// Android ones plus the list from here:
+			// https://stackoverflow.com/questions/4963300/which-notnull-java-annotation-should-i-use
+			switch (annotation.Type) {
+				case "Landroid/annotation/NonNull;":
+				case "Landroidx/annotation/RecentlyNonNull;":
+				case "Ljavax/validation/constraints/NotNull;":
+				case "Ledu/umd/cs/findbugs/annotations/NonNull;":
+				case "Ljavax/annotation/Nonnull;":
+				case "Lorg/jetbrains/annotations/NotNull;":
+				case "Llombok/NonNull;":
+				case "Landroid/support/annotation/NonNull;":
+				case "Lorg/eclipse/jdt/annotation/NonNull;":
+					return true;
+			}
+
+			return false;
+		}
+
 		IEnumerable<XElement> GetFields ()
 		{
 			foreach (var field in classFile.Fields.OrderBy (n => n.Name, StringComparer.OrdinalIgnoreCase)) {
@@ -437,6 +490,7 @@ IEnumerable<XElement> GetFields ()
 						new XAttribute ("type",                 SignatureToJavaTypeName (field.Descriptor)),
 						new XAttribute ("type-generic-aware",   GetGenericType (field)),
 						new XAttribute ("jni-signature",        field.Descriptor),
+						GetNotNull (field),
 						GetValue (field),
 						new XAttribute ("visibility",           visibility),
 						new XAttribute ("volatile",             (field.AccessFlags & FieldAccessFlags.Volatile) != 0));

tests/Xamarin.Android.Tools.Bytecode-Tests/NullableAnnotationTests.cs

@@ -0,0 +1,68 @@
+using System;
+
+using Xamarin.Android.Tools.Bytecode;
+
+using NUnit.Framework;
+using System.Linq;
+
+namespace Xamarin.Android.Tools.BytecodeTests
+{
+	[TestFixture]
+	public class NullableAnnotationTests : ClassFileFixture
+	{
+		[Test]
+		public void RuntimeInvisibleAnnotations ()
+		{
+			var c = LoadClassFile ("NotNullClass.class");
+
+			// Method with no annotations
+			var null_method = c.Methods.First (m => m.Name == "nullFunc");
+
+			Assert.AreEqual (0, null_method.Attributes.OfType<RuntimeInvisibleAnnotationsAttribute> ().Count ());
+			Assert.AreEqual (0, null_method.Attributes.OfType<RuntimeInvisibleParameterAnnotationsAttribute> ().Count ());
+
+			// Method with not-null parameter and return value annotations
+			var notnull_method = c.Methods.First (m => m.Name == "notNullFunc");
+			var return_ann = notnull_method.Attributes.OfType<RuntimeInvisibleAnnotationsAttribute> ().FirstOrDefault ()?.Annotations;
+			var param_ann = notnull_method.Attributes.OfType<RuntimeInvisibleParameterAnnotationsAttribute> ().FirstOrDefault ()?.Annotations;
+
+			Assert.NotNull (return_ann);
+			Assert.IsTrue (return_ann.Any (a => a.Type == "Landroid/annotation/NonNull;"));
+
+			Assert.NotNull (param_ann);
+			Assert.IsTrue (param_ann.Any (a => a.ParameterIndex == 0 && a.Annotations[0].Type == "Landroid/annotation/NonNull;"));
+
+			// Field with no annotations
+			var null_field = c.Fields.First (f => f.Name == "nullField");
+
+			Assert.AreEqual (0, null_field.Attributes.OfType<RuntimeInvisibleAnnotationsAttribute> ().Count ());
+			Assert.AreEqual (0, null_field.Attributes.OfType<RuntimeInvisibleParameterAnnotationsAttribute> ().Count ());
+
+			// Field with not-null annotation
+			var notnull_field = c.Fields.First (f => f.Name == "notNullField");
+
+			var field_ann = notnull_method.Attributes.OfType<RuntimeInvisibleAnnotationsAttribute> ().FirstOrDefault ()?.Annotations;
+
+			Assert.NotNull (field_ann);
+			Assert.IsTrue (field_ann.Any (a => a.Type == "Landroid/annotation/NonNull;"));
+		}
+
+		[Test]
+		public void NullableAnnotationOutput ()
+		{
+			var c = LoadClassFile ("NotNullClass.class");
+			var builder = new XmlClassDeclarationBuilder (c);
+			var xml = builder.ToXElement ();
+
+			var method = xml.Elements ("method").First (m => m.Attribute ("name").Value == "notNullFunc");
+			Assert.AreEqual ("true", method.Attribute ("return-not-null").Value);
+
+			var parameter = method.Element ("parameter");
+			Assert.AreEqual ("true", parameter.Attribute ("not-null").Value);
+
+			var field = xml.Elements ("field").First (f => f.Attribute ("name").Value == "notNullField");
+			Assert.AreEqual ("true", field.Attribute ("not-null").Value);
+		}
+	}
+}
+

tests/Xamarin.Android.Tools.Bytecode-Tests/Xamarin.Android.Tools.Bytecode-Tests.csproj

@@ -31,6 +31,7 @@
     <EmbeddedResource Include="Resources\*" />
     <EmbeddedResource Include="kotlin\**\*.class" />
 
+    <EmbeddedResource Include="$(IntermediateOutputPath)classes\com\xamarin\NotNullClass.class" />
     <EmbeddedResource Include="$(IntermediateOutputPath)classes\com\xamarin\IJavaInterface.class" />
     <EmbeddedResource Include="$(IntermediateOutputPath)classes\com\xamarin\IParameterInterface.class" />
     <EmbeddedResource Include="$(IntermediateOutputPath)classes\com\xamarin\JavaAnnotation.class" />
@@ -51,14 +52,14 @@
   </ItemGroup>
 
   <ItemGroup>
-    <TestJar Include="java\**\*.java" Exclude="java\java\util\Collection.java" />
+    <TestJar Include="java\**\*.java" Exclude="java\java\util\Collection.java,java\android\annotation\NonNull.java" />
     <TestJarNoParameters Include="java\java\util\Collection.java" />
     <TestKotlinJar Include="kotlin\**\*.kt" />
   </ItemGroup>
 
   <Target Name="BuildClasses" BeforeTargets="BeforeBuild" Inputs="@(TestJar)" Outputs="@(TestJar->'$(IntermediateOutputPath)classes\%(RecursiveDir)%(Filename).class')">
     <MakeDir Directories="$(IntermediateOutputPath)classes" />
-    <Exec Command="&quot;$(JavaCPath)&quot; -parameters $(_JavacSourceOptions) -g -d &quot;$(IntermediateOutputPath)classes&quot; @(TestJar->'%(Identity)', ' ')" />
+    <Exec Command="&quot;$(JavaCPath)&quot; -parameters $(_JavacSourceOptions) -g -d &quot;$(IntermediateOutputPath)classes&quot; java\android\annotation\NonNull.java @(TestJar->'%(Identity)', ' ')" />
     <Exec Command="&quot;$(JavaCPath)&quot; $(_JavacSourceOptions) -g -d &quot;$(IntermediateOutputPath)classes&quot; @(TestJarNoParameters->'%(Identity)', ' ')" />
   </Target>
 

tests/Xamarin.Android.Tools.Bytecode-Tests/java/android/annotation/NonNull.java

@@ -0,0 +1,7 @@
+package android.annotation;
+
+import java.lang.annotation.*;  
+import java.lang.reflect.*;  
+  
+public @interface NonNull {  
+}