Optional support of Json struct tags and (Un)Marshaler interfaces as fallback #154

SVilgelm · 2025-10-26T04:33:01Z

This PR makes Go struct tag resolution more flexible when types use json struct tags instead of yaml struct tags and ensures anonymous struct fields are inlined consistently under the fallback mode.
It also fallbacks to json.(Un)Marshaller interfaces if they are implemented.
Note: The fallback to the interface implemented by using double conversion: Obj -> JSON -> any -> YAML for marshaling or Node -> any -> JSON -> Obj for unmarshaling. There is no more direct way to invoke interfaces, so this trade-off is required for compatibility.

Example:

type Test struct {
  F string `json:foo`
}

func main() {
  text := []byte(`foo: "bar"`)
  var v Test
  d := NewDecoder(bytes.NewReader(text))
  d.FallbackToJSON(true)
  d.Decode(&v)
  fmt.Println(v.F)
  // Output:
  // bar
}

Fixes #153

Update getStructInfo function and add tests
Update struct tag parsing accept an optional fallback to the json struct tag and propagate that behavior through recursive calls.
- Add a fallbackToJSON parameter to getStructInfo so callers can request that empty yaml tags be replaced by the json struct tag when present.
- When fallbackToJSON is enabled, treat anonymous fields whose json struct tag exists (or which have no yaml struct tag but fallback is on) as inline if their type is a struct or pointer-to-struct.
- Propagate fallbackToJSON in recursive getStructInfo calls for nested types so tag resolution remains consistent.
- Add comprehensive tests for getStructInfo covering normal and fallback-to-json behavior.
Add FallbackToJSON option to encode and decoder
- Add fallbackToJSON bool fields to internal decoder and encoder
  structs.
- Pass fallbackToJSON into getStructInfo calls so struct tag lookup
  considers json struct tags when enabled.
- Expose Decoder.FallbackToJSON and Encoder.FallbackToJSON methods that
  set the option on the public types.
- Wire Decoder.Decode to transfer the public Decoder setting into the
  internal decoder instance; likewise for Encoder.
- Add examples (tests) demonstrating Decoder.FallbackToJSON and
  Encoder.FallbackToJSON behavior.
Support json.(Un)Marshaler interfaces
- implements encoder path for json.Marshaler to convert JSON output to
  YAML via an intermediate interface{} and then marshal that.
- adds decoder.callJSONUnmarshaler to route a decoded node through
  json.Unmarshal to satisfy json.Unmarshaler implementations and report
  errors with proper node position.
- extends decoder selection logic to try json.Unmarshaler as a fallback
  when FallbackToJSON(true) is set.
- updates tests (yaml_test.go) to cover both encoder and decoder JSON
  fallback behavior via a TestFallbackToJSON type that tracks when
  MarshalJSON/UnmarshalJSON are invoked and verifies round-trip
  behavior.
Normalize JSON
- change callJSONUnmarshaler to return bool and collect TypeError and
  UnmarshalError details from the initial node.Decode call.
- extract JSON marshaling/unmarshaling logic into unmarshalJSON helper.
- add normalizeJSON to convert YAML-parsed structures into JSON-safe
  structures: convert map[any]any to map[string]any (stringifying
  non-string keys via JSON), recursively normalize arrays and maps, and
  detect duplicate keys that would conflict after conversion.
- add tests for json.Unmarshaler support including maps with string and
  non-string keys to ensure correct normalization and error handling.

Copilot

Pull Request Overview

This PR adds support for optional JSON tag fallback in YAML struct parsing. When enabled, the YAML parser will use json tags as a fallback when yaml tags are not present, and automatically treats anonymous struct fields with JSON tags (or no tags when fallback is enabled) as inline.

Key Changes:

Modified getStructInfo function to accept a fallbackToJSON parameter that enables JSON tag fallback behavior
Added logic to automatically inline anonymous struct/pointer-to-struct fields when fallback mode is active
Added comprehensive unit tests covering normal and fallback-to-JSON scenarios

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
yaml.go	Core implementation: added `fallbackToJSON` parameter to `getStructInfo` and logic for JSON tag fallback and automatic inlining of anonymous fields
yaml_test.go	Comprehensive test suite for `getStructInfo` covering tag resolution with and without JSON fallback, including inline field scenarios
encode.go	Updated `getStructInfo` call to pass `false` for `fallbackToJSON` parameter
decode.go	Updated `getStructInfo` call to pass `false` for `fallbackToJSON` parameter

Comments suppressed due to low confidence (1)

yaml.go:629

