Set null value for empty document #725
Conversation
|
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #725 +/- ##
==========================================
+ Coverage 77.92% 77.95% +0.03%
==========================================
Files 22 22
Lines 7986 7993 +7
==========================================
+ Hits 6223 6231 +8
+ Misses 1348 1347 -1
Partials 415 415 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR addresses #724 by ensuring that an empty YAML document is decoded to a nil value.
- Added a new test (TestSetNullValue) for verifying that empty or explicit null documents yield a nil value.
- Modified decoding logic in decode.go to set the target to a zero value when no document is present.
- Adjusted error handling in DecodeContext by removing a conditional check for io.EOF.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| decode_test.go | Added tests that confirm an empty string or "null" source correctly sets nil. |
| decode.go | Updated decoding logic to handle empty documents by setting the target to zero and removed redundant EOF handling. |
Comments suppressed due to low confidence (1)
decode.go:1987
- The removal of special handling for io.EOF in DecodeContext may change error propagation behavior. Please confirm that this change aligns with the intended API semantics for yaml.Unmarshal.
if err := d.decode(ctx, rv); err != nil {
| @@ -3685,3 +3685,31 @@ releases: | |||
| t.Fatal(err) | |||
| } | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
[nitpick] Consider adding a brief comment above this test function to explain that it verifies empty or null YAML documents are decoded as nil.
| // TestSetNullValue verifies that empty or null YAML documents are decoded as nil. |
Copilot uses AI. Check for mistakes.
| @@ -1930,6 +1932,13 @@ func (d *Decoder) decodeInit(ctx context.Context) error { | |||
| func (d *Decoder) decode(ctx context.Context, v reflect.Value) error { | |||
| d.decodeDepth = 0 | |||
| d.anchorValueMap = make(map[string]reflect.Value) | |||
| if len(d.parsedFile.Docs) == 0 { | |||
There was a problem hiding this comment.
[nitpick] It would be helpful to add a comment clarifying why the condition 'if dst.Type().Kind() != reflect.Ptr || !dst.IsNil()' is used when setting the target value on an empty document.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Pull Request Overview
This PR addresses the issue "Set null value for empty document" by adding test cases and modifying the decoder logic to correctly handle empty or "null" YAML documents.
- Added TestSetNullValue in decode_test.go to check for proper nil assignment for empty documents and explicit "null" values.
- Modified decode.go to handle nil destination pointers and to set the zero value when the document is empty.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| decode_test.go | Added test cases to verify that empty and "null" YAML documents are unmarshaled to nil. |
| decode.go | Adjusted decoder behavior to set null/zero values and updated pointer validation logic. |
| @@ -1930,6 +1935,13 @@ func (d *Decoder) decodeInit(ctx context.Context) error { | |||
| func (d *Decoder) decode(ctx context.Context, v reflect.Value) error { | |||
| d.decodeDepth = 0 | |||
| d.anchorValueMap = make(map[string]reflect.Value) | |||
| if len(d.parsedFile.Docs) == 0 { | |||
There was a problem hiding this comment.
For an empty document, while the zero value is set in decodeInit, the function proceeds to check if len(d.parsedFile.Docs) is less than or equal to d.streamIndex, which then returns io.EOF even for valid empty documents. Consider adding a return statement after setting the zero value to prevent returning an unintended error.
Copilot uses AI. Check for mistakes.
fix #724