Skip to content

Commit

Permalink
[Java.Interop.Tools.JavaTypeSystem] 'annotated-visibility' on types (#…
Browse files Browse the repository at this point in the history
…1102)

Context: b274a67

When adding support for Android's `@RestrictTo` annotation in
b274a67, the XML Adjuster step was accidentally only modified to
pass through the `//*/@annotated-visibility` attribute for *members*
(`//method`, `//field`) and not for *types* (`//class`, `//interface`).

Fix this by storing and emitting the `//@annotated-visibility`
attribute for types as well.
  • Loading branch information
jpobst authored Apr 25, 2023
1 parent 07d5595 commit e7d2edb
Show file tree
Hide file tree
Showing 11 changed files with 66 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,15 @@ public static void Save (JavaTypeCollection types, XmlWriter writer)
static void SaveType (JavaTypeModel type, XmlWriter writer)
{
if (type is JavaClassModel cls)
SaveType (type, writer, "class", XmlConvert.ToString (cls.IsAbstract), cls.BaseType, cls.BaseTypeGeneric, cls.BaseTypeJni);
SaveType (type, writer, "class", XmlConvert.ToString (cls.IsAbstract), cls.BaseType, cls.BaseTypeGeneric, cls.BaseTypeJni, cls.AnnotatedVisibility);
else
SaveType (type, writer, "interface", "true", null, null, null);
SaveType (type, writer, "interface", "true", null, null, null, type.AnnotatedVisibility);

foreach (var nested in type.NestedTypes)
SaveType (nested, writer);
}

static void SaveType (JavaTypeModel cls, XmlWriter writer, string elementName, string abs, string? ext, string? extgen, string? jniExt)
static void SaveType (JavaTypeModel cls, XmlWriter writer, string elementName, string abs, string? ext, string? extgen, string? jniExt, string annotatedVisibility)
{
writer.WriteStartElement (elementName);

Expand All @@ -80,6 +80,7 @@ static void SaveType (JavaTypeModel cls, XmlWriter writer, string elementName, s
writer.WriteAttributeString ("static", XmlConvert.ToString (cls.IsStatic));
writer.WriteAttributeString ("visibility", cls.Visibility);
writer.WriteAttributeStringIfValue ("jni-signature", cls.ExtendedJniSignature);
writer.WriteAttributeStringIfValue ("annotated-visibility", annotatedVisibility);

if (cls.PropertyBag.TryGetValue ("merge.SourceFile", out var source))
writer.WriteAttributeString ("merge.SourceFile", source);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@ public static JavaClassModel ParseClass (JavaPackage package, XElement element)
javaDeprecated: element.XGetAttribute ("deprecated"),
javaStatic: element.XGetAttributeAsBool ("static"),
jniSignature: element.XGetAttribute ("jni-signature"),
baseTypeJni: element.XGetAttribute ("jni-extends")
baseTypeJni: element.XGetAttribute ("jni-extends"),
annotatedVisibility: element.XGetAttribute ("annotated-visibility")
);

if (element.XGetAttribute ("merge.SourceFile") is string source && source.HasValue ())
Expand Down Expand Up @@ -151,8 +152,9 @@ public static JavaInterfaceModel ParseInterface (JavaPackage package, XElement e
var deprecated = element.XGetAttribute ("deprecated");
var is_static = element.XGetAttribute ("static") == "true";
var jni_signature = element.XGetAttribute ("jni-signature");
var annotated_visibility = element.XGetAttribute ("annotated-visibility");

var model = new JavaInterfaceModel (package, nested_name, visibility, deprecated, is_static, jni_signature);
var model = new JavaInterfaceModel (package, nested_name, visibility, deprecated, is_static, jni_signature, annotated_visibility);

if (element.XGetAttribute ("merge.SourceFile") is string source && source.HasValue ())
model.PropertyBag.Add ("merge.SourceFile", source);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ static bool ShouldImport (TypeDefinition td)
javaDeprecated: obs_attr != null ? "deprecated" : "not-deprecated",
javaStatic: false,
jniSignature: FormatJniSignature (package, nested_name),
baseTypeJni: base_jni.HasValue () ? $"L{base_jni};" : string.Empty
baseTypeJni: base_jni.HasValue () ? $"L{base_jni};" : string.Empty,
annotatedVisibility: string.Empty
); ;

ParseImplementedInterfaces (type, model);
Expand Down Expand Up @@ -144,7 +145,8 @@ static bool ShouldImport (TypeDefinition td)
javaVisibility: type.IsPublic || type.IsNestedPublic ? "public" : "protected internal",
javaDeprecated: obs_attr != null ? "deprecated" : "not-deprecated",
javaStatic: false,
jniSignature: FormatJniSignature (package, nested_name)
jniSignature: FormatJniSignature (package, nested_name),
annotatedVisibility: ""
);

