Skip to content

[generator] Phase 2: Kotlin @JvmInline value class projection#1440

Open
jonathanpeppers wants to merge 8 commits into
mainfrom
jonathanpeppers/kotlin-inline-class-wrappers
Open

[generator] Phase 2: Kotlin @JvmInline value class projection#1440
jonathanpeppers wants to merge 8 commits into
mainfrom
jonathanpeppers/kotlin-inline-class-wrappers

Conversation

@jonathanpeppers
Copy link
Copy Markdown
Member

@jonathanpeppers jonathanpeppers commented Jun 3, 2026

Phase 2 of #1431. Builds on #1432 (sibling-collision dedup) by detecting Kotlin @JvmInline value class types in class-parse, surfacing them in api.xml, and projecting them into generator output as readonly struct wrappers — while JNI marshaling stays on the underlying primitive.

What this does

Given Kotlin source like:

@JvmInline
value class MyColor(val value: ULong)
@JvmInline
value class MyAlpha(val value: ULong)
@JvmInline
value class MyDp(val value: Float)

object Widgets {
    fun tint(c: MyColor) { ... }
    fun tint(a: MyAlpha) { ... }
    fun tint(d: MyDp)    { ... }
    fun pad(d: MyDp): MyDp           { ... }
    fun pad(x: MyDp, y: MyDp): MyDp  { ... }
}

The generator now emits:

public readonly partial struct MyColor : System.IEquatable<MyColor> {
    public readonly long Value;
    public MyColor (long value) { Value = value; }
    public static implicit operator long (MyColor value) => value.Value;
    public static implicit operator MyColor (long value) => new MyColor (value);
    // == / != / Equals / GetHashCode / ToString
}
// ...same shape for MyAlpha (long-backed) and MyDp (float-backed).

public static class Widgets {
    public static void  Tint (MyColor c) { ... }   // was: void Tint_Rn_QMJI (long c)
    public static void  Tint (MyAlpha a) { ... }   // was: void Tint_uzYZ1wI (long a)
    public static void  Tint (MyDp d)    { ... }   // was: void Tint_L3D9Hvg (long d)
    public static MyDp  Pad  (MyDp d)            { ... }
    public static MyDp  Pad  (MyDp x, MyDp y)    { ... }
}

The bodies of Tint/Pad still pass the JVM-erased primitive (long/float) across JNI — the implicit conversion operators on the struct make that compile without any thunk changes.

Bytecode layer

  • New ClassFile.KotlinInlineClassUnderlyingJniType, MethodInfo.KotlinInlineClassReturnJniType, MethodInfo.KotlinName, ParameterInfo.KotlinInlineClassJniType.
  • KotlinFixups.Fixup does a pre-pass DetectInlineClasses that recognizes @JvmInline + KotlinClassFlags.IsInlineClass + a single non-synthetic instance field with a primitive descriptor, then stamps each method's parameter/return JNI types when the Kotlin source-level type was an inline class and the JVM-erased descriptor matches the underlying primitive (so boxed/nullable positions are correctly skipped).
  • Recovers the unmangled Kotlin source name from metadata (tint instead of tint-Rn_QMJI) and surfaces it via MethodInfo.KotlinName.
  • XmlClassDeclarationBuilder emits kotlin-inline-class, kotlin-inline-class-underlying-jni-type, kotlin-inline-class-jni-type (parameter), kotlin-inline-class-return-jni-type (method), and managedName (unmangled Kotlin name) attributes.

Generator layer

  • ClassGen.IsKotlinInlineClass / ClassGen.KotlinInlineClassUnderlyingJniType.
  • Parameter.KotlinInlineClassJniType / Method.KotlinInlineClassReturnJniType.
  • XmlApiImporter reads the new attributes; the existing managedName codepath delivers the unmangled C# binding name without affecting JNI invocation.
  • Parameter.Validate / ReturnValue.Validate apply the projection by looking up the wrapper ClassGen via SymbolTable and overriding managed_type so Type/FullName return the struct while Symbol (and therefore JNI marshaling) stays on the underlying primitive.
  • MethodBase.Matches also compares Parameters[i].KotlinInlineClassJniType so two inline classes that share the same JVM primitive (e.g. multiple ULong-backed inline classes on Compose APIs) survive Phase 1's RemoveCollidingSiblings as distinct overloads.
  • Parameter.ShouldGenerateKeepAlive returns false for projected inline-class params (avoids needless GC.KeepAlive on a value-type wrapping a primitive).
  • New KotlinInlineClassStruct TypeWriter emits the wrapper struct.
  • JavaInteropCodeGenerator.WriteType routes inline-class ClassGen instances to the new writer instead of BoundClass.
  • New TypeNameUtilities.JniSignatureToJavaTypeName helper (handles nested types via $.).

