When two struct fields have the same tag, a matching TOML value is silently ignored

[johnmaguire]

PoC code:

package main

import (
	"fmt"

	"github.com/BurntSushi/toml"
)

type Config struct {
	A string `toml:"a"`
	B string `toml:"b"`
}

type ConfigDup struct {
	A  string `toml:"a"`
	A2 string `toml:"a"` // note that this is the same as the struct field above
}

func main() {
	myCfg := `
a = "foo"
b = "bar"
`

	var cfg Config
	var cfgDup ConfigDup

	_, err := toml.Decode(myCfg, &cfg)
	if err != nil {
		panic(err)
	}

	_, err = toml.Decode(myCfg, &cfgDup)
	if err != nil {
		panic(err)
	}

	fmt.Printf("cfg: %+v\n", cfg)
    fmt.Printf("cfgDup: %+v\n", cfgDup)
}

Output:

cfg: {A:foo B:bar}
cfgDup: {A: A2:}

Expected output:

cfg: {A:foo B:bar}
cfgDup: {A:foo A2:foo}

Or... an error returned from the Decode function (this would've been preferable for our case, where this was a typo.)

[arp242]

encoding/json seems to set only the first one:

package main

import (
	"encoding/json"
	"fmt"

	"github.com/BurntSushi/toml"
)

type Config struct {
	A  string `toml:"a"`
	A2 string `toml:"a"`
}

func main() {
	var c, c2 Config
	err := json.Unmarshal([]byte(`{"a": "XXXX"}`), &c)
	fmt.Printf("json: %s: %#v\n", err, c)

	_, err = toml.Decode(`a="XXXXX"`, &c2)
	fmt.Printf("toml: %s: %#v\n", err, c2)
}

Outputs:

json: %!s(<nil>): main.Config{A:"XXXX", A2:""}
toml: %!s(<nil>): main.Config{A:"", A2:""}

I guess that's better? Personally I think both are confusing and I'd prefer to return an error, but usually I try to follow the behaviour of encoding/json.

[johnmaguire]

@arp242 I think a returned error would be completely reasonable... in our case the declaration of the struct tag was a mistake/bug. We copied and pasted from the line above and failed to update the toml tag on the copy - an error would've meant we'd catch the bug prior to deploy to our development environment.

Following JSON's approach would mean nothing previously working would be broken, but the new config value would be ignored. (A better spot to be in than the one we experienced where the old value also stopped working, and we had to figure out why.)

[arp242]

I agree; but on the other hand being a drop-in replacement for encoding/json – warts and all – also has some value. It's not uncommon to (un)marshal the same struct from multiple formats (TOML, JSON, YAML, etc.)

A second concern is that changing the behaviour is essentially backwards-incompatible, and that it might break "accidentally works" types of scenarios. I have no idea if it'll break for one person, 10 people, or hundreds of people.