Remove trim and AOT warning suppressions

[simonrozsival]

Related to dotnet/android#10794.

This models Java peer activation, JNI type lookup, and native registration reflection requirements for trim/AOT analysis so dotnet/android can remove suppressions instead of hiding analyzer warnings.

• Enables IsAotCompatible for Java.Interop.csproj.
• Adds an ActivatePeer overload that carries the declaring type constructor preservation contract.
• Keeps the existing ConstructorInfo-based overload as a RUC compatibility shim.
• Splits constructor-only JNI type lookup from native-registration lookup so DAM requirements are explicit.
• Removes UnconditionalSuppressMessage usage from the Java.Interop project source.
• Removes IL analyzer pragmas from Java.Interop trim/AOT paths.

Validation:

• ./dotnet-local.sh build external/Java.Interop/src/Java.Interop/Java.Interop.csproj -v:minimal -nr:false
• ./dotnet-local.sh build external/Java.Interop/src/Java.Runtime.Environment/Java.Runtime.Environment.csproj -v:minimal -nr:false
• rg "UnconditionalSuppressMessage|pragma warning disable IL" external/Java.Interop/src/Java.Interop -g "*.cs"

Both builds succeeded with 0 warnings and 0 errors, and the scan has no matches.

[jonathanpeppers]

/review

[github-actions]

✅ Java.Interop PR Reviewer completed successfully!

[github-actions]

Analysis Summary

I reviewed the 10 requested files. Here are my findings:

Test Files

JavaVMFixture.cs — GetTypeForNativeRegistration override:

• The implementation is correct. It does call base.GetTypeForNativeRegistration() (line 113), but with reversed priority vs the other overrides: local TypeMappings are checked first, base is the fallback. This is reasonable — the base implementation currently returns null, so checking local mappings first avoids a pointless null return. See inline comment.

Test coverage for new API surface:

• GetType — Covered via JniTypeManagerTests.GetType() (line 122) and GetTypeSignature() (line 208), plus JavaVMFixture exercises the override.
• GetTypeAssignableTo — Indirectly tested via JniValueManagerTests.GetValue_FindBestMatchType() which triggers the peer creation path (now calling GetTypeAssignableTo). The JavaVMFixture override exercises it directly.
• GetTypeForNativeRegistration — Override in JavaVMFixture exists. No dedicated unit test for the base class behavior (returns null), but integration coverage comes from ManagedPeer call sites.
• GetInvokerTypeForPeerCreation — This is internal, tested indirectly via TryCreatePeerInstance in the value manager path. The new GetInvokerType_Generic test (line 33 of JniRuntime.JniTypeManagerTests.cs) validates the public GetInvokerType with generics but not GetInvokerTypeForPeerCreation specifically — however, the latter intentionally rejects generics (returns null for arguments.Length > 0), so this is fine.
• ActivatePeer new overload: Both JniRuntimeTest.cs and JniRuntime.JniValueManagerTests.cs updated their ActivatePeer overrides to include the new Type declaringType parameter. The old overload is kept as a compatibility shim in the base class.

JniValueMarshalerContractTests.cs — Expression.Constant validation:

• The change from new {marshalerType}() to {marshalerValue} (where marshalerValue = FullName) correctly validates the Expression.New → Expression.Constant shift. The same pattern is correctly applied in the JniInt32ArrayValueMarshalerContractTests override (line 507). See inline comment.

Generator Files

ClassGen.cs — __TypeRegistrations pattern change:

• Moving RequiresDynamicCode/RequiresUnreferencedCode from Lookup method to class level is correct and properly replaces the old UnconditionalSuppressMessage on Lookup. See inline comment.

ArraySymbol.cs — JNIEnv.GetArray changes:

• Both FromNative (line 95) and PreCallback (line 160) correctly switch from JNIEnv.GetArray(handle, ownership, typeof(T)) to JNIEnv.GetArray<T>(handle, ownership). This eliminates typeof() usage that would trigger trim warnings.

GeneratedEnumSymbol.cs — FromNative change:

• Line 30: Same pattern — JNIEnv.GetArray(...) with typeof() is replaced by JNIEnv.GetArray<{0}>(...). Correct.

Expected output __TypeRegistrations.cs:

• The expected test output correctly reflects the generator changes: attributes moved to class level (lines 17-20), UnconditionalSuppressMessage removed from Lookup (lines 37-39 old → removed). This will keep generator tests passing.

No Issues Found

All changes are internally consistent and correctly implement the trim/AOT annotation strategy described in the PR.

Generated by Java.Interop PR Reviewer for issue #1416 · ● 44.5M

[github-actions]

Note: Unlike GetType (line 59) and GetTypeAssignableTo (line 86), which call base first and fall back to local mappings, GetTypeForNativeRegistration does the opposite — it checks local mappings first and falls back to base last.

