FasterXML / FasterXML/jackson-module-kotlin

There are some problems with KNAI.findDefaultCreator

Offen
#547 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
bug
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.

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.