Skip to content

Tag omitzero respects IsZero() #444

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

lzap
Copy link

@lzap lzap commented May 30, 2025

This patch adds support for IsZero method semantics introduced in Go 1.24 for JSON. When a type has this method implemented as func (T) IsZero() bool and it returns true and TOML Go struct omitzero exists, the library will omit it completely.

This introduces a change, Go built-in type time.Time already has this method and previously when an empty time was marshaled it created TOML time = 0001-01-01T00:00:00Z while now this value is skipped. But this will be only done for those fields which are tagged with omitzero which is not an issue, no?

Solves: #443

Note this feature does not need Go 1.24, it only follows what Go team is doing for JSON but this will work with any Go version supported by toml.

@arp242
Copy link
Collaborator

arp242 commented Jun 4, 2025

previously when an empty time was marshaled it created TOML time = 0001-01-01T00:00:00Z

This should not be the case; unlike e.g. encoding/json it should skip empty structs with the omitempty tag; for example:

func main() {
	type s struct {
		I int       `toml:"i"`
		T time.Time `toml:"t,omitempty"`
	}

	{
		t, err := toml.Marshal(s{1, time.Now()})
		if err != nil {
			panic(err)
		}
		fmt.Printf("%q\n", string(t))
	}
	{
		t, err := toml.Marshal(s{2, time.Time{}})
		if err != nil {
			panic(err)
		}
		fmt.Printf("%q\n", string(t))
	}
}

Outputs:

"i = 1\nt = 2025-06-04T22:45:18.68785576+01:00\n"
"i = 2\n"

The entire situation is a bit unfortunate, as omitempty/omitzero tags have somewhat different meanings than in stdlib (this library goes back to before Go 1.0). I'm not sure what the best solution is 🤔

@lzap
Copy link
Author

lzap commented Jun 5, 2025

Oh so is my understanding that my patch is completely useless and the library already does this? Wow, this is not the first time this happened to me. I can close then.

@arp242
Copy link
Collaborator

arp242 commented Jun 5, 2025

Not entirely; ideally I would like this to behave identical to stdlib. I'm just not entirely sure what the best path for that is 🤔 I'd like to avoid a v2 if at all possible.

Just saying that you shouldn't need this patch for your particular issue.

@lzap
Copy link
Author

lzap commented Jun 5, 2025

Okay let me know if you want me to rewrite the patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants