[TrimmableTypeMap] Add standalone integration coverage for generator-consumed scanner fields

[Copilot]

The existing TrimmableTypeMap integration tests only compared fields shared by the legacy Cecil scanner and the new SRM-based scanner. This change adds standalone correctness coverage for scanner fields that are consumed by the generator pipeline but have no legacy equivalent: IsUnconditional, InvokerTypeName, CompatJniName, and MarshalMethodInfo.ManagedMethodName.

• Add standalone scanner integration tests

  • Add ScannerFieldIntegrationTests under tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/
  • Cover:
    • component-driven and cross-reference-driven IsUnconditional
    • Mono.Android sanity check that MCW bindings are not marked unconditional
    • InvokerTypeName resolution for interfaces and abstract types
    • CompatJniName format for user types
    • non-empty ManagedMethodName for marshal methods, with a known-method spot check

• Expand the user fixture to exercise missing paths

  • Add fixture types for:
    • [ContentProvider]
    • user-defined interface + invoker
    • abstract Java peer + invoker
  • Keep the fixture focused on generator-consumed scanner concepts rather than legacy/new parity

• Align CompatJniName generation with compat format

  • Update auto-generated compat JNI names to preserve dots in the namespace portion and only use / before the type name
  • Example:

// Before
UserApp.Models.UserModel -> userapp/models/UserModel

// After
UserApp.Models.UserModel -> userapp.models/UserModel

• What the new tests assert
  • UserApp.MainActivity, MyBackgroundService, BootReceiver, SettingsProvider => IsUnconditional == true
  • UserApp.MyBackupAgent becomes unconditional via [Application(BackupAgent=typeof(...))]
  • Android.Views.View+IOnClickListener resolves to Android.Views.View+IOnClickListenerInvoker
  • Android.Widget.AbsListView resolves to Android.Widget.AbsListViewInvoker
  • user-defined concrete types keep InvokerTypeName == null

Original prompt

---

This section details on the original issue you should resolve

<issue_title>[TrimmableTypeMap] Add integration tests for generator-consumed scanner fields</issue_title>
<issue_description># [TrimmableTypeMap] Add integration tests for generator-consumed scanner fields

Part of #10789

Context

PR #10827 added integration tests that validate the new JavaPeerScanner (SRM-based) against the legacy Cecil-based scanner. These tests cover the fields that overlap between the two scanners: JavaName, BaseJavaName, ImplementedInterfaces, MarshalMethods, ActivationCtors, and type flags (IsInterface, IsAbstract, IsGenericDefinition, DoNotGenerateAcw).

However, PR #10808 introduced the Generator pipeline (ModelBuilder → TypeMapAssemblyEmitter) which consumes several JavaPeerInfo fields that are new concepts with no legacy equivalent. These fields cannot be validated via legacy-vs-new comparison and need standalone correctness tests.

Fields not covered by integration tests

Field	Used by	Risk if wrong
IsUnconditional	ModelBuilder.IsUnconditionalEntry() — determines 2-arg (unconditional) vs 3-arg (trimmable) TypeMap attribute	Wrong trimming behavior — types could be incorrectly trimmed or unnecessarily preserved
InvokerTypeName	ModelBuilder.BuildProxyType() — proxy generation for interfaces/abstract types	Missing invoker = runtime MissingMethodException when activating interface wrappers
CompatJniName	AcwMapWriter — backward-compatible ACW name for acw-map.txt	Broken debugging, profiling, or tooling that relies on acw-map.txt
MarshalMethods.ManagedMethodName	Future UCO wrapper generation	Low risk now, but incorrect values would cause wrong method dispatch

Proposed tests

1. IsUnconditional_ComponentTypes

Scan UserTypesFixture.dll:

• Types with [Activity], [Service], [BroadcastReceiver], [ContentProvider] → IsUnconditional = true
• Plain Java peer types without component attributes → IsUnconditional = false
• Types referenced by [Application(BackupAgent=typeof(X))] → forced unconditional via cross-reference

2. IsUnconditional_MonoAndroid

Scan Mono.Android.dll:

