From d64033ed94dc16f83d30fd02893af21219f9b67c Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Thu, 30 Jan 2020 22:58:10 -0500 Subject: [PATCH] Fix reflection names in generated proguard files (#1088) * Fix reflection names in generated proguard files Proguard wants to speak reflection names * Opportunistic clean up proguard tests This ensures we fail the test if there are any unexpected ones --- .../kotlin/codegen/api/AdapterGenerator.kt | 10 +- .../moshi/kotlin/codegen/api/ProguardRules.kt | 6 +- .../codegen/JsonClassCodegenProcessorTest.kt | 205 ++++++++++-------- 3 files changed, 119 insertions(+), 102 deletions(-) diff --git a/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/AdapterGenerator.kt b/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/AdapterGenerator.kt index 00214ac..8f7e406 100644 --- a/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/AdapterGenerator.kt +++ b/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/AdapterGenerator.kt @@ -170,8 +170,8 @@ internal class AdapterGenerator( } val adapterConstructorParams = when (requireNotNull(primaryConstructor).parameters.size) { - 1 -> listOf(CN_MOSHI.canonicalName) - 2 -> listOf(CN_MOSHI.canonicalName, "${CN_TYPE.canonicalName}[]") + 1 -> listOf(CN_MOSHI.reflectionName()) + 2 -> listOf(CN_MOSHI.reflectionName(), "${CN_TYPE.reflectionName()}[]") // Should never happen else -> error("Unexpected number of arguments on primary constructor: $primaryConstructor") } @@ -186,7 +186,7 @@ internal class AdapterGenerator( } hasDefaultProperties = propertyList.any { it.hasDefault } parameterTypes = AsmType.getArgumentTypes(constructorSignature.removePrefix("")) - .map { it.toCanonicalString() } + .map { it.toReflectionString() } } return ProguardConfig( targetClass = className, @@ -568,7 +568,7 @@ internal class AdapterGenerator( /** Represents a prepared adapter with its [spec] and optional associated [proguardConfig]. */ internal data class PreparedAdapter(val spec: FileSpec, val proguardConfig: ProguardConfig?) -private fun AsmType.toCanonicalString(): String { +private fun AsmType.toReflectionString(): String { return when (this) { AsmType.VOID_TYPE -> "void" AsmType.BOOLEAN_TYPE -> "boolean" @@ -580,7 +580,7 @@ private fun AsmType.toCanonicalString(): String { AsmType.LONG_TYPE -> "long" AsmType.DOUBLE_TYPE -> "double" else -> when (sort) { - AsmType.ARRAY -> "${elementType.toCanonicalString()}[]" + AsmType.ARRAY -> "${elementType.toReflectionString()}[]" // Object type else -> className } diff --git a/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/ProguardRules.kt b/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/ProguardRules.kt index 7b797f0..5088d35 100644 --- a/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/ProguardRules.kt +++ b/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/ProguardRules.kt @@ -46,7 +46,7 @@ internal data class ProguardConfig( // private final {adapter fields} // } // - val targetName = targetClass.canonicalName + val targetName = targetClass.reflectionName() val adapterCanonicalName = ClassName(targetClass.packageName, adapterName).canonicalName // Keep the class name for Moshi's reflective lookup based on it appendln("-if class $targetName") @@ -65,7 +65,7 @@ internal data class ProguardConfig( qualifierProperties.asSequence() .flatMap { it.qualifiers.asSequence() } - .map(ClassName::canonicalName) + .map(ClassName::reflectionName) .sorted() .forEach { qualifier -> appendln("-if class $targetName") @@ -83,7 +83,7 @@ internal data class ProguardConfig( appendln("-if class $targetName") appendln("-keepnames class kotlin.jvm.internal.DefaultConstructorMarker") appendln("-if class $targetName") - appendln("-keepclassmembers class ${targetClass.canonicalName} {") + appendln("-keepclassmembers class $targetName {") val allParams = targetConstructorParams.toMutableList() val maskCount = if (targetConstructorParams.isEmpty()) { 0 diff --git a/kotlin/codegen/src/test/java/com/squareup/moshi/kotlin/codegen/JsonClassCodegenProcessorTest.kt b/kotlin/codegen/src/test/java/com/squareup/moshi/kotlin/codegen/JsonClassCodegenProcessorTest.kt index ee0c9ca..0286f05 100644 --- a/kotlin/codegen/src/test/java/com/squareup/moshi/kotlin/codegen/JsonClassCodegenProcessorTest.kt +++ b/kotlin/codegen/src/test/java/com/squareup/moshi/kotlin/codegen/JsonClassCodegenProcessorTest.kt @@ -432,6 +432,14 @@ class JsonClassCodegenProcessorTest { @JsonClass(generateAdapter = true) data class Complex(val firstName: FirstName = "", @MyQualifier val names: MutableList, val genericProp: T) + + object NestedType { + @JsonQualifier + annotation class NestedQualifier + + @JsonClass(generateAdapter = true) + data class NestedSimple(@NestedQualifier val firstName: String) + } @JsonClass(generateAdapter = true) class MultipleMasks( @@ -506,101 +514,110 @@ class JsonClassCodegenProcessorTest { )) assertThat(result.exitCode).isEqualTo(KotlinCompilation.ExitCode.OK) - assertThat(result.generatedFiles.find { it.name == "moshi-testPackage.Aliases.pro" }).hasContent(""" - -if class testPackage.Aliases - -keepnames class testPackage.Aliases - -if class testPackage.Aliases - -keep class testPackage.AliasesJsonAdapter { - public (com.squareup.moshi.Moshi); + result.generatedFiles.filter { it.extension == "pro" }.forEach { generatedFile -> + when (generatedFile.nameWithoutExtension) { + "moshi-testPackage.Aliases" -> assertThat(generatedFile).hasContent(""" + -if class testPackage.Aliases + -keepnames class testPackage.Aliases + -if class testPackage.Aliases + -keep class testPackage.AliasesJsonAdapter { + public (com.squareup.moshi.Moshi); + } + """.trimIndent()) + "moshi-testPackage.Simple" -> assertThat(generatedFile).hasContent(""" + -if class testPackage.Simple + -keepnames class testPackage.Simple + -if class testPackage.Simple + -keep class testPackage.SimpleJsonAdapter { + public (com.squareup.moshi.Moshi); + } + """.trimIndent()) + "moshi-testPackage.Generic" -> assertThat(generatedFile).hasContent(""" + -if class testPackage.Generic + -keepnames class testPackage.Generic + -if class testPackage.Generic + -keep class testPackage.GenericJsonAdapter { + public (com.squareup.moshi.Moshi,java.lang.reflect.Type[]); + } + """.trimIndent()) + "moshi-testPackage.UsingQualifiers" -> assertThat(generatedFile).hasContent(""" + -if class testPackage.UsingQualifiers + -keepnames class testPackage.UsingQualifiers + -if class testPackage.UsingQualifiers + -keep class testPackage.UsingQualifiersJsonAdapter { + public (com.squareup.moshi.Moshi); + private com.squareup.moshi.JsonAdapter stringAtMyQualifierAdapter; + } + -if class testPackage.UsingQualifiers + -keep @interface testPackage.MyQualifier + """.trimIndent()) + "moshi-testPackage.MixedTypes" -> assertThat(generatedFile).hasContent(""" + -if class testPackage.MixedTypes + -keepnames class testPackage.MixedTypes + -if class testPackage.MixedTypes + -keep class testPackage.MixedTypesJsonAdapter { + public (com.squareup.moshi.Moshi); + } + """.trimIndent()) + "moshi-testPackage.DefaultParams" -> assertThat(generatedFile).hasContent(""" + -if class testPackage.DefaultParams + -keepnames class testPackage.DefaultParams + -if class testPackage.DefaultParams + -keep class testPackage.DefaultParamsJsonAdapter { + public (com.squareup.moshi.Moshi); + } + -if class testPackage.DefaultParams + -keepnames class kotlin.jvm.internal.DefaultConstructorMarker + -if class testPackage.DefaultParams + -keepclassmembers class testPackage.DefaultParams { + public synthetic (java.lang.String,int,kotlin.jvm.internal.DefaultConstructorMarker); + } + """.trimIndent()) + "moshi-testPackage.Complex" -> assertThat(generatedFile).hasContent(""" + -if class testPackage.Complex + -keepnames class testPackage.Complex + -if class testPackage.Complex + -keep class testPackage.ComplexJsonAdapter { + public (com.squareup.moshi.Moshi,java.lang.reflect.Type[]); + private com.squareup.moshi.JsonAdapter mutableListOfStringAtMyQualifierAdapter; + } + -if class testPackage.Complex + -keep @interface testPackage.MyQualifier + -if class testPackage.Complex + -keepnames class kotlin.jvm.internal.DefaultConstructorMarker + -if class testPackage.Complex + -keepclassmembers class testPackage.Complex { + public synthetic (java.lang.String,java.util.List,java.lang.Object,int,kotlin.jvm.internal.DefaultConstructorMarker); + } + """.trimIndent()) + "moshi-testPackage.MultipleMasks" -> assertThat(generatedFile).hasContent(""" + -if class testPackage.MultipleMasks + -keepnames class testPackage.MultipleMasks + -if class testPackage.MultipleMasks + -keep class testPackage.MultipleMasksJsonAdapter { + public (com.squareup.moshi.Moshi); + } + -if class testPackage.MultipleMasks + -keepnames class kotlin.jvm.internal.DefaultConstructorMarker + -if class testPackage.MultipleMasks + -keepclassmembers class testPackage.MultipleMasks { + public synthetic (long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,int,int,int,kotlin.jvm.internal.DefaultConstructorMarker); + } + """.trimIndent()) + "moshi-testPackage.NestedType.NestedSimple" -> assertThat(generatedFile).hasContent(""" + -if class testPackage.NestedType${'$'}NestedSimple + -keepnames class testPackage.NestedType${'$'}NestedSimple + -if class testPackage.NestedType${'$'}NestedSimple + -keep class testPackage.NestedType_NestedSimpleJsonAdapter { + public (com.squareup.moshi.Moshi); + private com.squareup.moshi.JsonAdapter stringAtNestedQualifierAdapter; + } + -if class testPackage.NestedType${'$'}NestedSimple + -keep @interface testPackage.NestedType${'$'}NestedQualifier + """.trimIndent()) + else -> error("Unexpected proguard file! ${generatedFile.name}") } - """.trimIndent()) - - assertThat(result.generatedFiles.find { it.name == "moshi-testPackage.Simple.pro" }).hasContent(""" - -if class testPackage.Simple - -keepnames class testPackage.Simple - -if class testPackage.Simple - -keep class testPackage.SimpleJsonAdapter { - public (com.squareup.moshi.Moshi); - } - """.trimIndent()) - - assertThat(result.generatedFiles.find { it.name == "moshi-testPackage.Generic.pro" }).hasContent(""" - -if class testPackage.Generic - -keepnames class testPackage.Generic - -if class testPackage.Generic - -keep class testPackage.GenericJsonAdapter { - public (com.squareup.moshi.Moshi,java.lang.reflect.Type[]); - } - """.trimIndent()) - - assertThat(result.generatedFiles.find { it.name == "moshi-testPackage.UsingQualifiers.pro" }).hasContent(""" - -if class testPackage.UsingQualifiers - -keepnames class testPackage.UsingQualifiers - -if class testPackage.UsingQualifiers - -keep class testPackage.UsingQualifiersJsonAdapter { - public (com.squareup.moshi.Moshi); - private com.squareup.moshi.JsonAdapter stringAtMyQualifierAdapter; - } - -if class testPackage.UsingQualifiers - -keep @interface testPackage.MyQualifier - """.trimIndent()) - - assertThat(result.generatedFiles.find { it.name == "moshi-testPackage.MixedTypes.pro" }).hasContent(""" - -if class testPackage.MixedTypes - -keepnames class testPackage.MixedTypes - -if class testPackage.MixedTypes - -keep class testPackage.MixedTypesJsonAdapter { - public (com.squareup.moshi.Moshi); - } - """.trimIndent()) - - assertThat(result.generatedFiles.find { it.name == "moshi-testPackage.DefaultParams.pro" }).hasContent(""" - -if class testPackage.DefaultParams - -keepnames class testPackage.DefaultParams - -if class testPackage.DefaultParams - -keep class testPackage.DefaultParamsJsonAdapter { - public (com.squareup.moshi.Moshi); - } - -if class testPackage.DefaultParams - -keepnames class kotlin.jvm.internal.DefaultConstructorMarker - -if class testPackage.DefaultParams - -keepclassmembers class testPackage.DefaultParams { - public synthetic (java.lang.String,int,kotlin.jvm.internal.DefaultConstructorMarker); - } - """.trimIndent()) - - assertThat(result.generatedFiles.find { it.name == "moshi-testPackage.Complex.pro" }).hasContent(""" - -if class testPackage.Complex - -keepnames class testPackage.Complex - -if class testPackage.Complex - -keep class testPackage.ComplexJsonAdapter { - public (com.squareup.moshi.Moshi,java.lang.reflect.Type[]); - private com.squareup.moshi.JsonAdapter mutableListOfStringAtMyQualifierAdapter; - } - -if class testPackage.Complex - -keep @interface testPackage.MyQualifier - -if class testPackage.Complex - -keepnames class kotlin.jvm.internal.DefaultConstructorMarker - -if class testPackage.Complex - -keepclassmembers class testPackage.Complex { - public synthetic (java.lang.String,java.util.List,java.lang.Object,int,kotlin.jvm.internal.DefaultConstructorMarker); - } - """.trimIndent()) - - assertThat(result.generatedFiles.find { it.name == "moshi-testPackage.MultipleMasks.pro" }).hasContent(""" - -if class testPackage.MultipleMasks - -keepnames class testPackage.MultipleMasks - -if class testPackage.MultipleMasks - -keep class testPackage.MultipleMasksJsonAdapter { - public (com.squareup.moshi.Moshi); - } - -if class testPackage.MultipleMasks - -keepnames class kotlin.jvm.internal.DefaultConstructorMarker - -if class testPackage.MultipleMasks - -keepclassmembers class testPackage.MultipleMasks { - public synthetic (long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,long,int,int,int,kotlin.jvm.internal.DefaultConstructorMarker); - } - """.trimIndent()) + } } private fun prepareCompilation(vararg sourceFiles: SourceFile): KotlinCompilation {