[K2] Run unit tests against the latest Analysis API

[vmishenev]

To run the unit tests against the latest dev-version of Analysis API, the property flag
org.jetbrains.dokka.build.useTheLatestAnalysisAPI=true should be added to the project.

Later I will configure CI (probably TC) to run the test task with this flag by schedule (twice a week?).

[vmishenev]

Another implementation could be:
Extract the version pattern from the property directly (org.jetbrains.dokka.build.useTheLatestAnalysisAPI=2.0.20-dev-+) rather than from the toml file. It can be more convenient to change the pattern.

[whyoleg]

I think, that it will better to override version directly in the version catalog instead of via configurations as it will be more idiomatic.
F.e like described in this issue: gradle/gradle#20070

So in root settings.gradle.kts we should have something like:

dependencyResolutionManagement {
    versionCatalogs {
        create("libs") {
            val kotlinCompilerK2Version = providers.gradleProperty(
                "org.jetbrains.dokka.build.kotlin-compiler-k2-version"
            ).orNull
            if (kotlinCompilerK2Version != null) {
                version("kotlin-compiler-k2", kotlinCompilerK2Version)
            }
        }
    }
}

(property name is TBD, just an example)
And then, no additional configuration is needed in a project.

It's possible to just have a boolean flag, but I believe that it will be better if we provide this version on CI (as you suggested in the comment above) as it will allow us easier to update to newer version, by just changing property on TeamCity instead of changing it in the repository.

[IgnatBeresnev]

I thought about this for some time.

I personally like both approaches (the current one and what Oleg proposed), both have advantages and disadvantages. Oleg's proposal seems to cover some additional use cases though: it will be possible to run the tests with any custom version of kotlin-compiler-k2, not just the hardcoded and latest -dev one. This might come in useful when debugging or trying to narrow down the version in which a bug appeared (i.e test 2.0.20-dev-10, then 2.0.20-dev-1, then 2.0.20-dev-5, etc) without having to change the project's code - all done in CI

The proposed approach does have one disadvantage in my opinion: it's less obvious what is happening, we're overriding but a single specific version in the very common and unexpected place. In Vadim's approach the resolution logic is mostly isolated in analysis-kotlin-symbols, which is nice and easy to understand. But hopefully Gradle will provide some sort of a solution in gradle/gradle#20070, like -Pgradle.libs.kotlin-compiler-k2="2.0.20-dev-+", and this disadvantage won't be relevant anymore, and the resolution code will actually be easy to get rid of.

[IgnatBeresnev]

Oh, sorry, missed @vmishenev's comment above

Extract the version pattern from the property directly (org.jetbrains.dokka.build.useTheLatestAnalysisAPI=2.0.20-dev-+) rather than from the toml file. It can be more convenient to change the pattern

I guess I would mostly vote for passing the version as a property (instead of the boolean flag), not necessarily as Oleg proposed :)

[whyoleg]

The proposed approach does have one disadvantage in my opinion: it's less obvious what is happening, we're overriding but a single specific version in the very common and unexpected place.

TBH, for me, the variant with configurations in this PR is less obvious, because configurations are like for advanced usage and most of the time there is a solution which doesn't involve them and it's easier and more straightforward.

F.e with logic implemented in current PR we override all dependencies (not only which use kotlin-compiler-k2 version) to newer version. F.e in case we will have kotlin-compiler-k2=2.1.0 and kotlin-compiler-k2-override-version=2.2.0-dev-* and some dependency with version 2.1.0 we will update this dependency version and so receive a build failure. May be it will not happen, but when something like this happens, the issue is not obvious. It could be prevented of course, by checking group/module names of dependency.

Instead when we dynamically modify version inside Version Catalog we are explicitly changing only one version and so only those dependencies which use it via Gradle-native way.
Additionally, what if we would like to add one more property to configure testing with newer version of kotlin-compiler for descriptors (to check, that there is no regressions in analysis) or any other thing - I would expect to see all such version overrides in one place, not in separate projects.

While for current use-case both variants are fine, IMO we should use version catalogs native solution instead of configuration based.

[vmishenev]

with logic implemented in current PR we override all dependencies

Yes, I agree. It can theoretically lead to false-positive builds.

I would expect to see all such version overrides in one place, not in separate projects.

