feat: Make SerialDescriptorForNullable.original public

[Chuckame]

Closes #2631

[sandwwraith]

You should also do a couple more things:

1. Update public API dump (see https://github.com/Kotlin/kotlinx.serialization/blob/master/CONTRIBUTING.md#updating-the-public-api-dump)
2. Add some tests for the change — that x.nullable.unwrapNullable === x, that for @Serializable class X(val s: String?), val n = X.serializer().descriptor.getElementDescriptor(0) results in n.isNullable == true and n.unwrapNullable == String.serializer().descriptor, for your use case, etc.

[Chuckame]

Following the contributing guide, I've rebased onto dev (but the PR is on master, I cannot change it ⚠️), tests have been added, and doc+knit+apiDump executed @sandwwraith ! 🚀

[sandwwraith]

After looking at the api dump, I've realized that unwrap is actually a verb, and its counter-intuitive to have a verb in property name (https://kotlinlang.org/docs/coding-conventions.html#choose-good-names). So it should be either

1. A function SerialDescriptor.unwrapNullable()
   or
2. A property with a noun, e.g. unwrappedNullable.

I'm not sure what is better. Doing (1) will bring asymmetry with .nullable. (2) is confusing, because it is not clear what 'unwrapped' is since we do not have 'wrapped' anywhere else. Besides, 'unwrappedNullable' sounds like some flavor of a nullable thing, but in reality, returned descriptor may be non-nullable.

[Chuckame]

You're right, I'm feeling the same. I would prefer unwrappedNullable to not have this asymmetry. I'll commit now. If you feel not comfortable, could we have a third person reviewing this to help chosing a better name ?

[sandwwraith]

@shanshin or @qwwdfsad may help here, I guess

[Chuckame]

Or, we can do unwrappedOriginal and also unwrap WrappedSerialDescriptor.original (I've looked at all the SerialDescriptors and only SerialDescriptorForNullable and WrappedSerialDescriptor would have sense of unwrapping the original descriptor)

[pdvrieze]

@Chuckame It seems to be that the route of introducing a WrappedSerialDescriptor interface would make a lot of sense, it would also future proof the design to support other wrapping descriptors (and not special case the nullable descriptor only)

[Chuckame]

I like the idea, but it would mean also renaming the current data class WrappedSerialDescriptor to something else to allow creation of the interface. What do you think @sandwwraith ?

[sandwwraith]

the route of introducing a WrappedSerialDescriptor interface would make a lot of sense, it would also future proof the design to support other wrapping descriptors

IMO this is way out of scope of this PR.

[sandwwraith]

WDYT of nonNullableOriginal name? We can add to the documentation that this property "does not affect descriptors that were created as nullable by directly implementing SerialDescriptor interface."

[Chuckame]

New doc, is it okay for you @sandwwraith ? I also renamed to nonNullOriginal (pushed)

 * Returns non-nullable serial descriptor for the type, only if this descriptor has been auto-generated (plugin
 * generated descriptors) or with `.nullable` extension on a descriptor or serializer.
 *
 * Otherwise, returns this.
 * 
 * It may return nullable descriptor if this descriptor has been created manually as nullable by directly implementing SerialDescriptor interface.
 *

[sandwwraith]

Yep, looks good to me

[sandwwraith]

@Chuckame You need to update apiDump with tool: getNonNullOriginal alphabetically goes before getNullable

[Chuckame]

@sandwwraith it's ready, apiDump done, doc updated. When do you think it could be released ?

[sandwwraith]

@Chuckame We usually do releases around Kotlin's release. Given that 2.0.0 is just around the corner, I think it will be in about a month.

[Chuckame]

Ok thanks, so I'll wait this PR release before releasing on my side.

[sandwwraith]

Thank you!

[Chuckame]

Can I access to a snapshot build until the release ?

[sandwwraith]

Unfortunately, we don't do snapshot builds. You can build it yourself and publish it to your maven local (https://github.com/Kotlin/kotlinx.serialization/blob/master/docs/building.md) if you want to test the changes with your library