Run Detekt as Kotlin Compiler Plugin

[tyvsmith]

To improve the feedback cycle of errors to the developer and the possible performance when doing type resolution, Detekt could be implemented to run as a Kotlin compiler plugin. This would bring the behavior closer to a tool like ErrorProne.

We're investigating this internally at Uber to see if we can get any wins out of this approach.

A naive approach/prototype would be to use the AnalysisHandlerExtension, like the JVM ABI plugin and visit the KtFiles.

A more interested approach, would be to figure out wiring for the existing checkers based on CallChecker.

[arturbosch]

Thanks for reporting interest in this approach and posting the links.
I've done some experiments with the compiler api 2 years ago which did not go far.

[schalkms]

+1
I would also be interested in the advantages and disadvantages of this approach.
Could you please try to outline your findings and the current state of your experiment? @tyvsmith
We are very curious about that.
A Control flow checker would also be a nice addition. Some existing rules would already benefit from that.

[3flex]

I had a proof of concept of the "naive" approach working some time ago.

The main problems I saw:

• it's not an officially supported API
• AnalysisHandlerExtension only supports JVM last I checked - we'd ideally want support for other targets
• doesn't seem possible to use the embedded Kotlin compiler (but switching to regular compiler caused issues with classpath on Gradle if I recall correctly)

But it's a huge win in terms of performance because detekt can use the result of Kotlin compilation directly instead of effectively recompiling (and it also means detekt would use the exact same compiler configuration as when the Kotlin code is compiled). And the classpath conflicts for Gradle should go away if committing to this direction as detekt wouldn't need to be on the Gradle plugin classpath anymore.

I can't see how CallChecker would help with performance? But if it's a viable option without the pitfalls of the naive approach it could be a good direction.

[arturbosch]

I've prototyped an approach based on the AnalysisHandlerExtension with great results: https://github.com/detekt/detekt-compiler-plugin.

Detekt findings next to kotlinc findings

[3flex]

Nice! That's a good start. To get the most advantage though we'd want to be able to run detekt during the project compilation instead of as a separate task - that's what I meant when I said

doesn't seem possible to use the embedded Kotlin compiler

Because KotlinCompile Gradle tasks ultimately use the normal compiler, not embedded compiler, and to get the best speedup we want to use the type resolution result generated by the compile task itself so that work is only done once. Running detekt as a compiler plugin but having that compilation work happen twice probably won't be much faster than the current setup. Let me know if that doesn't make sense?

[tyvsmith]

Great job! Is the runtime much different using that approach or just a stepping stone to using it during the compilation stage?

[arturbosch]

Nice! That's a good start. To get the most advantage though we'd want to be able to run detekt during the project compilation instead of as a separate task - that's what I meant when I said

doesn't seem possible to use the embedded Kotlin compiler

Because KotlinCompile Gradle tasks ultimately use the normal compiler, not embedded compiler, and to get the best speedup we want to use the type resolution result generated by the compile task itself so that work is only done once. Running detekt as a compiler plugin but having that compilation work happen twice probably won't be much faster than the current setup. Let me know if that doesn't make sense?

I'm not sure if I understand you 100%. We are running inside the KotlinCompile task.
We declare a Gradle Plugin which declares a KotlinGradleSubPlugin which makes detekt run as a compiler plugin in KotlinCompile task. Therefore we have the best performance and a non empty BindingContext.
Will test this further soon how we can migrate our DetektExtension/flags.

Great job! Is the runtime much different using that approach or just a stepping stone to using it during the compilation stage?

It feels much faster. Also the KotlinCompile task is incremental and therefore we also only run on changed files.

[3flex]

I'll need to debug to see what's actually happening with the plugin, but from what I can see in the POC you're providing org.jetbrains.kotlin.com.intellij.openapi.project.Project (compiler-embeddable version) to the AnalysisHandlerExtension instead of com.intellij.openapi.project.Project, so I assume the analysis handler extension being used by the plugin is not the same as the one that's used by the compiler itself, therefore BindingContext is generated twice, once for compilation (using compiler) and again for detekt (using compiler-embeddable).

Yes it's all happening in the KotlinCompile Gradle task, and it's quicker than running detekt separately because there are fewer overheads, but in general the slowest part will still be creating BindingContext, so if it's happening twice then it's less of an improvement than it might seem, though any speedup is welcome.

As I said I haven't run this yet though so apologies if that's not the case, but I ran into similar issues when experimenting with this so wanted to flag it.

[arturbosch]

Closing this in favor of the new repository https://github.com/detekt/detekt-compiler-plugin.