[typemap] Diagnose unsupported export signatures - #12597
Conversation
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Review tier: Lite
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerScanner.cs — |
What changed in this PR
Adds early, localized diagnostics for unsupported [Export] / [ExportField] signature shapes in the trimmable typemap pipeline, so trimmable CoreCLR/NativeAOT fail deterministically (and before emitting typemap/Java/ACW-map outputs) instead of failing later in downstream steps.
Changes:
- Introduces XA4263 (“unsupported export signature type”) during trimmable scanning, while preserving XA4206 precedence for
[Export]on generic declaring types. - Tightens managed→JNI descriptor resolution to support “can we map this?” validation (via
TryManagedTypeToJniDescriptor) without changing the legacy fallback behavior used for signature generation. - Extends unit + integration parity fixtures to cover unsupported shapes and additional supported peer/interface/string-array/stream override cases, and documents XA4263.
| File | Description |
|---|---|
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/TestFixtures/TestTypes.cs | Adds fixture types covering unsupported [Export]/[ExportField] shapes (managed-only types, generics). |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TrimmableTypeMapGeneratorTests.cs | Adds generator-level assertions for XA4263/XA4206 and ensures unsupported members aren’t emitted into peers/Java sources. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/ExportSignatureTests.cs | Adds focused fixture test ensuring unsupported signatures produce no generated members. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/JavaSourceSemanticParityTests.cs | Extends semantic parity coverage for additional supported method shapes (peer/interface/string-array/stream mapping). |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/JavaSourceParityFixture/JavaSourceParityTypes.cs | Adds new exported members used by the semantic parity fixture (peer, listener, string array, stream mapping). |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/TrimmableTypeMapBuildTests.cs | Adds build-matrix coverage for unsupported export shapes and expands “no partial outputs” checks to include acw-map.txt. |
| src/Xamarin.Android.Build.Tasks/Tasks/GenerateTrimmableTypeMap.cs | Implements new logger hooks for XA4206 and XA4263 in the MSBuild task. |
| src/Xamarin.Android.Build.Tasks/Properties/Resources.resx | Adds localized XA4263 message template. |
| src/Xamarin.Android.Build.Tasks/Properties/Resources.Designer.cs | Adds generated accessor for XA4263 resource. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerScanner.cs | Adds export signature validation and refactors managed→JNI mapping into a Try* API for “supported mapping” checks. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/ITrimmableTypeMapLogger.cs | Adds logger interface hooks for XA4206 and XA4263. |
| Documentation/docs-mobile/TOC.yml | Wires XA4263 doc into the docs TOC. |
| Documentation/docs-mobile/messages/xa4263.md | Adds documentation for XA4263. |
| Documentation/docs-mobile/messages/index.md | Adds XA4263 to the error code index. |
Files not reviewed (1)
- src/Xamarin.Android.Build.Tasks/Properties/Resources.Designer.cs: Generated file
e40eedf to
7f245c2
Compare
b42a478 to
4cf3eb1
Compare
f31db8b to
51d724d
Compare
51d724d to
7f64a58
Compare
Reject unresolved managed, generic, and function-pointer export signatures during trimmable scanning with localized XA4263 diagnostics before any typemap, Java, or ACW-map output is written. Preserve legacy XA4206 precedence and supported Java mappings. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Honor ExportParameter return mappings on exported fields, reject incompatible special mappings before generation, and resolve Java peer descriptors by assembly identity through type forwarders. Add semantic, cross-assembly, and three-runtime regressions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Require special ExportParameter mappings to target exact scalar Stream or XmlReader types, and match Export, ExportParameter, and ExportField attributes by full Java.Interop identity throughout scanner validation and collection. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Ignore static exported constructors, diagnose unresolved instance constructor parameters without overlapping XA4260 shape ownership, preserve ExportParameter kinds through UCO activation, and resolve enum signatures by assembly identity. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Ignore Export name overrides on metadata constructors, assert UCO dispatch calls the managed constructor rather than a virtual method, and recursively reserve nested structural parameter shapes for XA4260. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Require Stream and XmlReader special export mappings to resolve from their framework assemblies or forwarders, preventing same-full-name user types from receiving framework adapters. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Recognize System.Private.Xml as the canonical XmlReader definition and verify both direct and multi-facade mappings without accepting same-full-name user types. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Exercise the resolved wrong-assembly branch by indexing a user assembly that defines System.Xml.XmlReader instead of relying on a missing assembly reference. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
6cd69fa to
c401d78
Compare
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
/review |
|
✅ Android PR Reviewer completed successfully!
|
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
❌ Needs Changes
Found 1 error: non-constructor by-ref, pointer, and rectangular-array exports are accepted by the new scanner validation even though trimmable codegen cannot emit them, so they still fail later without XA4263.
The assembly-identity/forwarder handling and scalar ExportParameter coverage are otherwise well structured. CI build 1578712 is still in progress; the completed Android Tools macOS lane and CLA check are green, with no reported failures yet.
Generated by Android PR Reviewer for #12597 · gpt56 · 856.3 AIC · ⌖ 25.7 AIC · ⊞ 25.7K
Comment /review to run again
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
/review |
|
✅ Android PR Reviewer completed successfully!
|
There was a problem hiding this comment.
❌ Reject
Findings: 1 error · 0 warnings · 0 suggestions.
The new XA4263 coverage is extensive, and all 44 CI checks are green. However, the scanner still recognizes primitive, string, ICharSequence, and non-generic collection mappings by managed full name alone. User-defined collision types can therefore bypass the new diagnostic and reach incompatible generated marshalling code. The inline comment identifies the affected validation path and requested regression coverage.
Generated by Android PR Reviewer for #12597 · gpt56 · 673.1 AIC · ⌖ 8.95 AIC · ⊞ 25.7K
Comment /review to run again
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Preserve undefined ExportParameterKind values through scanner validation so unsupported signatures report XA4263 instead of being treated as unspecified. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |

Summary
[Export]/[ExportField]signatures during trimmable scanning with localized XA4263 before typemap, Java, or ACW-map output[ExportParameter]mappingsJava.Interopidentity and resolve peer/enum/special framework types by assembly identity through forwardersFinal type-identity behavior
Special mappings require canonical framework identity, not just a managed full name:
System.IO.Stream:System.Runtimefacade or resolvedSystem.Private.CoreLibSystem.Xml.XmlReader:System.Xml.ReaderWriterfacade or resolvedSystem.Private.Xmlnetstandard → System.Xml.ReaderWriter → System.Private.Xmlare accepted through the existing cycle-safe forwarder resolverTests use minimal metadata assemblies for direct
System.Private.Xml, the two-hop facade chain, and a resolvedUser.Xmlassembly containingSystem.Xml.XmlReader. A separately compiled Android fixture and end-to-end builds retain the user-collision/no-output controls.Constructor diagnostics ownership
PR #12567 owns XA4260 for generic, byref, pointer, function-pointer, rectangular-array constructor parameters, including nested SZ-array forms. XA4263 owns unresolved managed types and invalid
[ExportParameter]kind/type/identity pairs. This PR does not duplicate XA4259/XA4261 analysis.An isolated composition with the latest #12567 reproduces its remaining follow-up: two XA4263-rejected overloads on the same type with default/missing
SuperArgumentsStringalso receive secondary XA4259 and XA4261. The coordinator will make that analyzer skip XA4263-rejected constructors after rebasing #12567 above this PR.Validation
GenerateExportedMembersgit diff --check; every added line at most 180 characters; full-name attribute audit cleanStacked on #12596. Tracks #12561.