Uh oh!
There was an error while loading. Please reload this page.
[typemap] Validate exported Java fields - #12596
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Lite
Findings: 1
New issues introduced by this change (2)
| Severity | Finding |
|---|---|
 | src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/TrimmableTypeMapBuildTests.cs — ❌ error — The typemap output assertion only checks for *.TypeMap.dll, but trimmable typemap… |
 | src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerScanner.cs — 💡 suggestion — ValidateExportField() introduces an extra per-method custom-attribute scan and… |
What changed in this PR
This PR tightens trimmable typemap parity with the legacy [ExportField] pipeline by validating invalid exported-field initializer shapes early (parameterized/void), emitting the corresponding XA4205/XA4208 diagnostics, and ensuring the generator stops before writing partial typemap/Java outputs. It also expands the Java-source semantic parity fixture and adds new host/device coverage for static/instance/protected exported fields.
Changes:
- Add
[ExportField]signature validation in the trimmable scanner and surface XA4205/XA4208 through the shared logger/task plumbing. - Extend generator + integration fixtures/tests to assert “no partial Java fields / marshal methods” for invalid
[ExportField]shapes and to validate ordered field semantics (static/instance/protected). - Add on-device coverage for instance primitive
[ExportField]initialization visibility.
| File | Description |
|---|---|
| tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/ExportTests.cs | Adds a device test validating instance primitive exported-field initialization is visible via JNI. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/TestFixtures/TestTypes.cs | Introduces invalid [ExportField] fixture types (parameterized + void return) for regression testing. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TrimmableTypeMapGeneratorTests.cs | Adds generator-level assertions for legacy XA4205/XA4208 diagnostics and absence of partial Java/typemap members. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/ExportFieldTests.cs | Adds focused unit coverage ensuring invalid export fields produce no Java field or marshal wrapper. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/JavaSourceSemanticParityTests.cs | Extends the semantic parity fixture to cover field ordering/modifiers and adds a Java consumer for reads. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/JavaSourceParityFixture/JavaSourceParityTypes.cs | Adds a protected exported field to increase fixture coverage for visibility and instance fields. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/TrimmableTypeMapBuildTests.cs | Adds build-time coverage asserting invalid [ExportField] stops before writing typemap/Java outputs and matches legacy diagnostics. |
| src/Xamarin.Android.Build.Tasks/Tasks/GenerateTrimmableTypeMap.cs | Plumbs XA4205/XA4208 into the MSBuild logger using localized Java.Interop resources. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerScanner.cs | Adds scanner-side validation to reject parameterized/void [ExportField] initializers before member emission. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/ITrimmableTypeMapLogger.cs | Extends the shared logger interface with new ExportField-specific diagnostic hooks. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 31, 2026
simonrozsivalforce-pushed
the
simonrozsival-export-field-parity
branch
from
September 1, 2026 12:42
03d31af to
658036bComparesimonrozsivalforce-pushed
the
simonrozsival-export-field-parity
branch
from
September 1, 2026 12:50
658036b to
43a6569Comparesimonrozsivalforce-pushed
the
simonrozsival-export-field-parity
branch
2 times, most recently
from
September 1, 2026 13:33
871230e to
27baef7CompareMatch legacy XA4205 and XA4208 validation in the trimmable scanner, before invalid field members can reach generated typemap or Java outputs. Extend semantic, build, and device coverage for valid static and instance ExportField behavior and classify legacy-unsupported field names. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Prove that parameter-count validation takes precedence over void-return validation for an initializer violating both rules, matching measured llvm-ir behavior across trimmable CoreCLR and NativeAOT without partial outputs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Match legacy XA4207 precedence for ExportField methods declared on generic types before parameter-count or void-return validation, and reject them before typemap or Java outputs are written. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Ignore unrelated attributes that share the ExportFieldAttribute simple name by matching the Java.Interop namespace consistently during validation, field collection, and marshal-method registration. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Remove duplicated scanner fixtures and keep focused build/runtime parity coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
simonrozsivalforce-pushed
the
simonrozsival-export-field-parity
branch
from
September 1, 2026 13:54
27baef7 to
5a3ce62Comparesimonrozsival
changed the base branch from
simonrozsival-jcw-interface-parity
to
mainSeptember 1, 2026 13:54
simonrozsival
commented
Sep 1, 2026
MemberAuthor
/review |
Contributor
✅ Android PR Reviewer completed successfully!
|
Contributor
There was a problem hiding this comment.
Review summary
Verdict:
The validation logic matches the measured legacy diagnostic precedence, invalid members are excluded before field/marshal-method collection, and the localized task logging is wired consistently. The remaining issue is regression coverage: the final matrix no longer exercises combined-invalid signatures, so the precedence behavior is not protected.
CI build #1576821 is still in progress with no reported failures at review time.
Generated by Android PR Reviewer for #12596 · gpt56 · 243.4 AIC · ⌖ 9 AIC · ⊞ 25.7K
Comment /review to run again
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
rolfbjarne
approved these changes
Sep 2, 2026
Uh oh!
There was an error while loading. Please reload this page.
simonrozsival added a commit
that referenced
this pull request
Sep 4, 2026
## Summary - reject unsupported `[Export]`/`[ExportField]` signatures during trimmable scanning with localized XA4263 before typemap, Java, or ACW-map output - preserve supported Java peers/interfaces, primitives, strings, ordinary arrays, enums, collections, and valid scalar `[ExportParameter]` mappings - match Export attributes by full `Java.Interop` identity and resolve peer/enum/special framework types by assembly identity through forwarders - validate exported constructors while preserving constructor identity and scalar adapter dispatch ## Final type-identity behavior Special mappings require canonical framework identity, not just a managed full name: - `System.IO.Stream`: `System.Runtime` facade or resolved `System.Private.CoreLib` - `System.Xml.XmlReader`: `System.Xml.ReaderWriter` facade or resolved `System.Private.Xml` - multi-hop facades such as `netstandard → System.Xml.ReaderWriter → System.Private.Xml` are accepted through the existing cycle-safe forwarder resolver - user assemblies defining the same full names are rejected with XA4263 for method parameter/return, ExportField return, and constructor parameter paths Tests use minimal metadata assemblies for direct `System.Private.Xml`, the two-hop facade chain, and a resolved `User.Xml` assembly containing `System.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 `SuperArgumentsString` also receive secondary XA4259 and XA4261. The coordinator will make that analyzer skip XA4263-rejected constructors after rebasing #12567 above this PR. ## Validation - 802 standalone trimmable tests - 27 integration tests, including semantic classfile comparison, javac, peer/enum collisions, and Stream/XmlReader collisions - 66 export-signature/runtime-identity and no-output build tests - valid llvm-ir CoreCLR, trimmable CoreCLR, and trimmable NativeAOT fixture builds - legacy `GenerateExportedMembers` - latest #12567 composition: stronger two-overload secondary-diagnostic reproduction and 838 full combined standalone tests - `git diff --check`; every added line at most 180 characters; full-name attribute audit clean Stacked on #12596. Tracks #12561.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
[ExportField]initializer validation in the trimmable scanner with localized XA4205/XA4207/XA4208 diagnosticsJava.Interop.ExportFieldAttributeby full identity during validation, field collection, and registration parsingBehavior
Parameterized and
void[ExportField]initializers now report XA4205 and XA4208 respectively, matching LLVM-IR. Generic declaring types report XA4207 before initializer signature validation in both trimmable runtimes.Combined-invalid coverage verifies the same diagnostic precedence: XA4205 wins for a parameterized
voidinitializer, while XA4207 wins when that initializer is declared on a generic type.The scanner skips invalid initializer members instead of generating invalid Java fields and marshal methods. Attribute matching now uses the full
Java.Interop.ExportFieldAttributeidentity so unrelated user-defined attributes with the same short name are ignored.Validation
Microsoft.Android.Sdk.TrimmableTypeMap.Teststests on the final headExportcoverage under LLVM-IR CoreCLR, trimmable CoreCLR, and trimmable NativeAOTgit diff --checkRoot of GitHub Stack #12627. Tracks #12561 and #11289.