Tests

  • Bytecode-Tests: 82 passed (2 pre-existing skips). New KotlinInlineClassCollisionTests exercise the real Kotlin .class fixtures.
  • generator-Tests: 7 unit tests in KotlinInlineClassTests plus 1 end-to-end test in KotlinInlineClassEndToEndTests that drives real Kotlin .class files (compiled by tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin-gradle/) through KotlinFixupsXmlClassDeclarationBuilderXmlApiImporterJava.Interop.Tools.Generator.Transformation.KotlinFixupsValidateJavaInteropCodeGenerator. The end-to-end test mirrors the real CodeGenerator.Generate pipeline and would have caught the RemoveCollidingSiblings regression that motivated the MethodBase.Matches fix.
  • 23 pre-existing generator-Tests failures are unrelated environment issues (Roslyn / FileStream); confirmed unchanged.

Build-system

The Kotlin/Gradle fixture target (BuildKotlinGradleProject) was extracted into a shared tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin-gradle.targets file imported by both Bytecode-Tests.csproj and generator-Tests.csproj, so the .class files are produced regardless of which test project the build hits first.

Out of scope / known limitations

  • Boxed inline-class positions (nullable like MyColor?, generics like List<MyColor>, supertype implementations) still emit Lcom/example/MyColor; references that no longer have a peer class binding. These were rare on the targeted Compose surface; users can unblock them via Metadata.xml (<remove-node>) until a follow-up.
  • Reference-backed inline classes (e.g. value class Tag(val raw: String)) are detected as inline classes but the projection path requires a primitive backing field; non-primitive-backed inline classes are skipped and fall through to the legacy peer-class path.
  • Generic inline classes (value class Wrap<T>(val v: T)).
  • Validation against the Compose material3-android AAR is not part of this PR.

Closes part of #1431. Phase 1 (sibling-collision dedup) was #1432.

Copilot AI and others added 2 commits June 3, 2026 11:00
Phase 2 of #1431. Surface inline-class info on the
api.xml so the generator can later project parameters/returns to
strongly-typed wrapper structs while keeping JNI marshaling on the
underlying primitive.

* Stamp ClassFile.KotlinInlineClassUnderlyingJniType with the JNI
  descriptor of the single non-synthetic instance field on every Kotlin
  '@JvmInline value class'.
