FasterXML / FasterXML/jackson-module-kotlin
There are some problems with KNAI.findDefaultCreator
- Vorherrschende Sprache
- Kotlin
- Sterne
- 1.2k
- Forks
- 187
- Ø Merge
- 9 Std. 48 Min.
- Gemergte PRs (30 T.)
- 16
Beschreibung
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.
```kotlin
private fun KFunction<*>.isPossibleSingleString(propertyNames: Set): Boolean = parameters.singleOrNull()?.let {
it.name !in propertyNames
&& it.type.javaType == String::class.java
&& !it.hasAnnotation()
} == 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`.
```kotlin
val anyConstructorHasJsonCreator = kClass.constructors
.filterOutSingleStringCallables(propertyNames)
.any { it.hasAnnotation() }
val anyCompanionMethodIsJsonCreator = member.type.rawClass.kotlin.companionObject?.declaredFunctions
?.filterOutSingleStringCallables(propertyNames)
?.any { it.hasAnnotation() && it.hasAnnotation() }
?: 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.
```kotlin
private fun KFunction<*>.isPossibleSingleString(propertyNames: Set): Boolean = parameters.size == 1 &&
parameters[0].name !in propertyNames &&
parameters[0].type.javaType == String::class.java &&
!parameters[0].hasAnnotation()
```
### 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.
```kotlin
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`.
```kotlin
val anyConstructorHasJsonCreator = kClass.constructors
.filterOutSingleStringCallables(propertyNames)
.any { it.hasAnnotation() }
val anyCompanionMethodIsJsonCreator = member.type.rawClass.kotlin.companionObject?.declaredFunctions
?.filterOutSingleStringCallables(propertyNames)
?.any { it.hasAnnotation() && it.hasAnnotation() }
?: 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.
```kotlin
val anyConstructorHasJsonCreator = kClass.constructors
.filterOutSingleStringCallables(propertyNames)
.any { it.hasAnnotation() }
val anyCompanionMethodIsJsonCreator = member.type.rawClass.kotlin.companionObject?.declaredFunctions
?.filterOutSingleStringCallables(propertyNames)
?.any { it.hasAnnotation() && it.hasAnnotation() }
?: false
```
On the other hand, if `JsonCreator.mode` is `DISABLED`, returning `false` seems to be correct.
Beitragsleitfaden
Für dieses Repository ist kein Beitragsleitfaden indexiert
Bewertung
Dieses Issue wurde noch nicht bewertet.