[BUG] ObjectMapper.readValue<T>(...) breaks null safety

[revintec]

Jackson-module-kotlin version 2.9.7, newer version should experience the same problem

changing original source code inline fun <reified T> ObjectMapper.readValue(content: String): T = readValue(content, jacksonTypeRef<T>()) adds trailing ... as T would fix the problem, however this way it provides poor exception message
Exception in thread "main" java.lang.NullPointerException: null cannot be cast to non-null type kotlin.Any

[dinomite]

Interesting, adding that cast to the extensions functions makes sense to me, but I'm a bit wary as I'm not certain what side effects it might have.

@viartemev do you have any insight?

@revintec would you make a PR with casts on the extension functions? Depending on how confident we are, this could be part of 2.12.1 or might wait until 2.13.

[revintec]

sorry for the late reply. as I discovered lately, it's more complicated than just add a as T cast

1. the exception message would not be user-friendly
2. can't cover generic class

  // should throw, instead we got a map with contaminated values
  println(JSON.readValue<Map<String,Any>>("{\"key\":null}"))

I think the right approach would be to add a isNullable alone side com.fasterxml.jackson.core.type.TypeReference._type and check null inside jackson-core when deserializing. but this requires more modification, thus I didn't come up with a PR for now

[k163377]

Starting from 2.19, a RuntimeJsonMappingException will be thrown if a value different from the given type parameter is read.

[cowtowncoder]

Is there specific reason to use RuntimeJsonMappingException over regular JsonMappingException -- I am guessing some method signature does not expose IOException?
(ideally RuntimeJsonMappingException should only used if regular checked one cannot).

Also note that this probably should be changed for 3.0 when merging, as all JacksonExceptions are unchecked so limitations no longer apply.

[k163377]

@cowtowncoder
The RuntimeJsonMappingException was used to unify the exceptions thrown by the checks.
As commented below, there were cases where other exceptions would be wrapped in RuntimeJsonMappingException.
#937 (comment)

Also note that this probably should be changed for 3.0 when merging, as all JacksonExceptions are unchecked so limitations no longer apply.

For exceptions used in 3.x, DatabindException will be used after consideration.

[cowtowncoder]

@k163377 sorry, yes, I realized we just discussed this recently. All good.