[TrimmableTypeMap] Skip unresolvable trimmable typemap peers

[simonrozsival]

Goal

Extract the scanner/diagnostics slice from #11617 so the trimmable typemap generator handles stale binding metadata before it reaches NativeAOT.

The trimmable typemap scanner can encounter Java peer types whose managed base type or implemented interface references a type that is missing from the resolved assembly set. This commonly happens when Android binding packages were built against different versions of another binding package. NativeAOT resolves rooted metadata eagerly, so rooting one of these unused stale peers into the generated typemap can fail the app build even when the app never uses the type.

This PR skips those unresolvable peers and reports XA4256 with enough information to identify the skipped type and stale reference.

Change map

Area	Additions are for	Removals are for
Scanner inputs	Adds AssemblyInput so the scanner keeps the source path for each resolved assembly. The path is used in XA4256 diagnostics.	Replaces tuple-only generator/scanner plumbing where the assembly path was unavailable.
Assembly indexing	Tracks exported type names alongside type definitions, so type-forwarded/exported references do not get misclassified as missing.	Avoids treating every unresolved TypeRef lookup as a hard missing type when metadata declares it as exported.
Peer resolution guard	Before emitting a Java peer, recursively checks the peer base type and implemented interfaces, including type specifications and generic arguments, for typerefs that point at missing types in loaded assemblies. Results are memoized per resolved type during a scan so shared base/interface graphs are not re-walked for every peer.	Removes the behavior of blindly carrying unresolvable stale peers into the generated trimmable typemap.
Diagnostics	Adds XA4256 resources and logging through GenerateTrimmableTypeMap, including skipped peer name, peer assembly, unresolved type, unresolved assembly, and expected assembly path.	Keeps the skipped peer actionable without turning unused stale metadata into a NativeAOT failure.
Docs/tests	Adds XA4256 docs and updates generator tests to use AssemblyInput.	N/A

Current design notes

What counts as unresolvable

The scanner only warns when a referenced assembly is present in the resolved assembly set but the referenced type cannot be found in that assembly's type definitions or exported types. If the referenced assembly itself is not present in the scanner cache, the existing behavior is preserved and the reference is ignored for this check.

What is checked

For each candidate Java peer, the scanner walks:

• the peer base type,
• implemented interfaces,
• nested base/interface references encountered through those types,
• TypeSpec signatures, including arrays and generic instantiations.

If any of those metadata references resolves to a loaded assembly but not to a type/exported type in that assembly, the peer is skipped and XA4256 is logged.

Why skip instead of fail

These peers are often stale binding metadata from packages that are otherwise compatible for the app's actual usage. Skipping the peer keeps unused stale metadata out of the closed-world NativeAOT typemap while still surfacing a warning for projects that do use the skipped type.

Example warning

warning XA4256: Skipping Java peer type 'Google.Android.Material.Shadow.ShadowDrawableWrapper' from assembly 'Xamarin.Google.Android.Material' because referenced type 'AndroidX.AppCompat.Graphics.Drawable.DrawableWrapper' from assembly 'Xamarin.AndroidX.AppCompat.AppCompatResources' could not be resolved in '/home/user/.nuget/packages/xamarin.androidx.appcompat.appcompatresources/1.6.0/lib/net6.0-android31.0/Xamarin.AndroidX.AppCompat.AppCompatResources.dll'. This type will not be included in the trimmable type map.

Review notes / intentional non-goals

• This PR only changes the trimmable typemap scanner/generator path.
• It does not broaden NativeAOT support for missing dependencies; it prevents unused stale Java peers from being rooted into generated typemap metadata.
• It intentionally keeps the diagnostic as a warning because the app may not use the skipped peer.

Test strategy

This PR updates host generator/scanner coverage for the new AssemblyInput path-aware scanner API, XA4256 logger plumbing, and Java peers skipped because unresolved metadata appears through a direct base type, implemented interface, or generic TypeSpec argument.

Local validation

dotnet test tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests.csproj --no-restore --nologo

[Copilot]

Pull request overview

This PR updates the trimmable typemap pipeline to tolerate stale/unresolvable binding metadata by skipping Java peers whose base type or implemented interfaces cannot be resolved from the provided assembly set, and surfaces the situation as a new warning (XA4256).

Changes:

• Add XA4256 warning plumbing (logger API + MSBuild task logger) and documentation/resources for the new message.
• Extend typemap inputs to carry an assembly path (AssemblyInput) so warnings can report the resolved assembly file involved.
• Update the typemap scanner/indexing to detect unresolvable base/interface references and skip affected peers rather than failing generation.

