[generator] Honor metadata JNI signature overrides

[jonathanpeppers]

---

Pull Request
title and
description
should follow the
commit-messages.md workflow documentation, and in particular should include:

• Useful description of why the change is necessary.
• Links to issues fixed
• Unit tests

<add-node> methods currently derive their JNI signature from managed parameter types even when metadata supplies explicit jni-signature and jni-type values. This breaks bindings where Java erasure differs from the strongly typed managed API and can cause NoSuchMethodError at runtime.

Preserve metadata-authored JNI overrides through XML import and use them consistently for registration and invocation IDs while leaving managed parameter types unchanged. Compatibility safeguards invalidate stale overrides after signature-changing transforms, ignore empty values, preserve enum-safe overrides, and reject malformed or ABI-incompatible signatures with a coded diagnostic.

The complete standalone generator suite passes with 478 tests.

Fixes #11841

[Copilot]

Pull request overview

This PR updates the Java.Interop generator pipeline to preserve and honor metadata-authored JNI signature/type overrides for <add-node>-inserted methods, ensuring generated [Register] attributes and invocation IDs can match erased Java signatures (fixing runtime NoSuchMethodError scenarios like #11841).

Changes:

• Preserve <method jni-signature> / <parameter jni-type> from metadata by copying them into managed-jni-signature / managed-jni-type during fixup application, with invalidation when signature-affecting transforms occur.
• Flow managed-jni-signature / managed-jni-type through XML import into the object model and use them for Method.JniSignature and Parameter.JniType.
• Add validation + a new coded warning for malformed signature overrides, along with expanded unit tests and a codegen test for correct __id emission.

Reviewed changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
external/Java.Interop/tools/generator/Java.Interop.Tools.Generator.ObjectModel/Parameter.cs	Adds per-parameter JNI type override support and ABI-compat validation.
external/Java.Interop/tools/generator/Java.Interop.Tools.Generator.ObjectModel/Method.cs	Adds per-method JNI signature override support, parsing/validation, and override-aware JniSignature.
external/Java.Interop/tools/generator/Java.Interop.Tools.Generator.ObjectModel/JniSignatureUtilities.cs	New helper for parsing JNI signatures and checking ABI compatibility.
external/Java.Interop/tools/generator/Java.Interop.Tools.Generator.Importers/XmlApiImporter.cs	Imports managed-jni-* attributes into the object model.
external/Java.Interop/tests/generator-Tests/Unit-Tests/XmlTests.cs	Adds validation tests for malformed overrides.
external/Java.Interop/tests/generator-Tests/Unit-Tests/XmlApiImporterTests.cs	Verifies importer wiring for managed-jni-signature / managed-jni-type.
external/Java.Interop/tests/generator-Tests/Unit-Tests/FixupXmlDocumentTests.cs	Tests preservation + invalidation rules for overrides during fixup operations.
external/Java.Interop/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs	Ensures generated code uses override signatures for __id/invocation IDs while keeping managed parameter types.
external/Java.Interop/src/Java.Interop.Tools.Generator/Utilities/Report.cs	Adds a new coded warning for invalid JNI signature overrides.
external/Java.Interop/src/Java.Interop.Tools.Generator/Metadata/FixupXmlDocument.cs	Preserves overrides into managed-jni-* and invalidates them when signature-affecting transforms occur.
external/Java.Interop/src/Java.Interop.Localization/Resources.resx	Adds localized text for the new warning.
external/Java.Interop/src/Java.Interop.Localization/Resources.Designer.cs	Updates generated resource accessors for the new warning string.

Files not reviewed (1)

• external/Java.Interop/src/Java.Interop.Localization/Resources.Designer.cs: Generated file

Suppressed comments (2)

external/Java.Interop/tools/generator/Java.Interop.Tools.Generator.ObjectModel/Parameter.cs:302

• JniTypeOverride is validated only against null, so an empty-string override will be treated as present and will always fail AreAbiCompatible, effectively dropping the method/parameter. If empty overrides should be ignored, use string.IsNullOrEmpty here too.

			if (JniTypeOverride != null && !JniSignatureUtilities.AreAbiCompatible (sym.JniName, JniTypeOverride)) {
				Report.LogCodedWarning (0, Report.WarningInvalidParameterType, this, JniTypeOverride, context.GetContextTypeMember ());
				return false;
			}

external/Java.Interop/tools/generator/Java.Interop.Tools.Generator.ObjectModel/Method.cs:273

• Validation only checks JniSignatureOverride == null, so an empty-string override will go through parsing and fail the method, even though empty overrides are intended to be ignored. Use string.IsNullOrEmpty so empty values behave like missing overrides.

			if (JniSignatureOverride == null)
				return true;

---

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

[jonathanpeppers]

/review

[github-actions]

✅ Android PR Reviewer completed successfully!

[github-actions]

⚠️ Needs Changes

Found 1 error and 1 warning: malformed empty reference descriptors currently pass validation, and parameter-moving/type-changing transforms can retain stale method overrides. Both findings are posted inline with regression-test guidance.

The override propagation through import, cloning, and code generation is otherwise coherent, and all 44 reported checks for Azure build #1547327 are green.

Generated by Android PR Reviewer for #12340 · gpt56 · 188.5 AIC · ⌖ 22.9 AIC · ⊞ 25.3K
_{Comment /review to run again}

[jonathanpeppers]

@dalexsoto review

[dalexsoto]

Requesting changes because runtime JNI descriptor validation still accepts generic T...; signatures, which are generic-signature grammar rather than valid method descriptors and can cause NoSuchMethodError. Constructor parameters also preserve generic jni-type values without preserving the constructor’s erased signature, allowing invalid (TT;)V IDs. Please separate generic ABI classification from strict runtime-descriptor parsing and either scope preservation to methods or fully support constructor overrides, with regressions.

[jonathanpeppers]

Addressed the requested changes in 952edf3.nnAdded three regressions first and confirmed all three failed on the prior implementation:n- generic method override (TT;)V was acceptedn- generic parameter override TT; was acceptedn- constructor parameter jni-type was promoted without constructor signature-override supportnnThe fix separates generic expected-symbol ABI parsing from strict runtime-descriptor parsing and limits metadata jni-type` promotion to method parameters. The focused regressions now pass, and the complete generator suite passes 486/486.

[dalexsoto]

Blocking on two current descriptor-validation regressions:

• PreserveJniOverrides() promotes every method parameter jni-type before Parameter.Validate() runs. Existing add-node metadata such as RxJava2 onNext(T) pairs jni-type="TT;" with the valid erased jni-signature="(Ljava/lang/Object;)V". The parameter override is rejected before the erased method signature is considered, so these existing methods are omitted. Please distinguish generic metadata annotations from runtime descriptor overrides and add this existing shape as a regression.
• TryReadType() accepts malformed object descriptors such as Ljava.lang.String;, which are then emitted verbatim. Please validate JNI internal class names (or reuse the existing descriptor parser) and add a dotted-name regression.

[jonathanpeppers]

@dalexsoto Addressed both findings in fc23add.nn- Generic parameter jni-type values such as TT; and [TT; remain generic metadata annotations instead of being promoted into runtime overrides. The erased method-level jni-signature is still preserved, covering the existing RxJava onNext(T) shape.n- Runtime object descriptors now reject dotted class names, including array element descriptors.n`nAdded regressions for the RxJava shape and dotted method/parameter overrides. They failed before the fix; focused tests and the complete generator suite now pass (490/490). Re-requesting review.

[dalexsoto]

Blocking because the direct-parameter <add-node> path still drops metadata-authored jni-type overrides. PreserveJniOverrides(metaitem) only visits parameters nested under descendant <method> elements (implementation). When metadata targets an existing method and directly adds <parameter jni-type=...>, no method descendant exists, so managed-jni-type is never created and import falls back to the managed type, potentially emitting the wrong runtime descriptor and NoSuchMethodError. This direct-parameter form is already supported and tested for signature invalidation. Please preserve its non-generic jni-type override while retaining the TT;/[TT; exemption, and add a regression.

[jonathanpeppers]

@dalexsoto Fixed in cc5a7a5. Add-node content is now cloned and processed per matched target, so a direct parameter added to a method preserves its non-generic jni-type override while method signature invalidation still occurs. Constructor targets remain excluded, and generic TT;/[TT; annotations remain unpromoted.nnAdded a direct-parameter regression and confirmed it failed before the fix. The focused add-node tests pass, and the complete generator suite passes 491/491. Re-requesting review.

[dalexsoto]

The direct-parameter add-node path now preserves non-generic JNI type overrides per target while retaining signature invalidation and generic/constructor exclusions. The prior blockers are resolved; no new blocking issues found.