Introduce PolymorphismStyle.None #494
Conversation
This implements charleskorn#378 by introducing a new `None` style that omits the tag/property entirely from the output yaml. Note this is asymmetric and reads are treated the same as `Tag` as reads require a tag. I added a few tests to JvmYamlWritingTest for both this case and some added coverage for the existing two cases as well.
There was a problem hiding this comment.
Thanks for the PR @ZacSweers!
Overall, this looks good to me.
In addition to the comment below, could you please also add a test for the behaviour when reading polymorphic and non-polymorphic content while PolymorphismStyle.None is in use?
| PolymorphismStyle.Tag -> throw MissingTypeTagException(node.path) | ||
| PolymorphismStyle.Tag, PolymorphismStyle.None -> throw MissingTypeTagException(node.path) | ||
| PolymorphismStyle.Property -> createPolymorphicMapDeserializer(node, yaml, context, configuration) | ||
| } | ||
| else -> throw IncorrectTypeException("Expected ${descriptor.kind.friendlyDescription}, but got a map", node.path) | ||
| } | ||
|
|
||
| is YamlTaggedNode -> when { | ||
| descriptor.kind is PolymorphicKind && configuration.polymorphismStyle == PolymorphismStyle.Tag -> YamlPolymorphicInput(node.tag, node.path, node.innerNode, yaml, context, configuration) | ||
| descriptor.kind is PolymorphicKind && configuration.polymorphismStyle != PolymorphismStyle.Property -> { | ||
| YamlPolymorphicInput(node.tag, node.path, node.innerNode, yaml, context, configuration) | ||
| } |
There was a problem hiding this comment.
I think it might be simpler to just fail if we encounter polymorphism or a tag and PolymorphismStyle.None is in use.
|
Thank you for working on this! I really appreciate it :) |
|
Will look when I'm back from vacation 👍 |
|
Done! |
There was a problem hiding this comment.
Thanks @ZacSweers! I've got some minor nits for the test names, but I'll update the branch and then merge this - thanks again for the PR.
|
I think there's a bug that I missed in kaml/src/jvmMain/kotlin/com/charleskorn/kaml/YamlOutput.kt Lines 175 to 177 in 2f0de20 I think in the None case, it should just omit the tag right? If so, I can PR a quick fix |
That makes sense to me - a PR with a test case and fix would be great. |
This implements #378 by introducing a new
Nonestyle that omits the tag/property entirely from the output yaml.Note this is asymmetric and reads are treated the same as
Tagas reads require a tag.I added a few tests to JvmYamlWritingTest for both this case and some added coverage for the existing two cases as well.