From 9440e5c7d0b72eea05d8c5c2868f3dbf6377d129 Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Mon, 15 Nov 2021 11:25:15 -0500 Subject: [PATCH] Update to KSP 1.0.1 and use new jvm modifiers resolver API (#1422) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Update to KSP 1.0.1 and use new jvm modifiers resolver API This allows us to fully support transient across compilation boundaries * Copy in KSP-supported KCT for now * Update CI refs * Move up transient check to the right place Wasn't actually looking at the added annotation later 🤧 * Try regular RC? * Skip that matrix for now --- .github/workflows/build.yml | 9 +- gradle/libs.versions.toml | 2 +- kotlin/codegen/build.gradle.kts | 4 +- .../moshi/kotlin/codegen/ksp/TargetTypes.kt | 15 +- .../java/com/tschuchort/compiletesting/Ksp.kt | 227 ++++++++++++++++++ .../test/extra/AbstractClassInModuleA.kt | 6 +- 6 files changed, 244 insertions(+), 19 deletions(-) create mode 100644 kotlin/codegen/src/test/java/com/tschuchort/compiletesting/Ksp.kt diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 745b810..ea17378 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -11,13 +11,14 @@ jobs: fail-fast: false matrix: kotlin-version: [ '1.5.31', '1.6.0-RC' ] - ksp-version: [ '1.5.31-1.0.0', '1.6.0-RC-1.0.0' ] + # TODO add back KSP 1.6.0-1.0.1 once it's out + ksp-version: [ '1.5.31-1.0.1' ] kotlin-test-mode: [ 'REFLECT', 'KSP', 'KAPT' ] exclude: - - kotlin-version: '1.5.31' - ksp-version: '1.6.0-RC-1.0.0' +# - kotlin-version: '1.5.31' +# ksp-version: '1.6.0-RC-1.0.1-RC' - kotlin-version: '1.6.0-RC' - ksp-version: '1.5.31-1.0.0' + ksp-version: '1.5.31-1.0.1' env: MOSHI_KOTLIN_VERSION: '${{ matrix.kotlin-version }}' diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 1c00f98..f2a14f2 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -19,7 +19,7 @@ jvmTarget = "1.8" kotlin = "1.5.31" kotlinCompileTesting = "1.4.4" kotlinpoet = "1.10.2" -ksp = "1.5.31-1.0.0" +ksp = "1.5.31-1.0.1" ktlint = "0.41.0" [plugins] diff --git a/kotlin/codegen/build.gradle.kts b/kotlin/codegen/build.gradle.kts index 8b4a679..2624dce 100644 --- a/kotlin/codegen/build.gradle.kts +++ b/kotlin/codegen/build.gradle.kts @@ -82,8 +82,10 @@ dependencies { compileOnly(libs.kotlin.compilerEmbeddable) // Always force the latest KSP version to match the one we're compiling against testImplementation(libs.ksp) + testImplementation(libs.ksp.api) testImplementation(libs.kotlin.compilerEmbeddable) - testImplementation(libs.kotlinCompileTesting.ksp) + // TODO reenable when it supports KSP 1.0.1+ +// testImplementation(libs.kotlinCompileTesting.ksp) // Copy these again as they're not automatically included since they're shaded testImplementation(project(":moshi")) diff --git a/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/ksp/TargetTypes.kt b/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/ksp/TargetTypes.kt index e3bfafa..3b0b1b3 100644 --- a/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/ksp/TargetTypes.kt +++ b/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/ksp/TargetTypes.kt @@ -39,7 +39,6 @@ import com.squareup.kotlinpoet.KModifier import com.squareup.kotlinpoet.ParameterizedTypeName.Companion.parameterizedBy import com.squareup.kotlinpoet.PropertySpec import com.squareup.kotlinpoet.TypeName -import com.squareup.kotlinpoet.jvm.transient import com.squareup.kotlinpoet.ksp.TypeParameterResolver import com.squareup.kotlinpoet.ksp.toClassName import com.squareup.kotlinpoet.ksp.toKModifier @@ -220,6 +219,7 @@ private fun KSAnnotated?.jsonIgnore(): Boolean { return this?.findAnnotationWithType()?.ignore ?: false } +@OptIn(KspExperimental::class) private fun declaredProperties( constructor: TargetConstructor, originalType: KSClassDeclaration, @@ -238,7 +238,9 @@ private fun declaredProperties( val propertySpec = property.toPropertySpec(resolver, resolvedType, typeParameterResolver) val name = propertySpec.name val parameter = constructor.parameters[name] - val isTransient = property.isAnnotationPresent(Transient::class) + val isTransient = Modifier.JAVA_TRANSIENT in property.modifiers || + property.isAnnotationPresent(Transient::class) || + Modifier.JAVA_TRANSIENT in resolver.effectiveJavaModifiers(property) result[name] = TargetProperty( propertySpec = propertySpec, parameter = parameter, @@ -255,7 +257,7 @@ private fun declaredProperties( private fun KSPropertyDeclaration.toPropertySpec( resolver: Resolver, resolvedType: KSType, - typeParameterResolver: TypeParameterResolver, + typeParameterResolver: TypeParameterResolver ): PropertySpec { return PropertySpec.builder( name = simpleName.getShortName(), @@ -264,13 +266,6 @@ private fun KSPropertyDeclaration.toPropertySpec( .mutable(isMutable) .addModifiers(modifiers.map { KModifier.valueOf(it.name) }) .apply { - // Check modifiers and annotation since annotation is source-only - // Note that this won't work properly until https://github.com/google/ksp/issues/710 is fixed - val isTransient = Modifier.JAVA_TRANSIENT in this@toPropertySpec.modifiers || - isAnnotationPresent(Transient::class) - if (isTransient) { - transient() - } addAnnotations( this@toPropertySpec.annotations .mapNotNull { diff --git a/kotlin/codegen/src/test/java/com/tschuchort/compiletesting/Ksp.kt b/kotlin/codegen/src/test/java/com/tschuchort/compiletesting/Ksp.kt new file mode 100644 index 0000000..b91b371 --- /dev/null +++ b/kotlin/codegen/src/test/java/com/tschuchort/compiletesting/Ksp.kt @@ -0,0 +1,227 @@ +/* + * Copyright (C) 2021 Square, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.tschuchort.compiletesting + +import com.google.devtools.ksp.AbstractKotlinSymbolProcessingExtension +import com.google.devtools.ksp.KspOptions +import com.google.devtools.ksp.processing.KSPLogger +import com.google.devtools.ksp.processing.SymbolProcessorProvider +import com.google.devtools.ksp.processing.impl.MessageCollectorBasedKSPLogger +import org.jetbrains.kotlin.cli.common.CLIConfigurationKeys +import org.jetbrains.kotlin.cli.common.messages.MessageRenderer +import org.jetbrains.kotlin.cli.common.messages.PrintingMessageCollector +import org.jetbrains.kotlin.cli.jvm.config.JavaSourceRoot +import org.jetbrains.kotlin.com.intellij.core.CoreApplicationEnvironment +import org.jetbrains.kotlin.com.intellij.mock.MockProject +import org.jetbrains.kotlin.com.intellij.psi.PsiTreeChangeAdapter +import org.jetbrains.kotlin.com.intellij.psi.PsiTreeChangeListener +import org.jetbrains.kotlin.compiler.plugin.ComponentRegistrar +import org.jetbrains.kotlin.config.CompilerConfiguration +import org.jetbrains.kotlin.resolve.jvm.extensions.AnalysisHandlerExtension +import org.jetbrains.kotlin.utils.addToStdlib.firstIsInstanceOrNull +import java.io.File + +/** + * The list of symbol processors for the kotlin compilation. + * https://goo.gle/ksp + */ +var KotlinCompilation.symbolProcessorProviders: List + get() = getKspRegistrar().providers + set(value) { + val registrar = getKspRegistrar() + registrar.providers = value + } + +/** + * The directory where generated KSP sources are written + */ +val KotlinCompilation.kspSourcesDir: File + get() = kspWorkingDir.resolve("sources") + +/** + * Arbitrary arguments to be passed to ksp + */ +var KotlinCompilation.kspArgs: MutableMap + get() = getKspRegistrar().options + set(value) { + val registrar = getKspRegistrar() + registrar.options = value + } + +/** + * Controls for enabling incremental processing in KSP. + */ +var KotlinCompilation.kspIncremental: Boolean + get() = getKspRegistrar().incremental + set(value) { + val registrar = getKspRegistrar() + registrar.incremental = value + } + +/** + * Controls for enabling incremental processing logs in KSP. + */ +var KotlinCompilation.kspIncrementalLog: Boolean + get() = getKspRegistrar().incrementalLog + set(value) { + val registrar = getKspRegistrar() + registrar.incrementalLog = value + } + +/** + * Controls for enabling all warnings as errors in KSP. + */ +var KotlinCompilation.kspAllWarningsAsErrors: Boolean + get() = getKspRegistrar().allWarningsAsErrors + set(value) { + val registrar = getKspRegistrar() + registrar.allWarningsAsErrors = value + } + +private val KotlinCompilation.kspJavaSourceDir: File + get() = kspSourcesDir.resolve("java") + +private val KotlinCompilation.kspKotlinSourceDir: File + get() = kspSourcesDir.resolve("kotlin") + +private val KotlinCompilation.kspResources: File + get() = kspSourcesDir.resolve("resources") + +/** + * The working directory for KSP + */ +private val KotlinCompilation.kspWorkingDir: File + get() = workingDir.resolve("ksp") + +/** + * The directory where compiled KSP classes are written + */ +// TODO this seems to be ignored by KSP and it is putting classes into regular classes directory +// but we still need to provide it in the KSP options builder as it is required +// once it works, we should make the property public. +private val KotlinCompilation.kspClassesDir: File + get() = kspWorkingDir.resolve("classes") + +/** + * The directory where compiled KSP caches are written + */ +private val KotlinCompilation.kspCachesDir: File + get() = kspWorkingDir.resolve("caches") + +/** + * Custom subclass of [AbstractKotlinSymbolProcessingExtension] where processors are pre-defined instead of being + * loaded via ServiceLocator. + */ +private class KspTestExtension( + options: KspOptions, + processorProviders: List, + logger: KSPLogger +) : AbstractKotlinSymbolProcessingExtension( + options = options, + logger = logger, + testMode = false +) { + private val loadedProviders = processorProviders + + override fun loadProviders() = loadedProviders +} + +/** + * Registers the [KspTestExtension] to load the given list of processors. + */ +private class KspCompileTestingComponentRegistrar( + private val compilation: KotlinCompilation +) : ComponentRegistrar { + var providers = emptyList() + + var options: MutableMap = mutableMapOf() + + var incremental: Boolean = false + var incrementalLog: Boolean = false + var allWarningsAsErrors: Boolean = false + + override fun registerProjectComponents(project: MockProject, configuration: CompilerConfiguration) { + if (providers.isEmpty()) { + return + } + val options = KspOptions.Builder().apply { + this.projectBaseDir = compilation.kspWorkingDir + + this.processingOptions.putAll(compilation.kspArgs) + + this.incremental = this@KspCompileTestingComponentRegistrar.incremental + this.incrementalLog = this@KspCompileTestingComponentRegistrar.incrementalLog + this.allWarningsAsErrors = this@KspCompileTestingComponentRegistrar.allWarningsAsErrors + + this.cachesDir = compilation.kspCachesDir.also { + it.deleteRecursively() + it.mkdirs() + } + this.kspOutputDir = compilation.kspSourcesDir.also { + it.deleteRecursively() + it.mkdirs() + } + this.classOutputDir = compilation.kspClassesDir.also { + it.deleteRecursively() + it.mkdirs() + } + this.javaOutputDir = compilation.kspJavaSourceDir.also { + it.deleteRecursively() + it.mkdirs() + } + this.kotlinOutputDir = compilation.kspKotlinSourceDir.also { + it.deleteRecursively() + it.mkdirs() + } + this.resourceOutputDir = compilation.kspResources.also { + it.deleteRecursively() + it.mkdirs() + } + configuration[CLIConfigurationKeys.CONTENT_ROOTS] + ?.filterIsInstance() + ?.forEach { + this.javaSourceRoots.add(it.file) + } + }.build() + + // Temporary until friend-paths is fully supported https://youtrack.jetbrains.com/issue/KT-34102 + @Suppress("invisible_member") + val messageCollectorBasedKSPLogger = MessageCollectorBasedKSPLogger( + PrintingMessageCollector( + compilation.internalMessageStreamAccess, + MessageRenderer.GRADLE_STYLE, + compilation.verbose + ), + allWarningsAsErrors + ) + val registrar = KspTestExtension(options, providers, messageCollectorBasedKSPLogger) + AnalysisHandlerExtension.registerExtension(project, registrar) + // Dummy extension point; Required by dropPsiCaches(). + CoreApplicationEnvironment.registerExtensionPoint(project.extensionArea, PsiTreeChangeListener.EP.name, PsiTreeChangeAdapter::class.java) + } +} + +/** + * Gets the test registrar from the plugin list or adds if it does not exist. + */ +private fun KotlinCompilation.getKspRegistrar(): KspCompileTestingComponentRegistrar { + compilerPlugins.firstIsInstanceOrNull()?.let { + return it + } + val kspRegistrar = KspCompileTestingComponentRegistrar(this) + compilerPlugins = compilerPlugins + kspRegistrar + return kspRegistrar +} diff --git a/kotlin/tests/extra-moshi-test-module/src/main/kotlin/com/squareup/moshi/kotlin/codegen/test/extra/AbstractClassInModuleA.kt b/kotlin/tests/extra-moshi-test-module/src/main/kotlin/com/squareup/moshi/kotlin/codegen/test/extra/AbstractClassInModuleA.kt index 87aba20..89e6657 100644 --- a/kotlin/tests/extra-moshi-test-module/src/main/kotlin/com/squareup/moshi/kotlin/codegen/test/extra/AbstractClassInModuleA.kt +++ b/kotlin/tests/extra-moshi-test-module/src/main/kotlin/com/squareup/moshi/kotlin/codegen/test/extra/AbstractClassInModuleA.kt @@ -18,9 +18,9 @@ package com.squareup.moshi.kotlin.codegen.test.extra import com.squareup.moshi.Json public abstract class AbstractClassInModuleA { - // Ignored to ensure processor sees them across module boundaries. - // @Transient doesn't work for this case because it's source-only and jvm modifiers aren't currently visible in KSP. - + // Ignored/transient to ensure processor sees them across module boundaries. + @Transient private lateinit var lateinitTransient: String + @Transient private var regularTransient: String = "regularTransient" // Note that we target the field because otherwise it is stored on the synthetic holder method for // annotations, which isn't visible from kapt @field:Json(ignore = true) private lateinit var lateinitIgnored: String