The cache lookup does not consider the fallbackToJSON parameter. Two calls with different fallbackToJSON values for the same type will incorrectly return the same cached result, leading to incorrect behavior. Consider incorporating fallbackToJSON into the cache key or disabling caching when this parameter varies.

func getStructInfo(st reflect.Type, fallbackToJSON bool) (*structInfo, error) {
	fieldMapMutex.RLock()
	sinfo, found := structMap[st]
	fieldMapMutex.RUnlock()
	if found {
		return sinfo, nil

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

yaml_test.go

Copilot

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

yaml.go:645

The cache lookup doesn't account for the fallbackToJSON parameter. When getStructInfo is called with different fallbackToJSON values for the same type, it will incorrectly return cached results from the first call. The cache key should include both the type and the fallback flag, or the cache should be disabled when fallback is enabled.

func getStructInfo(st reflect.Type, fallbackToJSON bool) (*structInfo, error) {
	fieldMapMutex.RLock()
	sinfo, found := structMap[st]
	fieldMapMutex.RUnlock()
	if found {
		return sinfo, nil
	}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

yaml.go

Copilot

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

yaml_test.go

yaml.go

decode.go

Copilot

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

yaml_test.go

encode.go

decode.go

Copilot

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

decode.go

encode.go

decode.go

SVilgelm · 2025-10-27T21:41:15Z

@ccoVeille Could you please review this PR? It is huge, but all changes are hidden behind FallbackToJSON option, so should be safe to merge.
The only concern I have that the PR is huge and maybe it is better to split it for better readability.

SVilgelm · 2025-10-28T00:19:13Z

go vet step fails:

# go.yaml.in/yaml/v4
Error: ./yaml_test.go:28:5: struct field tag `just_b` not compatible with reflect.StructTag.Get: bad syntax for struct tag pair
Error: ./yaml_test.go:127:5: struct field tag `just_b` not compatible with reflect.StructTag.Get: bad syntax for struct tag pair
Error: Process completed with exit code 2.

What should I do in this case? I can remove a test case for "Foo `foo`" scenario or govet CI step can be removed because it is covered by golangci-lint

Also, I'm going to fix the commit messages to comply with the commit check

ccoVeille · 2025-10-28T00:36:21Z

I was busy with personal project today.
I only reviewed your PR quickly.

There are a few things that I would have done differently, and one I'm worried about even if I didn't took the time to check because it's late here. And I don't have a computer with me.

So please bare with me, I might be wrong and/or tired 😅

I wouldn't have added a FallbackToJSON method to the decoder, but an Option pattern to a new method named DecodeWithOptions

But it would require #155 first.

The other things I'm worried about is the behavior of code when YAML and JSON tags are present. Especially when their logic differ.

type Whatever struct {
 Password string `yaml:"password" json:"pwd"`
 Secret string `yaml:"-" json:"secret"`
 Bar int `json:"bar"`
 Foo string `json:"foooo"`
}

With the following yaml

password: qwerty
pwd: ~
secret: danger
bar: 42
foo: 123
foooo: 456

I would expect to get

Whatever{
 Password: "qwerty" // and not "~"
 Secret: "" // because of yaml:"-"
 Bar: 42
 Foo: "123" // not "456" because of foooo
}

Here I'm trying to find extreme awkward examples, but I feel like they should be considered.

I wrote this on a phone. I didn't check. But I would expect this to be discussed with many people, because once added it would be difficult, if not impossible to change later.

Please note, while I do understand the need. I don't see an urge to add this feature quickly.

But I get the idea, and I like it. It requires some thoughts, discussions, and iterations.

SVilgelm · 2025-10-28T02:38:17Z

Thank you for reviewing.

Here I'm trying to find extreme awkward examples, but I feel like they should be considered.

I have covered this case and created the unit tests, the yaml tag always takes a precedent:

I wouldn't have added a FallbackToJSON method to the decoder, but an Option pattern to a new method named DecodeWithOptions

We can talk about this in #155. I am fully open for any ideas.

Please note, while I do understand the need. I don't see an urge to add this feature quickly.

Sure, no rush at all

I still have a question about govet step in CI. Should I drop a test case for a scenario when a structure tag is not compatible:

type Whatever struct {
 Password string `yaml:"password" json:"pwd"` // It is okay
 Secret string `secret` // <- It is not compatible, but supported by current code
}

Or we can drop the govet step in CI :) Any way it is covered by golangci-lint, which allows to add //nolint:govet comments to ignore those cases.

ccoVeille · 2025-10-28T07:12:58Z

Thank you for reviewing.

Here I'm trying to find extreme awkward examples, but I feel like they should be considered.

I have covered this case and created the unit tests, the yaml tag always takes a precedent:

https://github.com/yaml/go-yaml/pull/154/files#diff-5afd41e7e0a70c3a6f0e1572f6938d9aacf5bc590033f7e798a0fb6f741e51ddR29

https://github.com/yaml/go-yaml/pull/154/files#diff-5afd41e7e0a70c3a6f0e1572f6938d9aacf5bc590033f7e798a0fb6f741e51ddR53

I will take a look, thanks

I wouldn't have added a FallbackToJSON method to the decoder, but an Option pattern to a new method named DecodeWithOptions

We can talk about this in #155. I am fully open for any ideas.

Let's continue discussion in #155

Please note, while I do understand the need. I don't see an urge to add this feature quickly.

Sure, no rush at all

I still have a question about govet step in CI. Should I drop a test case for a scenario when a structure tag is not compatible:
type Whatever struct {
 Password string `yaml:"password" json:"pwd"` // It is okay
 Secret string `secret` // <- It is not compatible, but supported by current code
}

Could you open an issue for this?

I mean the fact secret is supported even if the tag is invalid.

May I challenge you here?

For me, it's normal and expected that it works.

Consider this:

type Whatever struct {
  Password string `yaml:"password" json:"pwd"` // It is okay
  Secret string `secret2` // <- what does current code?
}

With

password: qwerty
secret2: danger

I would expect a "bug" only if it would be decoded with

var _ = Whatever{Password: "qwerty", Secret: "danger"}

I mean I would expect the lib to fall back on default behavior of trying to match the name of field in the struct, as if no struct tag were provided.

What does json.Unmarshal with this on the same struct ?

{
  "pwd": "qwerty",
  "secret": "danger"
}

Does it fail because tags are invalid or does it decode Secret=danger ?

Here again, I didn't check what you reported, I based my reply on your comment and my thoughts.

Unless you agree with me, please reply to this in the issue you will create about it.

Or we can drop the govet step in CI :) Any way it is covered by golangci-lint, which allows to add //nolint:govet comments to ignore those cases.

The govet thing is a good thing. The govet is automatic with go test. I get the point of being able to write invalid code to test an invalid behavior. I faced it and removed govet by using golangci-lint too.

Please note to disable govet in go test, go test -vet=off can be used

https://pkg.go.dev/cmd/go#hdr-Testing_flags

SVilgelm · 2025-10-28T15:18:48Z

The govet is automatic with go test

not really:

25-10-28 7:54 % go test ./...                                                                                                                                               
ok      go.yaml.in/yaml/v4      2.453s                                                                                                                                      
?       go.yaml.in/yaml/v4/cmd/go-yaml  [no test files]                                                                                                                     
?       go.yaml.in/yaml/v4/internal/libyaml     [no test files]                                                                                                             
ok      go.yaml.in/yaml/v4/internal/testutil/assert     (cached)                                                                                                            
ok      go.yaml.in/yaml/v4/yts  0.619s

% go vet ./...                                                                                                                                                
# go.yaml.in/yaml/v4                                                                                                                                                        
# [go.yaml.in/yaml/v4]                                                                                                                                                      
./yaml_test.go:28:5: struct field tag `just_b` not compatible with reflect.StructTag.Get: bad syntax for struct tag pair                                                    
./yaml_test.go:127:5: struct field tag `just_b` not compatible with reflect.StructTag.Get: bad syntax for struct tag pair

As you can see, I ran go test locally and it does not fail, but go vet fails. Okay, let me create a separate issue for this case to have a discussion in #157

Update struct tag parsing accept an optional fallback to the `json` tag and propagate that behavior through recursive calls. - Add a fallbackToJSON parameter to getStructInfo so callers can request that empty yaml tags be replaced by the json tag when present. - When fallbackToJSON is enabled, treat anonymous fields whose json tag exists (or which have no yaml tag but fallback is on) as inline if their type is a struct or pointer-to-struct. - Propagate fallbackToJSON in recursive getStructInfo calls for nested types so tag resolution remains consistent. - Add comprehensive tests for getStructInfo covering normal and fallback-to-json behavior. This makes tag resolution more flexible when types use json tags instead of yaml tags and ensures anonymous struct fields are inlined consistently under the fallback mode.

Introduce a fallbackToJSON flag through Decoder and Encoder APIs and propagate it to internal encoder/decoder instances. This enables using `json` struct tags as a fallback when no `yaml` tag is present, and treats anonymous fields accordingly for inlining. Key changes: - Add fallbackToJSON bool fields to internal decoder and encoder structs. - Pass fallbackToJSON into getStructInfo calls so struct tag lookup considers json tags when enabled. - Expose Decoder.FallbackToJSON and Encoder.FallbackToJSON methods that set the option on the public types. - Wire Decoder.Decode to transfer the public Decoder setting into the internal decoder instance; likewise for Encoder. - Add examples (tests) demonstrating Decoder.FallbackToJSON and Encoder.FallbackToJSON behavior. This improves compatibility with structs that only define json tags and makes YAML (de)serialization more flexible for mixed-tagged types.

decode.go

ccoVeille · 2025-10-30T21:33:45Z

yaml.go

+// FallbackToJSON enables using the "json" struct tags as a fallback
+// when no "yaml" struct tag is present.
+// If enabled, the anonymous struct fields are also considered for inlining.
+func (dec *Decoder) FallbackToJSON(enable bool) {


Except this that I would prefer not to be added here, but via an Option passed as a variadic, I'm fine with the changes

The options are not implemented yet. I'll change this logic to Option when they are ready for sure

Add support for falling back to JSON (un)marshaling when types implement encoding/json interfaces. When encoder.FallbackToJSON is enabled, types implementing json.Marshaler are serialized by calling MarshalJSON, decoding that JSON into an interface{}, and then marshaling that value into YAML. When decoder.FallbackToJSON is enabled, types implementing json.Unmarshaler are handled by decoding the YAML node into an interface{}, marshaling that value to JSON and passing the bytes to UnmarshalJSON. This change: - implements encoder path for json.Marshaler to convert JSON output to YAML via an intermediate interface{} and then marshal that. - adds decoder.callJSONUnmarshaler to route a decoded node through json.Unmarshal to satisfy json.Unmarshaler implementations and report errors with proper node position. - extends decoder selection logic to try json.Unmarshaler as a fallback when FallbackToJSON(true) is set. - updates tests (yaml_test.go) to cover both encoder and decoder JSON fallback behavior via a TestFallbackToJSON type that tracks when MarshalJSON/UnmarshalJSON are invoked and verifies round-trip behavior. Errors from JSON (un)marshal are converted into UnmarshalError entries so line/column information is preserved. This enables interoperable use of JSON (un)marshalers with the YAML encoder/decoder when desired.

Co-authored-by: Copilot <[email protected]>

Decode node into an intermediate representation and properly invoke json.Unmarshaler by normalizing and serializing that intermediate value to JSON before calling UnmarshalJSON. - change callJSONUnmarshaler to return bool and collect TypeError and UnmarshalError details from the initial node.Decode call. - extract JSON marshaling/unmarshaling logic into unmarshalJSON helper. - add normalizeJSON to convert YAML-parsed structures into JSON-safe structures: convert map[any]any to map[string]any (stringifying non-string keys via JSON), recursively normalize arrays and maps, and detect duplicate keys that would conflict after conversion. - add tests for json.Unmarshaler support including maps with string and non-string keys to ensure correct normalization and error handling. This enables compatibility with types implementing json.Unmarshaler while preserving detailed error reporting and preventing JSON key conflicts.

Add a //nolint:govet comment to two test struct fields that use a tag without a key ("just_b"). The change silences govet warnings while preserving the malformed tag needed by tests that verify tag name handling and fallback to json behavior. This keeps the tests explicit about their intent and avoids linter noise during CI.

Update comment formatting to use ASCII arrow ("->") instead of Unicode arrow ("→") in several places of decode.go and encode.go. This change standardizes comment text and removes mixed character usage, improving readability and reducing the chance of encoding-related display issues in various editors and tooling. Functionality remains unchanged.

Co-authored-by: Copilot <[email protected]>

Move the Test_unmarshalJSON tests and supporting TestUnmarshaler type into a dedicated decode_internal_test.go file and remove the duplicate test code from yaml_test.go. This isolates JSON-unmarshaling-specific tests for unmarshalJSON into a focused test file, improving test organization and making the intent of these tests clearer. The tests cover maps with string, int, and any keys, including a duplicate-key error case, and assert correct conversion to the TestUnmarshaler struct.

Improve handling of errors returned from Node decoding when invoking json.Unmarshaler implementations by using errors.As to detect *TypeError instead of a type assertion. This ensures wrapped TypeError values are recognized and their internal errors are collected into the decoder's terrors slice.

Copilot AI review requested due to automatic review settings October 26, 2025 04:33