-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
encoding/json: UnmarshalTypeError.Error message seems insensible #43126
Comments
Related considerations:
Strawman proposal: the JSON path should be a valid expression in JavaScript to address the specific value from the root value.
|
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. |
One option is RFC 6901, which uses
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. |
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 |
The
|
… is set Fixes golang#58649 without relying on the mentioned refactoring/cleaning in golang#43126 With most efforts redirected towards encoding/json v2. I don't think golang#43126 will ever be addressed in v1. I don't think we should consider it as a requirement for this patch. This could have been added as part of golang@2596a0c encoding/json v2 is moving in the same direction by adding context by default as shown in https://github.com/go-json-experiment/json/blob/4e0381018ad68adc9c0cb7896d109c994429654e/errors.go#L364-L373 But we don't need to wait for v2 to become stable to improve UX and and save users some time, and help them keep their hair!
Change https://go.dev/cl/648055 mentions this issue: |
Consider this snippet:
On Go1.15, this prints:
Notice that it combines two different namespaces where it is:
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
The text was updated successfully, but these errors were encountered: