Skip to content

Commit

Permalink
Error out on nil returns from MarshalTOML/MarshalText
Browse files Browse the repository at this point in the history
encoding/json encodes it as "", but IMHO that's confusing, and since it
would generate invalid TOML before (`keyname =`) it's okay to change
this to an error.

Another alternative might be to skip the field if the return value is
nil, but that's probably also a bit confusing in some cases, and we can
change error to skip in the future and remain compatible, but we can't
do the reverse.
  • Loading branch information
arp242 committed Jun 7, 2022
1 parent 4786019 commit a8d6de8
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 0 deletions.
6 changes: 6 additions & 0 deletions encode.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,13 +213,19 @@ func (enc *Encoder) eElement(rv reflect.Value) {
if err != nil {
encPanic(err)
}
if s == nil {
encPanic(errors.New("MarshalTOML returned nil and no error"))
}
enc.w.Write(s)
return
case encoding.TextMarshaler:
s, err := v.MarshalText()
if err != nil {
encPanic(err)
}
if s == nil {
encPanic(errors.New("MarshalText returned nil and no error"))
}
enc.writeQuoted(string(s))
return
case time.Duration:
Expand Down
38 changes: 38 additions & 0 deletions encode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,44 @@ Fun = "why would you do this?"
}
}

type (
retNil1 string
retNil2 string
)

func (r retNil1) MarshalText() ([]byte, error) { return nil, nil }
func (r retNil2) MarshalTOML() ([]byte, error) { return nil, nil }

func TestEncodeEmpty(t *testing.T) {
t.Run("text", func(t *testing.T) {
var (
s struct{ Text retNil1 }
buf bytes.Buffer
)
err := NewEncoder(&buf).Encode(s)
if err == nil {
t.Fatalf("no error, but expected an error; output:\n%s", buf.String())
}
if buf.String() != "" {
t.Error("\n" + buf.String())
}
})

t.Run("toml", func(t *testing.T) {
var (
s struct{ Text retNil2 }
buf bytes.Buffer
)
err := NewEncoder(&buf).Encode(s)
if err == nil {
t.Fatalf("no error, but expected an error; output:\n%s", buf.String())
}
if buf.String() != "" {
t.Error("\n" + buf.String())
}
})
}

// Would previously fail on 32bit architectures; can test with:
// GOARCH=386 go test -c && ./toml.test
// GOARCH=arm GOARM=7 go test -c && qemu-arm ./toml.test
Expand Down

0 comments on commit a8d6de8

Please sign in to comment.