• All MCW binding types have DoNotGenerateAcw=true and should not be marked unconditional
• Sanity check: count of unconditional types = 0

3. InvokerTypeName_InterfacesAndAbstractTypes

Scan Mono.Android.dll:

• Every type with IsInterface = true should have a non-null InvokerTypeName
• The referenced invoker type should actually exist in the scanned assemblies
• Validate resolution via [Register] connector (primary) and {TypeName}Invoker convention (fallback)

4. InvokerTypeName_UserTypes

Scan UserTypesFixture.dll:

• Concrete user types → InvokerTypeName = null
• User-defined interfaces (if present) → invoker resolved correctly

5. CompatJniName_UserTypes

Scan UserTypesFixture.dll:

• Verify CompatJniName uses lowercased raw namespace format
• E.g., MyApp.Namespace.MyType → myapp.namespace/MyType

6. ManagedMethodName_MarshalMethods

Scan Mono.Android.dll:

• Every MarshalMethodInfo has non-null, non-empty ManagedMethodName
• Spot-check known types (e.g., Android.App.Activity has OnCreate method)

Notes

• These tests should live in tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/
• Unlike the existing comparison tests, these are standalone correctness tests — they validate the new scanner's output against known expectations, not against the legacy scanner
• The UserTypesFixture may need additional types (e.g., user-defined interfaces, [Application] with BackupAgent) to exercise all paths
  </issue_description>

Comments on the Issue (you are @copilot in this section)

• Fixes [TrimmableTypeMap] Add integration tests for generator-consumed scanner fields #10913

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[simonrozsival]

@copilot build fails:

       "/Users/runner/work/1/s/android/Xamarin.Android.sln" (default target) (1:2) ->
       "/Users/runner/work/1/s/android/tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/UserTypesFixture/UserTypesFixture.csproj" (default target) (29:8) ->
       (CoreCompile target) -> 
         /Users/runner/work/1/s/android/tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/UserTypesFixture/UserTypes.cs(70,3): error CS7036: There is no argument given that corresponds to the required parameter 'authorities' of 'ContentProviderAttribute.ContentProviderAttribute(string[])' [/Users/runner/work/1/s/android/tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/UserTypesFixture/UserTypesFixture.csproj]

    291 Warning(s)
    1 Error(s)

[Copilot]

@copilot build fails:

       "/Users/runner/work/1/s/android/Xamarin.Android.sln" (default target) (1:2) ->

...

Fixed in e9900e0. SettingsProvider now passes the required string[] authorities argument to ContentProviderAttribute, and the reported CS7036 error is gone in targeted validation.

[simonrozsival]

Code Review — PR #10920

Summary

Good PR that adds standalone integration tests for generator-consumed scanner fields (IsUnconditional, InvokerTypeName, CompatJniName, MarshalMethodInfo.ManagedMethodName) and a targeted behavioral fix to CompatJniName generation. The test coverage is comprehensive and the fixture additions are well-structured.

CompatJniName format change (JavaPeerScanner.cs)

The removal of .Replace('.', '/') changes the compat JNI name from userapp/models/UserModel to userapp.models/UserModel. I verified this is safe for downstream consumers:

• ACWMapGenerator applies .Replace('/', '.') before writing to acw-map.txt, so both formats produce the same final output: userapp.models.UserModel
• ManifestDocument does the same .Replace('/', '.') conversion
• The new format is only consumed by the new TrimmableTypeMap pipeline, not the legacy path

The JavaPeerInfo.cs doc comment already describes the expected format as "myapp.namespace/MyType", so the code change aligns implementation with documentation. ✅

Test structure (ScannerFieldIntegrationTests.cs)

• Good use of partial class to extend ScannerComparisonTests and reuse existing infrastructure (AllAssemblyPaths, AllUserTypesAssemblyPaths)
• Lazy<> caching for scan results is the right call — avoids redundant scanning across test methods
• Tests cover both positive and negative cases (e.g., PlainActivity should NOT be unconditional, concrete types should have InvokerTypeName == null)

Minor observations