By the way, I see the request on some local DSL to override a version https://github.com/gradle/gradle/issues/26029
since e.g., another implementation is to override a version only for the test configuration. (but does not work for our integration tests)

I think we can try https://docs.gradle.org/current/userguide/platforms.html#sec:overwriting-catalog-versions

[adam-enko]

I agree that it would be nicer to have the version defined in one place, and to allow it to be overridden with a specific version. Using the version catalog has some benefits (it's nice to have a version defined in one place, and it should be possible to override a version via a runtime property).

However, Gradle dependency management is tricky...

A version catalog doesn't always control the version. Example: if a project has a direct dependency on x.y.z:foo:1.0.0, and some other dependency has a transient dependency on x.y.z:foo:2.0.0, then Gradle will select the highest version.

So, while version catalogs are convenient, to control the version of dependencies we'd need to use constraints.

This is what I'd propose:

1. In DokkaBuild, add a val kotlinAnalysisVersionOverride: Provider<String>.

2. And then, in the project, define constraints to control the versions. This can either use the overridden version (if present), or the default value in the version catalog.

   // dokka-subprojects/analysis-kotlin-symbols/build.gradle.kts
   
   
   dependencies {
       constraints {
           listOf(
               libs.kotlin.high.level.api.api,
               libs.kotlin.high.level.api.impl,
               libs.kotlin.high.level.api.fir,
               // ...
           ).forEach {
               implementation(it) {
                   version {
                       strictly(
                           // find the optional override,
                           dokkaBuild.kotlinAnalysisVersionOverride
                               // otherwise, use the default version
                               .orElse(libs.versions.kotlin.compiler.k2)
                               .get()
                       )
                   }
               }
           }
       }
   }

For Dokka currently, the analysis dependencies are shadowed into the project's artifact, so I don't think that aligning the versions across multiple projects is required. If it is, and the constraints should be shared across subprojects, then the solution is to use the Java Platform Plugin to share the constraints.

[vmishenev]

That is interesting. Thank you for participating.
It seems overly strict.

A version catalog doesn't always control the version. Example: if a project has a direct dependency on x.y.z:foo:1.0.0, and some other dependency has a transient dependency on x.y.z:foo:2.0.0, then Gradle will select the highest version.

I thought we should have an existing case to introduce a version constraint.
Currently, I can not imagine any case with Analysis API surface that breaks using the API.

@whyoleg Do you have an opinion on what approach we should implement?

[whyoleg]

hm, I think that constraints is the direct replacement for what is proposed in current PR (using configurations), but with nicer API and more strict behaviour.
But, I'm not sure, that we will fall in the situation when for some reason, we would have transitive dependency on Analysis API of higher version :)
So we have 2 options:

• using constraints
• overriding version in version catalog

I would say, that the main difference for our use case is locality of the change:

• with constraints - we override version for single module
• with version catalog - we override version in whole project

Both variants are fine for me. Still I'm more in favour of changing the version via version catalog. No specific arguments for this, mostly it just fills more clear and natural for me: if we want to override version, we override it in version catalog.
BTW, @atyrin setup to test Dokka over newer Kotlin versions also overrides version via version catalog, so setup will be similar and so more understandable (IMO).

On additional note: I think we agreed that we will provide newer dev version via property (not just boolean flag) and so it would be good to add to the Version Catalog near kotlin-compiler-k2 that this version could be overridden by Gradle property.

[IgnatBeresnev]

I thought about this for some time.

I personally like both approaches (the current one and what Oleg proposed), both have advantages and disadvantages. Oleg's proposal seems to cover some additional use cases though: it will be possible to run the tests with any custom version of kotlin-compiler-k2, not just the hardcoded and latest -dev one. This might come in useful when debugging or trying to narrow down the version in which a bug appeared (i.e test 2.0.20-dev-10, then 2.0.20-dev-1, then 2.0.20-dev-5, etc) without having to change the project's code - all done in CI

The proposed approach does have one disadvantage in my opinion: it's less obvious what is happening, we're overriding but a single specific version in the very common and unexpected place. In Vadim's approach the resolution logic is mostly isolated in analysis-kotlin-symbols, which is nice and easy to understand. But hopefully Gradle will provide some sort of a solution in gradle/gradle#20070, like -Pgradle.libs.kotlin-compiler-k2="2.0.20-dev-+", and this disadvantage won't be relevant anymore, and the resolution code will actually be easy to get rid of.