Fix TaggedDecoder nullable decoding

[pschichtel]

The implementation of decodeNullableSerializableElement of TaggedDecoder was inconsistent with AbstractDecoder, which made it impossible to use null-capable serializers as seen in the new tests. This partially fixes #2455 by allowing me to create a nullable version of the JsonElement serializer.

[pschichtel]

I also tried changing the existing serializers to nullable, but that breaks encoding since the AbstractEncoder (and other places) then assume that the serializer can take null as an input. Actually changing the serializers to nullable types, but it still breaks the kotlinx.serialization.json.serializers.JsonSerializerInGenericsTest#testGenericsWithNulls test because it will now get JsonNull instances instead of null in most places. And even if I change the test to work correctly, I'm fairly certain this would at least be a source-incompatible change.

[pdvrieze]

First note, that TaggedEncoder through encodeNullableSerializableValue already correctly supports the case of the serializer supporting nullability itself. This means the lack of support in TaggedDecoder is a bug, not merely a potential feature.

The implementation however is incorrect in that it doesn't conform to the behavioural contract of nullable serializers (the test is broken to compensate). See the specific comments. The main decode function is easy to fix, but adding the overload for decodeNullableSerializableValue that takes the previous value would be good.

[sandwwraith]

Hi, thanks for your PR. Whether val x: JsonNull? = null and {"x":null} should deserialize to is a debatable question, and it probably is down to a personal preference or a specific application task. It was never meant to be differentiable for e.g. your use case (absence of value or default value).
However, as you correctly pointed out, we can't carry source-breaking or behavior-breaking changes, so JsonNullSerializer.descriptor stays like it is now — not-nullable.

I'm in favor of the fix that will allow you to write a custom serializer like in your test to solve the problem.
Unfortunately, as you can see, there are some failed tests in the Hocon module, which also uses TaggedDecoder. Would you mind to take a look at them?

[pschichtel]

I'm in favor of the fix that will allow you to write a custom serializer like in your test to solve the problem.

Yep, perfectly reasonable. While poking around in the decoders I realized that I probably have a very niche use-case and there doesn't seem to be an obvious way to integrate it without breaking things or introducing a configuration option specifically for this, which is probably not worth it.

Unfortunately, as you can see, there are some failed tests in the Hocon module, which also uses TaggedDecoder. Would you mind to take a look at them?

I guess for the last changes I only ran the JSON tests. I reverted the value -> element change and executed allTest locally, all tests are passing here.

[sandwwraith]

@pschichtel Can you also please open your PR against dev and rebase your commits? All commits with code should go to dev first

[pschichtel]

@sandwwraith changed it

[sandwwraith]

Thank you!