Show a summary per file

File	Description
tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TrimmableTypeMapGeneratorTests.cs	Updates generator tests for the new AssemblyInput API and adds test-logger support for the new warning.
src/Xamarin.Android.Build.Tasks/Tasks/GenerateTrimmableTypeMap.cs	Switches task input wiring to pass AssemblyInput (including assembly path) into the generator and logs XA4256 via resources.
src/Xamarin.Android.Build.Tasks/Properties/Resources.resx	Adds the XA4256 localized warning string and parameter documentation.
src/Xamarin.Android.Build.Tasks/Properties/Resources.Designer.cs	Regenerates strongly-typed resource accessor for XA4256.
src/Microsoft.Android.Sdk.TrimmableTypeMap/TrimmableTypeMapGenerator.cs	Updates generator API to accept IReadOnlyList<AssemblyInput> instead of tuple inputs.
src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerScanner.cs	Adds resolvability checks for base/interface type references and skips peers while logging XA4256.
src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/AssemblyIndex.cs	Tracks assembly path and exported (forwarded) types to improve resolution checks and warning output.
src/Microsoft.Android.Sdk.TrimmableTypeMap/ITrimmableTypeMapLogger.cs	Extends logger interface with LogUnresolvableJavaPeerSkippedWarning for XA4256.
src/Microsoft.Android.Sdk.TrimmableTypeMap/AssemblyInput.cs	Introduces AssemblyInput (name, path, PEReader) as the new generator/scanner input shape.
Documentation/docs-mobile/messages/xa4256.md	Adds end-user documentation for XA4256 (issue + solution guidance).
Documentation/docs-mobile/messages/index.md	Adds XA4256 to the messages index.

Copilot's findings

Files not reviewed (1)

• src/Xamarin.Android.Build.Tasks/Properties/Resources.Designer.cs: Generated file

• Files reviewed: 10/11 changed files
• Comments generated: 1

[simonrozsival]

/review

[github-actions]

✅ Android PR Reviewer completed successfully!

[github-actions]

⚠️ Needs Changes

Thanks for this — the core idea is solid and the implementation is mostly careful. Skipping peers whose managed base/interface references a type missing from the resolved assembly set (and emitting XA4256 as a warning rather than failing the closed-world NativeAOT build) is the right call, and the metadata-scanning code is generally clean.

What I'd like addressed before merge (details inline):

Warnings (3)

• Patterns / undocumented behavioral change — The new IsSupportedFrameworkGenericPeer gate (lines 273‐278) silently drops all generic-definition framework peers except four allow-listed collection types. This goes beyond "skip unresolvable peers," isn't in the change map, has no dedicated test, and the legacy scanner didn't do it — please confirm intent, document the rationale, add a test, and verify ScannerComparisonTests parity.
• Testing — The new test only exercises direct TypeRef base/interface refs. The trickiest new code — the hand-rolled signature-blob parser (IsResolvableSignatureType, GENERICINST/array/generic-param handling) — is untested. Add a case where the generic argument is the unresolvable type.
• Performance — IsResolvableJavaPeerType re-walks the full base+interface graph with a fresh visited set per peer; sibling types sharing ancestors re-validate the same types repeatedly. Memoize per (assembly, type) on the scanner instance.

Suggestion (1)

• Readability — Replace the raw ELEMENT_TYPE bytes in the signature switch with SignatureTypeCode/SignatureTypeKind (or named constants).

Things that look good 👍

• AssemblyIndex.ExportedTypeNames correctly accounts for type-forwarded types (e.g. System.Object forwarded from System.Runtime), avoiding false skips.
• Nested-type naming is consistent across the TypesByFullName and ResolveTypeReference/exported-type paths (parent+name, ns.name), so the resolvability check shouldn't false-negative on nested types.
• No null-forgiving !; [NotNullWhen(false)] out-params used correctly; the signature parser's default/empty cases err toward not skipping.
• XA4256 is unique, the Resources.resx entry is grouped/commented correctly, and the xa4256.md doc + TOC entry are present and consistent.
• The retained tuple Scan overload is still referenced by JavaPeerScannerTests, so it's not dead code.

CI — Not green yet: Package Tests macOS > Tests > APKs 1 is currently failing and several checks are still in progress (mergeable_state: blocked). This may well be an unrelated flake (APK/device legs are often flaky), but please confirm it's not caused by these changes and get CI to green before merge.