ParseImplementedInterfaces (type, model);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ namespace Java.Interop.Tools.JavaTypeSystem.Models
// Represents a Java built-in type like 'int' or 'float'
public class JavaBuiltInType : JavaTypeModel
{
public JavaBuiltInType (string name) : base (new JavaPackage ("", "", null), name, "public", false, true, "not deprecated", false, "") { }
public JavaBuiltInType (string name) : base (new JavaPackage ("", "", null), name, "public", false, true, "not deprecated", false, "", "") { }

public override void Resolve (JavaTypeCollection types, ICollection<JavaUnresolvableModel> unresolvables)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ public class JavaClassModel : JavaTypeModel
public JavaTypeReference? BaseTypeReference { get; private set; }
public List<JavaConstructorModel> Constructors { get; } = new List<JavaConstructorModel> ();

public JavaClassModel (JavaPackage javaPackage, string javaNestedName, string javaVisibility, bool javaAbstract, bool javaFinal, string javaBaseType, string javaBaseTypeGeneric, string javaDeprecated, bool javaStatic, string jniSignature, string baseTypeJni) :
base (javaPackage, javaNestedName, javaVisibility, javaAbstract, javaFinal, javaDeprecated, javaStatic, jniSignature)
public JavaClassModel (JavaPackage javaPackage, string javaNestedName, string javaVisibility, bool javaAbstract, bool javaFinal, string javaBaseType, string javaBaseTypeGeneric, string javaDeprecated, bool javaStatic, string jniSignature, string baseTypeJni, string annotatedVisibility) :
base (javaPackage, javaNestedName, javaVisibility, javaAbstract, javaFinal, javaDeprecated, javaStatic, jniSignature, annotatedVisibility)
{
BaseType = javaBaseType;
BaseTypeGeneric = javaBaseTypeGeneric;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ namespace Java.Interop.Tools.JavaTypeSystem.Models
{
public class JavaInterfaceModel : JavaTypeModel
{
public JavaInterfaceModel (JavaPackage javaPackage, string javaNestedName, string javaVisibility, string javaDeprecated, bool javaStatic, string jniSignature) :
base (javaPackage, javaNestedName, javaVisibility, false, false, javaDeprecated, javaStatic, jniSignature)
public JavaInterfaceModel (JavaPackage javaPackage, string javaNestedName, string javaVisibility, string javaDeprecated, bool javaStatic, string jniSignature, string annotatedVisibility) :
base (javaPackage, javaNestedName, javaVisibility, false, false, javaDeprecated, javaStatic, jniSignature, annotatedVisibility)
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ public abstract class JavaTypeModel : IJavaResolvable
public string NestedName { get; set; }

public string Visibility { get; }
public string AnnotatedVisibility { get; }
public bool IsAbstract { get; }
public bool IsFinal { get; }
public string Deprecated { get; }
Expand All @@ -36,12 +37,13 @@ public abstract class JavaTypeModel : IJavaResolvable

public Dictionary<string, string> PropertyBag { get; } = new Dictionary<string, string> ();

protected JavaTypeModel (JavaPackage javaPackage, string javaNestedName, string javaVisibility, bool javaAbstract, bool javaFinal, string deprecated, bool javaStatic, string jniSignature)
protected JavaTypeModel (JavaPackage javaPackage, string javaNestedName, string javaVisibility, bool javaAbstract, bool javaFinal, string deprecated, bool javaStatic, string jniSignature, string annotatedVisibility)
{
Package = javaPackage;
NestedName = javaNestedName.Replace ('$', '.');
Name = NestedName.LastSubset ('.');
Visibility = javaVisibility;
AnnotatedVisibility = annotatedVisibility;
IsAbstract = javaAbstract;
IsFinal = javaFinal;
Deprecated = deprecated;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
<TargetFrameworks>net472;$(DotNetTargetFramework)</TargetFrameworks>
<IsPackable>false</IsPackable>
<RootNamespace>Java.Interop.Tools.JavaTypeSystem.Tests</RootNamespace>
<LangVersion>8.0</LangVersion>
</PropertyGroup>

<Import Project="..\..\TargetFrameworkDependentValues.props" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public static JavaTypeCollection GetLoadedApi ()
return JavaXmlApiImporter.Parse (ApiPath);
}

public static JavaClassModel CreateClass (JavaPackage javaPackage, string javaNestedName, string javaVisibility = "public", bool javaAbstract = false, bool javaFinal = false, string javaBaseType = "java.lang.Object", string javaBaseTypeGeneric = "java.lang.Object", string javaDeprecated = "not deprecated", bool javaStatic = false, string jniSignature = "", string baseTypeJni = "java/lang/Object")
public static JavaClassModel CreateClass (JavaPackage javaPackage, string javaNestedName, string javaVisibility = "public", bool javaAbstract = false, bool javaFinal = false, string javaBaseType = "java.lang.Object", string javaBaseTypeGeneric = "java.lang.Object", string javaDeprecated = "not deprecated", bool javaStatic = false, string jniSignature = "", string baseTypeJni = "java/lang/Object", string annotatedVisibility = "")
{
if (string.IsNullOrWhiteSpace (jniSignature))
jniSignature = $"{(!string.IsNullOrWhiteSpace (javaPackage.Name) ? javaPackage.Name + "." : "")}{javaNestedName}".Replace ('.', '/');
Expand All @@ -30,7 +30,8 @@ public static JavaClassModel CreateClass (JavaPackage javaPackage, string javaNe
javaDeprecated: javaDeprecated,
javaStatic: javaStatic,
jniSignature: jniSignature,
baseTypeJni: baseTypeJni
baseTypeJni: baseTypeJni,
annotatedVisibility: annotatedVisibility
);

return klass;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
using System;
using System.IO;
using System.Linq;
using System.Xml;
using Java.Interop.Tools.JavaTypeSystem.Models;
using NUnit.Framework;

Expand All @@ -25,5 +27,35 @@ public void TestToString ()
var kls = pkg.Types.First (t => t.FullName == "android.database.ContentObservable");
Assert.AreEqual ("[Class] android.database.ContentObservable", kls.ToString ());
}

[Test]
public void AnnotatedVisibility ()
{
// Ensure the 'annotated-visibility' attribute gets passed through
using var sw = new StringWriter ();

using (var xml = XmlWriter.Create (sw))
JavaXmlApiExporter.Save (api, xml);

var doc = new XmlDocument { XmlResolver = null };
using var sreader = new StringReader (sw.ToString ());
using var reader = XmlReader.Create (sreader, new XmlReaderSettings { XmlResolver = null });
doc.Load (reader);

var annotated_class = doc.SelectSingleNode ("/api/package/class[@name='StateListAnimator']");
Assert.AreEqual ("TESTS", annotated_class.Attributes ["annotated-visibility"].InnerText);

var annotated_ctor = annotated_class ["constructor"];
Assert.AreEqual ("TESTS", annotated_ctor.Attributes ["annotated-visibility"].InnerText);

var annotated_method = annotated_class ["method"];
Assert.AreEqual ("TESTS", annotated_method.Attributes ["annotated-visibility"].InnerText);

var annotated_interface = doc.SelectSingleNode ("/api/package/interface[@name='DrmStore.ConstraintsColumns']");
Assert.AreEqual ("TESTS", annotated_interface.Attributes ["annotated-visibility"].InnerText);

var annotated_field = annotated_interface ["field"];
Assert.AreEqual ("TESTS", annotated_field.Attributes ["annotated-visibility"].InnerText);
}
}
}
15 changes: 10 additions & 5 deletions tests/Java.Interop.Tools.JavaTypeSystem-Tests/api-24.xml.in
Original file line number Diff line number Diff line change
Expand Up @@ -43190,7 +43190,8 @@
name="StateListAnimator"
jni-signature="Landroid/animation/StateListAnimator;"
static="false"
visibility="public">
visibility="public"
annotated-visibility="TESTS">
<implements
name="java.lang.Cloneable"
name-generic-aware="java.lang.Cloneable"
Expand All @@ -43203,7 +43204,8 @@
visibility="public"
bridge="false"
synthetic="false"
jni-signature="()V" />
jni-signature="()V"
annotated-visibility="TESTS" />
<method
abstract="false"
deprecated="not deprecated"
Expand All @@ -43217,7 +43219,8 @@
visibility="public"
bridge="false"
synthetic="false"
jni-signature="([ILandroid/animation/Animator;)V">
jni-signature="([ILandroid/animation/Animator;)V"
annotated-visibility="TESTS">
<parameter
name="specs"
type="int[]"
Expand Down Expand Up @@ -167236,7 +167239,8 @@
name="DrmStore.ConstraintsColumns"
jni-signature="Landroid/drm/DrmStore$ConstraintsColumns;"
static="true"
visibility="public">
visibility="public"
annotated-visibility="TESTS">
<field
deprecated="not deprecated"
final="true"
Expand All @@ -167249,7 +167253,8 @@
jni-signature="Ljava/lang/String;"
value="&quot;extended_metadata&quot;"
visibility="public"
volatile="false" />
volatile="false"
annotated-visibility="TESTS" />
<field
deprecated="not deprecated"
final="true"
Expand Down

0 comments on commit e7d2edb

Please sign in to comment.