Golang: Omitempty when field is required by some oneOf variants

[vbqz]

Hi,

I've bumped into an unexpected interaction between oneOf and the new --omit-empty flag.

If set, all non-required objects will be tagged with ",omitempty" (off by default)

I tried to condense the problem down to the example below. We're using oneOf to express variants, where the "b" field is only required in some of them. I expected this "partially required" to overrule the flag, since making B omitempty makes it hard to generate valid events of kind "one".

Does this make any sense or am I missing some nuance in the schema rules?

Schema

{
  "$schema": "https://json-schema.org/draft/2020-12/schema",
  "$id": "https://example.com/def.json",
  "title": "yada yada",
  "$defs": {
    "bar": {
      "type": [
        "string",
        "null"
      ]
    },
    "one": {
      "type": "object",
      "properties": {
        "kind": {
          "type": "string",
          "const": "one"
        },
        "b": {
          "$ref": "#/$defs/bar"
        }
      },
      "required": [
        "kind",
        "b"
      ]
    },
    "two": {
      "type": "object",
      "properties": {
        "kind": {
          "type": "string",
          "const": "two"
        },
        "b": {
          "$ref": "#/$defs/bar"
        }
      },
      "required": [
        "kind"
      ]
    }
  },
  "oneOf": [
    {
      "$ref": "#/$defs/one"
    },
    {
      "$ref": "#/$defs/two"
    }
  ]
}

Command (quicktype version 23.0.104):

npx quicktype --src-lang schema -o ./event.go --just-types-and-package --omit-empty def.json

Outcome:

package main

type Event struct {
        B    *string `json:"b,omitempty"`
        Kind Kind    `json:"kind"`
}

type Kind string

const (
        One Kind = "one"
        Two Kind = "two"
)

Thanks!

[rekram1-node]

@vbqz could you elaborate on your expectations of what should be done? Seems to me that this would be expected. You have two events, both of which will have a non nil "kind", both of these events can have a "B" but only one of them has it being required. Thereby making it optional. To me this makes sense but perhaps I misunderstand you or perhaps it is I who is mistaken.

[vbqz]

Hi @rekram1-node

I think you have the right idea of the schema, B is (conditionally) optional. The problem lies in the generated Go code.

If I try to create a valid event like:

Event {
    Kind: One,
    B:    nil,         // OK since bar can be "string" or "null".
}

then the omitempty tag on B will exclude my B: nil from the marshalled payload, it is simply not there. And then this payload does not fulfil any of the oneOf variants. In other words

we get  -> { "kind": "one" }
we want -> { "kind": "one", "b": null }

[rekram1-node]

@vbqz Okay then why use the omit-empty flag if you don't want it? Couldn't you just run:

npx quicktype --src-lang schema -o ./event.go --just-types-and-package def.json

[vbqz]

@rekram1-node Sure, that is what we're doing right now. However I opened this issue since it seems like using --omit-empty generated incorrect code.

The upside of --omit-empty would be that truly optional fields could be omitted. Avoiding null value noise for example seeing all fields of variants A or B when the items is actually variant C.

[rekram1-node]

@vbqz I see...

I assume your expected generated code would be the following correct?:

package main

type Event struct {
        B    *string `json:"b"`
        Kind Kind    `json:"kind"`
}

type Kind string

const (
        One Kind = "one"
        Two Kind = "two"
)

[rekram1-node]

@vbqz After looking at the omit empty flag description:
omit-empty If set, all non-required objects will be tagged with
",omitempty" (off by default)

And given that B is not required in all cases, I think that the behavior you are experiencing is the expected outcome at least in the current iteration. Perhaps it is too opinionated but given that description this would fall under that behavior.

[vbqz]

@rekram1-node Yes I think that would be the code.

Yes I was just about to post the flag description myself.
I'd argue the opposite since b is in a required list (albeit in a oneOf) it should not count as "non-required".

But I think we've exhausted this topic, it boils down to a difference of interpretation.