Generated by Android PR Reviewer for issue #11751 · 1.2K AIC · ⌖ 51.8 AIC · ⊞ 37.9K
_{Comment /review to run again}

Addressed

[simonrozsival]

/review

[github-actions]

✅ Android PR Reviewer completed successfully!

[github-actions]

🤖 Android PR Review — ⚠️ Needs Changes

Thanks for this — skipping unresolvable trimmable typemap peers and emitting a warning, rather than hard-failing the NativeAOT build on stale cross-version binding metadata, is the right approach. The implementation is solid: the recursive resolvability walk over base types, interfaces, TypeSpecs and generic arguments is memoized (resolvabilityCache), breaks cycles with a per-call visited set, and stays conservative — it only skips when the referenced assembly is in the scanned set but the type is genuinely absent (including type-forwards via the new ExportedTypeNames), so assemblies that simply aren't scanned are still treated as resolvable. The new XA4256 message, docs, and the switch to Ordinal string comparisons are all nice touches.

Blocking

• ❌ The PR doesn't compile. TrimmableTypeMapGeneratorTests.cs:299 hits CS8506 (mismatched switch-arm handle types), failing make jenkins (make: *** [jenkins] Error 2) on the Mac leg — and the same error is already present in the Linux leg's build log. One-line fix inline (annotate the result as EntityHandle). This is the root cause of the current red build.

Suggestion

• 💡 The new theory covers the positive skip paths, but the false-positive guard (referenced assembly not in the scanned set ⇒ must not skip) and the type-forwarding resolution path are untested. Details inline.

CI

• ❌ Mac macOS > Build: failure — error CS8506 in the test project.
• ⏳ Linux / Windows Build: still running, but their make jenkins step compiles the same test project and the Linux log already shows the identical CS8506, so they'll fail the same way until the build break is fixed.
• ✅ license/cla.

Once the compile error is fixed and CI goes green, this looks close to mergeable. Severity counts: 1 ❌ error, 1 💡 suggestion.

Generated by Android PR Reviewer for issue #11751 · 1.9K AIC · ⌖ 63.3 AIC · ⊞ 40K
_{Comment /review to run again}

[simonrozsival]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[simonrozsival]

/review

[github-actions]

✅ Android PR Reviewer completed successfully!

[github-actions]

🤖 Android PR Review — [TrimmableTypeMap] Skip unresolvable trimmable typemap peers

Verdict: ✅ LGTM (pending CI) — submitted as a comment, not an approval.

This is a well-engineered, well-tested change. The new resolvability guard walks the peer's base type, interfaces, and their TypeSpec/generic arguments, skips peers that reference types missing from the loaded assemblies, and emits XA4256 instead of letting NativeAOT fail later.

I independently verified the trickiest part — the interaction between the per-call visited cycle-guard and the shared resolvabilityCache:

• false results only ever come from a genuinely-missing TypeReference (a global fact), so caching them is always safe.
• true results may include tautological cycle-break edges (class Foo : Bar<Foo>), but since valid base/interface graphs are acyclic those back-edges are always generic arguments, so the memoized value stays correct.
• The cache-before-visited ordering and keying by (AssemblyName, TypeRow) are both sound and keep type-name building off the hot path.

XA4256 is a brand-new code and is consistent across Resources.resx, Resources.Designer.cs, xa4256.md, and the docs TOC. Type-forwarding handling (ExportedTypeNames, StringComparer.Ordinal) matches TypesByFullName's comparer. The new Xunit tests cover the base/interface/generic-arg skip shapes plus the important negative cases (assembly-not-scanned, type-forwarded base).

Findings

Severity	Count
❌ error	0
⚠️ warning	0
💡 suggestion	2

• 💡 Performance — reuse the per-walk visited HashSet instead of allocating one per candidate peer (inline).
• 💡 Patterns — question: are generic-parameter constraints intentionally excluded from the resolvability walk? (inline)

Both are optional; neither blocks merge.

Notes

• Earlier review rounds — missing skip test, the framework-generic allow-list, magic-byte constants, per-scan memoization, the EntityHandle/CS8506 fix, and the not-scanned/forwarded test cases — all appear fully addressed. Nice iteration.
• ⏳ At review time, CI (Azure DevOps) was still in progress with no failures. This sign-off is contingent on those checks going green — please don't merge on a red Xamarin.Android-PR.

Generated by Android PR Reviewer for issue #11751 · 1.6K AIC · ⌖ 48.1 AIC · ⊞ 40K
_{Comment /review to run again}

Addressed