Merge pull request #510 from k163377/refactor-hasCreatorAnnotation

Refactor KNAI.hasCreatorAnnotation

Author: dinomite

src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinNamesAnnotationIntrospector.kt

@@ -14,12 +14,13 @@ import com.fasterxml.jackson.databind.introspect.NopAnnotationIntrospector
 import com.fasterxml.jackson.databind.util.BeanUtil
 import java.lang.reflect.Constructor
 import java.lang.reflect.Method
-import java.util.*
+import java.util.Locale
 import kotlin.reflect.KClass
 import kotlin.reflect.KFunction
 import kotlin.reflect.KParameter
 import kotlin.reflect.full.companionObject
 import kotlin.reflect.full.declaredFunctions
+import kotlin.reflect.full.hasAnnotation
 import kotlin.reflect.full.memberProperties
 import kotlin.reflect.full.primaryConstructor
 import kotlin.reflect.jvm.internal.KotlinReflectionInternalError
@@ -59,68 +60,44 @@ internal class KotlinNamesAnnotationIntrospector(val module: KotlinModule, val c
     }
 
     @Suppress("UNCHECKED_CAST")
-    override fun hasCreatorAnnotation(member: Annotated): Boolean {
+    private fun hasCreatorAnnotation(member: AnnotatedConstructor): Boolean {
         // don't add a JsonCreator to any constructor if one is declared already
 
-        if (member is AnnotatedConstructor && !member.declaringClass.isEnum) {
-            // if has parameters, is a Kotlin class, and the parameters all have parameter annotations, then pretend we have a JsonCreator
-            if (member.parameterCount > 0 && member.declaringClass.isKotlinClass()) {
-                return cache.checkConstructorIsCreatorAnnotated(member) {
-                    val kClass = cache.kotlinFromJava(member.declaringClass as Class<Any>)
-                    val kConstructor = cache.kotlinFromJava(member.annotated as Constructor<Any>)
+        val kClass = cache.kotlinFromJava(member.declaringClass as Class<Any>)
+            .apply { if (this in ignoredClassesForImplyingJsonCreator) return false }
+        val kConstructor = cache.kotlinFromJava(member.annotated as Constructor<Any>) ?: return false
 
-                    if (kConstructor != null) {
-                        val isPrimaryConstructor = kClass.primaryConstructor == kConstructor ||
-                                (kClass.primaryConstructor == null && kClass.constructors.size == 1)
+        // TODO:  should we do this check or not?  It could cause failures if we miss another way a property could be set
+        // val requiredProperties = kClass.declaredMemberProperties.filter {!it.returnType.isMarkedNullable }.map { it.name }.toSet()
+        // val areAllRequiredParametersInConstructor = kConstructor.parameters.all { requiredProperties.contains(it.name) }
 
-                        val propertyNames = kClass.memberProperties.map { it.name }.toSet()
+        val propertyNames = kClass.memberProperties.map { it.name }.toSet()
 
-                        fun KFunction<*>.isPossibleSingleString(): Boolean {
-                            val result = parameters.size == 1 &&
-                                    parameters[0].name !in propertyNames &&
-                                    parameters[0].type.javaType == String::class.java &&
-                                    parameters[0].annotations.none { it.annotationClass.java == JsonProperty::class.java }
-                            return result
-                        }
+        return when {
+            kConstructor.isPossibleSingleString(propertyNames) -> false
+            kConstructor.parameters.any { it.name == null } -> false
+            !kClass.isPrimaryConstructor(kConstructor) -> false
+            else -> {
+                val anyConstructorHasJsonCreator = kClass.constructors
+                    .filterOutSingleStringCallables(propertyNames)
+                    .any { it.hasAnnotation<JsonCreator>() }
 
-                        fun Collection<KFunction<*>>.filterOutSingleStringCallables(): Collection<KFunction<*>> {
-                            return this.filter { !it.isPossibleSingleString() }
-                        }
+                val anyCompanionMethodIsJsonCreator = member.type.rawClass.kotlin.companionObject?.declaredFunctions
+                    ?.filterOutSingleStringCallables(propertyNames)
+                    ?.any { it.hasAnnotation<JsonCreator>() && it.hasAnnotation<JvmStatic>() }
+                    ?: false
 
-                        val anyConstructorHasJsonCreator = kClass.constructors.filterOutSingleStringCallables()
-                                .any { it.annotations.any { it.annotationClass.java == JsonCreator::class.java }
-                                }
-
-                        val anyCompanionMethodIsJsonCreator = member.type.rawClass.kotlin.companionObject?.declaredFunctions
-                                ?.filterOutSingleStringCallables()?.any {
-                                    it.annotations.any { it.annotationClass.java == JvmStatic::class.java } &&
-                                            it.annotations.any { it.annotationClass.java == JsonCreator::class.java }
-                                } ?: false
-
-                        // TODO:  should we do this check or not?  It could cause failures if we miss another way a property could be set
-                        // val requiredProperties = kClass.declaredMemberProperties.filter {!it.returnType.isMarkedNullable }.map { it.name }.toSet()
-                        // val areAllRequiredParametersInConstructor = kConstructor.parameters.all { requiredProperties.contains(it.name) }
-
-                        val areAllParametersValid = kConstructor.parameters.size == kConstructor.parameters.count { it.name != null }
-
-                        val isSingleStringConstructor = kConstructor.isPossibleSingleString()
-
-                        val implyCreatorAnnotation = isPrimaryConstructor
-                                && !(anyConstructorHasJsonCreator || anyCompanionMethodIsJsonCreator)
-                                && areAllParametersValid
-                                && !isSingleStringConstructor
-                                && kClass !in ignoredClassesForImplyingJsonCreator
-
-                        implyCreatorAnnotation
-                    } else {
-                        false
-                    }
-                }
+                !(anyConstructorHasJsonCreator || anyCompanionMethodIsJsonCreator)
             }
         }
-        return false
     }
 
+    override fun hasCreatorAnnotation(member: Annotated): Boolean =
+        if (member is AnnotatedConstructor && member.isKotlinConstructorWithParameters())
+            cache.checkConstructorIsCreatorAnnotated(member) { hasCreatorAnnotation(it) }
+        else
+            false
+
     @Suppress("UNCHECKED_CAST")
     private fun findKotlinParameterName(param: AnnotatedParameter): String? {
         return if (param.declaringClass.isKotlinClass()) {
@@ -165,3 +142,19 @@ internal class KotlinNamesAnnotationIntrospector(val module: KotlinModule, val c
     ReplaceWith("with(receiver) { declaringClass.declaredMethods.any { it.name.contains('-') } }")
 )
 private fun AnnotatedMethod.isInlineClass() = declaringClass.declaredMethods.any { it.name.contains('-') }
+
+// if has parameters, is a Kotlin class, and the parameters all have parameter annotations, then pretend we have a JsonCreator
+private fun AnnotatedConstructor.isKotlinConstructorWithParameters(): Boolean =
+    parameterCount > 0 && declaringClass.isKotlinClass() && !declaringClass.isEnum
+
+private fun KFunction<*>.isPossibleSingleString(propertyNames: Set<String>): Boolean = parameters.size == 1 &&
+        parameters[0].name !in propertyNames &&
+        parameters[0].type.javaType == String::class.java &&
+        !parameters[0].hasAnnotation<JsonProperty>()
+
+private fun Collection<KFunction<*>>.filterOutSingleStringCallables(propertyNames: Set<String>): Collection<KFunction<*>> =
+    this.filter { !it.isPossibleSingleString(propertyNames) }
+
+private fun KClass<*>.isPrimaryConstructor(kConstructor: KFunction<*>) = this.primaryConstructor.let {
+    it == kConstructor || (it == null && this.constructors.size == 1)
+}