There are some problems with KNAI.findDefaultCreator

[k163377]

I found several processes for KNAI.findDefaultCreator that are not working as intended, or the test results do not change even if they are removed.

1. JsonProperty.Access is ignored in KFunction<*>.isPossibleSingleString.

This function is decraled as follows.

private fun KFunction<*>.isPossibleSingleString(propertyNames: Set<String>): Boolean = parameters.singleOrNull()?.let {
    it.name !in propertyNames
        && it.type.javaType == String::class.java
        && !it.hasAnnotation<JsonProperty>()
} == true

From reading the Javadoc of JsonProperty.Access, it looks like the correct behavior is to ignore it during deserialization if the JsonProperty.access property is READ_ONLY.
https://javadoc.io/static/com.fasterxml.jackson.core/jackson-annotations/1.13.1/com/fasterxml/jackson/annotation/JsonProperty.Access.html

Giving JsonProperty to parameters with READ_ONLY looks like a strange edge case conceptually, but I think it should be fixed if we want to match Javadoc.

2. kConstructor.parameters.any { it.name == null } will not be true

Since KParameter.name is nullable, this decision seems reasonable, but it does not seem to be true in practice.

In the constructor, KParameter.name is null only if get the constructor of inner class as follows.
In this case, outer class is requested as an instance parameter, and its KParameter.name is null.

val anyConstructorHasJsonCreator = kClass.constructors
    .filterOutSingleStringCallables(propertyNames)
    .any { it.hasAnnotation<JsonCreator>() }

val anyCompanionMethodIsJsonCreator = member.type.rawClass.kotlin.companionObject?.declaredFunctions
    ?.filterOutSingleStringCallables(propertyNames)
    ?.any { it.hasAnnotation<JsonCreator>() && it.hasAnnotation<JvmStatic>() }
    ?: false

On the other hand, Jackson does not support deserialization to non-static classes, so this pattern can be ignored.

Also, if KParameter.name is null, it seems to fail with findKotlinParameterName even if this process does not fail.
In this case, the error message will be has no property name instead of no Creators.

I think it would be better to remove this decision, as it eliminates unnecessary processing and appears to make error messages easier to understand.

---

The content has been rewritten because the modification of #818 has changed the search process for creators.

Original

I found several processes for KNAI.hasCreatorAnnotation that are not working as intended, or the test results do not change even if they are removed.

1. There are some problems with KFunction<*>.isPossibleSingleString.

This function is decraled as follows.

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>()

1-1. Unable to get the intended result from functions retrieved with companionObject.declaredFunctions.

The parameters of functions retrieved with companionObject.declaredFunctions include the instance parameter.
In other words, parameters.size == 1 will be true only for functions with no parameters.

Therefore, this decision needs to be modified to valueParameters.size == 1.

1-2.

From reading the Javadoc of JsonProperty.Access, it looks like the correct behavior is to ignore it during deserialization if the JsonProperty.access property is READ_ONLY.
https://javadoc.io/static/com.fasterxml.jackson.core/jackson-annotations/1.13.1/com/fasterxml/jackson/annotation/JsonProperty.Access.html

Giving JsonProperty to parameters with READ_ONLY looks like a strange edge case conceptually, but I think it should be fixed if we want to match Javadoc.

2. Removing kConstructor.isPossibleSingleString does not change the test results.

The process I am referring to here is the following part.
Removing kConstructor.isPossible... from the branch of the when expression does not seem to change the test results.

val propertyNames = kClass.memberProperties.map { it.name }.toSet()

return when {
    kConstructor.isPossibleSingleString(propertyNames) -> false

    /* omission */

}

To begin with, what is the purpose of the isPossibleSingleString decision?
I did not understand why it specifically excludes functions that match this condition.
In addition, I think that this decision should be removed from the KotlinModule process, since there seems to be no bug reports despite the problems mentioned in 1-1.

3. kConstructor.parameters.any { it.name == null } will not be true

Since KParameter.name is nullable, this decision seems reasonable, but it does not seem to be true in practice.

In the constructor, KParameter.name is null only if get the constructor of inner class as follows.
In this case, outer class is requested as an instance parameter, and its KParameter.name is null.

val anyConstructorHasJsonCreator = kClass.constructors
    .filterOutSingleStringCallables(propertyNames)
    .any { it.hasAnnotation<JsonCreator>() }

val anyCompanionMethodIsJsonCreator = member.type.rawClass.kotlin.companionObject?.declaredFunctions
    ?.filterOutSingleStringCallables(propertyNames)
    ?.any { it.hasAnnotation<JsonCreator>() && it.hasAnnotation<JvmStatic>() }
    ?: false

On the other hand, Jackson does not support deserialization to non-static classes, so this pattern can be ignored.

Also, if KParameter.name is null, it seems to fail with findKotlinParameterName even if this process does not fail.
In this case, the error message will be has no property name instead of no Creators.

I think it would be better to remove this decision, as it eliminates unnecessary processing and appears to make error messages easier to understand.

4. JsonCreator.mode is being ignored.

In the following process, all cases where JsonCreator is included are handled in the same way.

val anyConstructorHasJsonCreator = kClass.constructors
    .filterOutSingleStringCallables(propertyNames)
    .any { it.hasAnnotation<JsonCreator>() }

val anyCompanionMethodIsJsonCreator = member.type.rawClass.kotlin.companionObject?.declaredFunctions
    ?.filterOutSingleStringCallables(propertyNames)
    ?.any { it.hasAnnotation<JsonCreator>() && it.hasAnnotation<JvmStatic>() }
    ?: false

On the other hand, if JsonCreator.mode is DISABLED, returning false seems to be correct.