Support non string map keys

[shuheiktgw]

Closes #680 & #331

I took over #681, resolved conflicts and added some unit tests. The original implementation looks very good to me.

• Describe the purpose for which you created this PR.
• Create test code that corresponds to the modification

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 50.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.94%. Comparing base (52dacb8) to head (2bc0fe2).
⚠️ Report is 2 commits behind head on master.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #756      +/-   ##
==========================================
- Coverage   77.97%   77.94%   -0.03%     
==========================================
  Files          22       22              
  Lines        8108     8113       +5     
==========================================
+ Hits         6322     6324       +2     
- Misses       1370     1372       +2     
- Partials      416      417       +1     

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull Request Overview

Adds support for non-string map keys in the YAML encoder and enhances test coverage for various key types

• Uses encodeValue to generate a MapKeyNode for non-string keys, falling back to fmt.Sprint
• Updates encodeMap logic in encode.go to handle dynamic key types
• Adds unit tests for map[int]string, map[float64]string, map[bool]string, map[any]string with array and struct keys

Reviewed Changes

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

File	Description
encode.go	Introduced logic to attempt encodeValue on each map key and fallback to string encoding
encode_test.go	Added tests for integer, float, boolean, array, and struct keys

Comments suppressed due to low confidence (1)

encode.go:716

• [nitpick] The variable name kn is ambiguous. Consider renaming it to something more descriptive like keyNodeValue to improve readability.

kn, err := e.encodeValue(ctx, reflect.ValueOf(key), column)

[goccy]

@shuheiktgw In this PR, only the encoding side is modified, but isn't it necessary to address the decoding side as well ?

[shuheiktgw]

@goccy Decoding seems to be working fine on my end. Is there any specific part you're concerned about? 👀

func Test1(t *testing.T) {
	content := `
1: one
2: two
`
	var m map[int]string
	if err := yaml.Unmarshal([]byte(content), &m); err != nil {
		t.Fatal(err)
	}

	if expected := map[int]string{1: "one", 2: "two"}; !reflect.DeepEqual(m, expected) {
		t.Fatalf("got %#v, want %#v", m, expected)
	}
}

[goccy]

@shuheiktgw It seems that cases where int type is used as a key have already been handled, which is great 😄 . However, cases where the key is something like a struct still don't seem to be supported, so it looks like it will need to be addressed later (since the encode side is handled in this PR).

	content := `
"{a: b}": one
`
	type T struct {
		A string `yaml:"a"`
	}
	var m map[T]string
	if err := yaml.Unmarshal([]byte(content), &m); err != nil {
		t.Fatal(err)
	}

Ah, However, I just realized that the encoding side is also incorrect. When a slice or struct is used as a key, the output is no longer a valid YAML string. Presumably, when values with structures like slice or struct are specified as keys, the ? keyword needs to be used to output the structure as is.

e.g.)

? a: b
: v

If it is difficult to address this in this PR, I think it would be fine to limit it to int, float, string, and bool.

[shuheiktgw]

@goccy Thanks for the information! Wow, I didn’t know about the ? keyword... I took a quick look at the code, and it seems like supporting encoding/decoding of complex map keys (like maps or sequences) wouldn’t be straightforward. How about we:

1. Add unit tests for decoding simple map keys (e.g., int, float, boolean) in this PR
2. Create a separate issue to track support for complex map key encoding/decoding

What do you think?

[herczegzsolt]

I think the encoding of struct values has always been incorrect. I've included the string-encoding in the original PR to be "backwards-compatible" with the old non-compliant way.

Since the encoding was always non-compliant, it may be a reasonable decision to ignore and break it anyway.

[shibe2]

Variable k already holds reflect.ValueOf(key).

[shibe2]

When keys are numeric, how should they be sorted?