[TrimmableTypeMap] Emit XA4212 for custom IJavaObject types on the trimmable path#12132
[TrimmableTypeMap] Emit XA4212 for custom IJavaObject types on the trimmable path#12132simonrozsival wants to merge 2 commits into
Conversation
… path Root-cause fix (replaces the earlier test skip). The XA4212 "custom IJavaObject not supported" diagnostic was produced only by the Cecil-based XAJavaTypeScanner, which runs on the managed path (AssemblyModifierPipeline in _RunAfterILLinkAdditionalSteps) and the llvm-ir path. On the trimmable typemap path — now the NativeAOT default — that post-ILLink pipeline is intentionally skipped as dead work, so a managed class implementing Android.Runtime.IJavaObject without deriving from Java.Lang.Object/Throwable was silently ignored: no diagnostic, and the type is absent from the typemap, failing at runtime. Detect this case directly in JavaPeerScanner, which already scans every type: a class that is not a Java peer, is not a System.Exception subclass, and implements Android.Runtime.IJavaObject (directly, transitively via an interface, or through a base class) now emits XA4212. Thread AndroidErrorOnCustomJavaObject through the GenerateTrimmableTypeMap task and generator so it errors by default and warns when disabled, matching the legacy behavior and message format. Un-skip the XA4212 test. Verified locally: XA4212(NativeAOT) and XA4212(CoreCLR) pass, 614 trimmable-typemap generator unit tests pass, and BuildBasicApplicationAppCompat (NativeAOT + CoreCLR) builds clean with no false-positive XA4212. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 94f450c7-781d-4019-a56c-fae1c456b9a7
There was a problem hiding this comment.
Pull request overview
This PR restores the XA4212 diagnostic (“custom IJavaObject not supported”) on the trimmable typemap path (now the NativeAOT default) by detecting offending managed types during the Reflection.Metadata-based scan, and threads the existing $(AndroidErrorOnCustomJavaObject) behavior through to the trimmable pipeline.
Changes:
- Add custom-
IJavaObjectdetection toJavaPeerScannerand emit XA4212 as error vs warning based on a newerrorOnCustomJavaObjectknob. - Thread
AndroidErrorOnCustomJavaObjectintoGenerateTrimmableTypeMapand into the trimmable typemap MSBuild target invocation. - Add XA4212 to
Resources.resx/Designer and extend the trimmable typemap logger interface + test logger.
Show a summary per file
| File | Description |
|---|---|
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TrimmableTypeMapGeneratorTests.cs | Extends test logger with XA4212 logging hooks for generator-level tests. |
| src/Xamarin.Android.Build.Tasks/Tasks/GenerateTrimmableTypeMap.cs | Threads ErrorOnCustomJavaObject into the trimmable generator and logs XA4212. |
| src/Xamarin.Android.Build.Tasks/Properties/Resources.resx | Adds localized XA4212 message. |
| src/Xamarin.Android.Build.Tasks/Properties/Resources.Designer.cs | Adds the strongly-typed XA4212 resource accessor. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.Trimmable.targets | Passes $(AndroidErrorOnCustomJavaObject) into GenerateTrimmableTypeMap. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/TrimmableTypeMapGenerator.cs | Adds errorOnCustomJavaObject plumbing into scanning. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerScanner.cs | Implements custom-IJavaObject detection and XA4212 emission in the scanner. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/ITrimmableTypeMapLogger.cs | Adds XA4212 error/warning logging methods to the generator logger contract. |
Copilot's findings
Files not reviewed (1)
- src/Xamarin.Android.Build.Tasks/Properties/Resources.Designer.cs: Generated file
- Files reviewed: 7/8 changed files
- Comments generated: 4
|
@copilot resolve the merge conflicts in this pull request and implement code review suggestions |
Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
Done in |
## Goal Make the trimmable typemap the **default** typemap for NativeAOT. **Supersedes #11822.** That PR was based directly on `main`; this one is instead **stacked on #12030** (`dev/simonrozsival/android-trimmable-revisit-arrays-generic-collections`), which reworks the AOT-safe array and collection factories. Rebasing onto #12030 let me **drop the array-proxy / boxed-nullable / `System.String`-array-element commits** (now handled by #12030's `MakeArrayType`/`MakeGenericType` factories) plus other work that has since landed on that branch, leaving just the focused NativeAOT default flip and its test adjustments. > **Scope:** for now this only **changes the default**. The existing `managed`/`llvm-ir` configurations remain reachable on NativeAOT (the runtime keeps its `ManagedTypeManager` / `JavaMarshalValueManager` fallbacks). Removing the non-trimmable NativeAOT paths and adding a hard error will be done in a **separate PR**. Contributes to #10794, #11012, #8724, #10788, #10793. ## Change map ### Core enablement - **`Microsoft.Android.Sdk.NativeAOT.targets`** — default `_AndroidTypeMapImplementation` `managed` → `trimmable`. - **`Xamarin.Android.Common.targets`** — run the post-ILLink `AssemblyModifierPipeline` for NativeAOT+trimmable; skip the project proguard config for NativeAOT+trimmable. (All gated on `trimmable`, so `managed`/`llvm-ir` NativeAOT is unchanged.) - **`Microsoft.Android.Sdk.TypeMap.Trimmable.targets`** — disable `ManagedPeerNativeRegistration` for trimmable. - Pass generated ACW keep rules to R8 on the trimmable path; keep `_PreTrimmingFixLegacyDesignerUpdateItems` off the trimmable path. ### Tests - `BaseTest` — `IgnoreNativeAotLinkedAssemblyChecks` / `IgnoreOnNativeAot` helpers. - Skip guards for NativeAOT cases that inspect illink's `linked/` output (`LinkerTests`, `IncrementalBuildTest`, `BuildTest2`); delete the obsolete `BuildTest2.NativeAOT` typemap test. - Warning-clean updates now that the trimmable default no longer emits the reflection-manager IL3050/IL3053 warnings. - `DotNetBuild` expects `mapping.txt` for NativeAOT release; add a NativeAOT regression test for R8-kept runtime ACWs. - The `XA4212` custom-`IJavaObject` diagnostic is skipped on the NativeAOT (trimmable) path here for simplicity; the root-cause fix that emits it from the trimmable generator moved to follow-up **#12132**. ## Local validation - `Microsoft.Android.Sdk.TrimmableTypeMap.Tests` → **614 pass**. - Full device/CI matrix + apkdesc/size baselines deferred to this PR's CI run. ## Follow-up - **#12132** — [NativeAOT] Emit XA4212 for custom IJavaObject types on the trimmable path. Adds the custom-`IJavaObject` detection to the trimmable typemap generator and re-enables the test skipped here.
Root-cause fix (replaces the earlier test skip). The XA4212 "custom IJavaObject not supported" diagnostic was produced only by the Cecil-based XAJavaTypeScanner, which runs on the managed path (AssemblyModifierPipeline in _RunAfterILLinkAdditionalSteps) and the llvm-ir path. On the trimmable typemap path — now the NativeAOT default — that post-ILLink pipeline is intentionally skipped as dead work, so a managed class implementing Android.Runtime.IJavaObject without deriving from Java.Lang.Object/Throwable was silently ignored: no diagnostic, and the type is absent from the typemap, failing at runtime.
Detect this case directly in JavaPeerScanner, which already scans every type: a class that is not a Java peer, is not a System.Exception subclass, and implements Android.Runtime.IJavaObject (directly, transitively via an interface, or through a base class) now emits XA4212. Thread AndroidErrorOnCustomJavaObject through the GenerateTrimmableTypeMap task and generator so it errors by default and warns when disabled, matching the legacy behavior and message format. Un-skip the XA4212 test.
Verified locally: XA4212(NativeAOT) and XA4212(CoreCLR) pass, 614 trimmable-typemap generator unit tests pass, and BuildBasicApplicationAppCompat (NativeAOT + CoreCLR) builds clean with no false-positive XA4212.