Skip to content
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

Merged
merged 3 commits into from Jul 5, 2023
Merged

Decompose Kotlin/Java analysis #3034

merged 3 commits into from Jul 5, 2023

Conversation

IgnatBeresnev
Copy link
Member

@IgnatBeresnev IgnatBeresnev commented Jun 6, 2023

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.

@qwwdfsad qwwdfsad requested a review from vmishenev June 17, 2023 18:50
@IgnatBeresnev IgnatBeresnev marked this pull request as ready for review June 18, 2023 21:49
Copy link
Member

@vmishenev vmishenev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have reviewed only the general concept and core API.

Also, could you regroup files in analysis-java-psi module.
image
(For example, 2 parsers are out of parsers package. Classes DocComment* can be placed in a separate package.)

gradle/libs.versions.toml Outdated Show resolved Hide resolved
@IgnatBeresnev
Copy link
Member Author

Added a detailed description for the PR, as well as addressed review comments, both left here as comments and those discussed in person.

@IgnatBeresnev
Copy link
Member Author

IgnatBeresnev commented Jul 4, 2023

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

@IgnatBeresnev IgnatBeresnev merged commit 9559158 into master Jul 5, 2023
12 checks passed
This was referenced Jul 26, 2023
@vmishenev vmishenev mentioned this pull request Aug 24, 2023
IgnatBeresnev pushed a commit that referenced this pull request Sep 5, 2023
#3034 broke the fix for #1599 by changing packages
@IgnatBeresnev IgnatBeresnev deleted the analysis-refactoring branch October 10, 2023 14:29
@IgnatBeresnev IgnatBeresnev added the topic: K2 Issues / PRs that are related to the K2 migration. See #2888 label Oct 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: K2 Issues / PRs that are related to the K2 migration. See #2888
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants