Skip to content
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

Unexpected behaviour with the TextMarshaler interface #357

Closed
alessandro-guido opened this issue Jun 4, 2022 · 4 comments · Fixed by #358
Closed

Unexpected behaviour with the TextMarshaler interface #357

alessandro-guido opened this issue Jun 4, 2022 · 4 comments · Fixed by #358

Comments

@alessandro-guido
Copy link

alessandro-guido commented Jun 4, 2022

The toml packages behaves unexpectedly when using the TextMarshaler interface. In particular, the toml package ignores a MarshalText implementation if the receiver is a pointer but the value to be serialized is not, like in the following example.

type Document struct {
	Field IntSlice
}

type IntSlice []int

func (items *IntSlice) MarshalText() ([]byte, error) {
    ...
}

I've tried to setup a complete example here: https://go.dev/play/p/tMLzMg5sqTe. The json package, IMHO correctly, makes use of the MarshalText defined as above. Changing the signature like below is required to make the toml package use the custom MarshalText function.

func (items IntSlice) MarshalText() ([]byte, error) {

Decoding interface is not affected by this problem, i.e. an UnmarshalText function is picked even if it has a pointer type receiver (tested locally, not shown in the example code).

@arp242
Copy link
Collaborator

arp242 commented Jun 4, 2022

Does this still happen in the latest master? Some changes were made last week (specifically: #354).

@alessandro-guido
Copy link
Author

Yes, it does. Tested with BurntSushi/toml@v1.1.1-0.20220529223624-8fccf3c6afbe

@arp242
Copy link
Collaborator

arp242 commented Jun 4, 2022

Alrighty, just checking!

There is actually already a test case for this: https://github.com/BurntSushi/toml/blob/master/encode_test.go#L342-L343 – although it tests that it "fails" rather than works.

I took a quick look, and encoding/json contains some additional code to handle this, in newTypeEncoder(). Need to look a bit at how that works and adjust Encoder.encoder accordingly here.

To set expectations: I probably won't have time for this in the next few weeks, but I'll review patches if you feel like taking a crack at it.

@kkHAIKE
Copy link
Contributor

kkHAIKE commented Jun 16, 2022

@arp242 make a fix for it, #358
😊

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

Successfully merging a pull request may close this issue.

3 participants