-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
add support for strict errors #796
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still not convinced (more context in thread kubernetes-sigs/yaml#61) that we should allow yaml.v3 to be non-spec compliant and allow duplicate fields at any time, although yaml.v2 allows duplicate fields by default.
JSON just "recommends" unique keys, for which it then makes sense that unique keys is a sense of strictness, but YAML mandates their uniqueness, hence I do not think of that as a strictness measure.
I think the change to separate strict and non-strict errors is good though.
decode.go
Outdated
doc *Node | ||
aliases map[*Node]bool | ||
terrors []string | ||
strictTerrors []string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should probably be strictErrors
? I guess t stands for type, but it is still a bit weird when reading
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed it
Signed-off-by: Inteon <[email protected]>
Signed-off-by: Inteon <[email protected]>
@luxas as requested, I now only consider "unknown field" errors as strict errors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -129,8 +129,8 @@ func (dec *Decoder) Decode(v interface{}) (err error) { | |||
out = out.Elem() | |||
} | |||
d.unmarshal(node, out) | |||
if len(d.terrors) > 0 { | |||
return &TypeError{d.terrors} | |||
if len(d.terrors) > 0 || len(d.strictErrors) > 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure if this is now common enough to have its own helper func attached to the decoder, something like if err := d.checkErr(); err != nil { return err }
... it would add some overhead due to one extra fn call so not sure if it's worth it.
I'm closing this PR, since I'm currently working on other things. |
based on #752
Now, "unknown field" errors are considered strict errors.
This means that unmarshalling will still return a valid response, but a strict error is returned too.
The library user can now decide eg. to use the result and also display a warning.
ref: kubernetes-sigs/yaml#61