* Stamp MethodInfo.KotlinInlineClassReturnJniType / ParameterInfo.
  KotlinInlineClassJniType when a method's Kotlin source-level return
  or parameter type was an inline class (the JVM-erased type is the
  inline class's backing primitive).
* Emit kotlin-inline-class / kotlin-inline-class-underlying-jni-type
  on <class>, kotlin-inline-class-jni-type on <parameter>, and
  kotlin-inline-class-return-jni-type on <method>.
* Bytecode tests cover the existing kotlin-gradle/ MyColor / MyAlpha /
  MyDp / Widgets fixture: ULong-backed and Float-backed detection,
  per-parameter stamping, return-type stamping, and round-trip through
  XmlClassDeclarationBuilder.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Phase 2 of #1431. Wires the inline-class XML attributes from the
class-parse layer (commit 35ccc4f) through the generator so:

* `<class kotlin-inline-class="true" .../>` is emitted as a
  `readonly partial struct` wrapper around the underlying primitive
  (e.g. `J` -> `long`, `F` -> `float`) instead of a peer-class binding.
  The struct exposes `Value`, implicit conversions, equality, and
  ToString.

* `<parameter kotlin-inline-class-jni-type="L...;" .../>` makes the
  generator project the parameter's managed type to the wrapper struct
  while keeping JNI marshaling on the underlying primitive. Because the
  struct has implicit conversion operators, existing `JniArgumentValue`
  thunks compile unchanged.

* `<method kotlin-inline-class-return-jni-type="L...;" .../>` does the
  same for return values via `ReturnValue.managed_type`.

Plumbing:
* `ClassGen.IsKotlinInlineClass`, `ClassGen.KotlinInlineClassUnderlyingJniType`
* `Parameter.KotlinInlineClassJniType`
* `Method.KotlinInlineClassReturnJniType`
* `Parameter.Validate`/`ReturnValue.Validate` apply the projection by
  looking up the wrapper `ClassGen` via `SymbolTable`.
* New `KotlinInlineClassStruct` TypeWriter emits the wrapper struct.
* New `TypeNameUtilities.JniSignatureToJavaTypeName` helper.

Out of scope (acknowledged limitations): boxed/nullable/generic
inline-class positions still resolve to the (now-replaced) peer
binding; reference-backed inline classes; generic inline classes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 3, 2026 16:14
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds Kotlin @JvmInline value class awareness end-to-end (bytecode → api.xml → generator) so inline-class parameters/returns can be projected as strongly-typed C# wrappers while preserving JNI marshaling on the JVM-erased underlying primitive.

Changes:

  • Detect Kotlin inline/value classes in class-parse, stamp underlying JNI primitive info, and emit new inline-class attributes into api.xml.
  • Import the new attributes in the generator and project method parameters/returns to inline-class wrapper types.
  • Emit readonly partial struct wrapper types for inline classes and add unit tests covering import/projection/round-tripping.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
tools/generator/Utilities/TypeNameUtilities.cs Adds helper to map JNI reference signatures to Java type names for symbol lookup.
tools/generator/SourceWriters/KotlinInlineClassStruct.cs New type writer emitting the C# readonly struct wrapper for inline classes.
tools/generator/Java.Interop.Tools.Generator.ObjectModel/ReturnValue.cs Projects return types to inline-class wrapper structs when stamped.
tools/generator/Java.Interop.Tools.Generator.ObjectModel/Parameter.cs Projects parameter types to inline-class wrapper structs when stamped; preserves clone data.
tools/generator/Java.Interop.Tools.Generator.ObjectModel/Method.cs Adds storage for stamped inline-class return JNI type.
tools/generator/Java.Interop.Tools.Generator.ObjectModel/ClassGen.cs Adds inline-class flags/underlying descriptor to the object model.
tools/generator/Java.Interop.Tools.Generator.Importers/XmlApiImporter.cs Reads new inline-class XML attributes into the generator model.
tools/generator/Java.Interop.Tools.Generator.CodeGeneration/JavaInteropCodeGenerator.cs Routes inline classes to the new struct writer.
tests/Xamarin.Android.Tools.Bytecode-Tests/Xamarin.Android.Tools.Bytecode-Tests.targets Adjusts embedded Kotlin fixture class list.
tests/Xamarin.Android.Tools.Bytecode-Tests/KotlinInlineClassCollisionTests.cs Adds tests asserting fixup stamping and XML attribute emission.
tests/generator-Tests/Unit-Tests/KotlinInlineClassTests.cs Adds tests covering XML import and the new helper.
src/Xamarin.Android.Tools.Bytecode/XmlClassDeclarationBuilder.cs Emits new inline-class XML attributes for classes/methods/parameters.
src/Xamarin.Android.Tools.Bytecode/Methods.cs Adds fields to carry stamped inline-class parameter/return JNI signatures.
src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinFixups.cs Detects inline classes and stamps inline-class parameter/return metadata for projection.
src/Xamarin.Android.Tools.Bytecode/ClassFile.cs Stores inline-class underlying JNI descriptor on the ClassFile.

Comment thread tools/generator/SourceWriters/KotlinInlineClassStruct.cs
Comment thread tools/generator/SourceWriters/KotlinInlineClassStruct.cs Outdated
Comment thread tools/generator/Utilities/TypeNameUtilities.cs Outdated
Comment thread src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinFixups.cs
Comment thread src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinFixups.cs Outdated
Comment thread src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinFixups.cs Outdated
Comment thread src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinFixups.cs
Comment thread src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinFixups.cs
jonathanpeppers and others added 3 commits June 3, 2026 11:26
Wires the four Kotlin .class files compiled by the kotlin-gradle/

fixture under tests/Xamarin.Android.Tools.Bytecode-Tests/ into a

generator-level test that exercises the full Phase 2 pipeline:

  bytecode (KotlinFixups + XmlClassDeclarationBuilder)

    -> api.xml string

    -> XmlApiImporter.Parse + Validate

    -> JavaInteropCodeGenerator.WriteType

and asserts the projected C# output emits readonly partial structs

for MyColor/MyAlpha/MyDp, projects them in Widgets.tint/pad method

signatures, and never falls back to peer-class bindings for them.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When the Kotlin compiler mangles a JVM method name for inline-class

binary compatibility (e.g. `tint(MyColor)` -> `tint-Rn_QMJI`), recover

the unmangled Kotlin source name and surface it in api.xml as the

`managedName` attribute. The mangled JVM name still appears in

`name`/`jni-signature` so JNI invocation targets the actual method.

With this and the Phase 2 inline-class -> struct projection, methods

that erase to colliding JVM signatures emit as plain C# overloads

distinguished by struct type, e.g.:

    void Tint (MyColor color);

    void Tint (MyAlpha alpha);

    void Tint (MyDp dp);

instead of the previous `Tint_Rn_QMJI` / `Tint_uzYZ1wI` mangled names.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1. KotlinInlineClassStruct.cs: drop unused `using System;` and unused

   `klass` field; gate `object?`/`string?` annotations on

   `opt.NullableOperator` so consumers without nullable enabled don't

   get CS8632 warnings.

2. TypeNameUtilities.JniSignatureToJavaTypeName: also translate `\$`

   (JNI nested-type separator) to `.` so SymbolTable.Lookup() resolves

   nested inline classes.

3. KotlinFixups.DetectInlineClasses: only stamp underlying-JNI type

   when there is exactly one non-synthetic instance field AND that

   field is a JVM primitive descriptor (Z/B/C/D/F/I/J/S). Reference-

   backed inline classes (e.g. `value class Tag(val s: String)`) are

   skipped — the wrapper struct currently emits a primitive `Value`

   field, so they would otherwise produce wrong bindings.

4. KotlinFixups.GetInlineClassJniType: now also takes the JVM-erased

   descriptor of the position being projected (param/return/property)

   and only returns a JNI-type when it equals the inline class's

   underlying primitive. Boxed / nullable / generic inline-class

   positions (where the JVM signature stays `L...;`) are no longer

   incorrectly stamped — they fall through to the legacy peer-class

   binding path so JNI marshaling stays consistent. Applied at all

   four stamping sites: function param, function return, property

   getter return, property setter param.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jonathanpeppers
Copy link
Copy Markdown
Member Author

/review

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 3, 2026

Java.Interop PR Reviewer completed successfully!

Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — well-designed feature with minor issues

Summary: This PR adds Kotlin @JvmInline value class projection through the full pipeline (bytecode → api.xml → generator → C# wrapper structs). The architecture is clean — using the existing managedName attribute for name unmangling, projecting managed types while keeping JNI marshaling on primitives via the sym / managed_type split, and carefully filtering to only primitive-backed inline classes.

Issue counts:

  • ⚠️ 2 warnings (silent fallback in JniPrimitiveToCSharpType; null-forgiving operator ! usage ×4)
  • 💡 1 suggestion (klass2 naming)

Positive callouts:

  • The two-pass approach in DetectInlineClasses (pre-scan all classes before processing methods) correctly handles cross-class references regardless of class ordering.
  • Good defensive filtering: only primitive-backed inline classes are projected; reference-backed and boxed positions are explicitly excluded with clear documentation of why.
  • The end-to-end test (KotlinInlineClassEndToEndTests) exercises the real Kotlin .class fixtures through the full pipeline — much stronger than testing against hand-crafted XML alone.
  • Thorough comments linking each piece to the issue tracker (#1431) and explaining the "why" behind design decisions (e.g., why boxed positions are skipped).
  • Method.Clone correctly copies KotlinInlineClassReturnJniType, and Parameter.Clone correctly copies KotlinInlineClassJniType.

CI: dotnet.java-interop is still queued/pending. license/cla passed.

Generated by Java.Interop PR Reviewer for issue #1440 · ● 10.6M

Comment thread src/Xamarin.Android.Tools.Bytecode/XmlClassDeclarationBuilder.cs Outdated
Comment thread tools/generator/SourceWriters/KotlinInlineClassStruct.cs Outdated
- XmlClassDeclarationBuilder: drop null-forgiving '!' (4 sites). The string.IsNullOrEmpty guard's [NotNullWhen(false)] annotation already narrows the type, so the operator was unnecessary and violated the repo convention banning '!'.

- JavaInteropCodeGenerator.WriteType: collapse the 'klass'/'klass2' duplicate pattern variables into a nested if-else inside a single ClassGen match.

- KotlinInlineClassStruct.JniPrimitiveToCSharpType: replace the silent '_ => \"long\"' fallback with ArgumentOutOfRangeException. DetectInlineClasses already filters to primitive descriptors, so this branch is unreachable for valid input - failing fast prevents a non-primitive from silently producing a wrong 'long' wrapper.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jonathanpeppers added a commit to jonathanpeppers/compose-net that referenced this pull request Jun 3, 2026
Replaces the seven hand-rolled `[Flags]` enums in ComposeDefaults.cs
(`ButtonDefault`, `TextDefault`, `IconButtonDefault`,
`FloatingActionButtonDefault`, `SurfaceDefault`, `AlertDialogDefault`,
`TextFieldDefault`) with a new declarative form of
`ComposeDefaultsAttribute` that takes a positional Kotlin parameter
list:

    [assembly: ComposeDefaults("ButtonDefault",
        "!onClick", "modifier", "enabled", ..., "!content")]

Names prefixed with `!` consume a bit position but emit no enum member,
covering Compose params the caller always provides (onClick, content,
text, value, etc.). Optional slot params stay as members so call sites
can clear bits per-call (the AlertDialog pattern).

These seven were hand-rolled because the dotnet/android-libraries
binder strips the Compose overloads with mangled JVM names from inline
classes (Text--4IGK_g, Surface-T9BRK9s, AlertDialog-Oix01E0, ...), so
no IMethodSymbol exists for the existing generic
`[ComposeDefaults<T>]` form to read. The declarative form is a
near-term workaround; once dotnet/java-interop#1440 lands and exposes
those overloads, each declarative attribute can be swapped one-for-one
for the generic form.

Generator changes:
- New non-generic `ComposeDefaultsAttribute(string, params string[])`.
- `ComposeDefaultsGenerator` dispatches on attribute kind. Shared
  `ComposeDefaultsEmitter` core driven by a `Slot` list, populated
  either from `IMethodSymbol` (generic) or from the names array
  (declarative).
- Two new tests covering the declarative path; existing eight unchanged.

Verified the generated enums match the deleted hand-rolled values
byte-for-byte (bit positions and `All` mask) by enabling
EmitCompilerGeneratedFiles and diffing AlertDialogDefault and
SurfaceDefault.

Also adds .github/copilot-instructions.md documenting the layout,
build/test commands, both attribute forms, the ComposeBridges JNI
recipe, the facade conventions (ComposableNode/Container,
ComposableLambdaN, named-slot AlertDialog pattern), the bindings
policy, and style.

### Fix CN1003 message to cover both [ComposeDefaults] forms

The diagnostic message previously said `[ComposeDefaults<T>(string)]`,
which was wrong for both forms: the generic overload takes two
strings, and the declarative overload isn't generic at all. Make
the message form-agnostic.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The generator-Tests project embeds the same Kotlin .class files compiled from kotlin-gradle/ as Bytecode-Tests, but it only ProjectReferences the Xamarin.Android.Tools.Bytecode library — not the Bytecode-Tests project that owned the BuildKotlinGradleProject MSBuild target. On a clean CI checkout, generator-Tests's Csc task tried to embed .class files that had never been produced, failing with 'Could not find a part of the path' on macOS.

Extract BuildKotlinGradleProject into a shared kotlin-gradle.targets file alongside the kotlin-gradle/ fixture and import it from both test projects. The target runs BeforeTargets=BeforeCompile so whichever test project the build hits first will produce the .class files.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jonathanpeppers
Copy link
Copy Markdown
Member Author

/review

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 4, 2026

Java.Interop PR Reviewer completed successfully!

Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Needs Changes

Well-structured PR with clean separation between the bytecode-layer detection and generator-layer projection. The new KotlinInlineClassStruct TypeWriter, the ApplyKotlinInlineClassProjection hooks in Parameter/ReturnValue.Validate, and the DetectInlineClasses pre-pass are all thoughtfully designed. The test coverage (4 bytecode tests + 7 generator unit tests + 1 end-to-end test) is solid for the isolated components.

Key Issue

Generator-layer RemoveCollidingSiblings interaction (1 ⚠️): The Phase 1 collision-dedup code in tools/generator/Java.Interop.Tools.Generator.Transformation/KotlinFixups.cs (from PR #1432) was explicitly temporary — its comment reads "Until step 2 of #1431 projects inline-class params...". This PR is step 2, but doesn't update that code. Since RemoveCollidingSiblings runs before Validate (where projection happens), overloads like Tint(MyColor) and Tint(MyAlpha) — both long-backed — would still collide on RawNativeType and one would be removed before the struct projection can disambiguate them. The end-to-end test masks this because it doesn't call KotlinFixups.Fixup(gens).

Suggestions (2 💡)

  • ShouldGenerateKeepAlive for projected parameters: After projection, Parameter.Type returns the struct's FullName, which falls through to the default true case in ShouldGenerateKeepAlive, generating unnecessary GC.KeepAlive for value types.
  • PascalCase vs MemberToPascalCase: The local helper only uppercases the first character; underscore-separated Kotlin names would produce casing inconsistent with the generator layer.

Positive Callouts

  • Clean DetectInlineClasses pre-pass with proper validation (JvmInline annotation + metadata flags + single primitive backing field).
  • The GetInlineClassJniType guard that only projects when the JVM-erased descriptor matches the underlying primitive — correctly skipping boxed/nullable positions — is a solid safety check.
  • Extracting the Gradle build into a shared kotlin-gradle.targets that both Bytecode-Tests and generator-Tests import is a nice build-system improvement.
  • Good decision to keep sym on the underlying primitive while projecting managed_type to the struct — this makes the implicit conversion operators handle the JNI boundary transparently.
Severity Count
⚠️ Warning 1
💡 Suggestion 2

Generated by Java.Interop PR Reviewer for issue #1440 · ● 18M

Comment thread tests/generator-Tests/Unit-Tests/KotlinInlineClassEndToEndTests.cs
Comment thread src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinFixups.cs
Issue 1 (correctness, warning): MethodBase.Matches only compared Parameters[i].RawNativeType, so two Kotlin @JvmInline value classes that share the same underlying JVM primitive (e.g. multiple ULong-backed inline classes on a Compose-style API) collided. Phase 1's RemoveCollidingSiblings then dropped one of the renamed overloads before Phase 2's projection ran in Validate.

Fix: also compare Parameters[i].KotlinInlineClassJniType so MyColor/MyAlpha (both 'J'-backed) survive RemoveCollidingSiblings as distinct overloads.

Updated the end-to-end test to call Java.Interop.Tools.Generator.Transformation.KotlinFixups.Fixup(gens) before Validate, mirroring CodeGenerator.cs line 209. Verified the test fails without the Matches change (Tint(MyAlpha) is dropped, the expected overload is missing from output) and passes with it.

Issue 3 (suggestion, perf): Parameter.ShouldGenerateKeepAlive returned true for projected inline-class parameters because the wrapper struct's FullName didn't match any primitive case. Added an early return when KotlinInlineClassJniType is set so we don't emit GC.KeepAlive for value-type wrappers around a primitive.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jonathanpeppers added a commit to jonathanpeppers/compose-net that referenced this pull request Jun 4, 2026
PR #33 already shipped CircularProgressIndicator, LinearProgressIndicator,
SegmentedButton + both rows, WideNavigationRail, and WideNavigationRailItem
for issue #5. This change completes the remaining actionable item from
that issue, `ModalWideNavigationRail`. SecureTextField is still deferred
on the `TextFieldState` story.

- Add `[ComposeBridge] ModalWideNavigationRail` targeting the stripped
  `ModalWideNavigationRail-k3FuEkE` JVM name (hashed because of the `Dp`
  `@JvmInline value class` on `collapsedShadowElevation`).
- Add `[ComposeDefaults] ModalWideNavigationRailDefault` enum, marking
  `state` and `content` as `!` (always supplied by the facade).
- New `ModalWideNavigationRail` facade (ComposableContainer with
  optional `Header` slot). Uses the bound (non-stripped)
  `WideNavigationRailStateKt.RememberWideNavigationRailState` so no
  extra bridge is needed for the state holder.
- Sample: add a "Modal" button to the Misc tab demonstrating the
  visibility-toggle pattern with an internal "Close" item.

The facade documents two limitations callers should know about:
1. The rail always remembers state with `WideNavigationRailValue.Expanded`
   because the state's `expand`/`collapse`/`snapTo` methods are Kotlin
   suspend functions (no coroutine story in C# yet).
2. Scrim-tap dismissal hides the rail visually but cannot notify C#;
   callers gate visibility on their own `MutableState<bool>` and add an
   internal close button.

Once dotnet/java-interop#1440 lands the bridge can be swapped for a
direct call to the binding.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jonathanpeppers added a commit to jonathanpeppers/compose-net that referenced this pull request Jun 4, 2026
Completes the remaining actionable item from #5 that was deferred in #33.

#33 already shipped `CircularProgressIndicator`, `LinearProgressIndicator`,
`SegmentedButton` + both rows, `WideNavigationRail`, and
`WideNavigationRailItem`. This change adds `ModalWideNavigationRail` —
the modal-overlay variant. `SecureTextField` is still deferred on the
`TextFieldState` story (out of scope).

## What's added

- **Bridge.** `[ComposeBridge] ModalWideNavigationRail` targets the
  stripped `androidx/compose/material3/WideNavigationRailKt::ModalWideNavigationRail-k3FuEkE`
  JVM name. The hash comes from the `Dp` `@JvmInline value class` on
  `collapsedShadowElevation`; once dotnet/java-interop#1440 lands the
  bridge can be swapped for a direct binding call.
- **Defaults enum.** `[ComposeDefaults] ModalWideNavigationRailDefault`
  with `state` and `content` marked `!` (the facade always supplies
  these — they are not enum members and not in `All`).
- **Facade.** New `ModalWideNavigationRail` class (`ComposableContainer`
  with optional `Header` slot). Uses the bound (non-stripped)
  `WideNavigationRailStateKt.RememberWideNavigationRailState` for the
  state holder, so no extra bridge is needed there.
- **Sample.** A "Modal rail" button on the Misc tab demonstrates the
  visibility-toggle pattern with an internal "Close" item.

## Documented limitations

`WideNavigationRailState`s `expand` / `collapse` / `snapTo` methods are
Kotlin `suspend` functions. We don't have a `LaunchedEffect` /
coroutine story for C# yet, so:

1. The facade always remembers state with
   `WideNavigationRailValue.Expanded`, so the rail opens immediately
   when mounted.
2. Scrim-tap dismissal hides the rail visually (because
   `hideOnCollapse = true`) but cannot notify C#. Callers gate
   visibility on their own `MutableState<bool>` and add an internal
   close button — the sample shows the pattern, and the XML doc on
   `ModalWideNavigationRail` calls this out explicitly.

## Known risk

The Misc tab still trips compose-net#42 ("LayoutNode insertAt
ArrayIndexOutOfBoundsException") when switching tabs after interacting
with several Misc widgets. `ModalWideNavigationRail` may share the same
underlying `$changed = 0` / lambda-identity story; tracked separately
in that issue.

## Verified

- `dotnet test src/ComposeNet.SourceGenerators.Tests` — 32/32 pass.
- `dotnet build src/ComposeNet.Compose` — 0 errors.
- `dotnet build src/ComposeNet.Sample` — 0 errors, 0 warnings.

Closes the modal-rail part of #5.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants