From 35e2671683c5b823f04eea6ea155e2a9d4a64e3f Mon Sep 17 00:00:00 2001 From: Jonathan Pobst Date: Mon, 25 Nov 2019 11:28:58 -0600 Subject: [PATCH] [XAT.Bytecode] Add support for reading NotNull metadata into api.xml. --- .../JavaApi.XmlModel.cs | 5 +- .../JavaApiXmlGeneratorExtensions.cs | 15 ++-- .../JavaApiXmlLoaderExtensions.cs | 7 +- .../Annotation.cs | 28 ++++++++ .../AttributeInfo.cs | 29 +++++++- .../XmlClassDeclarationBuilder.cs | 56 ++++++++++++++- .../NullableAnnotationTests.cs | 68 +++++++++++++++++++ ...amarin.Android.Tools.Bytecode-Tests.csproj | 5 +- .../java/android/annotation/NonNull.java | 7 ++ .../java/com/xamarin/NotNullClass.java | 18 +++++ .../Unit-Tests/AdjusterTests.cs | 55 +++++++++++++-- 11 files changed, 276 insertions(+), 17 deletions(-) create mode 100644 tests/Xamarin.Android.Tools.Bytecode-Tests/NullableAnnotationTests.cs create mode 100644 tests/Xamarin.Android.Tools.Bytecode-Tests/java/android/annotation/NonNull.java create mode 100644 tests/Xamarin.Android.Tools.Bytecode-Tests/java/com/xamarin/NotNullClass.java diff --git a/src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApi.XmlModel.cs b/src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApi.XmlModel.cs index 2eef2f488..ce6c2b91b 100644 --- a/src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApi.XmlModel.cs +++ b/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 () diff --git a/src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApiXmlGeneratorExtensions.cs b/src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApiXmlGeneratorExtensions.cs index 562f0c67f..83e44f7db 100644 --- a/src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApiXmlGeneratorExtensions.cs +++ b/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 parameters, IEnumerable 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 (); } diff --git a/src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApiXmlLoaderExtensions.cs b/src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApiXmlLoaderExtensions.cs index 5592ab177..218488f3c 100644 --- a/src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApiXmlLoaderExtensions.cs +++ b/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 (); } diff --git a/src/Xamarin.Android.Tools.Bytecode/Annotation.cs b/src/Xamarin.Android.Tools.Bytecode/Annotation.cs index 968aac350..471e4bfda 100644 --- a/src/Xamarin.Android.Tools.Bytecode/Annotation.cs +++ b/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 value) } } } + + public sealed class ParameterAnnotation + { + public int ParameterIndex { get; } + public IList Annotations { get; } = new List (); + 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})"; + } + } } diff --git a/src/Xamarin.Android.Tools.Bytecode/AttributeInfo.cs b/src/Xamarin.Android.Tools.Bytecode/AttributeInfo.cs index c2bbbd832..c58918a18 100644 --- a/src/Xamarin.Android.Tools.Bytecode/AttributeInfo.cs +++ b/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 Annotations { get; } = new List (); + + 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})"; } } diff --git a/src/Xamarin.Android.Tools.Bytecode/XmlClassDeclarationBuilder.cs b/src/Xamarin.Android.Tools.Bytecode/XmlClassDeclarationBuilder.cs index 4d41072a2..32758428f 100644 --- a/src/Xamarin.Android.Tools.Bytecode/XmlClassDeclarationBuilder.cs +++ b/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 GetMethodParameters (MethodInfo method) { + var annotations = method.Attributes?.OfType ().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 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 GetExceptions (MethodInfo method) } } + static XAttribute GetNotNull (MethodInfo method) + { + var annotations = method.Attributes?.OfType ().FirstOrDefault ()?.Annotations; + + if (annotations?.Any (a => IsNotNullAnnotation (a)) == true) + return new XAttribute ("return-not-null", "true"); + + return null; + } + + static XAttribute GetNotNull (IList 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 ().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 GetFields () { foreach (var field in classFile.Fields.OrderBy (n => n.Name, StringComparer.OrdinalIgnoreCase)) { @@ -437,6 +490,7 @@ IEnumerable 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)); diff --git a/tests/Xamarin.Android.Tools.Bytecode-Tests/NullableAnnotationTests.cs b/tests/Xamarin.Android.Tools.Bytecode-Tests/NullableAnnotationTests.cs new file mode 100644 index 000000000..e1020792c --- /dev/null +++ b/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 ().Count ()); + Assert.AreEqual (0, null_method.Attributes.OfType ().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 ().FirstOrDefault ()?.Annotations; + var param_ann = notnull_method.Attributes.OfType ().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 ().Count ()); + Assert.AreEqual (0, null_field.Attributes.OfType ().Count ()); + + // Field with not-null annotation + var notnull_field = c.Fields.First (f => f.Name == "notNullField"); + + var field_ann = notnull_method.Attributes.OfType ().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); + } + } +} + diff --git a/tests/Xamarin.Android.Tools.Bytecode-Tests/Xamarin.Android.Tools.Bytecode-Tests.csproj b/tests/Xamarin.Android.Tools.Bytecode-Tests/Xamarin.Android.Tools.Bytecode-Tests.csproj index a08480953..746ecdca5 100644 --- a/tests/Xamarin.Android.Tools.Bytecode-Tests/Xamarin.Android.Tools.Bytecode-Tests.csproj +++ b/tests/Xamarin.Android.Tools.Bytecode-Tests/Xamarin.Android.Tools.Bytecode-Tests.csproj @@ -31,6 +31,7 @@ + @@ -51,14 +52,14 @@ - + - + diff --git a/tests/Xamarin.Android.Tools.Bytecode-Tests/java/android/annotation/NonNull.java b/tests/Xamarin.Android.Tools.Bytecode-Tests/java/android/annotation/NonNull.java new file mode 100644 index 000000000..96a318968 --- /dev/null +++ b/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 { +} \ No newline at end of file diff --git a/tests/Xamarin.Android.Tools.Bytecode-Tests/java/com/xamarin/NotNullClass.java b/tests/Xamarin.Android.Tools.Bytecode-Tests/java/com/xamarin/NotNullClass.java new file mode 100644 index 000000000..f71a70ea6 --- /dev/null +++ b/tests/Xamarin.Android.Tools.Bytecode-Tests/java/com/xamarin/NotNullClass.java @@ -0,0 +1,18 @@ +package com.xamarin; + +import android.annotation.NonNull; + +public class NotNullClass { + + public void nullFunc (String value) { + } + + @NonNull + public void notNullFunc (@NonNull String value) { + } + + public String nullField; + + @NonNull + public String notNullField; +} diff --git a/tests/generator-Tests/Unit-Tests/AdjusterTests.cs b/tests/generator-Tests/Unit-Tests/AdjusterTests.cs index 831cb57a3..e6f68a741 100644 --- a/tests/generator-Tests/Unit-Tests/AdjusterTests.cs +++ b/tests/generator-Tests/Unit-Tests/AdjusterTests.cs @@ -1,8 +1,10 @@ -using Mono.Cecil; +using Mono.Cecil; using MonoDroid.Generation; using NUnit.Framework; using System.IO; using System.Linq; +using System.Xml; +using System.Xml.Linq; using Xamarin.Android.Tools.ApiXmlAdjuster; namespace generatortests @@ -43,6 +45,11 @@ public void SetUp () "); + + foreach (var type in module.Types.Where (t => t.IsClass && t.Namespace == "Java.Lang")) { + //Make sure we use this method instead of SymbolTable directly, to match what happens in generator.exe + Xamarin.Android.Binder.CodeGenerator.ProcessReferencedType (type, options); + } } [TearDown] @@ -62,14 +69,50 @@ public void TearDown () [Test] public void Process () { - foreach (var type in module.Types.Where (t => t.IsClass && t.Namespace == "Java.Lang")) { - //Make sure we use this method instead of SymbolTable directly, to match what happens in generator.exe - Xamarin.Android.Binder.CodeGenerator.ProcessReferencedType (type, options); - } - adjuster.Process (inputFile, options, options.SymbolTable.AllRegisteredSymbols (options).OfType ().ToArray (), outputFile, (int)Log.LoggingLevel.Debug); FileAssert.Exists (outputFile); } + + [Test] + public void AdjustNotNullAnnotations () + { + var input = @" + + + + + + + + + + +"; + + var api = new JavaApi (); + api.LoadReferences (options, options.SymbolTable.AllRegisteredSymbols (options).OfType ().ToArray ()); + + using (var sr = new StringReader (input)) + using (var xml = XmlReader.Create (sr)) + api.Load (xml, false); + + api.StripNonBindables (); + api.Resolve (); + api.CreateGenericInheritanceMapping (); + api.MarkOverrides (); + api.FindDefects (); + + using (var sb = new StringWriter ()) { + using (var writer = XmlWriter.Create (sb)) + api.Save (writer); + + var root = XElement.Parse (sb.ToString ()); + + Assert.AreEqual ("true", root.Element ("package").Element ("class").Element ("method").Attribute ("return-not-null").Value); + Assert.AreEqual ("true", root.Element ("package").Element ("class").Element ("method").Element ("parameter").Attribute ("not-null").Value); + Assert.AreEqual ("true", root.Element ("package").Element ("class").Element ("field").Attribute ("not-null").Value); + } + } } }