Merge pull request #1020 from k163377/mkpe

Fixed old `StrictNullChecks` to throw exceptions similar to those thrown by new `StrictNullChecks`

Author: k163377

release-notes/CREDITS-2.x

@@ -18,6 +18,7 @@ Contributors:
 # 2.20.0 (not yet released)
 
 WrongWrong (@k163377)
+* #1020: Fixed old StrictNullChecks to throw exceptions similar to those thrown by new StrictNullChecks
 * #1018: Use MethodHandle in processing related to value class
 * #969: Cleanup of deprecated contents
 * #967: Update settings for 2.20

release-notes/VERSION-2.x

@@ -17,6 +17,9 @@ Co-maintainers:
 ------------------------------------------------------------------------
 
 2.20.0 (not yet released)
+#1020: Exceptions thrown by the old StrictNullChecks are now the similar to the new StrictNullChecks.
+  This means that the old StrictNullChecks will no longer throw MissingKotlinParameterException.
+  See PR for what is thrown and how error messages change.
 #1018: Improved handling of `value class` has improved performance for both serialization and deserialization.
   In particular, for serialization, proper caching has improved throughput by a factor of 2 or more in the general cases.
   Also, replacing function execution by reflection with `MethodHandle` improved throughput by several percent for both serialization and deserialization.

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

@@ -11,6 +11,7 @@ import com.fasterxml.jackson.databind.deser.ValueInstantiator
 import com.fasterxml.jackson.databind.deser.ValueInstantiators
 import com.fasterxml.jackson.databind.deser.impl.PropertyValueBuffer
 import com.fasterxml.jackson.databind.deser.std.StdValueInstantiator
+import com.fasterxml.jackson.databind.exc.InvalidNullException
 import java.lang.reflect.TypeVariable
 import kotlin.reflect.KType
 import kotlin.reflect.KTypeProjection
@@ -103,31 +104,32 @@ internal class KotlinValueInstantiator(
             } else if (strictNullChecks) {
                 val arguments = paramType.arguments
 
-                var paramTypeStr: String? = null
-                var itemType: KType? = null
-
-                if (propType.isCollectionLikeType && arguments.markedNonNullAt(0) && (paramVal as Collection<*>).any { it == null }) {
-                    paramTypeStr = "collection"
-                    itemType = arguments[0].type
-                }
-
-                if (propType.isMapLikeType && arguments.markedNonNullAt(1) && (paramVal as Map<*, *>).any { it.value == null }) {
-                    paramTypeStr = "map"
-                    itemType = arguments[1].type
-                }
-
-                if (propType.isArrayType && arguments.markedNonNullAt(0) && (paramVal as Array<*>).any { it == null }) {
-                    paramTypeStr = "array"
-                    itemType = arguments[0].type
+                // To make the behavior the same as deserialization of each element using NullsFailProvider,
+                // first wrapWithPath with paramVal and key.
+                val ex = when {
+                    propType.isCollectionLikeType && arguments.markedNonNullAt(0) -> {
+                        (paramVal as Collection<*>).indexOf(null).takeIf { it >= 0 }?.let {
+                            InvalidNullException.from(ctxt, jsonProp.fullName, jsonProp.type)
+                                .wrapWithPath(paramVal, it)
+                        }
+                    }
+                    propType.isMapLikeType && arguments.markedNonNullAt(1) -> {
+                        (paramVal as Map<*, *>).entries.find { (_, v) -> v == null }?.let { (k, _) ->
+                            InvalidNullException.from(ctxt, jsonProp.fullName, jsonProp.type)
+                                .wrapWithPath(paramVal, k.toString())
+                        }
+                    }
+                    propType.isArrayType && arguments.markedNonNullAt(0) -> {
+                        (paramVal as Array<*>).indexOf(null).takeIf { it >= 0 }?.let {
+                            InvalidNullException.from(ctxt, jsonProp.fullName, jsonProp.type)
+                                .wrapWithPath(paramVal, it)
+                        }
+                    }
+                    else -> null
                 }
 
-                if (paramTypeStr != null && itemType != null) {
-                    throw MissingKotlinParameterException(
-                        parameter = paramDef,
-                        processor = ctxt.parser,
-                        msg = "Instantiation of $itemType $paramType failed for JSON property ${jsonProp.name} due to null value in a $paramType that does not allow null values"
-                    ).wrapWithPath(this.valueClass, jsonProp.name)
-                }
+                // Then, wrapWithPath with this property.
+                ex?.let { throw it.wrapWithPath(this.valueClass, jsonProp.name) }
             }
 
             bucket[paramDef] = paramVal

src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/StrictNullChecksTestOld.kt

@@ -1,7 +1,7 @@
 package com.fasterxml.jackson.module.kotlin.test
 
+import com.fasterxml.jackson.databind.exc.InvalidNullException
 import com.fasterxml.jackson.module.kotlin.KotlinFeature
-import com.fasterxml.jackson.module.kotlin.MissingKotlinParameterException
 import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper
 import com.fasterxml.jackson.module.kotlin.readValue
 import org.junit.jupiter.api.Assertions.assertArrayEquals
@@ -32,7 +32,7 @@ class StrictNullChecksTestOld {
 
     @Test
     fun testListOfInt() {
-        assertThrows<MissingKotlinParameterException> {
+        assertThrows<InvalidNullException> {
             val json = """{"samples":[1, null]}"""
             mapper.readValue<ClassWithListOfInt>(json)
         }
@@ -62,7 +62,7 @@ class StrictNullChecksTestOld {
 
     @Test
     fun testArrayOfInt() {
-        assertThrows<MissingKotlinParameterException> {
+        assertThrows<InvalidNullException> {
             val json = """{"samples":[1, null]}"""
             mapper.readValue<ClassWithArrayOfInt>(json)
         }
@@ -92,7 +92,7 @@ class StrictNullChecksTestOld {
 
     @Test
     fun testMapOfStringToIntWithNullValue() {
-        assertThrows<MissingKotlinParameterException> {
+        assertThrows<InvalidNullException> {
             val json = """{ "samples": { "key": null } }"""
             mapper.readValue<ClassWithMapOfStringToInt>(json)
         }
@@ -121,7 +121,7 @@ class StrictNullChecksTestOld {
     @Disabled // this is a hard problem to solve and is currently not addressed
     @Test
     fun testListOfGenericWithNullValue() {
-        assertThrows<MissingKotlinParameterException> {
+        assertThrows<InvalidNullException> {
             val json = """{"samples":[1, null]}"""
             mapper.readValue<TestClass<List<Int>>>(json)
         }
@@ -137,7 +137,7 @@ class StrictNullChecksTestOld {
     @Disabled // this is a hard problem to solve and is currently not addressed
     @Test
     fun testMapOfGenericWithNullValue() {
-        assertThrows<MissingKotlinParameterException> {
+        assertThrows<InvalidNullException> {
             val json = """{ "samples": { "key": null } }"""
             mapper.readValue<TestClass<Map<String, Int>>>(json)
         }
@@ -153,7 +153,7 @@ class StrictNullChecksTestOld {
     @Disabled // this is a hard problem to solve and is currently not addressed
     @Test
     fun testArrayOfGenericWithNullValue() {
-        assertThrows<MissingKotlinParameterException> {
+        assertThrows<InvalidNullException> {
             val json = """{"samples":[1, null]}"""
             mapper.readValue<TestClass<Array<Int>>>(json)
         }