Skip to content

Fix API match for type composition, Equatable, and Sendable#219

Merged
marcprux merged 2 commits intoskiptools:mainfrom
dfabulich:on-geometry-change-fix
Apr 4, 2026
Merged

Fix API match for type composition, Equatable, and Sendable#219
marcprux merged 2 commits intoskiptools:mainfrom
dfabulich:on-geometry-change-fix

Conversation

@dfabulich
Copy link
Copy Markdown
Contributor

I was working on adding support for onGeometryChange, and I ran into what I believe is a bug in the transpiler, which this PR attempts to fix.

(While I was working on it, @marcprux worked around this issue in an entirely different way in skiptools/skip-ui#364, making this PR technically no longer required. But, it seems best to file the PR anyway.)

We start with a test of the issue. My signature for onGeometryChange looked like this:

// SKIP DECLARE: fun <T : Any> onGeometryChange(for_: kotlin.reflect.KClass<T>, of: (GeometryProxy) -> T, action: (T) -> Unit): View
func onGeometryChange<T>(
    for type: T.Type,
    of transform: @escaping @Sendable (GeometryProxy) -> T,
    action: @escaping (T) -> Void
) -> some View where T : Equatable, T : Sendable

That's a complex declaration, including the use of Equatable and Sendable as generic constraints, and, sure enough, it didn't wanna transpile properly, so I used a // SKIP DECLARE to transpile it.

The bug is that, when I used Bool as my T, and tried to use it in a playground, the API failed to match, and so the transpiler didn't add the .Compose(composectx) tail call that would make the ComposeBuilder valid.

import SwiftUI
struct OnGeometryChangeExample: View {
    var body: some View {
        Text("test")
            .onGeometryChange(for: Bool.self, of: { _ in true }, action: { _ in })
    }
}

I added this as a failing test in SwiftUITests.swift.

During matching, T was replaced with (Equatable & Sendable).Type, which Bool failed to match in three different ways:

  1. The transpiler didn't know how to match any composition type (X & Y).Type
  2. The transpiler didn't know that everything is Equatable in Kotlin
  3. The transpiler didn't know that all of the primitive types are Sendable

So, I also added a new test, CompatibiltyScoreTests.swift, which is basically a snapshot test of the current compatibility scores. I added a bunch of regression tests (which I assumed had correct results), and I added four failing tests:

    func testEquatableNamedScore() async throws {
        XCTAssertEqual(TypeSignature.bool.compatibilityScore(target: .named("Equatable", []), codebaseInfo: context), 1.9)
    }
    // XCTAssertEqual failed: ("nil") is not equal to ("Optional(1.9)")

    func testSendableNamedScore() async throws {
        XCTAssertEqual(TypeSignature.bool.compatibilityScore(target: .named("Sendable", []), codebaseInfo: context), 1.9)
    }
    // XCTAssertEqual failed: ("nil") is not equal to ("Optional(1.9)")

    func testEquatableSendableCompositionScore() async throws {
        let target: TypeSignature = .composition([.named("Equatable", []), .named("Sendable", [])])
        XCTAssertEqual(TypeSignature.bool.compatibilityScore(target: target, codebaseInfo: context), 1.9)
    }
    // XCTAssertEqual failed: ("nil") is not equal to ("Optional(1.9)")
    
    func testInheritanceWithComposition() async throws {
        context = try await setUpContext(swift: """
        protocol P: Sendable {}
        class A: P {}
        protocol Q {}
        """)
        let positiveTarget: TypeSignature = .composition([.named("Equatable", []), .named("P", [])])
        let negativeTarget: TypeSignature = .composition([.named("Equatable", []), .named("Q", [])])
        XCTAssertEqual(TypeSignature.named("A", []).compatibilityScore(target: positiveTarget, codebaseInfo: context), 1.9)
        XCTAssertNil(TypeSignature.named("A", []).compatibilityScore(target: negativeTarget, codebaseInfo: context))
    }
    // XCTAssertEqual failed: ("nil") is not equal to ("Optional(1.9)")

Then, I fixed those tests.

  1. I added support for strippedTarget case .named in compatibilityScore:

    case .named:
        if strippedTarget.isEquatable { return 1.9 }
        if strippedTarget.isSendable { return isSwiftBuiltinSendable ? 1.9 : nil }
        return nil
    private var isSwiftBuiltinSendable: Bool {
        switch self {
        case .bool, .int, .int8, .int16, .int32, .int64, .int128,
             .uint, .uint8, .uint16, .uint32, .uint64, .uint128,
             .double, .float, .string, .character:
            return true
        case .optional(let inner):
            return inner.isSwiftBuiltinSendable
        default:
            return false
        }
    }

    That fixed the first two failing tests in CompatibiltyScoreTests.swift.

  2. I added support for strippedTarget case .composition, delegating to inheritanceCompatibilityScore, and, in inheritanceCompatibilityScore, in case .composition, I called compatibilityScore on each of the composed protocolTypes. If any of them were nil, we return nil, and otherwise we take the average of the results. (Is that the right thing to do??) That then fixed the other two failing tests in CompatibiltyScoreTests.swift, as well as the SwiftUI test.

Skip Pull Request Checklist:

  • REQUIRED: I have signed the Contributor Agreement
  • REQUIRED: I have tested my change locally with swift test
  • OPTIONAL: I have tested my change on an iOS simulator or device
  • OPTIONAL: I have tested my change on an Android emulator or device

  • AI was used to generate or assist with generating this PR. Please specify below how you used AI to help you, and what steps you have taken to manually verify the changes.

Cursor assisted me in autogenerating CompatibilityScoreTests.swift, which I then manually tweaked and read very carefully. It also attempted a fix for the tests, but I then rewrote the solution by hand, once I understood what was wrong.

This is my very first attempt to work on the transpiler. I added a lot of tests, but I don't feel confident that I know what I'm doing.

@cla-bot cla-bot bot added the cla-signed label Mar 24, 2026
@dfabulich
Copy link
Copy Markdown
Contributor Author

Flaky CI, but the core tests are passing.

@marcprux marcprux merged commit bb0796e into skiptools:main Apr 4, 2026
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants