-
Notifications
You must be signed in to change notification settings - Fork 409
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Decompose Kotlin/Java analysis #3034
Conversation
a65f23c
to
333dc06
Compare
0a8fc22
to
9816012
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...analysis-markdown-jb/src/main/kotlin/org/jetbrains/dokka/analysis/markdown/jb/MarkdownApi.kt
Outdated
Show resolved
Hide resolved
...in/kotlin/org/jetbrains/dokka/analysis/kotlin/descriptors/ide/IdeDescriptorAnalysisPlugin.kt
Outdated
Show resolved
Hide resolved
...ain/kotlin/org/jetbrains/dokka/analysis/kotlin/descriptors/compiler/java/KotlinDocComment.kt
Outdated
Show resolved
Hide resolved
...ain/kotlin/org/jetbrains/dokka/analysis/kotlin/descriptors/compiler/java/KotlinDocComment.kt
Outdated
Show resolved
Hide resolved
...brains/dokka/analysis/kotlin/descriptors/compiler/java/KotlinAnalysisSourceRootsExtractor.kt
Show resolved
Hide resolved
9816012
to
97b1715
Compare
Added a detailed description for the PR, as well as addressed review comments, both left here as comments and those discussed in person. |
The PR will be merged into master prematurely to avoid having an enormous long-lived branch just collecting merge conflicts, so it's not perfect to say the least, and some of the code needs to be revisited in the near future. Created an issue to keep track of such work: #3058 |
This refactoring unblocks #2888 and #2889
Dokka uses a number of intellij / kotlin-compiler / kotlin-ide-plugin dependencies for source code analysis. However, they were added and used in a bit of a chaotic way, with some external compiler types even leaking into public API. This all led to increasing problems with maintainability when it came to migrating to the newer versions of the Kotlin compiler and the IntelliJ platform specifically.
Moreover, because the use of these dependencies was intertwined, it was not possible to say with confidence which dependency was used for what exactly, and it made it difficult to reason about the scope of work in that area. For instance, we couldn't say for sure which parts exactly would be affected by the migration to the K2 compiler, and whether anything would break for downstream clients (like Google's plugin).
This PR aims to address the higher level issues by separating the analysis into several big pieces, as well as allows us to provide two analysis implementations - one based on K1 and the other based on K2.
High level implementation
Kotlin Analysis API
A simple module with public API that only depends on the
core
module. Should contain interfaces / simple classes only, no complicated logic.This module must contain all of the public API that has anything to do with analyzing Kotlin sources. The implementation of the Kotlin Analysis API is internal and not exposed to the user. The API and module itself is implementation agnostic.
The implementations for the API interfaces will be injected during runtime, with the ability to switch between different implementations (K1, K2).
This allows us to clearly separate the public API from the internal implementation, and allows us to gradually migrate users from the K1 to the K2 compiler simply by changing a runtime dependency.
Kotlin Analysis Descriptors
Provides K1 (descriptor-based) implementations for the Kotlin Analysis API.
The module is internal and is not meant to be used / depended upon directly.
Kotlin Analysis Symbols
Provides K2 (symbol-based) implementations for the Kotlin Analysis API (#2995).
The module is internal and is not meant to be used / depended upon directly.
Java Analysis PSI
An internal module that encapsulates the analysis of Java source code, including mixed-language scenarios (i.e a Kotlin class that extends a Java class or visa versa).
The module is independent from the Kotlin analysis implementation, but it declares a number of interfaces for mixed-language scenarios, the implementations for which should be provided during runtime.
Lower level details
The code is very messy as the primary goal was to decompose the all-in-one analysis as much as possible, not to make it beautiful. To minimize bugs, the original behaviour was preserved where possible, even if it was ugly / incorrect / difficult to understand.
We plan to have a series of PRs addressing local problems and specific areas of code, starting with the internal refactoring and the documentation for the Java Analysis module.