1. Redundant null guard (line 57-59): After Assert.False(string.IsNullOrEmpty(invokerTypeName), ...), the if (invokerTypeName is not null) check is unreachable when the assert fails (since Assert.All collects exceptions per element). The guard is harmless but unnecessary — if Assert.False passes, invokerTypeName is guaranteed non-null.

2. File name vs class name: ScannerFieldIntegrationTests.cs contains partial class ScannerComparisonTests. This is fine for partial class organization, though a reader might initially expect a class named ScannerFieldIntegrationTests. The naming accurately describes the purpose of the tests in this file, which is reasonable.

3. Test IsUnconditional_MonoAndroid has a redundant second assertion: Assert.Equal(0, peers.Count(p => p.IsUnconditional)) already proves no peer is unconditional, making the subsequent Assert.All(peers.Where(p => p.DoNotGenerateAcw), ...) check logically redundant. However, the second assertion adds documentation value by explicitly stating the MCW binding relationship. Fine to keep.

UserTypes fixture additions

The new fixture types (SettingsProvider, IWidgetListener + invoker, AbstractWidget + invoker) are well-crafted:

• SettingsProvider completes the four Android component types (Activity, Service, BroadcastReceiver, ContentProvider)
• Interface + invoker pattern correctly uses [Register] with the connector third argument
• Abstract type + invoker follows the established DoNotGenerateAcw = true pattern

Verdict

No blocking issues. The behavioral change is correct and well-tested. The test coverage matches the spec from issue #10913.

[simonrozsival]

🤖 AI Review Summary

Verdict: ⚠️ Needs Changes (CI failures must be resolved before merge)

Found 0 blocking issues in the diff. The code is clean and well-structured. Two minor suggestions below.

CI Status

• ❌ dotnet-android Windows Build & Smoke Test — failed
• ❌ Xamarin.Android-PR — failed
• ✅ dotnet-android Linux Build — passed
• ✅ dotnet-android macOS Build — passed

The Windows build failure needs investigation. If it's a pre-existing flake unrelated to this PR, a re-run should clear it.

What this PR does

1. Behavioral fix: Removes .Replace('.', '/') from ComputeAutoJniNames — changes compat JNI name format from userapp/models/TypeName to userapp.models/TypeName. This is safe because downstream consumers (ACWMapGenerator, ManifestDocument) apply .Replace('/', '.') before writing to acw-map.txt, producing the same final output either way. The new format also matches the JavaPeerInfo.CompatJniName doc comment.
2. New integration tests: 6 standalone correctness tests for IsUnconditional, InvokerTypeName, CompatJniName, and MarshalMethodInfo.ManagedMethodName.
3. Fixture expansion: Adds ContentProvider, user-defined interface + invoker, and abstract type + invoker to the UserTypesFixture.

Checked against review rules