This ordering difference is intentional and correct for native registration (you want the test's registered types to take priority over the base class's default null return), but it's worth noting the asymmetry. If the base class implementation ever gains real logic (currently returns null), the reversed priority order will matter.

[github-actions]

The change from JNIEnv.GetArray({1}, {2}, typeof({3})) to JNIEnv.GetArray<{2}>({1}, {4}) switches from the non-generic GetArray overload (which takes a Type parameter via typeof(), triggering trim warnings) to the generic GetArray<T>() overload. This is correct — the generic overload preserves the type statically, so the trimmer can analyze it without warnings. The format string index renumbering is correct.

[github-actions]

The change from new {marshalerType}() to {marshalerValue} (using FullName) correctly reflects the underlying Expression.New → Expression.Constant shift in JniValueMarshaler.CreateSelf(). The ToCSharpCode() renderer for Expression.Constant emits the value's ToString() (which for types defaults to FullName), so using marshalerValue = marshaler.GetType().FullName here is the right expected string.

This is a sound change: Expression.Constant(this, marshalerType) captures the existing marshaler instance rather than constructing a new one, which avoids the trimmer warning from Expression.New needing to preserve the parameterless constructor. The test correctly validates this new behavior.

[github-actions]

Moving RequiresDynamicCode/RequiresUnreferencedCode from the Lookup method to the class level is the right approach. The Lookup method calls Type.GetType(managedType) which triggers IL2057 (the trim warning about Type.GetType with a dynamically-computed string). Placing the attributes at the class level instead of only on Lookup:

1. Ensures all members (including the lookup_*_package delegates that call Lookup) are covered
2. Replaces the per-method UnconditionalSuppressMessage with a proper caller-notification via the class-level attributes
3. The #if NET5_0_OR_GREATER guard is correct since these attributes were introduced in .NET 5

This is a clean improvement — the whole registration class is inherently trim-incompatible, so annotating the class is more accurate than suppressing on individual methods.

[github-actions]

GetNativeRegistrationTypeFromSignature is dead code — it's defined but never called anywhere in the codebase. The actual RegisterNativeMembers path now calls TypeManager.GetTypeForNativeRegistration directly (lines 294 and 314). Consider removing this unused method to avoid confusion.

[github-actions]

The [return: DynamicallyAccessedMembers(MethodsConstructors)] annotation claims the returned Type preserves constructors, all methods, and non-public nested types. Since typeMappings stores arbitrary Type values provided by the caller with no DAM contract, this is technically an unsound annotation — the trimmer will trust it, but there's no static guarantee the returned types actually have those members preserved.

Same concern as the base GetTypeForNativeRegistration — this is safe as long as callers (e.g., dotnet/android's linker steps) ensure the types in typeMappings are fully preserved.

[github-actions]

Good: The GetBaseDirectory() change from Assembly.Location to AppContext.BaseDirectory properly removes the IL3000 suppression. AppContext.BaseDirectory is trim/AOT-safe and works in single-file/NativeAOT scenarios where Assembly.Location returns empty string. This is a clean improvement. 👍

[github-actions]

The old GetListType iterated Type.GetInterfaces() to find any type implementing IList<T> (e.g. List<string>, JavaObjectArray<Foo> subclasses, etc.). The new version only matches: raw arrays, the three exact Java generic array types, and IList<T> itself.

This is clearly intentional to eliminate the trim-unsafe GetInterfaces() call, but it narrows the matching: a concrete type like List<string> will no longer be routed to the object-array marshaler path. The impact should be limited since Java interop types are typically JavaObjectArray<T> or plain arrays, but it's worth a quick sanity check that no test or downstream consumer relies on List<T> → array-marshaler resolution.

[github-actions]

GetNativeRegistrationTypeFromSignature is defined but never called anywhere in the codebase. It appears to be dead code left over from the refactoring — RegisterNativeMembers now calls GetTypeForNativeRegistration inline (lines 294, 314) and CreateMissingNativeRegistrationTypeException directly.

Consider removing it to keep the codebase clean, or was this intended as a convenience for future callers?

[github-actions]

Nit: The return type annotation [return: DynamicallyAccessedMembers (MethodsConstructors)] promises that the returned Type has methods, constructors, and nested types preserved. However, when the type comes from typeMappings (line 78), there's no static guarantee that the dictionary values carry that preservation — the IDictionary<string, Type> parameter in the constructor has no DAM annotation.

This is the same trust pattern the old code had (the suppression was essentially asserting the same thing), so it's not a regression. But since the goal of this PR is to move away from suppressions, it may be worth adding a [DynamicallyAccessedMembers] constraint to the typeMappings dictionary entries via a wrapper or documenting why this trust is safe (e.g. "callers must ensure registered types are preserved").