rickclephas / KMP-NativeCoroutines

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

Properly support overrides of annotated declarations #160

Closed ygnessin closed 7 months ago

ygnessin commented 8 months ago

My project uses this library alongside MocKMP. MocKMP uses KSP to generate mock implementations of interfaces for unit tests based on annotations.

Up until 1.0.0-ALPHA-15, these 2 libraries worked well together, but starting with 1.0.0-ALPHA-16 the following issue started to occur.

My interface:

public interface IExample {
    @NativeCoroutines
    public suspend fun invoke(): Unit
}

My unit test, using MockKMP:

@UsesMocks(IExample::class)
class ExampleTestCase : TestsWithMocks() {
    override fun setUpMocks() = injectMocks(mocker)

    @Mock
    lateinit var example: IExample

    // ... tests
}

KSP-generated MockKMP code:

internal class MockIExample(
  private val mocker: Mocker,
) : IExample {
  @NativeCoroutines
  public override suspend fun invoke(): Unit = this.mocker.registerSuspend(this, "invoke()")

  public override fun toString(): String = this.mocker.register(this, "toString()", default = {
      super.toString() })
}

When compiling my tests, I get this error: e: file:///<redacted>/MockIExample.kt:11:3 NativeCoroutines is only supported on public declarations

This is despite the fact that as you can see in the generated code above, invoke() is clearly declared a public override suspend fun. Line 11 points to the @NativeCoroutines annotation.

My suspicion is that the problem has to do with the order in which the KSP processors of MocKMP and KMP-NativeCoroutines are run. But I truly don't know enough about this area to debug further.

Please let me know what other information I can provide. I would also be willing to contribute a fix if you can point me in the right direction.

Thank you for your help, and thank you for the wonderful library!

rickclephas commented 8 months ago

Thanks for reporting this use case.

The invoke function is indeed public, but since MockIExample is internal, the function effectively becomes internal as well.

However, the real issue here is in how KMP-NativeCoroutines handles subclasses and overrides. At the moment, the annotations aren't supported on override declarations (because we only want to generate code for the base declaration).

To solve this, the annotations should be allowed on overrides as long as they match with the annotations on the base declaration.

ygnessin commented 8 months ago

Thanks for the quick response!

Please let me know if there's anything at all I can do to help with this.

zacsst commented 8 months ago

Hey there, 



New to this library, and love it. I think my question is related to this issue so posting here. Please let me know if I should open something separate.

I’m moving some code from my iOS app written in swift to KMM to stand up an android version of the app. Most of the code I’m moving over handles business logic and storing data. I’ll get to view model stuff later.

I have two protocols I’m moving over right now, a UserRepository and a UserMetadataService. They each have a Firebase… implementation.

In the FirebaseUserRepository implementation, it listens to auth state changes in Firebase.auth, and fetches user metadata through FirestoreUserMetadataService.

The issue I’m running into is when I can’t depend on the interface in my swift code, it seems none of the functions are generated. I’ll paste what I mean below. Would love to know if there's a solution for this! I'd really like to keep using interfaces for the sake of unit testing.

image image image

image image

rickclephas commented 8 months ago

@zacsst you are actually seeing an unrelated limitation in the Kotlin-ObjC interop regarding extensions. You can either call the extension directly or create a nice extension on the Swift side.

zacsst commented 8 months ago

@rickclephas thanks for the reply. Any suggestions for unit testing classes that depend on the nice extension?

I've tried having another extension in my test code with the same name, that calls a test specific function that can returns whatever I preset, but the original extension is always called.

rickclephas commented 8 months ago

@zacsst I am not really sure what you are trying to accomplish with the unit tests. Could you please open a new issue with some code samples?

rickclephas commented 7 months ago

Starting with v1.0.0-ALPHA-27 annotations are allowed on both override and actual declarations. Additional checks have been added to make sure that the base and expect declarations are annotated with the same annotation. Note: KMP-NativeCoroutines is only generating code for the base / expect declarations.