Skip to content

Commit 9027591

Browse files
committed
Fix Java.Interop.Export-Tests
A funny thing happened on the way through CI: `Java.Interop.Export-Tests` *with* `jnimarshalmethod-gen` failed! Failed AddExportMethods [169 ms] Error Message: Java.Interop.JavaException : Could not initialize class com.xamarin.interop.export.ExportType Stack Trace: at Java.Interop.JniEnvironment.StaticMethods.GetStaticMethodID(JniObjectReference type, String name, String signature) in /Users/runner/work/1/s/src/Java.Interop/obj/Release/net7.0/JniEnvironment.g.cs:line 21407 at Java.Interop.JniType.GetStaticMethod(String name, String signature) in /Users/runner/work/1/s/src/Java.Interop/Java.Interop/JniType.cs:line 315 at Java.InteropTests.MarshalMemberBuilderTest.AddExportMethods() at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor) at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr) --- End of managed Java.Interop.JavaException stack trace --- java.lang.NoClassDefFoundError: Could not initialize class com.xamarin.interop.export.ExportType The cause? e1822f0 updated `jnimarshalmethod-gen`: - registrations.Add (new ExpressionMethodRegistration (name, signature, mmDef)); + // Assume that `JavaCallableAttribute.Name` is "public" JCW name, and JCW's declare `n_`-prefixed `native` methods… + registrations.Add (new ExpressionMethodRegistration ("n_" + method.Name, signature, mmDef)); That is, instead of attempting to register e.g. `ExportType.action()V` (via `[JavaCallable("action")]`), it was instead attempting to register `ExportType.n_action()V`, because of the introduced `"n_" + method.Name` change. The "problem" is that `jnimarshalmethod-gen` was written in the context of Java.Interop, *not* .NET Android, and the existing `Java.Interop.Export-Tests` unit tests assume that there is no `n_`-prefix on native method declarations. There are two plausible solutions: update the unit tests to conform to existing .NET Android convention, and use an `n_` prefix on `native` method declarations. Or update `jcw-gen` so that when using `jcw-gen --codegen-target=JavaInterop1`, the `JavaCallableAttribute.Name` value is used for the `native` method declaration. Because @jonpryor is a glutton for punishment and "cleanliness", let's try the latter. `JavaCallableWrapperGenerator` already has a `.CodeGenerationTarget` property, How Hard Could It Be™? Turns out, harder than expected: `JavaCallableWrapperGenerator` was doing *lots* of work in its constructor, including the all important bit of populating `Signature` values, and the constructor runs *before* the `.CodeGenerationTarget` property is set. This is a somewhat straightforward fix: turn most of the `JavaCallableWrapperGenerator` constructor into a `Initialize()` method, and call `Initialize()` from the public methods. This creates a semantic change: some exceptions which were thrown by the constructor are now thrown from the public methods. We deem this as acceptable because all known uses of `JavaCallableWrapperGenerator` ~immediately call `.Generate()` after creating the instance, so the exception will still be thrown from a site near where it previously would have been thrown. The more annoying aspect is `Signature` initialization: we need to pass in `.CodeGenerationTarget`, which is Yet Another Constructor Parameter, and we already have A Lot™. Attempt to bring sanity to this mess by introducing a new `SignatureOptions` type to hold the `Signature` parameters. Update `jnimarshalmethod-gen` to undo the change which broke `Java.Interop.Export-Tests`. Update `tests/Java.Interop.Tools.JavaCallableWrappers-Tests` to add a test for `.CodeGenerationTarget==JavaInterop1`. Add `$(NoWarn)` to `Java.Interop.Tools.JavaCallableWrappers-Tests.csproj` in order to "work around" the warnings-as-errors: …/src/Java.Interop.NamingCustomAttributes/Java.Interop/ExportFieldAttribute.cs(19,63): error CA1019: Remove the property setter from Name or reduce its accessibility because it corresponds to positional argument name …/src/Java.Interop.NamingCustomAttributes/Android.Runtime/RegisterAttribute.cs(53,4): error CA1019: Remove the property setter from Name or reduce its accessibility because it corresponds to positional argument name …/src/Java.Interop.NamingCustomAttributes/Java.Interop/ExportFieldAttribute.cs(12,16): error CA1813: Avoid unsealed attributes … This is "weird"; the warnings/errors appear to come in because `Java.Interop.Tools.JavaCallableWrappers-Tests.csproj` now has: <Compile Include="..\..\src\Java.Interop\Java.Interop\JniTypeSignatureAttribute.cs" /> which appears to pull in `src/Java.Interop/.editorconfig`, which makes CA1019 and CA1813 errors. (insert massively confused face here. Like, wat?)
1 parent e1822f0 commit 9027591

