rickclephas / KMP-NativeCoroutines

Library to use Kotlin Coroutines from Swift code in KMP apps
MIT License
1.03k stars 31 forks source link

[K2] Compilation error on overridden declarations #188

Open dmitry-stakhov opened 2 weeks ago

dmitry-stakhov commented 2 weeks ago

Steps: 1 . Enable k2Mode in Gradle plugin.

nativeCoroutines {
    suffix = "Platform"
    k2Mode = true
}
  1. Add the next code:
    
    import com.rickclephas.kmp.nativecoroutines.NativeCoroutines

interface Base1 { @NativeCoroutines suspend fun foo() }

interface Base2 { @NativeCoroutines suspend fun foo() }

class Derived1: Base1 { @NativeCoroutines override suspend fun foo() { println("Derived1") } }

class Derived2: Base2, Base1 by Derived1() { @NativeCoroutines override suspend fun foo() { println("Derived2") } }


3. Run `./gradlew podPublishDebugXCFramework` 

Expected: compilation successful

Actual: compilation error

Cannot infer visibility for 'fooPlatform'. Please specify it explicitly. Class 'Derived2' must override 'fooPlatform' because it inherits multiple interface methods for it.


Note:
If we want to generate Native coroutine overload only to one interface, we get an another error.
```kotlin
interface Base1 {
    @NativeCoroutines
    suspend fun foo()
}

interface Base2 {
    suspend fun foo()
}

class Derived1: Base1 {
    @NativeCoroutines
    override suspend fun foo() {}
}

class Derived2: Base2, Base1 by Derived1() {
    override suspend fun foo() {}
}

Error:

Refined declaration 'suspend fun foo(): Unit' overrides declarations with different or no refinement from:
interface Base2 : Any
rickclephas commented 2 weeks ago

Thanks for testing the K2 mode and reporting this!

Actual: compilation error

Cannot infer visibility for 'fooPlatform'. Please specify it explicitly.
Class 'Derived2' must override 'fooPlatform' because it inherits multiple interface methods for it.

This is an interesting use case. It indeed doesn't work at the moment. And unfortunately since the generated declarations are final you can't even override them manually. (depending on the source-set even impossible, since the generated declarations are only available on Apple targets)

Could you possibly share some more details on this use case? Why do you need/have two different interfaces with the same function declaration? What prevents you from sharing that declaration through another interface?

Note: If we want to generate Native coroutine overload only to one interface, we get an another error.

Refined declaration 'suspend fun foo(): Unit' overrides declarations with different or no refinement from:
interface Base2 : Any

That's actually an expected error from the Kotlin compiler. @NativeCoroutines inherits the behaviour from @HiddenFromObjC which is required to be applied on all overridden declarations.

dmitry-stakhov commented 2 weeks ago

Why do you need/have two different interfaces with the same function declaration?

Historically we have the interface Api to request the data from the backend. This interface is implemented with some class ApiImpl that overrides all api suspend methods. Since recently the backend team has started published their api as OpenApi yaml conventions, so we decided to integrate the Api generation utilizing KSP. OpenApi generator produces the new interface ApiGen and ApiImplGen class similar to handwritten ones. The handwritten and generated Api are similar, but not identical. Api (legacy) contains api calls to old api that are not posted in OpenApi spec anymore, at the same time the generated Api has the new methods not implemented in legacy Api interface. This means we need to combine them / use both in the app to have access to all api. Hence to avoid passing different instances of handwritten and generated Api implementations we merge them by using the next structure:

class ApiImpl : Api, ApiGen by ApiImplGen() {
 // override Api methods
}

Answering your question:

  1. We have some methods overlap for Api and ApiGen due to the nature of these interfaces.
  2. We cannot remove existing in Api method from ApiGen because we want to migrate to it.
  3. We technically can remove existing in ApiGen methods from legacy Api, but it will take time to migrate all methods usage to ApiGen.

This is why currently we have 2 options:

  1. wait the native coroutines generation fix and switch to K2 mode now.
  2. remove all colliding methods from Api and migrate to ApiGen methods in the app (will take some time because of testing) -> switch to K2 mode after this.