Skip to content

Commit

Permalink
Upgrade to Kotlin 1.6 + always instantiate annotations (#1425)
Browse files Browse the repository at this point in the history
  • Loading branch information
ZacSweers committed Nov 17, 2021
1 parent 9440e5c commit 7355328
Show file tree
Hide file tree
Showing 15 changed files with 96 additions and 383 deletions.
16 changes: 2 additions & 14 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,7 @@ jobs:
strategy:
fail-fast: false
matrix:
kotlin-version: [ '1.5.31', '1.6.0-RC' ]
# 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.1-RC'
- kotlin-version: '1.6.0-RC'
ksp-version: '1.5.31-1.0.1'

env:
MOSHI_KOTLIN_VERSION: '${{ matrix.kotlin-version }}'
MOSHI_KSP_VERSION: '${{ matrix.ksp-version }}'

steps:
- name: Checkout
Expand All @@ -48,10 +36,10 @@ jobs:
java-version: '17'

- name: Test
run: ./gradlew build check --stacktrace -PkotlinTestMode=${{ matrix.kotlin-test-mode }} -PkotlinVersion=${{ matrix.kotlin-version }}
run: ./gradlew build check --stacktrace -PkotlinTestMode=${{ matrix.kotlin-test-mode }}

- name: Publish (default branch only)
if: github.repository == 'square/moshi' && github.ref == 'refs/heads/master' && matrix.kotlin-version == '1.5.31' && matrix.kotlin-test-mode == 'reflect'
if: github.repository == 'square/moshi' && github.ref == 'refs/heads/master' && matrix.kotlin-test-mode == 'reflect'
run: ./gradlew publish
env:
ORG_GRADLE_PROJECT_mavenCentralUsername: '${{ secrets.SONATYPE_NEXUS_USERNAME }}'
Expand Down
23 changes: 2 additions & 21 deletions gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -14,34 +14,15 @@
# limitations under the License.
#

# For Dokka https://github.com/Kotlin/dokka/issues/1405
# Memory for Dokka https://github.com/Kotlin/dokka/issues/1405
# --add-opens for GJF https://github.com/google/google-java-format#jdk-16
org.gradle.jvmargs=-Xmx2048m -Dfile.encoding=UTF-8 \
--add-opens=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.jvm=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED

kotlin.daemon.jvmargs=-Dfile.encoding=UTF-8 \
--add-opens=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.jvm=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED

# Kapt doesn't read the above jvm args when workers are used
# https://youtrack.jetbrains.com/issue/KT-48402
kapt.use.worker.api=false
kapt.include.compile.classpath=false

GROUP=com.squareup.moshi
Expand Down
6 changes: 3 additions & 3 deletions gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@
autoService = "1.0"
gjf = "1.11.0"
jvmTarget = "1.8"
kotlin = "1.5.31"
kotlin = "1.6.0"
kotlinCompileTesting = "1.4.4"
kotlinpoet = "1.10.2"
ksp = "1.5.31-1.0.1"
ksp = "1.6.0-1.0.1"
ktlint = "0.41.0"

[plugins]
dokka = { id = "org.jetbrains.dokka", version = "1.5.30" }
dokka = { id = "org.jetbrains.dokka", version = "1.5.31" }
japicmp = { id = "me.champeau.gradle.japicmp", version = "0.2.9" }
ksp = { id = "com.google.devtools.ksp", version.ref = "ksp" }
mavenPublish = { id = "com.vanniktech.maven.publish", version = "0.17.0" }
Expand Down
1 change: 1 addition & 0 deletions kotlin/codegen/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ tasks.withType<KotlinCompile>().configureEach {
}
}

