Fixed old StrictNullChecks to throw exceptions similar to those thrown by new StrictNullChecks
#1020
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR aligns the old strict-null-check behavior with the new implementation by replacing MissingKotlinParameterException with InvalidNullException and updating the value instantiator to throw InvalidNullException for null elements in collections, maps, and arrays.
- Tests now expect
InvalidNullExceptioninstead ofMissingKotlinParameterException. KotlinValueInstantiatorimports and usesInvalidNullException.from(...)to construct and wrap exceptions when encountering null items.- Removed the old branching logic and replaced it with a
whenblock that creates and throws the new exception type.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/StrictNullChecksTestOld.kt | Updated tests to assert InvalidNullException and removed the old exception import. |
| src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinValueInstantiator.kt | Added import for InvalidNullException and replaced the old null-check branches with logic that constructs and throws InvalidNullException. |
Comments suppressed due to low confidence (2)
src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinValueInstantiator.kt:109
- [nitpick] Variable name
exis quite generic. Rename it to something more descriptive likenullExceptionto clarify its purpose.
val ex = when {
src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/StrictNullChecksTestOld.kt:35
- Consider extending this test to verify that the thrown
InvalidNullExceptionincludes the correct JSON path or property name/index, ensuring the wrapping behavior is fully covered.
assertThrows<InvalidNullException> {
| 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 { |
There was a problem hiding this comment.
[nitpick] The when block that builds InvalidNullException contains repeated patterns for collection and array cases. Consider extracting a helper function to handle index-based null checks and exception wrapping to reduce duplication and improve readability.
Copilot uses AI. Check for mistakes.
k163377
changed the title
StrictNullChecks to throw exceptions similar to those thrown by new StrictNullChecksStrictNullChecks to throw exceptions similar to those thrown by new StrictNullChecks
To deprecate the old
StrictNullChecksoption, changes were made to make the exceptions thrown by the old process the similar to the newStrictNullChecks.This means that the old
StrictNullCheckswill no longer throwMissingKotlinParameterException.In the situation where this error occurs, it was inappropriate to use
MissingKotlinParameterExceptionbecause the input was not missing.This is also relevant for #617.
The following is a comparison of the respective messages displayed when
printStackTraceis performed on the result ofassertThrowsforcom.fasterxml.jackson.module.kotlin.test.StrictNullChecksTest(NewStrictNullChecks) andStrictNullChecksTestOld.As for messages, it has been improved so that even the key of a value that was
nullcan be checked.ClassWithArrayOfInt
NewStrictNullChecks
Fixed old
StrictNullChecksConventional old
StrictNullChecksClassWithListOfInt
NewStrictNullChecks
Fixed old
StrictNullChecksConventional old
StrictNullChecksClassWithMapOfStringToInt
NewStrictNullChecks
Fixed old
StrictNullChecksConventional old
StrictNullChecks