[typemap] Match legacy JCW interface declarations - #12585
Conversation
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
Review tier: Lite
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
 |
src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerScanner.cs — interfaces.Any(...) inside the per-interface loop creates an O(n²) scan and… |
What changed in this PR
This PR updates the trimmable typemap Java peer scanning/generation pipeline so generated Java callable wrappers emit an interface implements list that matches legacy CecilImporter semantics (stable order, no redundant parent interfaces, no duplicate Java names), while still preserving the full ordered direct-interface list from managed metadata for other semantics.
Changes:
- Add
JavaCallableWrapperInterfaceJavaNamesonJavaPeerInfoand compute it in the scanner by filtering redundant parent interfaces and deduplicating Java names in first-seen order. - Update JCW Java source generation and Java-name validation to use the filtered interface list when available.
- Extend the shared semantic parity fixture + integration/build tests to cover parent/derived redundancy, colliding managed aliases, stable ordering, and compilation via
javac.
| File | Description |
|---|---|
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/JavaSourceSemanticParityTests.cs | Extends semantic parity coverage for redundant/colliding interfaces and adds Java stub types required for javac compilation. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/JavaSourceParityFixture/JavaSourceParityTypes.cs | Expands the managed fixture with interface hierarchy + alias collision scenarios used by both legacy and trimmable pipelines. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/TrimmableTypeMapBuildTests.cs | Extends build validation to ensure the semantic parity fixture compiles across llvm-ir/trimmable and CoreCLR/NativeAOT configurations. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/TrimmableTypeMapGenerator.cs | Validates interface names against the JCW-emitted interface list (when available) to match generated Java behavior. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerScanner.cs | Computes both the full direct-interface list and the filtered JCW declaration list (dedupe + most-derived filtering). |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerInfo.cs | Adds an internal property to carry the JCW-specific interface declaration list alongside the full direct-interface list. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/JcwJavaSourceGenerator.cs | Emits the JCW implements clause from the filtered interface list to avoid redundant/duplicate declarations. |
caa6506 to
252fd7d
Compare
b477490 to
41401a8
Compare
41401a8 to
f7caab7
Compare
Filter redundant parent interfaces and duplicate Java names from generated callable-wrapper declarations while retaining the complete ordered interface list for scanner and typemap semantics. Add focused scanner and Java source regression coverage for parent, alias, and ordering behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
f7caab7 to
8d1c574
Compare
|
/review |
|
✅ Android PR Reviewer completed successfully!
|
Replace the captured LINQ predicate with explicit iteration and reuse the interface-assignability visited set across comparisons. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ LGTM on code (CI pending)
Findings: 0 errors · 0 warnings · 1 suggestion
The split between the complete direct-interface list and the filtered JCW declaration list preserves typemap/alias semantics while matching Java interface inheritance and erasure. The first-seen ordering, duplicate JNI-name handling, parent elimination, generator fallback, and source-name validation are coherent and covered by focused same-assembly regression tests.
Azure DevOps build 1576771 is still running: CLA has passed, with the currently published dotnet-android lanes in progress or queued and no failures reported yet.
Generated by Android PR Reviewer for #12585 · gpt56 · 190.5 AIC · ⌖ 8.92 AIC · ⊞ 25.7K
Comment /review to run again
| } | ||
|
|
||
| [Fact] | ||
| public void Scan_JavaCallableWrapperInterfaces_OmitsParentsAndDuplicateJavaNames () |
There was a problem hiding this comment.
🤖 💡 Testing — Please consider adding a cross-assembly variant of this regression: put the derived interface in the scanned app assembly and its parent in a referenced fixture/framework assembly. IsInterfaceAssignableFrom() now depends on TryResolveType() and ResolveEntityHandle() while crossing assembly indexes, but this fixture keeps both interfaces in TestFixtures, so the most common app-interface → bound-framework-interface path is not exercised.
Rule: Cover cross-assembly scanner behavior.
Summary
CecilImporterRegression coverage
Before the fix, the trimmable generator emitted the parent interface, derived interface, and a second managed alias for the same derived Java interface. The duplicate Java name produced an invalid
implementsdeclaration.The focused fixture declares all four inputs directly. Scanner coverage verifies the complete ordered metadata list remains available for typemap semantics, while generator coverage verifies the JCW emits only the derived and unrelated interfaces in stable order.
Validation
Microsoft.Android.Sdk.TrimmableTypeMap.Testsgit diff --checkTracks #12561.