touchlab / SKIE

SKIE - Swift Kotlin Interface Enhancer
https://skie.touchlab.co/
Apache License 2.0
709 stars 8 forks source link

@FlowInterop.Disabled does not work in the constructor #97

Open X1nto opened 1 month ago

X1nto commented 1 month ago

Questions:

What is the problem?

Applying @FlowInterop.Disabled to a constructor property still generates initializers with SkieSwiftFlow.

When does the problem occur?

During Swift compilation(?) The initializer in the generated header does use Kotlinx_coroutines_coreFlow, but Xcode fails to see coreFlow and still treats it as SkieFlow

image image

How do we reproduce the issue?

Add this class to the iosMain source set of the shared module:

class Cockroach<T: Any>(
    @FlowInterop.Disabled
    private val flow: Flow<T>
)

Try using the class from Swift:

let cockroach = Cockroach(flow: ) //This flow initializer should accept Kotlinx_coroutines_coreFlow, but it's SkieSwiftFlow.
image

What has changed since the last time SKIE worked in your project?

If only I knew.

What versions of SKIE, Kotlin, and Gradle do you use?

Skie 0.8.3 Kotlin 2.0.0 Gradle 8.9 Xcode 15.4

What is your SKIE Gradle configuration?

Default configuration, no skie {} block.

There must be something that I'm missing. I've been going crazy these past 7 hours trying to fix this problem. The generated header being correct is what makes me actually mad, since it's probably some obscure Xcode bug that no one has ever encountered.

TadeasKriz commented 1 month ago

Hi @X1nto, thanks for the report. The header always shows Kotlinx_coroutines_coreFlow, because SKIE uses the .apinotes file to override the signature. If you look there you should see SkieKotlinFlow. As to why the annotation doesn't work, that's a good question. Could you try putting it on the constructor itself instead of the property?

class Cockroach<T: Any> @FlowInterop.Disabled constructor(
    private val flow: Flow<T>
)

Let me know if that changes anything.

X1nto commented 1 month ago

My first thought was to put the annotation on the constructor, but the targets for the annotation are functions and properties.

image image
FilipDolnik commented 1 month ago

Hi! The annotation doesn't work because it targets the property, not the value parameter. Skie currently doesn't support Flow configuration for individual value parameters - it's a limitation of our current configuration framework.

As for why the annotation does not support constructors: I don't think it's intentional. We likely forgot to include the constructor target in there, so this is a "bug".

X1nto commented 1 month ago

Do you accept PRs? I'll try and see if I can fix it

FilipDolnik commented 1 month ago

Theoretically, yes, but we need to add tests as well, and those are not public, so a PR wouldn't save us any work in this case, unfortunately.