File tree

8 files changed

+218
-46
lines changed

8 files changed

+218
-46
lines changed

src/Java.Interop.Export/Java.Interop/JavaCallableAttribute.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1+
#nullable enable
12
using System;
23

34
namespace Java.Interop {
45

5-
[AttributeUsage (AttributeTargets.Method, AllowMultiple=false)]
6+
[AttributeUsage (AttributeTargets.Constructor | AttributeTargets.Method, AllowMultiple=false)]
67
public sealed class JavaCallableAttribute : Attribute {
78

89
public JavaCallableAttribute ()

src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers.csproj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
<PropertyGroup>
44
<TargetFramework>netstandard2.0</TargetFramework>
55
<AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath>
6-
<LangVersion>8.0</LangVersion>
6+
<LangVersion>11.0</LangVersion>
77
<Nullable>enable</Nullable>
88
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
99
<SignAssembly>true</SignAssembly>

src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers/JavaCallableWrapperGenerator.cs

Lines changed: 138 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,18 @@ void AddNestedTypes (TypeDefinition type)
160160
type.IsSubclassOf ("Android.Content.ContentProvider", cache)))
161161
Diagnostic.Error (4203, LookupSource (type), Localization.Resources.JavaCallableWrappers_XA4203, jniName);
162162

