Repository: goccy/go-yaml

Issue:
bug(encode): `omitempty` is not following strictly encoding/json definition
**Describe the bug**

As discussed on [the global `omitempty` option PR](https://github.com/goccy/go-yaml/pull/691#discussion_r2039813449), we noticed go-yaml is implementing `omitempty`in a different way than Go std encoding/json which may cause some surprises. Initially we thought `omitempty` is implemented as `omitzero`, but in fact go-yaml `omitempty` is a mix of features from std encoding `omitempty` and `omitzero` features (: 

Technically, those those two are [a distinct separate options e.g. in `encoding/json`](https://pkg.go.dev/github.com/go-json-experiment/json#section-readme:~:text=omitzero%3A%20When%20marshaling%2C%20the%20%22omitzero%22) with `omitzero` being introduced [only recently to the std json lib.](https://tip.golang.org/doc/go1.24#encodingjsonpkgencodingjson). 

The difference is a [bit nuanced](https://github.com/golang/go/discussions/63397#discussioncomment-7201224), but the key difference can be distilled into two main things:

* `omitzero` should [not omit `[]int{}` , `omitempty` should](https://go.dev/play/p/nQkKeZAaptv). go-yaml works [well here, just do not implement omitzero](https://go.dev/play/p/YGeK8oHy3Bn).
* `omitempty` should [not treat nested structs zero value as empty](https://go.dev/play/p/bn5B1PDoFmf), but go-yaml [does](https://go.dev/play/p/siC70aKxkmj)
* `omitzero` should check `IsZero() bool` interface, `omitempty` not.

This "bug" is often a feature, because generally what user wants is something close to `omitzero`. Perhaps this is [why `gopkg.in/v2` implemented `omitempty` as such](https://pkg.go.dev/gopkg.in/yaml.v2#section-readme:~:text=Only%20include%20the%20field%20if%20it%27s%20not%20set%20to%20the%20zero) and [go-yaml simply took that](https://github.com/goccy/go-yaml/blob/ee37df774be8ac7daf35270a9be0f4de1811affe/encode.go#L734).

Thoughts? @goccy

**Version Variables**
 - Go version: 1.24
 - go-yaml's Version: all until now 1.17.1



Discussion:
@bwplotka: Note that generally the [difference between of `omitempty` vs `omitzero` is full of nuances](https://github.com/golang/go/discussions/63397#discussioncomment-7201224) (e.g. [`omitzero` for `[]int{}` should NOT omit that field as it's empty but NOT zero, `omitempty` should omit it](https://github.com/golang/go/issues/22480)).

One would need to go one by one through all cases OR this library might choose to simplify things and never (e.g. in v1) support omitempty as NOT omitzero 
@goccy: Thank you for your reports. I think the steps from 0 to 3 are fine for me too. I think this is the best way to introduce omitempty and omitzero without breaking changes. However, for the last step 4, I would prefer not to release v2 in order to change the behaviour of omitempty, as releasing v2 is not something that can be done casually, as I would like to apply other improvements to it. In other words, we should proceed in the direction of introducing steps up to 3.@bwplotka: So I updated my issue, with more accurate info. I initially thought `omitempty` go-yaml is implemented as `omitzero`, but looking closer it's a mix of both. Let me update the proposal here:

**Proposal**

This library was relased under stable version 1.x so I don't think it's sensible to "fix" this in v1 and break existing users. I would propose the following:

0. We document this behaviour of `omitempty` in API docs. (the fact it's not standard).
1. ~We rename non-released `OmitEmpty` global option to `OmitZero` while we can. After all it controls omitzero not omitempty with the current implementation.~  I think we should actually keep it `omitempty` as this is our current omit empty definition and that's ok. We can't fix it in v1 and perhaps we should never actually fix it, if it's useful to users 👍🏽 
2. We add support for `omitzero` struct tags, ~which in this version of go-yaml will do exactly the same as `omitempty`~ which needs to be a bit different if we want to match standard definition (I think we should), especially that `int[]` behaviuour.
3. We add a new encoding option which makes `omitempty` behaves like omit empty not omitzero. This "fixes" the bug without breaking changes and allows those who are surprised to decide what behaviour they want.
4. In v2 we reconsider what omitempty should do by default.

> I would prefer not to release v2 in order to change the behaviour of omitempty, as releasing v2 is not something that can be done casually, as I would like to apply other improvements to it. In other words, we should proceed in the direction of introducing steps up to 3.

Yea, totally, that's why I don't think there's a rush, with 0-3 steps go-yaml can work without v2 for decades and it will be ok 👍🏽 
@bwplotka: 0 step fix https://github.com/goccy/go-yaml/pull/696 

Just to reiterate, we skip 1 as I think we should keep global `OmitEmpty` option as-is.

I guess we could create dedicated issue for `omitzero`?  (2 step), also for 3 step?@goccy: @bwplotka 
Before the next release, we want to add the OmitZero option and align the current behavior of the OmitEmpty option with the behavior of encoding/json ( fix current OmitEmpty option's implementation ). After that, we will add the omitzero tag in another PR. Finally, the OmitEmpty / OmitZero options will exhibit the correct behavior as options, and in struct tags, both omitempty and omitzero will exhibit the behavior of omitzero. After allowing sufficient time, we plan to change the behavior of omitempty as a breaking change.

Until the omitzero tag is added, let's use this Issue.@bwplotka: Makes sense except the:

> After allowing sufficient time, we plan to change the behavior of omitempty as a breaking change

My 2c:

I would be careful with that. Changing the default behaviour of omitempty to true omitempty will likely "break" (produce unexpected) encoding on a simple (usually automatic) minor upgrade of this module. This can easily break production environments as a result (e.g. client uses go-yaml on a structure with unsupported not filled fields by the server. Upgrade causes omitempty to not apply anymore and the field is present, causing server to crash etc). 

I would suggest we enable opt-in true omitempty and listen to the community feedback if this module should switch that to opt-out in v1 or in v2 (in 10years).@bwplotka: Unless by breaking change you mean v2, then ok.@goccy: > I would suggest we enable opt-in true omitempty and listen to the community feedback if this module should switch that to opt-out in v1 or in v2

Yes, this is how I intend to proceed in practice. How much time it will take depends on my maintenance costs and support from the community. However, for now, we need to set aside the behavior of the omitempty tag and address renaming the OmitEmpty option to the OmitZero option. 

@bwplotka Could you handle this? If it's difficult, I will take care of it.@goccy: @bwplotka I've completed all the steps I initially planned. Please review them at your convenience.

https://github.com/goccy/go-yaml/pull/729@goccy: @bwplotka I apologize, but this issue is a blocker for the next release, so if you could review the PR, please let me know. If you don't have time to review, I plan to merge it.

https://github.com/goccy/go-yaml/pull/729@bwplotka: Will check today!@bwplotka: Reviewed! Feel free to comment, will check tomorrow for replies

Solution patch:
diff --git a/encode.go b/encode.go
index e0c795e..813d7fc 100644
--- a/encode.go
+++ b/encode.go
@@ -38,6 +38,7 @@ type Encoder struct {
 	anchorNameMap              map[string]struct{}
 	anchorCallback             func(*ast.AnchorNode, interface{}) error
 	customMarshalerMap         map[reflect.Type]func(interface{}) ([]byte, error)
+	omitEmpty                  bool
 	autoInt                    bool
 	useLiteralStyleIfMultiline bool
 	commentMap                 map[*Path][]*Comment
@@ -824,7 +825,7 @@ func (e *Encoder) encodeStruct(ctx context.Context, value reflect.Value, column
 		}
 		fieldValue := value.FieldByName(field.Name)
 		structField := structFieldMap[field.Name]
-		if structField.IsOmitEmpty && e.isZeroValue(fieldValue) {
+		if (e.omitEmpty || structField.IsOmitEmpty) && e.isZeroValue(fieldValue) {
 			// omit encoding
 			continue
 		}
diff --git a/option.go b/option.go
index 7dcd2ba..3849249 100644
--- a/option.go
+++ b/option.go
@@ -215,6 +215,15 @@ func AutoInt() EncodeOption {
 	}
 }
 
+// OmitEmpty forces the encoder to assume an `omitempty` struct tag is
+// set on all the fields. See `Marshal` commentary for the `omitempty` tag logic.
+func OmitEmpty() EncodeOption {
+	return func(e *Encoder) error {
+		e.omitEmpty = true
+		return nil
+	}
+}
+
 // CommentPosition type of the position for comment.
 type CommentPosition int
 



Test patch:
diff --git a/encode_test.go b/encode_test.go
index a82988f..2eba495 100644
--- a/encode_test.go
+++ b/encode_test.go
@@ -465,7 +465,7 @@ func TestEncoder(t *testing.T) {
 			nil,
 		},
 
-		// Conditional flag
+		// Omitempty flag.
 		{
 			"a: 1\n",
 			struct {
@@ -482,7 +482,6 @@ func TestEncoder(t *testing.T) {
 			}{0, 0},
 			nil,
 		},
-
 		{
 			"a:\n  \"y\": \"\"\n",
 			struct {
@@ -496,7 +495,6 @@ func TestEncoder(t *testing.T) {
 			}{}},
 			nil,
 		},
-
 		{
 			"a: {}\n",
 			struct {
@@ -510,7 +508,6 @@ func TestEncoder(t *testing.T) {
 			}{}},
 			nil,
 		},
-
 		{
 			"a: {x: 1}\n",
 			struct {
@@ -518,7 +515,6 @@ func TestEncoder(t *testing.T) {
 			}{&struct{ X, y int }{1, 2}},
 			nil,
 		},
-
 		{
 			"{}\n",
 			struct {
@@ -526,7 +522,6 @@ func TestEncoder(t *testing.T) {
 			}{nil},
 			nil,
 		},
-
 		{
 			"a: {x: 0}\n",
 			struct {
@@ -534,7 +529,6 @@ func TestEncoder(t *testing.T) {
 			}{&struct{ X, y int }{}},
 			nil,
 		},
-
 		{
 			"a: {x: 1}\n",
 			struct {
@@ -567,8 +561,29 @@ func TestEncoder(t *testing.T) {
 			},
 			nil,
 		},
+		// OmitEmpty global option.
+		{
+			"a: 1\n",
+			struct {
+				A int
+				B int `yaml:"b,omitempty"`
+			}{1, 0},
+			[]yaml.EncodeOption{
+				yaml.OmitEmpty(),
+			},
+		},
+		{
+			"{}\n",
+			struct {
+				A int
+				B int `yaml:"b,omitempty"`
+			}{0, 0},
+			[]yaml.EncodeOption{
+				yaml.OmitEmpty(),
+			},
+		},
 
-		// Flow flag
+		// Flow flag.
 		{
 			"a: [1, 2]\n",
 			struct {