Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/skills/android-reviewer/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment thread
jonathanpeppers marked this conversation as resolved.
- `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).

Expand Down
17 changes: 17 additions & 0 deletions .github/skills/android-reviewer/references/interop-rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,23 @@ when the diff contains interop markers (`JniObjectReference`, `JniPeerMembers`,

---

## Java Type Names

Determine the required representation at each changed call site:
Comment thread
jonathanpeppers marked this conversation as resolved.

| 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 |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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. |
Loading