• ✅ Mono formatting (tabs, space before ( and [)
• ✅ No null-forgiving operator (!)
• ✅ No string.Empty or Array.Empty<T>()
• ✅ No empty catch blocks
• ✅ Exception messages include context
• ✅ .Ordinal used for identifier comparisons
• ✅ Lazy<> caching avoids redundant scanning
• ✅ Bug fix has corresponding regression test (CompatJniName_UserTypes)

Positive callouts

• 👍 ContentProvider fixture completes all four Android component types in the test fixture.
• 👍 Negative test cases (PlainActivity NOT unconditional, concrete types have null InvokerTypeName) add real confidence.
• 👍 NullableExtensions.IsNullOrEmpty() rule correctly not applied — extensions aren't available in the TrimmableTypeMap test project.

---

Review generated by android-reviewer from review guidelines.

[simonrozsival]

🤖 💡 Code organization — This if (invokerTypeName is not null) guard is redundant. Within Assert.All, if Assert.False on line 53 throws, the exception is caught per-element and lines 54–56 are never reached for that element. When this line is reached, invokerTypeName is guaranteed non-null.

Harmless, but removing the guard would make the intent clearer — the Assert.Contains always runs if we get here.

Rule: Reduce indentation with early returns (Postmortem #62)

[Copilot]

Addressed in 1fa4c16. I removed the redundant null guard so the Assert.Contains now runs unconditionally once the non-empty assertion passes.

[simonrozsival]

🤖 💡 Documentation — The phrase "no slash replacement" could be misread as "this code doesn't use slashes" (it does, as the namespace/type separator). Consider rephrasing to emphasize what changed: // Compat format preserves dots in namespace: UserApp.Models -> userapp.models/TypeName

Rule: Comments explain "why", not "what" (Postmortem #59)

[Copilot]

Addressed in 1fa4c16. I reworded the comment to state that the compat format preserves namespace dots, which makes the intent clearer.

[simonrozsival]

@copilot address the review

[simonrozsival]

@copilot

🤖 AI Review Summary

Verdict: ⚠️ Needs Changes

Found 0 blocking code issues in the diff. The code is clean and well-structured. Two minor suggestions below.

CI Status

• ❌ dotnet-android Windows Build & Smoke Test — failed
• ❌ Xamarin.Android-PR — failed
• ✅ dotnet-android Linux Build — passed
• ✅ dotnet-android macOS Build — passed

CI must be green before merge. If the Windows failure is a pre-existing flake, a re-run should clear it.

What this PR does

1. Behavioral fix: Removes .Replace('.', '/') from ComputeAutoJniNames — compat JNI name changes from userapp/models/TypeName to userapp.models/TypeName. Safe because downstream consumers (ACWMapGenerator, ManifestDocument) apply .Replace('/', '.') before writing to acw-map.txt, producing identical output either way. New format matches the JavaPeerInfo.CompatJniName doc comment.
2. New integration tests: 6 standalone correctness tests for IsUnconditional, InvokerTypeName, CompatJniName, and MarshalMethodInfo.ManagedMethodName.
3. Fixture expansion: Adds ContentProvider, user-defined interface + invoker, and abstract type + invoker to UserTypesFixture.

Checked against review rules

• ✅ Mono formatting (tabs, space before ( and [)
• ✅ No null-forgiving operator (!)
• ✅ No string.Empty or Array.Empty<T>()
• ✅ No empty catch blocks
• ✅ Exception messages include context
• ✅ .Ordinal used for identifier comparisons
• ✅ Lazy<> caching avoids redundant scanning
• ✅ Bug fix has corresponding regression test (CompatJniName_UserTypes)

👍 Positive callouts

• ContentProvider fixture completes all four Android component types.
• Negative test cases (PlainActivity NOT unconditional, concrete types have null InvokerTypeName) add real confidence.
• NullableExtensions.IsNullOrEmpty() rule correctly not applied — extensions aren't available in the TrimmableTypeMap test project.

---

Review generated by android-reviewer from review guidelines.

[Copilot]

🤖 AI Review Summary

Verdict: ⚠️ Needs Changes (CI failures must be resolved before merge)...

Followed up in 1fa4c16. The review suggestions are addressed, and I investigated the reported Windows failure: the failed checks are the Azure DevOps build for this PR, but deeper log access from this environment is blocked by missing Azure DevOps authentication. Local targeted validation stayed green for the TrimmableTypeMap unit suite, and the integration-test build graph here still stops on existing Mono.Android metadata/API-compat failures unrelated to these review-only edits.

@copilot address the review

Done in 1fa4c16. I addressed the two review suggestions and checked the reported Windows CI failure; the remaining log-level investigation is blocked here by missing Azure DevOps auth, but the local targeted TrimmableTypeMap validation remains green.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• azcliprod.blob.core.windows.net
  • Triggering command: /usr/bin/../../opt/az/bin/python3 /usr/bin/../../opt/az/bin/python3 -Im azure.cli --version (dns block)
• dev.azure.com
  • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node /home/REDACTED/work/_temp/ghcca-node/node/bin/node --enable-source-maps /home/REDACTED/work/_temp/copilot-developer-action-main/dist/index.js (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

[Copilot]

@copilot

🤖 AI Review Summary

...

The code-review suggestions are already addressed in 1fa4c16. On CI, a fresh rerun is now in progress for this PR: Windows, macOS, and Linux jobs are currently running on Azure DevOps build 1333693, so I’m treating the earlier failure as pending confirmation from the rerun rather than making additional code changes without new failure details.