Skip to content

Commit

Permalink
[generator] Only use [JniTypeSignatureAttribute] for JavaInterop1 (…
Browse files Browse the repository at this point in the history 
…#1266)

Context: #1268
Context: 78d5937

Commit 78d5937 included this snippet:

> Update `generator` to support using `Java.Base.dll` as a referenced
> assembly for binding purposes, in particular by supporting the use
> of `[JniTypeSignatureAttribute]` on already bound types.

Aside: the symbol table used by `generator` uses the "full Java name"
as the key.  Which means that after 78d5937, types with
`JniTypeSignatureAttribute.SimpleReference` values are now added to
the symbol table, which in turn means that when given:

	// C#
	[JniTypeSignature ("B", ArrayRank=1, IsKeyword=true, GenerateJavaPeer=false)]
	public sealed partial class JavaSByteArray : JavaPrimitiveArray<SByte> {
	}

when `generator` needs to find type information corresponding to the
Java name of `B`, it will find `JavaSByteArray`!

This apparently breaks binding the [`androidx.work:work-runtime`][0]
Maven package, as [`androidx.work.WorkRequest.Builder.setId(UUID)`][1]
has a return type of `B`, a generic type parameter:

	// Java
	public abstract /* partial */ class androidx.work.WorkRequest.Builder<
	    B extends androidx.work.WorkRequest$Builder<B, ?>,
	    W extends androidx.work.WorkRequest>
	{
	  public final B setId(java.util.UUID);
	}

As a workaround, only use `[JniTypeSignature]` when using
`generator --codegen-target=JavaInterop1`.
`generator --codegen-target=XAJavaInterop1` will only look for
`[RegisterAttribute]` when constructing the symbol table.

This commit plumbs `CodeGenerationOptions` deeper into `generator`
and a new `ApiImporterOptions` into `Java.Interop.Tools.JavaTypeSystem`
that is used to determine whether we should be importing
`[JniTypeSignatureAttribute]` types.

TODO? #1268

[0]: https://maven.google.com/web/index.html#androidx.work:work-runtime
[1]: https://developer.android.com/reference/androidx/work/WorkRequest.Builder?hl=en#setId(java.util.UUID)
  • Loading branch information
@jpobst
jpobst authored Oct 21, 2024
1 parent 56cfab9 commit f863351
Show file tree
Hide file tree
Showing 7 changed files with 90 additions and 68 deletions.
9 changes: 9 additions & 0 deletions src/Java.Interop.Tools.JavaTypeSystem/Adapters/ApiImporterOptions.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
using System;
using System.Collections.ObjectModel;

namespace Java.Interop.Tools.JavaTypeSystem;

public class ApiImporterOptions
{
public Collection<string> SupportedTypeMapAttributes { get; } = ["Android.Runtime.RegisterAttribute"];
}
68 changes: 34 additions & 34 deletions src/Java.Interop.Tools.JavaTypeSystem/Adapters/ManagedApiImporter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@ public static class ManagedApiImporter
[Obsolete ("Use the TypeDefinitionCache overload for better performance.", error: true)]
public static JavaTypeCollection Parse (AssemblyDefinition assembly, JavaTypeCollection collection) => throw new NotSupportedException ();

public static JavaTypeCollection Parse (AssemblyDefinition assembly, JavaTypeCollection collection, TypeDefinitionCache resolver)
public static JavaTypeCollection Parse (AssemblyDefinition assembly, JavaTypeCollection collection, TypeDefinitionCache resolver, ApiImporterOptions options)
{
var types_to_add = new List<JavaTypeModel> ();

foreach (var md in assembly.Modules)
foreach (var td in md.Types) {
if (!ShouldSkipType (td, resolver) && ParseType (td, collection) is JavaTypeModel type)
if (!ShouldSkipType (td, resolver, options) && ParseType (td, collection, options) is JavaTypeModel type)
types_to_add.Add (type);
}

Expand All @@ -33,21 +33,21 @@ public static JavaTypeCollection Parse (AssemblyDefinition assembly, JavaTypeCol
return collection;
}

public static JavaTypeModel? ParseType (TypeDefinition type, JavaTypeCollection collection)
public static JavaTypeModel? ParseType (TypeDefinition type, JavaTypeCollection collection, ApiImporterOptions options)
{
if (!type.IsPublic && !type.IsNested)
return null;

if (!ShouldImport (type))
return null;

var model = type.IsInterface ? (JavaTypeModel?) ParseInterface (type, collection) : ParseClass (type, collection);
var model = type.IsInterface ? (JavaTypeModel?) ParseInterface (type, collection, options) : ParseClass (type, collection, options);

if (model is null)
return null;

foreach (var nested in type.NestedTypes)
if (ParseType (nested, collection) is JavaTypeModel nested_model)
if (ParseType (nested, collection, options) is JavaTypeModel nested_model)
model.NestedTypes.Add (nested_model);

return model;
Expand Down Expand Up @@ -89,19 +89,19 @@ static bool ShouldImport (TypeDefinition td)
return true;
}

public static JavaClassModel? ParseClass (TypeDefinition type, JavaTypeCollection collection)
public static JavaClassModel? ParseClass (TypeDefinition type, JavaTypeCollection collection, ApiImporterOptions options)
{
// TODO: type parameters?
var obs_attr = GetObsoleteAttribute (type.CustomAttributes);
var reg_attr = GetRegisterAttribute (type.CustomAttributes);
var reg_attr = GetRegisterAttribute (type.CustomAttributes, options);

if (reg_attr is null)
return null;

var encoded_fullname = ((string) reg_attr.ConstructorArguments [0].Value).Replace ('/', '.');
var (package, nested_name) = DecodeRegisterJavaFullName (encoded_fullname);

var base_jni = GetBaseTypeJni (type);
var base_jni = GetBaseTypeJni (type, options);

var model = new JavaClassModel (
javaPackage: GetOrCreatePackage (collection, package, type.Namespace),
Expand All @@ -118,20 +118,20 @@ static bool ShouldImport (TypeDefinition td)
annotatedVisibility: string.Empty
); ;

ParseImplementedInterfaces (type, model);
ParseImplementedInterfaces (type, model, options);

foreach (var method in type.Methods.Where (m => !m.IsConstructor))
if (ParseMethod (method, model) is JavaMethodModel m)
if (ParseMethod (method, model, options) is JavaMethodModel m)
model.Methods.Add (m);

return model;
}

public static JavaInterfaceModel? ParseInterface (TypeDefinition type, JavaTypeCollection collection)
public static JavaInterfaceModel? ParseInterface (TypeDefinition type, JavaTypeCollection collection, ApiImporterOptions options)
{
// TODO: type paramters?
var obs_attr = GetObsoleteAttribute (type.CustomAttributes);
var reg_attr = GetRegisterAttribute (type.CustomAttributes);
var reg_attr = GetRegisterAttribute (type.CustomAttributes, options);

if (reg_attr is null)
return null;
Expand All @@ -149,22 +149,22 @@ static bool ShouldImport (TypeDefinition td)
annotatedVisibility: ""
);

ParseImplementedInterfaces (type, model);
ParseImplementedInterfaces (type, model, options);

foreach (var method in type.Methods)
if (ParseMethod (method, model) is JavaMethodModel m)
if (ParseMethod (method, model, options) is JavaMethodModel m)
model.Methods.Add (m);

return model;
}

public static JavaMethodModel? ParseMethod (MethodDefinition method, JavaTypeModel declaringType)
public static JavaMethodModel? ParseMethod (MethodDefinition method, JavaTypeModel declaringType, ApiImporterOptions options)
{
if (method.IsPrivate || method.IsAssembly)
return null;

var obs_attr = GetObsoleteAttribute (method.CustomAttributes);
var reg_attr = GetRegisterAttribute (method.CustomAttributes);
var reg_attr = GetRegisterAttribute (method.CustomAttributes, options);

if (reg_attr is null)
return null;
Expand Down Expand Up @@ -225,7 +225,7 @@ static void AddReferenceTypeRecursive (JavaTypeModel type, JavaTypeCollection co
AddReferenceTypeRecursive (nested, collection);
}

static bool ShouldSkipType (TypeDefinition type, TypeDefinitionCache cache)
static bool ShouldSkipType (TypeDefinition type, TypeDefinitionCache cache, ApiImporterOptions options)
{
// We want to use Java's collection types instead of our managed adapter.
// eg: 'Java.Util.ArrayList' over 'Android.Runtime.JavaList'
Expand All @@ -246,28 +246,28 @@ static bool ShouldSkipType (TypeDefinition type, TypeDefinitionCache cache)
? type.Module.GetType (type.FullName.Substring (0, type.FullName.IndexOf ('`')))
: null;

if (ShouldSkipGeneric (type, non_generic_type, cache))
if (ShouldSkipGeneric (type, non_generic_type, cache, options))
return true;

return false;
}

static bool ShouldSkipGeneric (TypeDefinition? a, TypeDefinition? b, TypeDefinitionCache cache)
static bool ShouldSkipGeneric (TypeDefinition? a, TypeDefinition? b, TypeDefinitionCache cache, ApiImporterOptions options)
{
if (a == null || b == null)
return false;
if (!a.ImplementsInterface ("Android.Runtime.IJavaObject", cache) || !b.ImplementsInterface ("Android.Runtime.IJavaObject", cache))
return false;

return GetRegisteredJavaTypeName (a) == GetRegisteredJavaTypeName (b);
return GetRegisteredJavaTypeName (a, options) == GetRegisteredJavaTypeName (b, options);
}

static string? TypeReferenceToJavaType (TypeReference type)
static string? TypeReferenceToJavaType (TypeReference type, ApiImporterOptions options)
{
var retval = GetRegisteredJavaName (type);
var retval = GetRegisteredJavaName (type, options);

if (retval != null && type is GenericInstanceType generic) {
var parameters = generic.GenericArguments.Select (ga => GetRegisteredJavaName (ga.Resolve ())).ToArray ();
var parameters = generic.GenericArguments.Select (ga => GetRegisteredJavaName (ga.Resolve (), options)).ToArray ();

if (parameters.WhereNotNull ().Any ())
retval += $"<{string.Join (", ", parameters.WhereNotNull ())}>";
Expand All @@ -276,14 +276,14 @@ static bool ShouldSkipGeneric (TypeDefinition? a, TypeDefinition? b, TypeDefinit
return retval;
}

static string? GetRegisteredJavaName (TypeReference type)
static string? GetRegisteredJavaName (TypeReference type, ApiImporterOptions options)
{
var td = type.Resolve ();

return GetRegisteredJavaTypeName (td);
return GetRegisteredJavaTypeName (td, options);
}

static void ParseImplementedInterfaces (TypeDefinition type, JavaTypeModel model)
static void ParseImplementedInterfaces (TypeDefinition type, JavaTypeModel model, ApiImporterOptions options)
{
foreach (var iface_impl in type.Interfaces) {
var iface = iface_impl.InterfaceType;
Expand All @@ -292,7 +292,7 @@ static void ParseImplementedInterfaces (TypeDefinition type, JavaTypeModel model
if (iface_def is null || iface_def.IsNotPublic)
continue;

if (GetRegisterAttribute (iface_def.CustomAttributes) is CustomAttribute reg_attr) {
if (GetRegisterAttribute (iface_def.CustomAttributes, options) is CustomAttribute reg_attr) {
var jni = (string) reg_attr.ConstructorArguments [0].Value;
var name = jni.Replace ('/', '.').Replace ('$', '.');

Expand All @@ -301,7 +301,7 @@ static void ParseImplementedInterfaces (TypeDefinition type, JavaTypeModel model
}
}

static string GetBaseTypeJni (TypeDefinition type)
static string GetBaseTypeJni (TypeDefinition type, ApiImporterOptions options)
{
// Find a Java base type, ignoring generic types, if nothing else it will be Java.Lang.Object
TypeDefinition? base_type = type;
Expand All @@ -319,7 +319,7 @@ static string GetBaseTypeJni (TypeDefinition type)
if (base_type.HasGenericParameters || base_type.IsGenericInstance)
continue;

if (GetRegisterAttribute (base_type.CustomAttributes) is CustomAttribute reg_attr)
if (GetRegisterAttribute (base_type.CustomAttributes, options) is CustomAttribute reg_attr)
return (string) reg_attr.ConstructorArguments [0].Value;
}

Expand All @@ -329,19 +329,19 @@ static string GetBaseTypeJni (TypeDefinition type)
static CustomAttribute? GetObsoleteAttribute (Collection<CustomAttribute> attributes) =>
attributes.FirstOrDefault (a => a.AttributeType.FullNameCorrected () == "System.ObsoleteAttribute");

static CustomAttribute? GetRegisterAttribute (Collection<CustomAttribute> attributes) =>
static CustomAttribute? GetRegisterAttribute (Collection<CustomAttribute> attributes, ApiImporterOptions options) =>
attributes.FirstOrDefault (a => {
var attrType = a.AttributeType.FullNameCorrected ();
return attrType == "Android.Runtime.RegisterAttribute" ||
attrType == "Java.Interop.JniTypeSignatureAttribute";

return options.SupportedTypeMapAttributes.Contains (attrType);
});

static string? GetRegisteredJavaTypeName (TypeDefinition type)
static string? GetRegisteredJavaTypeName (TypeDefinition type, ApiImporterOptions options)
{
if (GetSpecialCase (type) is string s)
return s;

if (GetRegisterAttribute (type.CustomAttributes) is CustomAttribute reg_attr)
if (GetRegisterAttribute (type.CustomAttributes, options) is CustomAttribute reg_attr)
return ((string) reg_attr.ConstructorArguments [0].Value).Replace ('/', '.');

return null;
Expand Down
1 change: 0 additions & 1 deletion src/Java.Interop.Tools.JavaTypeSystem/Java.Interop.Tools.JavaTypeSystem.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
<PropertyGroup>
<TargetFramework>$(DotNetTargetFramework)</TargetFramework>
<Nullable>enable</Nullable>
<LangVersion>8.0</LangVersion>
</PropertyGroup>

<Import Project="..\..\TargetFrameworkDependentValues.props" />
Expand Down
26 changes: 13 additions & 13 deletions tests/generator-Tests/Unit-Tests/ManagedTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ public void Method ()
{
var type = module.GetType ("Com.Mypackage.Foo");
var @class = CecilApiImporter.CreateClass (type, options);
var method = CecilApiImporter.CreateMethod (@class, type.Methods.First (m => m.Name == "Bar"));
var method = CecilApiImporter.CreateMethod (@class, type.Methods.First (m => m.Name == "Bar"), options);
Assert.IsTrue (method.Validate (new CodeGenerationOptions (), new GenericParameterDefinitionList (), new CodeGeneratorContext ()), "method.Validate failed!");

Assert.AreEqual ("public", method.Visibility);
Expand All @@ -183,8 +183,8 @@ public void Method_Matches_True ()
var type = module.GetType ("Com.Mypackage.Foo");
var @class = CecilApiImporter.CreateClass (type, options);
var unknownTypes = type.Methods.First (m => m.Name == "UnknownTypes");
var methodA = CecilApiImporter.CreateMethod (@class, unknownTypes);
var methodB = CecilApiImporter.CreateMethod (@class, unknownTypes);
var methodA = CecilApiImporter.CreateMethod (@class, unknownTypes, options);
var methodB = CecilApiImporter.CreateMethod (@class, unknownTypes, options);
Assert.IsTrue (methodA.Matches (methodB), "Methods should match!");
}

Expand All @@ -196,8 +196,8 @@ public void Method_Matches_False ()
var unknownTypesA = type.Methods.First (m => m.Name == "UnknownTypes");
var unknownTypesB = type.Methods.First (m => m.Name == "UnknownTypesReturn");
unknownTypesB.Name = "UnknownTypes";
var methodA = CecilApiImporter.CreateMethod (@class, unknownTypesA);
var methodB = CecilApiImporter.CreateMethod (@class, unknownTypesB);
var methodA = CecilApiImporter.CreateMethod (@class, unknownTypesA, options);
var methodB = CecilApiImporter.CreateMethod (@class, unknownTypesB, options);
//Everything the same besides return type
Assert.IsFalse (methodA.Matches (methodB), "Methods should not match!");
}
Expand All @@ -207,7 +207,7 @@ public void MethodWithParameters ()
{
var type = module.GetType ("Com.Mypackage.Foo");
var @class = CecilApiImporter.CreateClass (type, options);
var method = CecilApiImporter.CreateMethod (@class, type.Methods.First (m => m.Name == "BarWithParams"));
var method = CecilApiImporter.CreateMethod (@class, type.Methods.First (m => m.Name == "BarWithParams"), options);
Assert.IsTrue (method.Validate (new CodeGenerationOptions (), new GenericParameterDefinitionList (), new CodeGeneratorContext ()), "method.Validate failed!");
Assert.AreEqual ("(ZID)Ljava/lang/String;", method.JniSignature);
Assert.AreEqual ("java.lang.String", method.Return);
Expand Down Expand Up @@ -237,7 +237,7 @@ public void Ctor ()
{
var type = module.GetType ("Com.Mypackage.Foo");
var @class = CecilApiImporter.CreateClass (type, options);
var ctor = CecilApiImporter.CreateCtor (@class, type.Methods.First (m => m.IsConstructor && !m.IsStatic));
var ctor = CecilApiImporter.CreateCtor (@class, type.Methods.First (m => m.IsConstructor && !m.IsStatic), options);
Assert.IsTrue (ctor.Validate (new CodeGenerationOptions (), new GenericParameterDefinitionList (), new CodeGeneratorContext ()), "ctor.Validate failed!");

Assert.AreEqual ("public", ctor.Visibility);
Expand All @@ -251,7 +251,7 @@ public void Field ()
{
var type = module.GetType ("Com.Mypackage.Foo");
var @class = CecilApiImporter.CreateClass (type, options);
var field = CecilApiImporter.CreateField (type.Fields.First (f => f.Name == "Value"));
var field = CecilApiImporter.CreateField (type.Fields.First (f => f.Name == "Value"), options);
Assert.IsTrue (field.Validate (new CodeGenerationOptions (), new GenericParameterDefinitionList (), new CodeGeneratorContext ()), "field.Validate failed!");

Assert.AreEqual ("Value", field.Name);
Expand Down Expand Up @@ -303,23 +303,23 @@ public void TypeNullability ()
var type = module.GetType ("NullableTestTypes.NullableClass");
var gen = CecilApiImporter.CreateClass (module.GetType ("NullableTestTypes.NullableClass"), options);

var not_null_field = CecilApiImporter.CreateField (type.Fields.First (f => f.Name == "not_null_field"));
var not_null_field = CecilApiImporter.CreateField (type.Fields.First (f => f.Name == "not_null_field"), options);
Assert.AreEqual (true, not_null_field.NotNull);

var null_field = CecilApiImporter.CreateField (type.Fields.First (f => f.Name == "null_field"));
var null_field = CecilApiImporter.CreateField (type.Fields.First (f => f.Name == "null_field"), options);
Assert.AreEqual (false, null_field.NotNull);

var null_method = CecilApiImporter.CreateMethod (gen, type.Methods.First (f => f.Name == "NullableReturnMethod"));
var null_method = CecilApiImporter.CreateMethod (gen, type.Methods.First (f => f.Name == "NullableReturnMethod"), options);
Assert.AreEqual (false, null_method.ReturnNotNull);
Assert.AreEqual (true, null_method.Parameters.First (f => f.Name == "notnull").NotNull);
Assert.AreEqual (false, null_method.Parameters.First (f => f.Name == "nullable").NotNull);

var not_null_method = CecilApiImporter.CreateMethod (gen, type.Methods.First (f => f.Name == "NotNullReturnMethod"));
var not_null_method = CecilApiImporter.CreateMethod (gen, type.Methods.First (f => f.Name == "NotNullReturnMethod"), options);
Assert.AreEqual (true, not_null_method.ReturnNotNull);
Assert.AreEqual (true, not_null_method.Parameters.First (f => f.Name == "notnull").NotNull);
Assert.AreEqual (false, not_null_method.Parameters.First (f => f.Name == "nullable").NotNull);

var ctor = CecilApiImporter.CreateCtor (gen, type.Methods.First (f => f.Name == ".ctor"));
var ctor = CecilApiImporter.CreateCtor (gen, type.Methods.First (f => f.Name == ".ctor"), options);
Assert.AreEqual (true, ctor.Parameters.First (f => f.Name == "notnull").NotNull);
Assert.AreEqual (false, ctor.Parameters.First (f => f.Name == "nullable").NotNull);
}
Expand Down
2 changes: 1 addition & 1 deletion tools/generator/CodeGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ static void Run (CodeGeneratorOptions options, DirectoryAssemblyResolver resolve
// Resolve types using Java.Interop.Tools.JavaTypeSystem
if (is_classparse && !options.UseLegacyJavaResolver) {
var output_xml = api_xml_adjuster_output ?? Path.Combine (Path.GetDirectoryName (filename), Path.GetFileName (filename) + ".adjusted");
JavaTypeResolutionFixups.Fixup (filename, output_xml, resolver, references.Distinct ().ToArray (), resolverCache);
JavaTypeResolutionFixups.Fixup (filename, output_xml, resolver, references.Distinct ().ToArray (), resolverCache, options);

if (only_xml_adjuster)
return;
Expand Down
Loading

0 comments on commit f863351

Please sign in to comment.