Kotlin IntelliJ plugin crashes on StatefulWorkflow and action builder

[zach-klippenstein]

There is a bug in the plugin that crashes the parser when it encounters action. @kirillzh filed JetBrains an issue about it.

I've created a minimal Android Studio project that demonstrates this bug: https://github.com/zach-klippenstein/repro-KT-34524

Open the project in Android Studio and open the file app/src/main/java/com/example/kt_34524repro/SampleWorkflow.kt to see the bug in action.

There are a few other issues that look to be related or duplicates:

• https://youtrack.jetbrains.com/issue/KT-32042
• https://youtrack.jetbrains.com/issue/KT-34759

[zach-klippenstein]

Verified this still occurs with plugin version 1.3.70-eap-274-Studio4.0-1.

[zach-klippenstein]

Posted about this issue in the Kotlin Slack's #compiler channel.

[zach-klippenstein]

We just heard from a contact at JetBrains that they're aware of this issue and considering how to prioritize it. There's maybe even a small chance it could be fixed in 1.3.70, but no guarantees.

[zach-klippenstein]

A related issue, https://youtrack.jetbrains.com/issue/KT-34759, has been marked as Fixed.

Our issue, https://youtrack.jetbrains.com/issue/KT-34524, is still In Progress.

[zach-klippenstein]

Our issue (https://youtrack.jetbrains.com/issue/KT-34524) has had its Target versions updated to 1.4-M2. Unfortunately, this means we won't see a fix probably at least for a couple months. However, it does mean that it's likely being actively worked on and so there's not much point in us trying to solve it ourselves.

[rjrjr]

After an offline conversation w/Zach, going to see if changing RenderContext to an inner class of StatefulWorkflow improves things any.

[rjrjr]

One of the differences between Mutator and Updater is that the latter made OutputT a function argument instead of a return value. To make that work we had to change the OutputT on RenderContext to an in type, while it's an out type on Workflow.

Given the timing of this bug, I'll bet you anything that this mismatch is related.

[rjrjr]

I'm playing with dropping in and out from the OutputT declarations. But wouldn't it be funny if the fix were as simple as changing the name of the type param on RenderContext from OutputT to O?

[rjrjr]

Dropping in and out didn't change anything.

[rjrjr]

Wellllll…

This change does indeed fix the IDE. There are issues.

• Kotlin provides no way to subclass an abstract inner class (https://youtrack.jetbrains.com/issue/KT-9860?_ga=2.150270710.1358334955.1584993739-1879603116.1549995965), meaning that RealRenderTester would have to be implemented in a different way. Or it would have to be defined inside of StatefulWorkflow itself.

• StatefulWorkflow classes get a bit more pleasant — no need to declare the param types on the context argument of render(). But StatelessWorkflow gets worse:

    override fun render(
        props: Unit,
        context: StatefulWorkflow<Unit, Unit, Nothing, Unit>.RenderContext
    ) {

• I think I saw a couple of spots where inline action {} breaks down with Nothing output type, not sure.

[rjrjr]

Looking at our internal stats, StatelessWorkflow is pretty popular, accounts for about a quarter of all Workflow implementations.

[zach-klippenstein]

But StatelessWorkflow gets worse:

If we go this route, I think we should just define a different context type for StatelessWorkflow. Then we can drop that weird Nothing type parameter altogether.

[rjrjr]

That makes a lot of sense. Winds up being a net usability win.

[rjrjr]

But we'll wind up needing two implementations of RenderTester. Yuck.

Maybe there's a base context type they can share, and then the inner classes are little facades with the two workflow base classes.

abstract class AbstractRenderContext<P, S, O> { ... }

abstract class StatefulWorkflow<P, S, O, R>() : Workflow<P, O , R> {
  inner class RenderContext : AbstractRenderContext<P, S, O> { ... }
}

class StatefulRenderTester <P, S, O, R>: AbstractRenderContext<P, S, O> { }

abstract class StatelessWorkflow<P, O, R>() : Workflow<P, Unit, O , R> {
  inner class RenderContext : AbstractRenderContext<P, Unit, Nothing> { ... }
}

class StatelessRenderTester <P, O, R>: AbstractRenderContext<P, Unit, O> { }

[rjrjr]

That's not right, but you get the idea.

[zach-klippenstein]

Yea, something like that. I'm not too worried about exploding the number of RenderContext types because so far all the abstract *Workflow classes I've written/thought about writing, that aren't StatefulWorkflow/StatelessWorkflow, don't actually need a RenderContext at all.

[zach-klippenstein]

Good news, upgrading Android Studio to Kotlin plugin 1.4-M1 seems to fix the issue in our repro project (https://github.com/zach-klippenstein/repro-KT-34524). The kotlin file is parsed, the action {} builder is highlighted as missing and offered to be imported, and the uncommented prose is highlighted as invalid syntax. Haven't tested with our big internal codebase yet, but this is a good step in the right direction!

[zach-klippenstein]

1.4-M1 didn't fix for everyone. But latest comms from Jetbrains is that it's been fixed in M3. I'll mark as blocked until then and leave it open, but there's no action items on our part. I will remove from the 1.0.0 milestone as well.

[zach-klippenstein]

One report that plugin 1.4-M3 has fixed this issue in AS 4.0 (internal message).