golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.29k stars 17.58k forks source link

encoding/json: UnmarshalTypeError.Error message seems insensible #43126

Open dsnet opened 3 years ago

dsnet commented 3 years ago

Consider this snippet:

type GoStruct1 struct {
    AlphaField GoStruct2 `json:"alpha_field"`
}
type GoStruct2 struct {
    BravoField []GoStruct3 `json:"bravo_field"`
}
type GoStruct3 struct {
    StringField string `json:"string_field"`
}

func main() {
    fmt.Println(json.Unmarshal([]byte(`{"alpha_field":{"bravo_field":[{"string_field": 3.14159}]}}`), new(GoStruct1)))
}

On Go1.15, this prints:

json: cannot unmarshal number into Go struct field GoStruct3.alpha_field.bravo_field.string_field of type string

Notice that it combines two different namespaces where it is:

{{GoTypeName}}.{{JSONPath}}

and that the message says that it is a "Go struct field" when it isn't. Also, it uses GoStruct3, when it makes little sense. At minimum, it should at least used GoStruct1.

\cc @mvdan

dsnet commented 3 years ago

Related considerations:

Strawman proposal: the JSON path should be a valid expression in JavaScript to address the specific value from the root value.

mvdan commented 3 years ago

I agree with the strawman proposal. In that proposal, what would you use in place of GoStruct3?

Also, is there a "json path" spec we can simply adhere to? or do we simply try to get as close to a JS expression as we reasonably can?

cc @cuonglm who worked on the original feature. I did think about proposing to make it more consistent with slices and such, but at the end of the day I thought the addition was already a big step forward.

dsnet commented 3 years ago

Also, is there a "json path" spec we can simply adhere to?

One option is RFC 6901, which uses / instead of . as the delimiter. This would be the visual difference between .fooName["quoted/name"].barName[5] and /fooName/quoted~1Name/barName/5. I kind of like the simplicity and visual aesthetics of this. It only requires escaping of the ~ and / as ~0 and ~1 respectively (note that ASCII control characters in U+0000 to U+0020 are left as is).

do we simply try to get as close to a JS expression as we reasonably can?

The grammar for a JS identifier is pretty complicated. Fortunately, the Go identifier grammar is a strict subset of the JS identifier grammar, so we could just use that as a shortcut.

dsnet commented 3 years ago

Another oddity. If the target type is a Go map, then the JSON object names for those are dropped: https://play.golang.org/p/DP-bAr_6TUy

fl0cke commented 3 years ago

The GoStruct3 at the beginning is extremely confusing. I would either