// --add-opens for kapt to work. KGP covers this for us but local JVMs in tests do not
tasks.withType<Test>().configureEach {
jvmArgs(
"--add-opens=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,32 +186,14 @@ public class AdapterGenerator(
result.addAnnotation(COMMON_SUPPRESS)
result.addType(generatedAdapter)
val proguardConfig = if (generateProguardRules) {
generatedAdapter.createProguardRule(target.instantiateAnnotations)
generatedAdapter.createProguardRule()
} else {
null
}
return PreparedAdapter(result.build(), proguardConfig)
}

private fun TypeSpec.createProguardRule(instantiateAnnotations: Boolean): ProguardConfig {
val adapterProperties = if (instantiateAnnotations) {
// Don't need to do anything special if we instantiate them directly!
emptySet()
} else {
propertySpecs
.asSequence()
.filter { prop ->
prop.type.rawType() == JsonAdapter::class.asClassName()
}
.filter { prop -> prop.annotations.isNotEmpty() }
.mapTo(mutableSetOf()) { prop ->
QualifierAdapterProperty(
name = prop.name,
qualifiers = prop.annotations.mapTo(mutableSetOf()) { it.typeName.rawType() }
)
}
}

private fun TypeSpec.createProguardRule(): ProguardConfig {
val adapterConstructorParams = when (requireNotNull(primaryConstructor).parameters.size) {
1 -> listOf(CN_MOSHI.reflectionName())
2 -> listOf(CN_MOSHI.reflectionName(), "${CN_TYPE.reflectionName()}[]")
Expand All @@ -237,7 +219,6 @@ public class AdapterGenerator(
adapterConstructorParams = adapterConstructorParams,
targetConstructorHasDefaults = hasDefaultProperties,
targetConstructorParams = parameterTypes,
qualifierProperties = adapterProperties
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,15 @@ import com.squareup.kotlinpoet.TypeName
import com.squareup.kotlinpoet.TypeVariableName
import com.squareup.kotlinpoet.WildcardTypeName
import com.squareup.kotlinpoet.asClassName
import com.squareup.kotlinpoet.asTypeName
import com.squareup.kotlinpoet.joinToCode
import com.squareup.moshi.JsonAdapter
import com.squareup.moshi.Types
import java.util.Locale

/** A JsonAdapter that can be used to encode and decode a particular field. */
@InternalMoshiCodegenApi
public data class DelegateKey(
private val type: TypeName,
private val jsonQualifiers: List<AnnotationSpec>,
private val instantiateAnnotations: Boolean
) {
public val nullable: Boolean get() = type.isNullable

Expand All @@ -67,22 +64,13 @@ public data class DelegateKey(

val (initializerString, args) = when {
jsonQualifiers.isEmpty() -> ", %M()" to arrayOf(MemberName("kotlin.collections", "emptySet"))
instantiateAnnotations -> {
", setOf(%L)" to arrayOf(jsonQualifiers.map { it.asInstantiationExpression() }.joinToCode())
}
else -> {
", %T.getFieldJsonQualifierAnnotations(javaClass, " +
"%S)" to arrayOf(Types::class.asTypeName(), adapterName)
", setOf(%L)" to arrayOf(jsonQualifiers.map { it.asInstantiationExpression() }.joinToCode())
}
}
val finalArgs = arrayOf(*standardArgs, *args, propertyName)

return PropertySpec.builder(adapterName, adapterTypeName, KModifier.PRIVATE)
.apply {
if (!instantiateAnnotations) {
addAnnotations(jsonQualifiers)
}
}
.initializer("%N.adapter(%L$initializerString, %S)", *finalArgs)
.build()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ public data class ProguardConfig(
val adapterConstructorParams: List<String>,
val targetConstructorHasDefaults: Boolean,
val targetConstructorParams: List<String>,
val qualifierProperties: Set<QualifierAdapterProperty>
) {
public fun outputFilePathWithoutExtension(canonicalName: String): String {
return "META-INF/proguard/moshi-$canonicalName"
Expand All @@ -66,21 +65,8 @@ public data class ProguardConfig(
// Keep the constructor for Moshi's reflective lookup
val constructorArgs = adapterConstructorParams.joinToString(",")
appendLine(" public <init>($constructorArgs);")
// Keep any qualifier properties
for (qualifierProperty in qualifierProperties) {
appendLine(" private com.squareup.moshi.JsonAdapter ${qualifierProperty.name};")
}
appendLine("}")

qualifierProperties.asSequence()
.flatMap { it.qualifiers.asSequence() }
.map(ClassName::reflectionName)
.sorted()
.forEach { qualifier ->
appendLine("-if class $targetName")
appendLine("-keep @interface $qualifier")
}

if (targetConstructorHasDefaults) {
// If the target class has default parameter values, keep its synthetic constructor
//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ public data class TargetType(
val typeVariables: List<TypeVariableName>,
val isDataClass: Boolean,
val visibility: KModifier,
val instantiateAnnotations: Boolean
) {

init {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,12 +144,11 @@ public class JsonClassCodegenProcessor : AbstractProcessor() {
types,
element,
cachedClassInspector,
instantiateAnnotations
) ?: return null

val properties = mutableMapOf<String, PropertyGenerator>()
for (property in type.properties.values) {
val generator = property.generator(messager, element, elements, type.instantiateAnnotations)
val generator = property.generator(messager, element, elements)
if (generator != null) {
properties[property.name] = generator
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,8 @@ import com.squareup.moshi.kotlin.codegen.api.rawType
import com.squareup.moshi.kotlin.codegen.api.unwrapTypeAlias
import kotlinx.metadata.KmConstructor
import kotlinx.metadata.jvm.signature
import java.lang.annotation.ElementType
import java.lang.annotation.Retention
import java.lang.annotation.RetentionPolicy
import java.lang.annotation.Target
import javax.annotation.processing.Messager
import javax.lang.model.element.AnnotationMirror
import javax.lang.model.element.Element
Expand All @@ -72,7 +70,6 @@ private val VISIBILITY_MODIFIERS = setOf(
KModifier.PROTECTED,
KModifier.PUBLIC
)
private val ANNOTATION_INSTANTIATION_MIN_VERSION = KotlinVersion(1, 6, 0)

private fun Collection<KModifier>.visibility(): KModifier {
return find { it in VISIBILITY_MODIFIERS } ?: KModifier.PUBLIC
Expand Down Expand Up @@ -126,7 +123,6 @@ internal fun targetType(
types: Types,
element: TypeElement,
cachedClassInspector: MoshiCachedClassInspector,
instantiateAnnotationsEnabled: Boolean
): TargetType? {
val typeMetadata = element.getAnnotation(Metadata::class.java)
if (typeMetadata == null) {
Expand Down Expand Up @@ -208,11 +204,6 @@ internal fun targetType(
}
}

val instantiateAnnotations = instantiateAnnotationsEnabled && run {
val (major, minor, patch) = typeMetadata.metadataVersion
val languageVersion = KotlinVersion(major, minor, patch)
languageVersion >= ANNOTATION_INSTANTIATION_MIN_VERSION
}
val kotlinApi = cachedClassInspector.toTypeSpec(kmClass)
val typeVariables = kotlinApi.typeVariables
val appliedType = AppliedType.get(element)
Expand Down Expand Up @@ -329,7 +320,6 @@ internal fun targetType(
typeVariables = typeVariables,
isDataClass = KModifier.DATA in kotlinApi.modifiers,
visibility = resolvedVisibility,
instantiateAnnotations = instantiateAnnotations
)
}

Expand Down Expand Up @@ -431,7 +421,6 @@ internal fun TargetProperty.generator(
messager: Messager,
sourceElement: TypeElement,
elements: Elements,
instantiateAnnotations: Boolean
): PropertyGenerator? {
if (jsonIgnore) {
if (!hasDefault) {
Expand All @@ -442,7 +431,7 @@ internal fun TargetProperty.generator(
)
return null
}
return PropertyGenerator(this, DelegateKey(type, emptyList(), instantiateAnnotations), true)
return PropertyGenerator(this, DelegateKey(type, emptyList()), true)
}

if (!isVisible) {
Expand Down Expand Up @@ -474,16 +463,6 @@ internal fun TargetProperty.generator(
)
}
}
if (!instantiateAnnotations) {
annotationElement.getAnnotation(Target::class.java)?.let {
if (ElementType.FIELD !in it.value) {
messager.printMessage(
ERROR,
"JsonQualifier @${qualifierRawType.simpleName} must support FIELD target"
)
}
}
}
}

val jsonQualifierSpecs = qualifiers.map {
Expand All @@ -494,7 +473,7 @@ internal fun TargetProperty.generator(

return PropertyGenerator(
this,
DelegateKey(type, jsonQualifierSpecs, instantiateAnnotations)
DelegateKey(type, jsonQualifierSpecs)
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import com.squareup.moshi.JsonClass
import com.squareup.moshi.kotlin.codegen.api.AdapterGenerator
import com.squareup.moshi.kotlin.codegen.api.Options.OPTION_GENERATED
import com.squareup.moshi.kotlin.codegen.api.Options.OPTION_GENERATE_PROGUARD_RULES
import com.squareup.moshi.kotlin.codegen.api.Options.OPTION_INSTANTIATE_ANNOTATIONS
import com.squareup.moshi.kotlin.codegen.api.Options.POSSIBLE_GENERATED_NAMES
import com.squareup.moshi.kotlin.codegen.api.ProguardConfig
import com.squareup.moshi.kotlin.codegen.api.PropertyGenerator
Expand Down Expand Up @@ -64,8 +63,6 @@ private class JsonClassSymbolProcessor(
}
}
private val generateProguardRules = environment.options[OPTION_GENERATE_PROGUARD_RULES]?.toBooleanStrictOrNull() ?: true
private val instantiateAnnotations = (environment.options[OPTION_INSTANTIATE_ANNOTATIONS]?.toBooleanStrictOrNull() ?: true) &&
environment.kotlinVersion.isAtLeast(1, 6)

override fun process(resolver: Resolver): List<KSAnnotated> {
val generatedAnnotation = generatedOption?.let {
Expand Down Expand Up @@ -125,11 +122,11 @@ private class JsonClassSymbolProcessor(
resolver: Resolver,
originalType: KSDeclaration,
): AdapterGenerator? {
val type = targetType(originalType, resolver, logger, instantiateAnnotations) ?: return null
val type = targetType(originalType, resolver, logger) ?: return null

val properties = mutableMapOf<String, PropertyGenerator>()
for (property in type.properties.values) {
val generator = property.generator(logger, resolver, originalType, type.instantiateAnnotations)
val generator = property.generator(logger, resolver, originalType)
if (generator != null) {
properties[property.name] = generator
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ internal fun TargetProperty.generator(
logger: KSPLogger,
resolver: Resolver,
originalType: KSDeclaration,
instantiateAnnotations: Boolean
): PropertyGenerator? {
if (jsonIgnore) {
if (!hasDefault) {
Expand All @@ -53,7 +52,7 @@ internal fun TargetProperty.generator(
)
return null
}
return PropertyGenerator(this, DelegateKey(type, emptyList(), instantiateAnnotations), true)
return PropertyGenerator(this, DelegateKey(type, emptyList()), true)
}

if (!isVisible) {
Expand Down Expand Up @@ -81,15 +80,6 @@ internal fun TargetProperty.generator(
)
}
}
if (!instantiateAnnotations) {
annotationElement.findAnnotationWithType<Target>()?.let {
if (AnnotationTarget.FIELD !in it.allowedTargets) {
logger.error(
"JsonQualifier @${qualifierRawType.simpleName} must support FIELD target"
)
}
}
}
}
}

Expand All @@ -101,7 +91,7 @@ internal fun TargetProperty.generator(

return PropertyGenerator(
this,
DelegateKey(type, jsonQualifierSpecs, instantiateAnnotations)
DelegateKey(type, jsonQualifierSpecs)
)
}

Expand Down

0 comments on commit 7355328

Please sign in to comment.