From 6bcc873805674af161889d7d707694bbd5d33b43 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Tue, 21 Jul 2026 14:30:27 -0500 Subject: [PATCH 1/2] Strengthen Java name conversion reviews Distinguish JNI, Java binary, Java source, and managed nested type names during review. Load interop guidance for trimmable typemap changes and require nested-name assertions at serialized output boundaries. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1a389fd8-0521-4de1-b2d4-ba67eff72ff8 --- .github/skills/android-reviewer/SKILL.md | 2 +- .../references/interop-rules.md | 17 +++++++++++++++++ .../references/testing-rules.md | 1 + 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/.github/skills/android-reviewer/SKILL.md b/.github/skills/android-reviewer/SKILL.md index 6ed99c3a413..f394353f6d4 100644 --- a/.github/skills/android-reviewer/SKILL.md +++ b/.github/skills/android-reviewer/SKILL.md @@ -72,7 +72,7 @@ Based on the file types identified in step 2, read the appropriate rule files fr - `references/csharp-rules.md` — When any `.cs` files changed. Covers nullable, async, error handling, performance, and code organization. - `references/msbuild-rules.md` — When `.targets`, `.props`, `.projitems`, or `.csproj` files changed, or when MSBuild task C# files changed (e.g., files under `src/Xamarin.Android.Build.Tasks/` or `external/xamarin-android-tools/src/Microsoft.Android.Build.BaseTasks/`). - `references/native-rules.md` — When `.c`, `.cpp`, `.h`, or `.hpp` files changed. Covers memory management, C++ best practices, symbol visibility, and platform-specific code. -- `references/interop-rules.md` — When both C# and native files changed, when the diff contains P/Invoke or JNI interop code (e.g., `DllImport`, `[Register]` attribute changes, `JNIEnv` calls, `[MarshalAs]`, `[StructLayout]`, `JniObjectReference`, `JniPeerMembers`, `JniTransition`), or when files under `external/Java.Interop/`, `src/Mono.Android/`, or `src/native/` changed. +- `references/interop-rules.md` — When interop markers or Java/JNI name conversions appear, or files under `external/Java.Interop/`, `src/Mono.Android/`, `src/native/`, or `src/Microsoft.Android.Sdk.TrimmableTypeMap/` change. - `references/testing-rules.md` — When test files changed (e.g., files under `tests/`, `**/Tests/`, or test project directories). - `references/security-rules.md` — When any code files changed (C#, C/C++, or MSBuild). diff --git a/.github/skills/android-reviewer/references/interop-rules.md b/.github/skills/android-reviewer/references/interop-rules.md index 8728fe6241e..8532ca5edd7 100644 --- a/.github/skills/android-reviewer/references/interop-rules.md +++ b/.github/skills/android-reviewer/references/interop-rules.md @@ -21,6 +21,23 @@ when the diff contains interop markers (`JniObjectReference`, `JniPeerMembers`, --- +## Java Type Names + +Determine the required representation at each changed call site: + +| Representation | Nested-class example | Typical consumers | +|----------------|----------------------|-------------------| +| **JNI internal name** | `android/view/View$OnClickListener` | JNI registration, descriptors after removing `L`/`;`, runtime JNI lookup | +| **Java binary name** | `android.view.View$OnClickListener` | `Class.forName`, Android manifests, ACW maps, ProGuard/R8 class rules | +| **Java source name** | `android.view.View.OnClickListener` | Generated `.java` type references such as `extends`, `implements`, parameters, and return types | +| **Managed nested type name** | `Android.Views.View+IOnClickListener` | Managed metadata and assembly-qualified names | + +- Read conversion helpers instead of trusting their names, and compare their full behavior with any inline logic they replace. +- Trace serialized names to the final consumer; correctness is per call site. +- Preserve `$` for binary-name consumers. Replace it with `.` only for Java source names. + +--- + ## P/Invoke & Marshaling Checks | Check | What to look for | diff --git a/.github/skills/android-reviewer/references/testing-rules.md b/.github/skills/android-reviewer/references/testing-rules.md index 2f36333a36a..760894e0a4f 100644 --- a/.github/skills/android-reviewer/references/testing-rules.md +++ b/.github/skills/android-reviewer/references/testing-rules.md @@ -20,3 +20,4 @@ Guidance for test code. The repo-specific conventions (e.g., `BaseTest`, | **Generator tests must include Invoker types** | Tests for generated binding code (under `external/Java.Interop/tools/generator/` and related test projects) should verify both the interface/class output and the `*Invoker` type behavior. Invoker codegen has historically had subtle bugs with default interface methods and virtual dispatch. | | **JVM-dependent tests** | Tests that require a running JVM must be in projects that configure the JVM environment (e.g., `Java.Interop-Tests`). Verify that test classes requiring a JVM are not placed in unit-test-only projects, where they will silently skip or fail with obscure errors. | | **Expected codegen output tests** | Generator tests that compare against expected output files should be updated when the expected format changes. Stale expected-output files cause spurious test failures that mask real regressions. | +| **Test nested Java/JNI names at boundaries** | Name-conversion tests must include a `$` nested type and assert the final manifest, map, rule, or generated-source output—not only the helper. | From 7338276664f9bf7074bef2c8ad1965ab5af8d6a7 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Tue, 21 Jul 2026 14:43:58 -0500 Subject: [PATCH 2/2] Align interop review scope descriptions Describe rule selection as based on changed files and diff content, and update the interop reference introduction for Java/JNI name handling. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1a389fd8-0521-4de1-b2d4-ba67eff72ff8 --- .github/skills/android-reviewer/SKILL.md | 2 +- .../skills/android-reviewer/references/interop-rules.md | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/.github/skills/android-reviewer/SKILL.md b/.github/skills/android-reviewer/SKILL.md index f394353f6d4..79d90520801 100644 --- a/.github/skills/android-reviewer/SKILL.md +++ b/.github/skills/android-reviewer/SKILL.md @@ -62,7 +62,7 @@ Review the CI results. **Never post ✅ LGTM if any required CI check is failing ### 5. Load review rules -Based on the file types identified in step 2, read the appropriate rule files from this skill's `references/` directory. +Based on the changed files and diff content identified in step 2, read the appropriate rule files from this skill's `references/` directory. **Always load:** - `references/repo-conventions.md` — Formatting, style, and patterns specific to this repository. diff --git a/.github/skills/android-reviewer/references/interop-rules.md b/.github/skills/android-reviewer/references/interop-rules.md index 8532ca5edd7..26e1a3f37e4 100644 --- a/.github/skills/android-reviewer/references/interop-rules.md +++ b/.github/skills/android-reviewer/references/interop-rules.md @@ -1,9 +1,8 @@ # Managed ↔ Native Interop Review Rules -Rules for the boundary between C# and C/C++ code — P/Invoke declarations, JNI -bindings, and shared structs. Load when both managed and native files change, or -when the diff contains interop markers (`JniObjectReference`, `JniPeerMembers`, -`DllImport`, `[Register]`, `JNIEnv`, `[MarshalAs]`, `[StructLayout]`). +Rules for managed/native boundaries and Java/JNI name handling. Load for interop +markers, Java/JNI name conversions, or changes under Java.Interop, Mono.Android, +native runtime, or TrimmableTypeMap paths. ---