Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[K2] Compilation error on overridden declarations #188

Open
dmitry-stakhov opened this issue Aug 27, 2024 · 2 comments
Open

[K2] Compilation error on overridden declarations #188

dmitry-stakhov opened this issue Aug 27, 2024 · 2 comments

Comments

@dmitry-stakhov
Copy link

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") }
}
  1. 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.

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
Copy link
Owner

rickclephas commented Aug 27, 2024

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
Copy link
Author

dmitry-stakhov commented Aug 28, 2024

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.

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

No branches or pull requests

2 participants