163+
this.outerType = outerType;
164+
}
165+
166+
bool initialized;
167+
string? outerType;
168+
169+
void Initialize ()
170+
{
171+
if (initialized)
172+
return;
173+
initialized = true;
174+
163175
foreach (MethodDefinition minfo in type.Methods.Where (m => !m.IsConstructor)) {
164176
var baseRegisteredMethod = GetBaseRegisteredMethod (minfo);
165177
if (baseRegisteredMethod != null)
@@ -298,7 +310,7 @@ void AddConstructor (MethodDefinition ctor, TypeDefinition type, string? outerTy
298310
if (!string.IsNullOrEmpty (eattr.Name)) {
299311
// Diagnostic.Warning (log, "Use of ExportAttribute.Name property is invalid on constructors");
300312
}
301-
ctors.Add (new Signature (ctor, eattr, cache));
313+
ctors.Add (new Signature (new (cache, CodeGenerationTarget, ctor, eattr)));
302314
curCtors.Add (ctor);
303315
return;
304316
}
@@ -307,7 +319,10 @@ void AddConstructor (MethodDefinition ctor, TypeDefinition type, string? outerTy
307319
if (rattr != null) {
308320
if (ctors.Any (c => c.JniSignature == rattr.Signature))
309321
return;
310-
ctors.Add (new Signature (ctor, rattr, managedParameters, outerType, cache));
322+
ctors.Add (new Signature (new (cache, CodeGenerationTarget, ctor, rattr) {
323+
ManagedParameters = managedParameters,
324+
OuterType = outerType,
325+
}));
311326
curCtors.Add (ctor);
312327
return;
313328
}
@@ -328,12 +343,26 @@ void AddConstructor (MethodDefinition ctor, TypeDefinition type, string? outerTy
328343
}
329344

330345
if (baseCtors.Any (m => m.Parameters.AreParametersCompatibleWith (ctor.Parameters, cache))) {
331-
ctors.Add (new Signature (".ctor", jniSignature, "", managedParameters, outerType, null));
346+
ctors.Add (new Signature (new (cache, CodeGenerationTarget) {
347+
RegisterName = ".ctor",
348+
RegisterSignature = jniSignature,
349+
RegisterConnector = "",
350+
ManagedParameters = managedParameters,
351+
OuterType = outerType,
352+
IsDynamicallyRegistered = true,
353+
}));
332354
curCtors.Add (ctor);
333355
return;
334356
}
335357
if (baseCtors.Any (m => !m.HasParameters)) {
336-
ctors.Add (new Signature (".ctor", jniSignature, "", managedParameters, outerType, ""));
358+
ctors.Add (new Signature (new (cache, CodeGenerationTarget) {
359+
RegisterName = ".ctor",
360+
RegisterSignature = jniSignature,
361+
RegisterConnector = "",
362+
ManagedParameters = managedParameters,
363+
OuterType = outerType,
364+
IsDynamicallyRegistered = true,
365+
}));
337366
curCtors.Add (ctor);
338367
return;
339368
}
@@ -489,15 +518,17 @@ void AddMethod (MethodDefinition? registeredMethod, MethodDefinition implemented
489518
Diagnostic.Error (4217, LookupSource (implementedMethod), Localization.Resources.JavaCallableWrappers_XA4217, attr.Name);
490519

491520
bool shouldBeDynamicallyRegistered = methodClassifier?.ShouldBeDynamicallyRegistered (type, registeredMethod, implementedMethod, attr.OriginAttribute) ?? true;
492-
var msig = new Signature (implementedMethod, attr, cache, shouldBeDynamicallyRegistered);
521+
var msig = new Signature (new (cache, CodeGenerationTarget, implementedMethod, attr) {
522+
IsDynamicallyRegistered = shouldBeDynamicallyRegistered,
523+
});
493524
if (!registeredMethod.IsConstructor && !methods.Any (m => m.Name == msig.Name && m.Params == msig.Params))
494525
methods.Add (msig);
495526
}
496527
foreach (ExportAttribute attr in GetExportAttributes (implementedMethod)) {
497528
if (type.HasGenericParameters)
498529
Diagnostic.Error (4206, LookupSource (implementedMethod), Localization.Resources.JavaCallableWrappers_XA4206);
499530

500-
var msig = new Signature (implementedMethod, attr, cache);
531+
var msig = new Signature (new (cache, CodeGenerationTarget, implementedMethod, attr));
501532
if (!string.IsNullOrEmpty (attr.SuperArgumentsString)) {
502533
// Diagnostic.Warning (log, "Use of ExportAttribute.SuperArgumentsString property is invalid on methods");
503534
}
@@ -508,7 +539,7 @@ void AddMethod (MethodDefinition? registeredMethod, MethodDefinition implemented
508539
if (type.HasGenericParameters)
509540
Diagnostic.Error (4207, LookupSource (implementedMethod), Localization.Resources.JavaCallableWrappers_XA4207);
510541

511-
var msig = new Signature (implementedMethod, attr, cache);
542+
var msig = new Signature (new (cache, CodeGenerationTarget, implementedMethod, attr));
512543
if (!implementedMethod.IsConstructor && !methods.Any (m => m.Name == msig.Name && m.Params == msig.Params)) {
513544
methods.Add (msig);
514545
exported_fields.Add (new JavaFieldInfo (implementedMethod, attr.Name, cache));
@@ -557,6 +588,8 @@ string GetManagedParameters (MethodDefinition ctor, string? outerType)
557588

558589
public void Generate (TextWriter writer)
559590
{
591+
Initialize ();
592+
560593
if (!string.IsNullOrEmpty (package)) {
561594
writer.WriteLine ("package " + package + ";");
562595
writer.WriteLine ();
@@ -773,7 +806,7 @@ void GenerateRegisterType (TextWriter sw, JavaCallableWrapperGenerator self, str
773806

774807
foreach (Signature method in self.methods) {
775808
if (method.IsDynamicallyRegistered) {
776-
sw.Write ("\t\t\t\"", method.Method);
809+
sw.Write ("\t\t\t\"");
777810
sw.Write (method.Method);
778811
sw.WriteLine ("\\n\" +");
779812
}
@@ -839,65 +872,121 @@ bool CannotRegisterInStaticConstructor (TypeDefinition type)
839872
return JavaNativeTypeManager.IsApplication (type, cache) || JavaNativeTypeManager.IsInstrumentation (type, cache);
840873
}
841874

842-
class Signature {
875+
record struct SignatureOptions (IMetadataResolver Cache, JavaPeerStyle Style) {
843876

844-
public Signature (MethodDefinition method, RegisterAttribute register, IMetadataResolver cache, bool shouldBeDynamicallyRegistered = true) : this (method, register, null, null, cache, shouldBeDynamicallyRegistered) {}
877+
public SignatureOptions (IMetadataResolver cache, JavaPeerStyle style, MethodDefinition method)
878+
: this (cache, style)
879+
{
880+
Method = method;
881+
IsStatic = method.IsStatic;
882+
JavaAccess = JavaCallableWrapperGenerator.GetJavaAccess (method.Attributes & MethodAttributes.MemberAccessMask);
883+
Annotations = JavaCallableWrapperGenerator.GetAnnotationsString ("\t", method.CustomAttributes, cache);
884+
IsDynamicallyRegistered = true;
885+
}
845886

846-
public Signature (MethodDefinition method, RegisterAttribute register, string? managedParameters, string? outerType, IMetadataResolver cache, bool shouldBeDynamicallyRegistered = true)
847-
: this (register.Name, register.Signature, register.Connector, managedParameters, outerType, null)
887+
public SignatureOptions (IMetadataResolver cache, JavaPeerStyle style, MethodDefinition method, RegisterAttribute register)
888+
: this (cache, style, method)
848889
{
849-
Annotations = JavaCallableWrapperGenerator.GetAnnotationsString ("\t", method.CustomAttributes, cache);
850-
IsDynamicallyRegistered = shouldBeDynamicallyRegistered;
890+
Register = register;
851891
}
852892

853-
public Signature (MethodDefinition method, ExportAttribute export, IMetadataResolver cache)
854-
: this (method.Name, GetJniSignature (method, cache), "__export__", null, null, export.SuperArgumentsString)
893+
public SignatureOptions (IMetadataResolver cache, JavaPeerStyle style, MethodDefinition method, ExportAttribute export)
894+
: this (cache, style, method)
855895
{
856-
IsExport = true;
857-
IsStatic = method.IsStatic;
858-
JavaAccess = JavaCallableWrapperGenerator.GetJavaAccess (method.Attributes & MethodAttributes.MemberAccessMask);
859-
ThrownTypeNames = export.ThrownNames;
860-
JavaNameOverride = export.Name;
861-
Annotations = JavaCallableWrapperGenerator.GetAnnotationsString ("\t", method.CustomAttributes, cache);
896+
IsExport = true;
897+
SuperCall = export.SuperArgumentsString;
898+
ThrownTypeNames = export.ThrownNames;
899+
JavaNameOverride = export.Name;
900+
if (style == JavaPeerStyle.JavaInterop1) {
901+
RegisterName = export.Name;
902+
}
862903
}
863904

864-
public Signature (MethodDefinition method, ExportFieldAttribute exportField, IMetadataResolver cache)
865-
: this (method.Name, GetJniSignature (method, cache), "__export__", null, null, null)
905+
public SignatureOptions (IMetadataResolver cache, JavaPeerStyle style, MethodDefinition method, ExportFieldAttribute exportField)
906+
: this (cache, style, method)
866907
{
908+
IsExport = true;
909+
Annotations = null;
910+
867911
if (method.HasParameters)
868912
Diagnostic.Error (4205, JavaCallableWrapperGenerator.LookupSource (method), Localization.Resources.JavaCallableWrappers_XA4205);
869913
if (method.ReturnType.MetadataType == MetadataType.Void)
870914
Diagnostic.Error (4208, JavaCallableWrapperGenerator.LookupSource (method), Localization.Resources.JavaCallableWrappers_XA4208);
871-
IsExport = true;
872-
IsStatic = method.IsStatic;
873-
JavaAccess = JavaCallableWrapperGenerator.GetJavaAccess (method.Attributes & MethodAttributes.MemberAccessMask);
915+
}
916+
874917

875-
// annotations are processed within JavaFieldInfo, not the initializer method. So we don't generate them here.
918+
public string? RegisterName;
919+
public string? RegisterSignature;
920+
public string? RegisterConnector;
921+
public RegisterAttribute Register {
922+
set {
923+
RegisterName = value.Name;
924+
RegisterSignature = value.Signature;
925+
RegisterConnector = value.Connector;
926+
}
927+
}
928+
public MethodDefinition Method {
929+
set {
930+
RegisterName = value.Name;
931+
RegisterSignature = GetJniSignature (value, Cache);
932+
RegisterConnector = "__export__";
933+
Annotations = JavaCallableWrapperGenerator.GetAnnotationsString ("\t", value.CustomAttributes, Cache);
934+
}
876935
}
936+
public bool IsDynamicallyRegistered;
937+
public string? ManagedParameters;
938+
public string? OuterType;
939+
public string? SuperCall;
940+
public string? Annotations;
941+
public string? JavaAccess;
942+
public string? JavaNameOverride;
943+
public bool IsExport;
944+
public bool IsStatic;
945+
public string[]? ThrownTypeNames;
946+
}
877947

878-
public Signature (string name, string? signature, string? connector, string? managedParameters, string? outerType, string? superCall)
948+
class Signature {
949+
950+
public Signature (SignatureOptions options)
879951
{
880-
ManagedParameters = managedParameters;
881-
JniSignature = signature ?? throw new ArgumentNullException ("`connector` cannot be null.", nameof (connector));
882-
Method = "n_" + name + ":" + JniSignature + ":" + connector;
883-
Name = name;
952+
if (options.RegisterName == null) {
953+
throw new ArgumentNullException ("`RegisterName` cannot be null.", nameof (options.RegisterName));
954+
}
955+
if (options.RegisterSignature == null) {
956+
throw new ArgumentNullException ("`RegisterSignature` cannot be null.", nameof (options.RegisterSignature));
957+
}
958+
Annotations = options.Annotations;
959+
IsDynamicallyRegistered = options.IsDynamicallyRegistered;
960+
IsExport = options.IsExport;
961+
IsStatic = options.IsStatic;
962+
JavaAccess = options.JavaAccess;
963+
JavaNameOverride = options.JavaNameOverride;
964+
JniSignature = options.RegisterSignature;
965+
ManagedParameters = options.ManagedParameters;
966+
Method = options.RegisterName + ":" + JniSignature + ":" + options.RegisterConnector;
967+
Name = options.RegisterName;
968+
ThrownTypeNames = options.ThrownTypeNames;
969+
970+
if (options.Style == JavaPeerStyle.XAJavaInterop1) {
971+
Method = "n_" + Method;
972+
}
884973

885974
var jnisig = JniSignature;
886975
int closer = jnisig.IndexOf (')');
887976
string ret = jnisig.Substring (closer + 1);
888977
retval = JavaNativeTypeManager.Parse (ret)?.Type;
889978
string jniparms = jnisig.Substring (1, closer - 1);
890-
if (string.IsNullOrEmpty (jniparms) && string.IsNullOrEmpty (superCall))
979+
if (string.IsNullOrEmpty (jniparms) && string.IsNullOrEmpty (options.SuperCall))
891980
return;
892981
var parms = new StringBuilder ();
893982
var scall = new StringBuilder ();
894983
var acall = new StringBuilder ();
895984
bool first = true;
896985
int i = 0;
897986
foreach (JniTypeName jti in JavaNativeTypeManager.FromSignature (jniparms)) {
898-
if (outerType != null) {
899-
acall.Append (outerType).Append (".this");
900-
outerType = null;
987+
if (options.OuterType != null) {
988+
acall.Append (options.OuterType).Append (".this");
989+
options.OuterType = null;
901990
continue;
902991
}
903992
string? parmType = jti.Type;
@@ -913,7 +1002,7 @@ public Signature (string name, string? signature, string? connector, string? man
9131002
++i;
9141003
}
9151004
this.parms = parms.ToString ();
916-
this.call = superCall != null ? superCall : scall.ToString ();
1005+
this.call = options.SuperCall != null ? options.SuperCall : scall.ToString ();
9171006
this.ActivateCall = acall.ToString ();
9181007
}
9191008

@@ -1050,13 +1139,21 @@ void GenerateMethod (Signature method, TextWriter sw)
10501139
sw.Write (' ');
10511140
if (method.IsStatic)
10521141
sw.Write ("static ");
1142+
if (CodeGenerationTarget == JavaPeerStyle.JavaInterop1) {
1143+
sw.Write ("native ");
1144+
}
10531145
sw.Write (method.Retval);
10541146
sw.Write (' ');
10551147
sw.Write (method.JavaName);
10561148
sw.Write (" (");
10571149
sw.Write (method.Params);
10581150
sw.Write (')');
1059-
sw.WriteLine (method.ThrowsDeclaration);
1151+
sw.Write (method.ThrowsDeclaration);
1152+
if (CodeGenerationTarget == JavaPeerStyle.JavaInterop1) {
1153+
sw.WriteLine (";");
1154+
return;
1155+
}
1156+
sw.WriteLine ();
10601157
sw.WriteLine ("\t{");
10611158
#if MONODROID_TIMING
10621159
sw.WriteLine ("\t\tandroid.util.Log.i(\"MonoDroid-Timing\", \"{0}.{1}: time: \"+java.lang.System.currentTimeMillis());", name, method.Name);
@@ -1127,6 +1224,8 @@ StreamWriter OpenStream (string outputPath)
11271224
/// </summary>
11281225
public string GetDestinationPath (string outputPath)
11291226
{
1227+
Initialize ();
1228+
11301229
var dir = package.Replace ('.', Path.DirectorySeparatorChar);
11311230
return Path.Combine (outputPath, dir, name + ".java");
11321231
}

src/Java.Interop/Java.Interop/JniTypeSignatureAttribute.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ public sealed class JniTypeSignatureAttribute : Attribute {
1111

1212
public JniTypeSignatureAttribute (string simpleReference)
1313
{
14+
#if !JCW_ONLY_TYPE_NAMES
1415
JniRuntime.JniTypeManager.AssertSimpleReference (simpleReference, nameof (simpleReference));
16+
#endif // !JCW_ONLY_TYPE_NAMES
1517

1618
SimpleReference = simpleReference;
1719
}

tests/Java.Interop.Tools.JavaCallableWrappers-Tests/Java.Interop.Tools.JavaCallableWrappers-Tests.csproj

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
<TargetFramework>$(DotNetTargetFramework)</TargetFramework>
55
<IsPackable>false</IsPackable>
66
<DefineConstants>$(DefineConstants);HAVE_CECIL;JCW_ONLY_TYPE_NAMES</DefineConstants>
7+
<NoWarn>$(NoWarn);CA1019;CA1813</NoWarn>
78
</PropertyGroup>
89

910
<Import Project="..\..\TargetFrameworkDependentValues.props" />
@@ -29,4 +30,9 @@
2930

3031
<Import Project="..\..\src\Java.Interop.NamingCustomAttributes\Java.Interop.NamingCustomAttributes.projitems" Label="Shared" Condition="Exists('..\..\src\Java.Interop.NamingCustomAttributes\Java.Interop.NamingCustomAttributes.projitems')" />
3132

33+
<ItemGroup>
34+
<Compile Include="..\..\src\Java.Interop\Java.Interop\JniTypeSignatureAttribute.cs" />
35+
<Compile Include="..\..\src\Java.Interop.Export\Java.Interop\JavaCallableAttribute.cs" />
36+
</ItemGroup>
37+
3238
</Project>

0 commit comments

Comments
 (0)