From c5e98e5be8d6e84152d5f28b927ae077c4292d50 Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Thu, 20 Jan 2022 10:23:22 -0500 Subject: [PATCH] Small cleanups from a few past PRs (#1509) --- .../squareup/moshi/AdapterMethodsFactory.kt | 68 ++++++++++--------- .../com/squareup/moshi/ClassJsonAdapter.kt | 18 +++-- .../src/main/java/com/squareup/moshi/Moshi.kt | 4 +- .../src/main/java/com/squareup/moshi/Types.kt | 28 ++++---- 4 files changed, 65 insertions(+), 53 deletions(-) diff --git a/moshi/src/main/java/com/squareup/moshi/AdapterMethodsFactory.kt b/moshi/src/main/java/com/squareup/moshi/AdapterMethodsFactory.kt index 85e6d8f..68467ce 100644 --- a/moshi/src/main/java/com/squareup/moshi/AdapterMethodsFactory.kt +++ b/moshi/src/main/java/com/squareup/moshi/AdapterMethodsFactory.kt @@ -95,7 +95,7 @@ internal class AdapterMethodsFactory( } companion object { - fun get(adapter: Any): AdapterMethodsFactory { + operator fun invoke(adapter: Any): AdapterMethodsFactory { val toAdapters = mutableListOf() val fromAdapters = mutableListOf() @@ -132,7 +132,7 @@ internal class AdapterMethodsFactory( * Returns an object that calls a `method` method on `adapter` in service of * converting an object to JSON. */ - fun toAdapter(adapter: Any, method: Method): AdapterMethod { + private fun toAdapter(adapter: Any, method: Method): AdapterMethod { method.isAccessible = true val returnType = method.genericReturnType val parameterTypes = method.genericParameterTypes @@ -142,10 +142,11 @@ internal class AdapterMethodsFactory( returnType == Void.TYPE && parametersAreJsonAdapters(2, parameterTypes) return when { + // void pointToJson(JsonWriter jsonWriter, Point point) { + // void pointToJson(JsonWriter jsonWriter, Point point, JsonAdapter adapter, ...) { methodSignatureIncludesJsonWriterAndJsonAdapter -> { - // void pointToJson(JsonWriter jsonWriter, Point point) { - // void pointToJson(JsonWriter jsonWriter, Point point, JsonAdapter adapter, ...) { val qualifierAnnotations = parameterAnnotations[1].jsonAnnotations + object : AdapterMethod( adaptersOffset = 2, type = parameterTypes[1], @@ -156,7 +157,7 @@ internal class AdapterMethodsFactory( nullable = true ) { override fun toJson(moshi: Moshi, writer: JsonWriter, value: Any?) { - invoke(writer, value) + invokeMethod(writer, value) } } } @@ -174,10 +175,14 @@ internal class AdapterMethodsFactory( method = method, nullable = nullable ) { + private lateinit var delegate: JsonAdapter + override fun bind(moshi: Moshi, factory: JsonAdapter.Factory) { super.bind(moshi, factory) - delegate = if (Types.equals(parameterTypes[0], returnType) && qualifierAnnotations == returnTypeAnnotations) { + val shouldSkip = Types.equals(parameterTypes[0], returnType) && + qualifierAnnotations == returnTypeAnnotations + delegate = if (shouldSkip) { moshi.nextAdapter(factory, returnType, returnTypeAnnotations) } else { moshi.adapter(returnType, returnTypeAnnotations) @@ -185,19 +190,20 @@ internal class AdapterMethodsFactory( } override fun toJson(moshi: Moshi, writer: JsonWriter, value: Any?) { - val intermediate = invoke(value) - delegate.toJson(writer, intermediate) + delegate.toJson(writer, invokeMethod(value)) } } } else -> { throw IllegalArgumentException( - """Unexpected signature for $method. -@ToJson method signatures may have one of the following structures: - void toJson(JsonWriter writer, T value) throws ; - void toJson(JsonWriter writer, T value, JsonAdapter delegate, ) throws ; - R toJson(T value) throws ; -""" + """ + Unexpected signature for $method. + @ToJson method signatures may have one of the following structures: + void toJson(JsonWriter writer, T value) throws ; + void toJson(JsonWriter writer, T value, JsonAdapter delegate, ) throws ; + R toJson(T value) throws ; + + """.trimIndent() ) } } @@ -217,7 +223,7 @@ internal class AdapterMethodsFactory( * Returns an object that calls a `method` method on `adapter` in service of * converting an object from JSON. */ - fun fromAdapter(adapter: Any, method: Method): AdapterMethod { + private fun fromAdapter(adapter: Any, method: Method): AdapterMethod { method.isAccessible = true val returnType = method.genericReturnType val returnTypeAnnotations = method.jsonAnnotations @@ -240,7 +246,7 @@ internal class AdapterMethodsFactory( method = method, nullable = true ) { - override fun fromJson(moshi: Moshi, reader: JsonReader) = invoke(reader) + override fun fromJson(moshi: Moshi, reader: JsonReader) = invokeMethod(reader) } } parameterTypes.size == 1 && returnType != Void.TYPE -> { @@ -269,18 +275,20 @@ internal class AdapterMethodsFactory( override fun fromJson(moshi: Moshi, reader: JsonReader): Any? { val intermediate = delegate.fromJson(reader) - return invoke(intermediate) + return invokeMethod(intermediate) } } } else -> { throw IllegalArgumentException( - """Unexpected signature for $method. -@FromJson method signatures may have one of the following structures: - R fromJson(JsonReader jsonReader) throws ; - R fromJson(JsonReader jsonReader, JsonAdapter delegate, ) throws ; - R fromJson(T value) throws ; -""" + """ + Unexpected signature for $method. + @FromJson method signatures may have one of the following structures: + R fromJson(JsonReader jsonReader) throws ; + R fromJson(JsonReader jsonReader, JsonAdapter delegate, ) throws ; + R fromJson(T value) throws ; + + """.trimIndent() ) } } @@ -322,9 +330,7 @@ internal class AdapterMethodsFactory( val jsonAnnotations = parameterAnnotations[i].jsonAnnotations jsonAdapters[i - adaptersOffset] = if (Types.equals(this.type, type) && annotations == jsonAnnotations) { - moshi.nextAdapter( - factory, type, jsonAnnotations - ) + moshi.nextAdapter(factory, type, jsonAnnotations) } else { moshi.adapter(type, jsonAnnotations) } @@ -337,9 +343,9 @@ internal class AdapterMethodsFactory( open fun fromJson(moshi: Moshi, reader: JsonReader): Any? = throw AssertionError() /** Invoke the method with one fixed argument, plus any number of JSON adapter arguments. */ - protected fun invoke(initialFixedArgumentForAdapterMethod: Any?): Any? { + protected fun invokeMethod(arg: Any?): Any? { val args = arrayOfNulls(1 + jsonAdapters.size) - args[0] = initialFixedArgumentForAdapterMethod + args[0] = arg jsonAdapters.copyInto(args, 1, 0, jsonAdapters.size) return try { @@ -350,10 +356,10 @@ internal class AdapterMethodsFactory( } /** Invoke the method with two fixed arguments, plus any number of JSON adapter arguments. */ - protected fun invoke(fixedArgument1: Any?, fixedArgument2: Any?): Any? { + protected fun invokeMethod(arg0: Any?, arg1: Any?): Any? { val args = arrayOfNulls(2 + jsonAdapters.size) - args[0] = fixedArgument1 - args[1] = fixedArgument2 + args[0] = arg0 + args[1] = arg1 jsonAdapters.copyInto(args, 2, 0, jsonAdapters.size) return try { diff --git a/moshi/src/main/java/com/squareup/moshi/ClassJsonAdapter.kt b/moshi/src/main/java/com/squareup/moshi/ClassJsonAdapter.kt index ef5da27..f625536 100644 --- a/moshi/src/main/java/com/squareup/moshi/ClassJsonAdapter.kt +++ b/moshi/src/main/java/com/squareup/moshi/ClassJsonAdapter.kt @@ -39,7 +39,6 @@ import java.lang.reflect.Type * * Fields from platform classes are omitted from both serialization and deserialization unless they * are either public or protected. This includes the following packages and their subpackages: - * * * `android.*` * * `androidx.*` * * `java.*` @@ -47,7 +46,6 @@ import java.lang.reflect.Type * * `kotlin.*` * * `kotlinx.*` * * `scala.*` - * */ internal class ClassJsonAdapter( private val classFactory: ClassFactory, @@ -103,12 +101,12 @@ internal class ClassJsonAdapter( internal class FieldBinding(val name: String, val field: Field, val adapter: JsonAdapter) { fun read(reader: JsonReader, value: Any?) { val fieldValue = adapter.fromJson(reader) - field[value] = fieldValue + field.set(value, fieldValue) } @Suppress("UNCHECKED_CAST") // We require that field's values are of type T. fun write(writer: JsonWriter, value: Any?) { - val fieldValue = field[value] as T + val fieldValue = field.get(value) as T adapter.toJson(writer, fieldValue) } } @@ -145,19 +143,24 @@ internal class ClassJsonAdapter( require(!rawType.isAnonymousClass) { "Cannot serialize anonymous class ${rawType.name}" } + require(!rawType.isLocalClass) { "Cannot serialize local class ${rawType.name}" } + val isNonStaticNestedClass = rawType.enclosingClass != null && !isStatic(rawType.modifiers) require(!isNonStaticNestedClass) { "Cannot serialize non-static nested class ${rawType.name}" } + require(!isAbstract(rawType.modifiers)) { "Cannot serialize abstract class ${rawType.name}" } + require(!rawType.isKotlin) { "Cannot serialize Kotlin type ${rawType.name}. Reflective serialization of Kotlin classes without using kotlin-reflect has undefined and unexpected behavior. Please use KotlinJsonAdapterFactory from the moshi-kotlin artifact or use code gen from the moshi-kotlin-codegen artifact." } + val classFactory = ClassFactory.get(rawType) val fields = sortedMapOf>() var parentType = type @@ -216,8 +219,11 @@ internal class ClassJsonAdapter( /** Returns true if fields with `modifiers` are included in the emitted JSON. */ private fun includeField(platformType: Boolean, modifiers: Int): Boolean { - if (isStatic(modifiers) || isTransient(modifiers)) return false - return isPublic(modifiers) || isProtected(modifiers) || !platformType + return if (isStatic(modifiers) || isTransient(modifiers)) { + false + } else { + isPublic(modifiers) || isProtected(modifiers) || !platformType + } } } } diff --git a/moshi/src/main/java/com/squareup/moshi/Moshi.kt b/moshi/src/main/java/com/squareup/moshi/Moshi.kt index 0b05e6e..66a69bc 100644 --- a/moshi/src/main/java/com/squareup/moshi/Moshi.kt +++ b/moshi/src/main/java/com/squareup/moshi/Moshi.kt @@ -213,7 +213,7 @@ public class Moshi internal constructor(builder: Builder) { } public fun add(adapter: Any): Builder = apply { - add(AdapterMethodsFactory.get(adapter)) + add(AdapterMethodsFactory(adapter)) } @Suppress("unused") @@ -236,7 +236,7 @@ public class Moshi internal constructor(builder: Builder) { @Suppress("unused") public fun addLast(adapter: Any): Builder = apply { - addLast(AdapterMethodsFactory.get(adapter)) + addLast(AdapterMethodsFactory(adapter)) } @CheckReturnValue diff --git a/moshi/src/main/java/com/squareup/moshi/Types.kt b/moshi/src/main/java/com/squareup/moshi/Types.kt index 6f85d4e..81e22fc 100644 --- a/moshi/src/main/java/com/squareup/moshi/Types.kt +++ b/moshi/src/main/java/com/squareup/moshi/Types.kt @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +@file:Suppress("PLATFORM_CLASS_MAPPED_TO_KOTLIN") package com.squareup.moshi import com.squareup.moshi.internal.EMPTY_TYPE_ARRAY @@ -30,6 +31,7 @@ import java.lang.reflect.WildcardType import java.util.Collections import java.util.Properties import javax.annotation.CheckReturnValue +import java.lang.annotation.Annotation as JavaAnnotation /** Factory methods for types. */ @CheckReturnValue @@ -62,7 +64,7 @@ public object Types { */ @JvmStatic public fun generatedJsonAdapterName(className: String): String { - return className.replace("$", "_") + "JsonAdapter" + return "${className.replace("$", "_")}JsonAdapter" } /** @@ -81,8 +83,7 @@ public object Types { return null } for (annotation in annotations) { - @Suppress("PLATFORM_CLASS_MAPPED_TO_KOTLIN") - if ((jsonQualifier == (annotation as java.lang.annotation.Annotation).annotationType())) { + if ((jsonQualifier == (annotation as JavaAnnotation).annotationType())) { val delegateAnnotations = LinkedHashSet(annotations) delegateAnnotations.remove(annotation) return Collections.unmodifiableSet(delegateAnnotations) @@ -178,7 +179,7 @@ public object Types { } is WildcardType -> getRawType(type.upperBounds[0]) else -> { - val className = if (type == null) "null" else type.javaClass.name + val className = type?.javaClass?.name?.toString() throw IllegalArgumentException("Expected a Class, ParameterizedType, or GenericArrayType, but <$type> is of type $className") } } @@ -259,19 +260,18 @@ public object Types { val field = clazz.getDeclaredField(fieldName) field.isAccessible = true val fieldAnnotations = field.declaredAnnotations - val annotations: MutableSet = LinkedHashSet(fieldAnnotations.size) - for (annotation in fieldAnnotations) { - @Suppress("PLATFORM_CLASS_MAPPED_TO_KOTLIN") - if ((annotation as java.lang.annotation.Annotation).annotationType() - .isAnnotationPresent(JsonQualifier::class.java) - ) { - annotations.add(annotation) + return buildSet(fieldAnnotations.size) { + for (annotation in fieldAnnotations) { + val hasJsonQualifier = (annotation as JavaAnnotation).annotationType() + .isAnnotationPresent(JsonQualifier::class.java) + if (hasJsonQualifier) { + add(annotation) + } } } - return Collections.unmodifiableSet(annotations) } catch (e: NoSuchFieldException) { throw IllegalArgumentException( - "Could not access field " + fieldName + " on class " + clazz.canonicalName, e + "Could not access field $fieldName on class ${clazz.canonicalName}", e ) } } @@ -298,7 +298,7 @@ public object Types { annotationType.isInstance(o) } "hashCode" -> 0 - "toString" -> "@" + annotationType.name + "()" + "toString" -> "@${annotationType.name}()" else -> method.invoke(proxy, *args) } } as T