Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overhaul internals to use KType #1549

Draft
wants to merge 27 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
d7ac1d9
Introduce JsonAdapter.KFactory
ZacSweers Jun 22, 2022
7709ef9
Make JsonAdapter's generic the source of truth for nullability
ZacSweers Jun 22, 2022
a947fd8
Import JavaArray as an alias
ZacSweers Jun 22, 2022
d84d2a4
Implement KType and KTypeProjection for internals use
ZacSweers Jun 22, 2022
b1ae4c7
Use KType for internal type modeling
ZacSweers Jun 22, 2022
51b0960
Remove null handling in deprecated adapter(KType) function
ZacSweers Jun 22, 2022
74ceced
Support new nullability in AdapterGenerator
ZacSweers Jun 22, 2022
fbbd4fd
Update NonNullJsonAdapter + doc behavior
ZacSweers Jun 22, 2022
12969ff
Update various adapters for new nullability behavior
ZacSweers Jun 22, 2022
9e1d218
Update various tests for new nullability and error message changes
ZacSweers Jun 22, 2022
60409d4
Make NonNullJsonAdapter error message match Util.unexpectedNull better
ZacSweers Jun 22, 2022
995605b
Introduce notion of NullAwareJsonAdapter to simplify things
ZacSweers Jun 23, 2022
613c2f0
Move KType extensions to new KTypes.kt file
ZacSweers Jun 23, 2022
6514561
Ignore test for now pending CR
ZacSweers Jun 23, 2022
9fed481
KType move import fixes
ZacSweers Jun 23, 2022
e9e8f98
Update some test error messages for new null message
ZacSweers Jun 23, 2022
e58fb7c
Add more KType helper APIs for code gen
ZacSweers Jun 23, 2022
1d5fd9c
Update code gen to use KTypes instead
ZacSweers Jun 23, 2022
5d4b030
Add dots for readability
ZacSweers Jun 23, 2022
5e43aae
Update doc
ZacSweers Jun 23, 2022
00247f5
Update japicmp
ZacSweers Jun 23, 2022
3c8b186
Implement KType in Builder + remove experimental annotations where po…
ZacSweers Jun 23, 2022
174f718
Use stable typeOf() in generated adapters now
ZacSweers Jun 23, 2022
aad3145
Introduce notion of platform types and make Type conversions nullable…
ZacSweers Jun 24, 2022
b8ba12b
Restore accidental commit
ZacSweers Jun 24, 2022
ea155db
Fix test infinitely recursing
ZacSweers Jun 24, 2022
186ff87
Fix another accidental change
ZacSweers Jun 24, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ class JsonStringJsonAdapterFactory : JsonAdapter.Factory {
override fun fromJson(reader: JsonReader): String =
reader.nextSource().use(BufferedSource::readUtf8)

override fun toJson(writer: JsonWriter, value: String?) {
writer.valueSink().use { sink -> sink.writeUtf8(checkNotNull(value)) }
override fun toJson(writer: JsonWriter, value: String) {
writer.valueSink().use { sink -> sink.writeUtf8(value) }
}
}
}
Expand All @@ -62,7 +62,7 @@ fun main() {
.add(JsonStringJsonAdapterFactory())
.build()

val example: ExampleClass = moshi.adapter(ExampleClass::class.java).fromJson(json)!!
val example: ExampleClass = moshi.adapter(ExampleClass::class.java).fromJson(json)

check(example.type == 1)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
package com.squareup.moshi.recipes

import com.squareup.moshi.Moshi
import com.squareup.moshi.adapter
import com.squareup.moshi.recipes.models.Card

class ReadJsonListKt {
Expand All @@ -36,7 +35,7 @@ class ReadJsonListKt {

fun readJsonList() {
val jsonAdapter = Moshi.Builder().build().adapter<List<Card>>()
val cards = jsonAdapter.fromJson(jsonString)!!
val cards = jsonAdapter.fromJson(jsonString)
println(cards)
cards[0].run {
println(rank)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import java.util.Date
replaceWith = ReplaceWith("com.squareup.moshi.adapters.Rfc3339DateJsonAdapter"),
level = DeprecationLevel.ERROR
)
public class Rfc3339DateJsonAdapter : JsonAdapter<Date>() {
public class Rfc3339DateJsonAdapter : JsonAdapter<Date?>() {

private val delegate = Rfc3339DateJsonAdapter()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public class EnumJsonAdapter<T : Enum<T>> internal constructor(
private val enumType: Class<T>,
private val fallbackValue: T?,
private val useFallbackValue: Boolean,
) : JsonAdapter<T>() {
) : JsonAdapter<T?>() {

private val constants: Array<T>
private val options: Options
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public class PolymorphicJsonAdapterFactory<T> internal constructor(
private val labelKey: String,
private val labels: List<String>,
private val subtypes: List<Type>,
private val fallbackJsonAdapter: JsonAdapter<Any>?
private val fallbackJsonAdapter: JsonAdapter<Any?>?
) : Factory {
/** Returns a new factory that decodes instances of `subtype`. */
public fun withSubtype(subtype: Class<out T>, label: String): PolymorphicJsonAdapterFactory<T> {
Expand Down Expand Up @@ -133,7 +133,7 @@ public class PolymorphicJsonAdapterFactory<T> internal constructor(
* it within your implementation of [JsonAdapter.fromJson]
*/
public fun withFallbackJsonAdapter(
fallbackJsonAdapter: JsonAdapter<Any>?
fallbackJsonAdapter: JsonAdapter<Any?>?
): PolymorphicJsonAdapterFactory<T> {
return PolymorphicJsonAdapterFactory(
baseType = baseType,
Expand All @@ -152,8 +152,8 @@ public class PolymorphicJsonAdapterFactory<T> internal constructor(
return withFallbackJsonAdapter(buildFallbackJsonAdapter(defaultValue))
}

private fun buildFallbackJsonAdapter(defaultValue: T?): JsonAdapter<Any> {
return object : JsonAdapter<Any>() {
private fun buildFallbackJsonAdapter(defaultValue: T?): JsonAdapter<Any?> {
return object : JsonAdapter<Any?>() {
override fun fromJson(reader: JsonReader): Any? {
reader.skipValue()
return defaultValue
Expand Down Expand Up @@ -181,8 +181,8 @@ public class PolymorphicJsonAdapterFactory<T> internal constructor(
private val labels: List<String>,
private val subtypes: List<Type>,
private val jsonAdapters: List<JsonAdapter<Any>>,
private val fallbackJsonAdapter: JsonAdapter<Any>?
) : JsonAdapter<Any>() {
private val fallbackJsonAdapter: JsonAdapter<Any?>?
) : JsonAdapter<Any?>() {
/** Single-element options containing the label's key only. */
private val labelKeyOptions: Options = Options.of(labelKey)

Expand Down Expand Up @@ -223,12 +223,13 @@ public class PolymorphicJsonAdapterFactory<T> internal constructor(
override fun toJson(writer: JsonWriter, value: Any?) {
val type: Class<*> = value!!.javaClass
val labelIndex = subtypes.indexOf(type)
val adapter: JsonAdapter<Any> = if (labelIndex == -1) {
val adapter: JsonAdapter<Any?> = if (labelIndex == -1) {
requireNotNull(fallbackJsonAdapter) {
"Expected one of $subtypes but found $value, a ${value.javaClass}. Register this subtype."
}
} else {
jsonAdapters[labelIndex]
@Suppress("UNCHECKED_CAST")
jsonAdapters[labelIndex] as JsonAdapter<Any?>
}
writer.beginObject()
if (adapter !== fallbackJsonAdapter) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import java.util.Date
* .build();
* ```
*/
public class Rfc3339DateJsonAdapter : JsonAdapter<Date>() {
public class Rfc3339DateJsonAdapter : JsonAdapter<Date?>() {
@Synchronized
@Throws(IOException::class)
override fun fromJson(reader: JsonReader): Date? {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ public class AdapterGenerator(
// https://github.com/square/moshi/issues/1502
private val valueParam = ParameterSpec.builder(
"value",
originalTypeName.copy(nullable = true)
originalTypeName
)
.build()
private val jsonAdapterTypeName = JsonAdapter::class.asClassName().parameterizedBy(
Expand Down Expand Up @@ -260,7 +260,8 @@ public class AdapterGenerator(

val typeRenderer: TypeRenderer = object : TypeRenderer() {
override fun renderTypeVariable(typeVariable: TypeVariableName): CodeBlock {
val index = typeVariables.indexOfFirst { it == typeVariable }
val nonNull = typeVariable.copy(nullable = false)
val index = typeVariables.indexOfFirst { it == nonNull }
check(index != -1) { "Unexpected type variable $typeVariable" }
return CodeBlock.of("%N[%L]", typesParam, index)
}
Expand Down Expand Up @@ -419,23 +420,12 @@ public class AdapterGenerator(
// Proceed as usual
if (property.hasLocalIsPresentName || property.hasConstructorDefault) {
result.beginControlFlow("%L ->", propertyIndex)
if (property.delegateKey.nullable) {
result.addStatement(
"%N = %N.fromJson(%N)",
property.localName,
nameAllocator[property.delegateKey],
readerParam
)
} else {
val exception = unexpectedNull(property, readerParam)
result.addStatement(
"%N = %N.fromJson(%N) ?: throw·%L",
property.localName,
nameAllocator[property.delegateKey],
readerParam,
exception
)
}
result.addStatement(
"%N = %N.fromJson(%N)",
property.localName,
nameAllocator[property.delegateKey],
readerParam
)
if (property.hasConstructorDefault) {
val inverted = (1 shl maskIndex).inv()
if (input is ParameterComponent && input.parameter.hasDefault) {
Expand All @@ -453,25 +443,13 @@ public class AdapterGenerator(
}
result.endControlFlow()
} else {
if (property.delegateKey.nullable) {
result.addStatement(
"%L -> %N = %N.fromJson(%N)",
propertyIndex,
property.localName,
nameAllocator[property.delegateKey],
readerParam
)
} else {
val exception = unexpectedNull(property, readerParam)
result.addStatement(
"%L -> %N = %N.fromJson(%N) ?: throw·%L",
propertyIndex,
property.localName,
nameAllocator[property.delegateKey],
readerParam,
exception
)
}
result.addStatement(
"%L -> %N = %N.fromJson(%N)",
propertyIndex,
property.localName,
nameAllocator[property.delegateKey],
readerParam
)
}
if (property.hasConstructorParameter) {
constructorPropertyTypes += property.target.type.asTypeBlock()
Expand Down Expand Up @@ -672,14 +650,6 @@ public class AdapterGenerator(
.addParameter(writerParam)
.addParameter(valueParam)

result.beginControlFlow("if (%N == null)", valueParam)
result.addStatement(
"throw·%T(%S)",
NullPointerException::class,
"${valueParam.name} was null! Wrap in .nullSafe() to write nullable values."
)
result.endControlFlow()

result.addStatement("%N.beginObject()", writerParam)
nonTransientProperties.forEach { property ->
// We manually put in quotes because we know the jsonName is already escaped
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,25 +15,17 @@
*/
package com.squareup.moshi.kotlin.codegen.api

import com.squareup.kotlinpoet.ARRAY
import com.squareup.kotlinpoet.BOOLEAN
import com.squareup.kotlinpoet.BYTE
import com.squareup.kotlinpoet.CHAR
import com.squareup.kotlinpoet.ClassName
import com.squareup.kotlinpoet.CodeBlock
import com.squareup.kotlinpoet.DOUBLE
import com.squareup.kotlinpoet.FLOAT
import com.squareup.kotlinpoet.INT
import com.squareup.kotlinpoet.LONG
import com.squareup.kotlinpoet.LambdaTypeName
import com.squareup.kotlinpoet.MemberName
import com.squareup.kotlinpoet.ParameterizedTypeName
import com.squareup.kotlinpoet.SHORT
import com.squareup.kotlinpoet.TypeName
import com.squareup.kotlinpoet.TypeVariableName
import com.squareup.kotlinpoet.WildcardTypeName
import com.squareup.moshi.Types

/**
* Renders literals like `Types.newParameterizedType(List::class.java, String::class.java)`.
* Renders literals like `List::class.parameterizedBy(String::class.asKTypeProjection(...))`.
* Rendering is pluggable so that type variables can either be resolved or emitted as other code
* blocks.
*/
Expand All @@ -44,83 +36,17 @@ internal abstract class TypeRenderer {
if (typeName.annotations.isNotEmpty()) {
return render(typeName.copy(annotations = emptyList()), forceBox)
}
if (typeName.isNullable) {
return renderObjectType(typeName.copy(nullable = false))
}

return when (typeName) {
is ClassName -> {
if (forceBox) {
renderObjectType(typeName)
} else {
CodeBlock.of("%T::class.java", typeName)
}
}

is ParameterizedTypeName -> {
// If it's an Array type, we shortcut this to return Types.arrayOf()
if (typeName.rawType == ARRAY) {
CodeBlock.of(
"%T.arrayOf(%L)",
Types::class,
renderObjectType(typeName.typeArguments[0])
)
} else {
val builder = CodeBlock.builder().apply {
add("%T.", Types::class)
val enclosingClassName = typeName.rawType.enclosingClassName()
if (enclosingClassName != null) {
add("newParameterizedTypeWithOwner(%L, ", render(enclosingClassName))
} else {
add("newParameterizedType(")
}
add("%T::class.java", typeName.rawType)
for (typeArgument in typeName.typeArguments) {
add(", %L", renderObjectType(typeArgument))
}
add(")")
}
builder.build()
}
}

is WildcardTypeName -> {
val target: TypeName
val method: String
when {
typeName.inTypes.size == 1 -> {
target = typeName.inTypes[0]
method = "supertypeOf"
}
typeName.outTypes.size == 1 -> {
target = typeName.outTypes[0]
method = "subtypeOf"
}
else -> throw IllegalArgumentException(
"Unrepresentable wildcard type. Cannot have more than one bound: $typeName"
)
}
CodeBlock.of("%T.%L(%L)", Types::class, method, render(target, forceBox = true))
}

is TypeVariableName -> renderTypeVariable(typeName)

is ClassName, is LambdaTypeName, is ParameterizedTypeName, is WildcardTypeName -> {
CodeBlock.of(
"%M<%T>()",
MemberName("kotlin.reflect", "typeOf"),
typeName
)
}
else -> throw IllegalArgumentException("Unrepresentable type: $typeName")
}
}

private fun renderObjectType(typeName: TypeName): CodeBlock {
return if (typeName.isPrimitive()) {
CodeBlock.of("%T::class.javaObjectType", typeName)
} else {
render(typeName)
}
}

private fun TypeName.isPrimitive(): Boolean {
return when (this) {
BOOLEAN, BYTE, SHORT, INT, LONG, CHAR, FLOAT, DOUBLE -> true
else -> false
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class ComplexGenericsInheritanceTest {
val json =
"""{"data":{"name":"foo"},"data2":"bar","data3":"baz"}"""

val instance = adapter.fromJson(json)!!
val instance = adapter.fromJson(json)
val testInstance = PersonResponse().apply {
data = Person("foo")
}
Expand All @@ -51,7 +51,7 @@ class ComplexGenericsInheritanceTest {
val json =
"""{"data":{"name":"foo"},"data2":"bar","data3":"baz"}"""

val instance = adapter.fromJson(json)!!
val instance = adapter.fromJson(json)
val testInstance = NestedPersonResponse().apply {
data = Person("foo")
}
Expand All @@ -67,7 +67,7 @@ class ComplexGenericsInheritanceTest {
val json =
"""{"data":{"name":"foo"},"data2":"bar","data3":"baz"}"""

val instance = adapter.fromJson(json)!!
val instance = adapter.fromJson(json)
val testInstance = UntypedNestedPersonResponse<Person>().apply {
data = Person("foo")
}
Expand All @@ -83,7 +83,7 @@ class ComplexGenericsInheritanceTest {
val json =
"""{"layer4E":{"name":"layer4E"},"layer4F":{"data":{"name":"layer4F"},"data2":"layer4F","data3":"layer4F"},"layer3C":[1,2,3],"layer3D":"layer3D","layer2":"layer2","layer1":"layer1"}"""

val instance = adapter.fromJson(json)!!
val instance = adapter.fromJson(json)
val testInstance = Layer4(
layer4E = Person("layer4E"),
layer4F = UntypedNestedPersonResponse<Person>().apply {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class DefaultConstructorTest {
val json =
"""{"required":"requiredClass"}"""
val instance = Moshi.Builder().build().adapter<TestClass>()
.fromJson(json)!!
.fromJson(json)
check(instance == expected) {
"No match:\nActual : $instance\nExpected: $expected"
}
Expand All @@ -37,7 +37,7 @@ class DefaultConstructorTest {
val json =
"""{"required":"requiredClass","optional":"customOptional","optional2":4,"dynamicSelfReferenceOptional":"setDynamic","dynamicOptional":5,"dynamicInlineOptional":6}"""
val instance = Moshi.Builder().build().adapter<TestClass>()
.fromJson(json)!!
.fromJson(json)
check(instance == expected) {
"No match:\nActual : $instance\nExpected: $expected"
}
Expand All @@ -48,7 +48,7 @@ class DefaultConstructorTest {
val json =
"""{"required":"requiredClass","optional":"customOptional"}"""
val instance = Moshi.Builder().build().adapter<TestClass>()
.fromJson(json)!!
.fromJson(json)
check(instance == expected) {
"No match:\nActual : $instance\nExpected: $expected"
}
Expand Down
Loading