wbrawner/moshi
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Support properties that don't have a backing field.

Browse source 
Currently our main loop to gather PropertyGenerators goes over the backing fields.
This needs to change to iterate over the properties themselves. That leads to a lot
of churn. The net result is slightly more compatibility with the reflective adapter.
This commit is contained in:
Jesse Wilson 2018-04-09 00:08:15 -04:00
parent e80cf48484
commit d1df4740d5
5 changed files with 226 additions and 183 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

154
kotlin-codegen/compiler/src/main/java/com/squareup/moshi/JsonClassCodeGenProcessor.kt
View file

@ -28,6 +28,7 @@ import me.eugeniomarletti.kotlin.metadata.KotlinMetadataUtils
import me.eugeniomarletti.kotlin.metadata.classKind
import me.eugeniomarletti.kotlin.metadata.declaresDefaultValue
import me.eugeniomarletti.kotlin.metadata.getPropertyOrNull
import me.eugeniomarletti.kotlin.metadata.hasSetter
import me.eugeniomarletti.kotlin.metadata.isDataClass
import me.eugeniomarletti.kotlin.metadata.isInnerClass
import me.eugeniomarletti.kotlin.metadata.isPrimary
@ -41,6 +42,7 @@ import org.jetbrains.kotlin.serialization.ProtoBuf.Modality
import org.jetbrains.kotlin.serialization.ProtoBuf.Property
import org.jetbrains.kotlin.serialization.ProtoBuf.ValueParameter
import org.jetbrains.kotlin.serialization.ProtoBuf.Visibility
import org.jetbrains.kotlin.util.capitalizeDecapitalize.decapitalizeAsciiOnly
import java.io.File
import javax.annotation.processing.ProcessingEnvironment
import javax.annotation.processing.Processor
@ -118,12 +120,12 @@ class JsonClassCodeGenProcessor : KotlinAbstractProcessor(), KotlinMetadataUtils
return true
}
private fun processElement(element: Element): AdapterGenerator? {
val metadata = element.kotlinMetadata
private fun processElement(model: Element): AdapterGenerator? {
val metadata = model.kotlinMetadata
if (metadata !is KotlinClassMetadata) {
messager.printMessage(
ERROR, "@JsonClass can't be applied to $element: must be a Kotlin class", element)
ERROR, "@JsonClass can't be applied to $model: must be a Kotlin class", model)
return null
}
@ -133,27 +135,28 @@ class JsonClassCodeGenProcessor : KotlinAbstractProcessor(), KotlinMetadataUtils
when {
classProto.classKind != Class.Kind.CLASS -> {
messager.printMessage(
ERROR, "@JsonClass can't be applied to $element: must be a Kotlin class", element)
ERROR, "@JsonClass can't be applied to $model: must be a Kotlin class", model)
return null
}
classProto.isInnerClass -> {
messager.printMessage(
ERROR, "@JsonClass can't be applied to $element: must not be an inner class", element)
ERROR, "@JsonClass can't be applied to $model: must not be an inner class",
model)
return null
}
classProto.modality == Modality.ABSTRACT -> {
messager.printMessage(
ERROR, "@JsonClass can't be applied to $element: must not be abstract", element)
ERROR, "@JsonClass can't be applied to $model: must not be abstract", model)
return null
}
classProto.visibility == Visibility.LOCAL -> {
messager.printMessage(
ERROR, "@JsonClass can't be applied to $element: must not be local", element)
ERROR, "@JsonClass can't be applied to $model: must not be local", model)
return null
}
}
val typeName = element.asType().asTypeName()
val typeName = model.asType().asTypeName()
val className = when (typeName) {
is ClassName -> typeName
is ParameterizedTypeName -> typeName.rawType
@ -185,62 +188,106 @@ class JsonClassCodeGenProcessor : KotlinAbstractProcessor(), KotlinMetadataUtils
nameResolver.getString(it.name)
}
// The compiler might emit methods just so it has a place to put annotations. Find these.
val annotatedElements = mutableMapOf<Property, ExecutableElement>()
for (enclosedElement in element.enclosedElements) {
if (enclosedElement !is ExecutableElement) continue
val property = classData.getPropertyOrNull(enclosedElement) ?: continue
annotatedElements[property] = enclosedElement
val annotationHolders = mutableMapOf<Property, ExecutableElement>()
val fields = mutableMapOf<String, VariableElement>()
val setters = mutableMapOf<String, ExecutableElement>()
val getters = mutableMapOf<String, ExecutableElement>()
for (element in model.enclosedElements) {
if (element is VariableElement) {
fields[element.name] = element
} else if (element is ExecutableElement) {
when {
element.name.startsWith("get") -> {
getters[element.name.substring("get".length).decapitalizeAsciiOnly()] = element
}
element.name.startsWith("is") -> {
getters[element.name.substring("is".length).decapitalizeAsciiOnly()] = element
}
element.name.startsWith("set") -> {
setters[element.name.substring("set".length).decapitalizeAsciiOnly()] = element
}
}
val property = classData.getPropertyOrNull(element)
if (property != null) {
annotationHolders[property] = element
}
}
}
val propertyGenerators = mutableListOf<PropertyGenerator>()
for (enclosedElement in element.enclosedElements) {
if (enclosedElement !is VariableElement) continue
val propertiesByName = mutableMapOf<String, PropertyGenerator>()
for (property in properties.values) {
val name = nameResolver.getString(property.name)
val fieldElement = fields[name]
val setterElement = setters[name]
val getterElement = getters[name]
val element = fieldElement ?: setterElement ?: getterElement!!
val name = enclosedElement.simpleName.toString()
val property = properties[name] ?: continue
val parameter = parameters[name]
val parameterElement = if (parameter != null) {
val parameterIndex = protoConstructor.valueParameterList.indexOf(parameter)
constructor.parameters[parameterIndex]
} else {
null
var parameterIndex: Int = -1
var parameterElement: VariableElement? = null
if (parameter != null) {
parameterIndex = protoConstructor.valueParameterList.indexOf(parameter)
parameterElement = constructor.parameters[parameterIndex]
}
val annotatedElement = annotatedElements[property]
val annotationHolder = annotationHolders[property]
if (property.visibility != Visibility.INTERNAL
&& property.visibility != Visibility.PROTECTED
&& property.visibility != Visibility.PUBLIC) {
messager.printMessage(ERROR, "property $name is not visible", enclosedElement)
messager.printMessage(ERROR, "property $name is not visible", element)
return null
}
val hasDefault = parameter?.declaresDefaultValue ?: true
if (Modifier.TRANSIENT in enclosedElement.modifiers) {
if (Modifier.TRANSIENT in element.modifiers) {
if (!hasDefault) {
throw IllegalArgumentException("No default value for transient property $name")
messager.printMessage(
ERROR, "No default value for transient property $name", element)
return null
}
continue
continue // This property is transient and has a default value. Ignore it.
}
if (!property.hasSetter && parameter == null) {
continue // This property is not settable. Ignore it.
}
val delegateKey = DelegateKey(
property.returnType.asTypeName(nameResolver, classProto::getTypeParameter, true),
jsonQualifiers(enclosedElement, annotatedElement, parameterElement))
jsonQualifiers(element, annotationHolder, parameterElement))
propertyGenerators += PropertyGenerator(
propertiesByName[name] = PropertyGenerator(
delegateKey,
name,
jsonName(name, enclosedElement, annotatedElement, parameterElement),
parameter != null,
jsonName(name, element, annotationHolder, parameterElement),
parameterIndex,
hasDefault,
property.returnType.asTypeName(nameResolver, classProto::getTypeParameter))
}
for (parameterElement in constructor.parameters) {
val name = parameterElement.name
val valueParameter = parameters[name]!!
if (properties[name] == null && !valueParameter.declaresDefaultValue) {
messager.printMessage(
ERROR, "No property for required constructor parameter $name", parameterElement)
return null
}
}
// Sort properties so that those with constructor parameters come first.
propertyGenerators.sortBy { if (it.hasConstructorParameter) -1 else 1 }
val propertyGenerators = propertiesByName.values.toMutableList()
propertyGenerators.sortBy {
if (it.hasConstructorParameter) {
it.parameterIndex
} else {
Integer.MAX_VALUE
}
}
val genericTypeNames = classProto.typeParameterList
.map {
@ -268,9 +315,9 @@ class JsonClassCodeGenProcessor : KotlinAbstractProcessor(), KotlinMetadataUtils
}
return AdapterGenerator(
className,
className = className,
propertyList = propertyGenerators,
originalElement = element,
originalElement = model,
hasCompanionObject = hasCompanionObject,
visibility = classProto.visibility!!,
genericTypeNames = genericTypeNames,
@ -280,18 +327,18 @@ class JsonClassCodeGenProcessor : KotlinAbstractProcessor(), KotlinMetadataUtils
/** Returns the JsonQualifiers on the field and parameter of a property. */
private fun jsonQualifiers(
field: VariableElement,
method: ExecutableElement?,
element: Element,
annotationHolder: ExecutableElement?,
parameter: VariableElement?
): Set<AnnotationMirror> {
val fieldQualifiers = field.qualifiers
val methodQualifiers = method.qualifiers
val elementQualifiers = element.qualifiers
val annotationHolderQualifiers = annotationHolder.qualifiers
val parameterQualifiers = parameter.qualifiers
// TODO(jwilson): union the qualifiers somehow?
return when {
fieldQualifiers.isNotEmpty() -> fieldQualifiers
methodQualifiers.isNotEmpty() -> methodQualifiers
elementQualifiers.isNotEmpty() -> elementQualifiers
annotationHolderQualifiers.isNotEmpty() -> annotationHolderQualifiers
parameterQualifiers.isNotEmpty() -> parameterQualifiers
else -> setOf()
}
@ -300,28 +347,22 @@ class JsonClassCodeGenProcessor : KotlinAbstractProcessor(), KotlinMetadataUtils
/** Returns the @Json name of a property, or `propertyName` if none is provided. */
private fun jsonName(
propertyName: String,
field: VariableElement,
method: ExecutableElement?,
element: Element,
annotationHolder: ExecutableElement?,
parameter: VariableElement?
): String {
val fieldJsonName = field.jsonName
val methodJsonName = method.jsonName
val fieldJsonName = element.jsonName
val annotationHolderJsonName = annotationHolder.jsonName
val parameterJsonName = parameter.jsonName
return when {
fieldJsonName != null -> fieldJsonName
methodJsonName != null -> methodJsonName
annotationHolderJsonName != null -> annotationHolderJsonName
parameterJsonName != null -> parameterJsonName
else -> propertyName
}
}
private fun errorMustBeKotlinClass(element: Element) {
messager.printMessage(ERROR,
"@${JsonClass::class.java.simpleName} can't be applied to $element: must be a Kotlin class",
element)
}
private fun AdapterGenerator.generateAndWrite(generatedOption: TypeElement?) {
val fileSpec = generateFile(generatedOption)
val adapterName = fileSpec.members.filterIsInstance<TypeSpec>().first().name!!
@ -348,3 +389,8 @@ class JsonClassCodeGenProcessor : KotlinAbstractProcessor(), KotlinMetadataUtils
return getAnnotation(Json::class.java)?.name
}
}
private val Element.name: String
get() {
return simpleName.toString()
}

5
kotlin-codegen/compiler/src/main/java/com/squareup/moshi/PropertyGenerator.kt
View file

@ -25,7 +25,7 @@ internal class PropertyGenerator(
val delegateKey: DelegateKey,
val name: String,
val serializedName: String,
val hasConstructorParameter: Boolean,
val parameterIndex: Int,
val hasDefault: Boolean,
val typeName: TypeName
) {
@ -35,6 +35,9 @@ internal class PropertyGenerator(
val isRequired
get() = !delegateKey.nullable && !hasDefault
val hasConstructorParameter
get() = parameterIndex != -1
/** We prefer to use 'null' to mean absent, but for some properties those are distinct. */
val differentiateAbsentFromNull
get() = delegateKey.nullable && hasDefault

34
kotlin-codegen/compiler/src/test/java/com/squareup/moshi/CompilerTest.kt
View file

@ -154,4 +154,38 @@ class CompilerTest {
assertThat(result.systemErr).contains(
"error: @JsonClass can't be applied to expression\$annotations(): must be a Kotlin class")
}
@Test fun requiredTransientConstructorParameterFails() {
val call = KotlinCompilerCall(temporaryFolder.root)
call.inheritClasspath = true
call.addService(Processor::class, JsonClassCodeGenProcessor::class)
call.addKt("source.kt", """
|import com.squareup.moshi.JsonClass
|
|@JsonClass(generateAdapter = true)
|class RequiredTransientConstructorParameter(@Transient var a: Int)
|""".trimMargin())
val result = call.execute()
assertThat(result.exitCode).isEqualTo(ExitCode.COMPILATION_ERROR)
assertThat(result.systemErr).contains(
"error: No default value for transient property a")
}
@Test fun nonPropertyConstructorParameter() {
val call = KotlinCompilerCall(temporaryFolder.root)
call.inheritClasspath = true
call.addService(Processor::class, JsonClassCodeGenProcessor::class)
call.addKt("source.kt", """
|import com.squareup.moshi.JsonClass
|
|@JsonClass(generateAdapter = true)
|class NonPropertyConstructorParameter(a: Int, val b: Int)
|""".trimMargin())
val result = call.execute()
assertThat(result.exitCode).isEqualTo(ExitCode.COMPILATION_ERROR)
assertThat(result.systemErr).contains(
"error: No property for required constructor parameter a")
}
}

92
kotlin-codegen/integration-test/src/test/kotlin/com/squareup/moshi/GeneratedAdaptersTest.kt
View file

@ -20,6 +20,7 @@ import org.intellij.lang.annotations.Language
import org.junit.Assert.fail
import org.junit.Test
import java.util.Locale
import java.util.SimpleTimeZone
class GeneratedAdaptersTest {
@ -81,10 +82,11 @@ class GeneratedAdaptersTest {
}
@JsonClass(generateAdapter = true)
data class DefaultValues(val foo: String,
val bar: String = "",
val nullableBar: String? = null,
val bazList: List<String> = emptyList())
data class DefaultValues(
val foo: String,
val bar: String = "",
val nullableBar: String? = null,
val bazList: List<String> = emptyList())
@Test
fun nullableArray() {
@ -626,6 +628,88 @@ class GeneratedAdaptersTest {
var v26: Int, var v27: Int, var v28: Int, var v29: Int, var v30: Int,
var v31: Int, var v32: Int, var v33: Int)
@Test fun extendsPlatformClassWithPrivateField() {
val moshi = Moshi.Builder().build()
val jsonAdapter = moshi.adapter(ExtendsPlatformClassWithPrivateField::class.java)
val encoded = ExtendsPlatformClassWithPrivateField(3)
assertThat(jsonAdapter.toJson(encoded)).isEqualTo("""{"a":3}""")
val decoded = jsonAdapter.fromJson("""{"a":4,"id":"B"}""")!!
assertThat(decoded.a).isEqualTo(4)
assertThat(decoded.id).isEqualTo("C")
}
@JsonClass(generateAdapter = true)
internal class ExtendsPlatformClassWithPrivateField(var a: Int) : SimpleTimeZone(0, "C")
@Test fun unsettablePropertyIgnored() {
val moshi = Moshi.Builder().build()
val jsonAdapter = moshi.adapter(UnsettableProperty::class.java)
val encoded = UnsettableProperty()
encoded.b = 5
assertThat(jsonAdapter.toJson(encoded)).isEqualTo("""{"b":5}""")
val decoded = jsonAdapter.fromJson("""{"a":4,"b":6}""")!!
assertThat(decoded.a).isEqualTo(-1)
assertThat(decoded.b).isEqualTo(6)
}
@JsonClass(generateAdapter = true)
class UnsettableProperty {
val a: Int = -1
var b: Int = -1
}
@Test fun getterOnlyNoBackingField() {
val moshi = Moshi.Builder().build()
val jsonAdapter = moshi.adapter(GetterOnly::class.java)
val encoded = GetterOnly(3, 5)
assertThat(jsonAdapter.toJson(encoded)).isEqualTo("""{"a":3,"b":5}""")
val decoded = jsonAdapter.fromJson("""{"a":4,"b":6}""")!!
assertThat(decoded.a).isEqualTo(4)
assertThat(decoded.b).isEqualTo(6)
assertThat(decoded.total).isEqualTo(10)
}
@JsonClass(generateAdapter = true)
class GetterOnly(var a: Int, var b: Int) {
val total : Int
get() = a + b
}
@Test fun getterAndSetterNoBackingField() {
val moshi = Moshi.Builder().build()
val jsonAdapter = moshi.adapter(GetterAndSetter::class.java)
val encoded = GetterAndSetter(3, 5)
assertThat(jsonAdapter.toJson(encoded)).isEqualTo("""{"a":3,"b":5,"total":8}""")
// Whether b is 6 or 7 is an implementation detail. Currently we call constructors then setters.
val decoded1 = jsonAdapter.fromJson("""{"a":4,"b":6,"total":11}""")!!
assertThat(decoded1.a).isEqualTo(4)
assertThat(decoded1.b).isEqualTo(7)
assertThat(decoded1.total).isEqualTo(11)
// Whether b is 6 or 7 is an implementation detail. Currently we call constructors then setters.
val decoded2 = jsonAdapter.fromJson("""{"a":4,"total":11,"b":6}""")!!
assertThat(decoded2.a).isEqualTo(4)
assertThat(decoded2.b).isEqualTo(7)
assertThat(decoded2.total).isEqualTo(11)
}
@JsonClass(generateAdapter = true)
class GetterAndSetter(var a: Int, var b: Int) {
var total : Int
get() = a + b
set(value) {
b = value - a
}
}
@Retention(AnnotationRetention.RUNTIME)
@JsonQualifier
annotation class Uppercase

124
kotlin-codegen/integration-test/src/test/kotlin/com/squareup/moshi/KotlinCodeGenTest.kt
View file

@ -20,9 +20,6 @@ import org.junit.Assert.fail
import org.junit.Ignore
import org.junit.Test
import java.io.ByteArrayOutputStream
import java.util.Locale
import java.util.SimpleTimeZone
import kotlin.annotation.AnnotationRetention.RUNTIME
class KotlinCodeGenTest {
@Ignore @Test fun duplicatedValue() {
@ -53,20 +50,6 @@ class KotlinCodeGenTest {
class RepeatedValue(var a: Int, var b: Int?)
@Ignore @Test fun requiredTransientConstructorParameterFails() {
val moshi = Moshi.Builder().build()
try {
moshi.adapter(RequiredTransientConstructorParameter::class.java)
fail()
} catch (expected: IllegalArgumentException) {
assertThat(expected).hasMessage("No default value for transient constructor parameter #0 " +
"a of fun <init>(kotlin.Int): " +
"com.squareup.moshi.KotlinJsonAdapterTest.RequiredTransientConstructorParameter")
}
}
class RequiredTransientConstructorParameter(@Transient var a: Int)
@Ignore @Test fun supertypeConstructorParameters() {
val moshi = Moshi.Builder().build()
val jsonAdapter = moshi.adapter(SubtypeConstructorParameters::class.java)
@ -105,20 +88,6 @@ class KotlinCodeGenTest {
var b: Int = -1
}
@Ignore @Test fun extendsPlatformClassWithPrivateField() {
val moshi = Moshi.Builder().build()
val jsonAdapter = moshi.adapter(ExtendsPlatformClassWithPrivateField::class.java)
val encoded = ExtendsPlatformClassWithPrivateField(3)
assertThat(jsonAdapter.toJson(encoded)).isEqualTo("""{"a":3}""")
val decoded = jsonAdapter.fromJson("""{"a":4,"id":"B"}""")!!
assertThat(decoded.a).isEqualTo(4)
assertThat(decoded.id).isEqualTo("C")
}
internal class ExtendsPlatformClassWithPrivateField(var a: Int) : SimpleTimeZone(0, "C")
@Ignore @Test fun extendsPlatformClassWithProtectedField() {
val moshi = Moshi.Builder().build()
val jsonAdapter = moshi.adapter(ExtendsPlatformClassWithProtectedField::class.java)
@ -216,84 +185,6 @@ class KotlinCodeGenTest {
}
}
@Ignore @Test fun unsettablePropertyIgnored() {
val moshi = Moshi.Builder().build()
val jsonAdapter = moshi.adapter(UnsettableProperty::class.java)
val encoded = UnsettableProperty()
encoded.b = 5
assertThat(jsonAdapter.toJson(encoded)).isEqualTo("""{"b":5}""")
val decoded = jsonAdapter.fromJson("""{"a":4,"b":6}""")!!
assertThat(decoded.a).isEqualTo(-1)
assertThat(decoded.b).isEqualTo(6)
}
class UnsettableProperty {
val a: Int = -1
var b: Int = -1
}
@Ignore @Test fun getterOnlyNoBackingField() {
val moshi = Moshi.Builder().build()
val jsonAdapter = moshi.adapter(GetterOnly::class.java)
val encoded = GetterOnly(3, 5)
assertThat(jsonAdapter.toJson(encoded)).isEqualTo("""{"a":3,"b":5}""")
val decoded = jsonAdapter.fromJson("""{"a":4,"b":6}""")!!
assertThat(decoded.a).isEqualTo(4)
assertThat(decoded.b).isEqualTo(6)
assertThat(decoded.total).isEqualTo(10)
}
class GetterOnly(var a: Int, var b: Int) {
val total : Int
get() = a + b
}
@Ignore @Test fun getterAndSetterNoBackingField() {
val moshi = Moshi.Builder().build()
val jsonAdapter = moshi.adapter(GetterAndSetter::class.java)
val encoded = GetterAndSetter(3, 5)
assertThat(jsonAdapter.toJson(encoded)).isEqualTo("""{"a":3,"b":5,"total":8}""")
// Whether b is 6 or 7 is an implementation detail. Currently we call constructors then setters.
val decoded1 = jsonAdapter.fromJson("""{"a":4,"b":6,"total":11}""")!!
assertThat(decoded1.a).isEqualTo(4)
assertThat(decoded1.b).isEqualTo(7)
assertThat(decoded1.total).isEqualTo(11)
// Whether b is 6 or 7 is an implementation detail. Currently we call constructors then setters.
val decoded2 = jsonAdapter.fromJson("""{"a":4,"total":11,"b":6}""")!!
assertThat(decoded2.a).isEqualTo(4)
assertThat(decoded2.b).isEqualTo(7)
assertThat(decoded2.total).isEqualTo(11)
}
class GetterAndSetter(var a: Int, var b: Int) {
var total : Int
get() = a + b
set(value) {
b = value - a
}
}
@Ignore @Test fun nonPropertyConstructorParameter() {
val moshi = Moshi.Builder().build()
try {
moshi.adapter(NonPropertyConstructorParameter::class.java)
fail()
} catch(expected: IllegalArgumentException) {
assertThat(expected).hasMessage(
"No property for required constructor parameter #0 a of fun <init>(" +
"kotlin.Int, kotlin.Int): ${NonPropertyConstructorParameter::class.qualifiedName}")
}
}
class NonPropertyConstructorParameter(a: Int, val b: Int)
@Ignore @Test fun kotlinEnumsAreNotCovered() {
val moshi = Moshi.Builder().build()
val adapter = moshi.adapter(UsingEnum::class.java)
@ -306,19 +197,4 @@ class KotlinCodeGenTest {
enum class KotlinEnum {
A, B
}
// TODO(jwilson): resolve generic types?
@Retention(RUNTIME)
@JsonQualifier
annotation class Uppercase
class UppercaseJsonAdapter {
@ToJson fun toJson(@Uppercase s: String) : String {
return s.toUpperCase(Locale.US)
}
@FromJson @Uppercase fun fromJson(s: String) : String {
return s.toLowerCase(Locale.US)
}
}
}