Update to KSP 1.0.1 and use new jvm modifiers resolver API (#1422)

* 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

@@ -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 }}'

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]

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"))

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<Json>()?.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 {

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<SymbolProcessorProvider>
+  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<String, String>
+  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<SymbolProcessorProvider>,
+  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<SymbolProcessorProvider>()
+
+  var options: MutableMap<String, String> = 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<JavaSourceRoot>()
+        ?.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<KspCompileTestingComponentRegistrar>()?.let {
+    return it
+  }
+  val kspRegistrar = KspCompileTestingComponentRegistrar(this)
+  compilerPlugins = compilerPlugins + kspRegistrar
+  return kspRegistrar
+}

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