sellmair / kompass

Kotlin Multiplatform Router for Android and iOS
MIT License
345 stars 12 forks source link

Can't use objects as destination #46

Closed janheinrichmerker closed 5 years ago

janheinrichmerker commented 5 years ago

The annotation processor fails to generate the serialization methods for object destinations.

Suppose the following destination:

@Destination
object HomeDestination: PodDestination()

Kompass generates the following method:

fun Kompass.Companion.bundleAsHomeDestination(bundle: Bundle): HomeDestination = HomeDestination()

This calls HomeDestination() but should instead call HomeDestination.

janheinrichmerker commented 5 years ago

BTW Very nice library! I like the architecture (and the ship analogy)! Would love to contribute more and I'll see if I can create some PRs.

sellmair commented 5 years ago

Hey @heinrichreimer, I am currently ill but very open for contributions. If you like: Take this issue and I will help you out solving it? ☺️

sellmair commented 5 years ago

The code that generates this code is here: io/sellmair/kompass/compiler/destination/visitor/KompassCompanionBundleAsDestinationVisitor.kt Line 60

    private fun FunSpec.Builder.buildImplementationHeader(tree: DestinationRenderTree) {
        addCode("return ${tree.element.simpleName}(")
    }

You can see, that I am always assuming that I can call the constructor here. Therefore I am wrongly opening the '(' bracket

janheinrichmerker commented 5 years ago

@sellmair Sure, I'll give it a try. Hope, you get well soon!

janheinrichmerker commented 5 years ago

As I understand your code, we could keep the original implementation for classes (just renaming buildImplementation to buildImplementationForClass) and then propose a new function:

private fun FunSpec.Builder.buildImplementationForObject(tree: DestinationRenderTree) {
    addCode("return ${tree.element.simpleName}")
}

The only thing left would be replacing the implementation of buildImplementation:

private fun FunSpec.Builder.buildImplementation(tree: DestinationRenderTree) {
    val isObject: Boolean = ...
    if (isObject) {
        buildImplementationForObject(tree)
    } else {
        buildImplementationForClass(tree)
    }
}

But... I have found no efficient way for guessing if the tree.element which in turn is an Element is an object or not. Given we have a KClass of it, it would be as easy as checking whether elementClass.objectInstance != null. However, the only way I found to get that, would be the following, which - to be honest - doesn't look very clean:

val elementClass = Class.forName(tree.element.qualifiedName.toString()).kotlin

I haven't tested my thoughts, but maybe you have another idea on how to tackle that issue?

sellmair commented 5 years ago

I like the splitting of the function in 'buildImplementationForClass' and 'buildImplementationForObject' 👍

In order to check if the element is an object or not: Isn't there a synthetic annotation placed for us during stubs generation? Maybe we could ask for that annotation to determine this? This works e.g. for nullability ☺️

sellmair commented 5 years ago

Closed with prototype 0.2.0 #49