Skip to content

Commit

Permalink
Set JDK versions when running the compiler frontend (#1063)
Browse files Browse the repository at this point in the history
* Set JDK versions when running the compiler frontend

This is necessary to load JPMS modules from the JDK.

Unfortunately there's no nice API to fetch the JDK version
from the Gradle KotlinCompile task. It exposes an API to write
this data but not to read it.

* Link to Kotlin YouTrack issue
  • Loading branch information
squarejesse authored Jun 28, 2023
1 parent ed71365 commit af21f4c
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ package app.cash.zipline.api.validator.fir
import java.io.File
import org.jetbrains.kotlin.fir.FirSession
import org.jetbrains.kotlin.fir.analysis.checkers.isSupertypeOf
import org.jetbrains.kotlin.fir.analysis.checkers.toRegularClassSymbol
import org.jetbrains.kotlin.fir.analysis.checkers.toClassLikeSymbol
import org.jetbrains.kotlin.fir.declarations.FirDeclaration
import org.jetbrains.kotlin.fir.declarations.FirFunction
import org.jetbrains.kotlin.fir.declarations.FirProperty
Expand All @@ -41,10 +41,12 @@ import org.jetbrains.kotlin.name.Name
import org.jetbrains.kotlin.types.Variance

fun readFirZiplineApi(
javaHome: File,
jdkRelease: Int,
sources: Collection<File>,
classpath: Collection<File>,
): FirZiplineApi {
return KotlinFirLoader(sources, classpath).use { loader ->
return KotlinFirLoader(javaHome, jdkRelease, sources, classpath).use { loader ->
val output = loader.load("zipline-api-dump")
FirZiplineApiReader(output).read()
}
Expand Down Expand Up @@ -142,15 +144,15 @@ internal class FirZiplineApiReader(

/** See [app.cash.zipline.kotlin.asString]. */
private fun FirTypeRef.asString(): String {
val classSymbol = toRegularClassSymbol(session) ?: error("unexpected class: $this")
val classLikeSymbol = toClassLikeSymbol(session) ?: error("unexpected class: $this")

val typeRef = when (this) {
is FirResolvedTypeRef -> delegatedTypeRef ?: this
else -> this
}

return buildString {
append(classSymbol.classId.asSingleFqName().asString())
append(classLikeSymbol.classId.asSingleFqName().asString())

if (typeRef is FirUserTypeRef) {
val typeArguments = typeRef.qualifier.lastOrNull()?.typeArgumentList?.typeArguments
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import org.jetbrains.kotlin.com.intellij.openapi.vfs.VirtualFileManager
import org.jetbrains.kotlin.com.intellij.psi.search.GlobalSearchScope
import org.jetbrains.kotlin.config.CommonConfigurationKeys
import org.jetbrains.kotlin.config.CompilerConfiguration
import org.jetbrains.kotlin.config.JVMConfigurationKeys
import org.jetbrains.kotlin.diagnostics.DiagnosticReporterFactory
import org.jetbrains.kotlin.fir.pipeline.FirResult
import org.jetbrains.kotlin.metadata.jvm.deserialization.JvmProtoBufUtil
Expand All @@ -50,6 +51,8 @@ import org.jetbrains.kotlin.platform.jvm.JvmPlatforms
* https://github.com/cashapp/redwood/blob/afe1c9f5f95eec3cff46837a4b2749cbaf72af8b/redwood-tooling-schema/src/main/kotlin/app/cash/redwood/tooling/schema/schemaParserFir.kt#L28
*/
internal class KotlinFirLoader(
private val javaHome: File,
private val jdkRelease: Int,
private val sources: Collection<File>,
private val classpath: Collection<File>,
) : AutoCloseable {
Expand All @@ -76,10 +79,10 @@ internal class KotlinFirLoader(
configuration.put(CommonConfigurationKeys.MODULE_NAME, targetName)
configuration.put(CLIConfigurationKeys.MESSAGE_COLLECTOR_KEY, messageCollector)
configuration.put(CommonConfigurationKeys.USE_FIR, true)
configuration.put(JVMConfigurationKeys.JDK_HOME, javaHome)
configuration.put(JVMConfigurationKeys.JDK_RELEASE, jdkRelease)
configuration.addKotlinSourceRoots(sources.map { it.absolutePath })
// TODO Figure out how to add the JDK modules to the classpath. Currently importing the stdlib
// allows a typealias to resolve to a JDK type which doesn't exist and thus breaks analysis.
configuration.addJvmClasspathRoots(classpath.filter { "kotlin-stdlib-" !in it.path })
configuration.addJvmClasspathRoots(classpath.toList())

val environment = KotlinCoreEnvironment.createForProduction(
disposable,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import java.io.File
import org.junit.Test

internal class FirZiplineApiReaderTest {
private val javaHome = File(System.getProperty("java.home"))
private val jdkRelease = Runtime.version().feature()
private val sources = System.getProperty("zipline.internal.sources")
.split(File.pathSeparator)
.map(::File)
Expand All @@ -33,7 +35,7 @@ internal class FirZiplineApiReaderTest {

@Test
fun happyPath() {
val ziplineApi = readFirZiplineApi(sources, classpath)
val ziplineApi = readFirZiplineApi(javaHome, jdkRelease, sources, classpath)
assertThat(ziplineApi).isEqualTo(
FirZiplineApi(
listOf(
Expand All @@ -56,6 +58,14 @@ internal class FirZiplineApiReaderTest {
FirZiplineFunction("var terse: kotlin.Boolean"),
),
),
FirZiplineService(
name = ImportsJdkTypes::class.qualifiedName!!,
functions = listOf(
FirZiplineFunction("fun close(): kotlin.Unit"),
FirZiplineFunction("fun jvmIoException(java.io.IOException): kotlin.String"),
FirZiplineFunction("fun okioIoException(okio.IOException): kotlin.String"),
),
),
FirZiplineService(
name = UnnecessaryEchoService::class.qualifiedName!!,
functions = listOf(
Expand All @@ -72,16 +82,31 @@ internal class FirZiplineApiReaderTest {

/** This should be included in the output. */
interface EchoService : ZiplineService {
fun echo(request: String): String
val greeting: String
var terse: Boolean
fun echo(request: String): String
}

/** This should be included in the output. */
interface ExtendedEchoService : EchoService {
fun echoAll(requests: List<String>): List<String>
}

/** This uses externally-defined types. */
interface ImportsJdkTypes : ZiplineService {
/**
* In this test we can also use a JDK class directly. This isn't the case for production code,
* where `ZiplineService` declarations should be defined in `commonMain`.
*/
fun jvmIoException(e: java.io.IOException): String

/**
* Okio's IOException is a 3rd-party class that's typealiased to a JDK class. In the generated
* TOML file this should use the `okio.IOExeption` name, and not the name it's aliased to.
*/
fun okioIoException(e: okio.IOException): String
}

/** This should be included in the output, but without additional methods. */
interface UnnecessaryEchoService : EchoService {
override fun echo(request: String): String
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import com.github.ajalt.clikt.parameters.options.option
import com.github.ajalt.clikt.parameters.options.required
import com.github.ajalt.clikt.parameters.options.split
import com.github.ajalt.clikt.parameters.types.file
import com.github.ajalt.clikt.parameters.types.int
import com.github.ajalt.clikt.parameters.types.path
import java.io.File
import kotlin.io.path.exists
Expand Down Expand Up @@ -65,6 +66,16 @@ class ValidateZiplineApi(
.required()
.help("Path to the TOML file that this command will read and write")

private val javaHome by option("--java-home")
.path()
.required()
.help("JAVA_HOME to build against")

private val jdkRelease by option("--jdk-release")
.int()
.required()
.help("JDK release version to build against")

private val sources by option("--sources")
.file()
.split(File.pathSeparator)
Expand All @@ -84,7 +95,7 @@ class ValidateZiplineApi(
else -> TomlZiplineApi(listOf())
}

val actualZiplineApi = readFirZiplineApi(sources, classpath)
val actualZiplineApi = readFirZiplineApi(javaHome.toFile(), jdkRelease, sources, classpath)

when (val decision = makeApiCompatibilityDecision(expectedZiplineApi, actualZiplineApi)) {
is ActualApiHasProblems -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@ abstract class ValidateZiplineApiTask @Inject constructor(
@get:OutputFile
abstract val ziplineApiFile: RegularFileProperty

@get:Input
abstract val javaHome: Property<String>

@get:Input
abstract val jdkRelease: Property<Int>

@get:InputFiles
internal val sourcepath = fileCollectionFactory.configurableFiles("sourcepath")

Expand Down Expand Up @@ -76,6 +82,8 @@ abstract class ValidateZiplineApiTask @Inject constructor(
it.mode.set(mode)
it.projectDirectory.set(project.projectDir)
it.tomlFile.set(tomlFileRelative)
it.javaHome.set(File(javaHome.get()))
it.jdkRelease.set(jdkRelease.get())
it.sources.setFrom(sourcepath)
it.classpath.setFrom(classpath)
}
Expand All @@ -92,6 +100,8 @@ private interface ZiplineApiValidatorParameters : WorkParameters {
val mode: Property<Mode>
val projectDirectory: DirectoryProperty
val tomlFile: RegularFileProperty
val javaHome: RegularFileProperty
val jdkRelease: Property<Int>
val sources: ConfigurableFileCollection
val classpath: ConfigurableFileCollection
}
Expand All @@ -118,6 +128,10 @@ private abstract class ZiplineApiValidatorWorker @Inject constructor(
subcommand,
"--toml-file",
parameters.tomlFile.get().asFile.toRelativeString(workingDirectory),
"--java-home",
parameters.javaHome.get().asFile.toString(),
"--jdk-release",
parameters.jdkRelease.get().toString(),
"--sources",
parameters.sources.files.joinToString(File.pathSeparator),
"--class-path",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import org.gradle.api.Task
import org.gradle.api.artifacts.Configuration
import org.gradle.api.provider.Provider
import org.gradle.api.tasks.TaskProvider
import org.gradle.internal.jvm.Jvm
import org.jetbrains.kotlin.gradle.dsl.KotlinMultiplatformExtension
import org.jetbrains.kotlin.gradle.plugin.KotlinCompilation
import org.jetbrains.kotlin.gradle.plugin.KotlinCompilerPluginSupportPlugin
Expand All @@ -31,7 +32,6 @@ import org.jetbrains.kotlin.gradle.targets.js.ir.JsIrBinary
import org.jetbrains.kotlin.gradle.targets.js.ir.KotlinJsIrTarget
import org.jetbrains.kotlin.gradle.targets.js.webpack.KotlinWebpack
import org.jetbrains.kotlin.gradle.tasks.KotlinCompile
import org.jetbrains.kotlin.gradle.tasks.KotlinCompileTool
import org.slf4j.LoggerFactory

@Suppress("unused") // Created reflectively by Gradle.
Expand Down Expand Up @@ -135,7 +135,7 @@ class ZiplinePlugin : KotlinCompilerPluginSupportPlugin {

private fun registerZiplineApiTask(
project: Project,
compileTask: KotlinCompileTool,
compileTask: KotlinCompile,
cliConfiguration: Configuration,
mode: Mode,
rollupTask: TaskProvider<Task>,
Expand All @@ -153,6 +153,18 @@ class ZiplinePlugin : KotlinCompilerPluginSupportPlugin {
task.configure { task ->
task.cliClasspath.from(cliConfiguration)
task.ziplineApiFile.set(project.file("api/zipline-api.toml"))

// TODO: the validation uses the wrong JDK. We should be getting the JDK from the
// KotlinCompile task (as defaultKotlinJavaToolchain.get().buildJvm), but it doesn't
// make that available for querying. Hack it to use Gradle's 'current' JVM.
// https://youtrack.jetbrains.com/issue/KT-59735
val buildJvm = Jvm.current()
task.javaHome.set(buildJvm.javaHome.path)
task.jdkRelease.set(
buildJvm.javaVersion?.getMajorVersion()?.toInt()
?: Runtime.version().feature(),
)

task.sourcepath.setFrom(compileTask.sources)
task.classpath.setFrom(compileTask.libraries)
}
Expand Down

0 comments on commit af21f4c

Please sign in to comment.