Repository: goccy/go-yaml (go)

Issue:
Paths for quoted keys don't filter correctly
I had some cases where comments from a comment map didn't show up in the marshaled yaml, and I realized these cases all have keys that require quotes.

Here's a test demonstrating this:
https://go.dev/play/p/U7B7jbXcKg4

I traced it down to path filter not matching in these cases. There is a PR incoming.

Pull request:
Fix quoted comments
fixes #369 

This resolves the quoted comments issue by updating the path filter account for quoted selectors.

Slightly related to this, I found that `Path.String()` wasn't normalizing its output. I had to remove the `*PathBuilder` receiver from `normalizeSelectorName()` to make this work.

The changes to `context.go` are not related to the rest of this PR. While tracing this I noticed that `context.mode` isn't used, so I removed it.

Patch:
diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md
new file mode 100644
index 0000000..1aad475
--- /dev/null
+++ b/.github/ISSUE_TEMPLATE/bug_report.md
@@ -0,0 +1,29 @@
+---
+name: Bug report
+about: Create a report to help us improve
+title: ''
+labels: bug
+assignees: ''
+
+---
+
+**Describe the bug**
+A clear and concise description of what the bug is.
+
+**To Reproduce**
+
+Please provide a minimum yaml content that can be reproduced.
+We are more than happy to use [Go Playground](https://go.dev/play)
+
+**Expected behavior**
+A clear and concise description of what you expected to happen.
+
+**Screenshots**
+If applicable, add screenshots to help explain your problem.
+
+**Version Variables**
+ - Go version: [e.g. 1.21 ]
+ - go-yaml's Version: [e.g. v1.11.1 ]
+
+**Additional context**
+Add any other context about the problem here.
diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md
new file mode 100644
index 0000000..e301d68
--- /dev/null
+++ b/.github/ISSUE_TEMPLATE/feature_request.md
@@ -0,0 +1,20 @@
+---
+name: Feature request
+about: Suggest an idea for this project
+title: ''
+labels: feature request
+assignees: ''
+
+---
+
+**Is your feature request related to a problem? Please describe.**
+A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]
+
+**Describe the solution you'd like**
+A clear and concise description of what you want to happen.
+
+**Describe alternatives you've considered**
+A clear and concise description of any alternative solutions or features you've considered.
+
+**Additional context**
+Add any other context or screenshots about the feature request here.
diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md
new file mode 100644
index 0000000..a1d6c8e
--- /dev/null
+++ b/.github/pull_request_template.md
@@ -0,0 +1,4 @@
+Before submitting your PR, please confirm the following.
+
+- [ ] Describe the purpose for which you created this PR.  
+- [ ] Create test code that corresponds to the modification
\ No newline at end of file
diff --git a/CHANGELOG.md b/CHANGELOG.md
index db9f0ce..afbfda9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,12 @@
+# 1.11.1 - 2023-09-14
+
+### Fix bugs
+
+- Handle `\r` in a double-quoted string the same as `\n` ( #372 )
+- Replace loop with n.Values = append(n.Values, target.Values...) ( #380 )
+- Skip encoding an inline field if it is null ( #386 )
+- Fix comment parsing with null value ( #388 )
+
 # 1.11.0 - 2023-04-03
 
 ### Features
diff --git a/ast/ast.go b/ast/ast.go
index f535a24..b4d5ec4 100644
--- a/ast/ast.go
+++ b/ast/ast.go
@@ -1506,9 +1506,7 @@ func (n *SequenceNode) Replace(idx int, value Node) error {
 func (n *SequenceNode) Merge(target *SequenceNode) {
 	column := n.Start.Position.Column - target.Start.Position.Column
 	target.AddColumn(column)
-	for _, value := range target.Values {
-		n.Values = append(n.Values, value)
-	}
+	n.Values = append(n.Values, target.Values...)
 }
 
 // SetIsFlowStyle set value to IsFlowStyle field recursively.
diff --git a/encode.go b/encode.go
index 7d8d81e..3b9b298 100644
--- a/encode.go
+++ b/encode.go
@@ -823,6 +823,10 @@ func (e *Encoder) encodeStruct(ctx context.Context, value reflect.Value, column
 			}
 			mapNode, ok := value.(ast.MapNode)
 			if !ok {
+				// if an inline field is null, skip encoding it
+				if _, ok := value.(*ast.NullNode); ok {
+					continue
+				}
 				return nil, xerrors.Errorf("inline value is must be map or struct type")
 			}
 			mapIter := mapNode.MapRange()
diff --git a/parser/context.go b/parser/context.go
index 99f18b1..42cc4f8 100644
--- a/parser/context.go
+++ b/parser/context.go
@@ -13,7 +13,6 @@ type context struct {
 	idx    int
 	size   int
 	tokens token.Tokens
-	mode   Mode
 	path   string
 }
 
@@ -56,7 +55,6 @@ func (c *context) copy() *context {
 		idx:    c.idx,
 		size:   c.size,
 		tokens: append(token.Tokens{}, c.tokens...),
-		mode:   c.mode,
 		path:   c.path,
 	}
 }
@@ -145,10 +143,6 @@ func (c *context) afterNextNotCommentToken() *token.Token {
 	return nil
 }
 
-func (c *context) enabledComment() bool {
-	return c.mode&ParseComments != 0
-}
-
 func (c *context) isCurrentCommentToken() bool {
 	tk := c.currentToken()
 	if tk == nil {
@@ -193,7 +187,6 @@ func newContext(tokens token.Tokens, mode Mode) *context {
 		idx:    0,
 		size:   len(filteredTokens),
 		tokens: token.Tokens(filteredTokens),
-		mode:   mode,
 		path:   "$",
 	}
 }
diff --git a/parser/parser.go b/parser/parser.go
index 568e6ad..6989a37 100644
--- a/parser/parser.go
+++ b/parser/parser.go
@@ -156,15 +156,38 @@ func (p *parser) createMapValueNode(ctx *context, key ast.MapKeyNode, colonToken
 		ctx.insertToken(ctx.idx, nullToken)
 		return ast.Null(nullToken), nil
 	}
-
+	var comment *ast.CommentGroupNode
+	if tk.Type == token.CommentType {
+		comment = p.parseCommentOnly(ctx)
+		if comment != nil {
+			comment.SetPath(ctx.withChild(key.GetToken().Value).path)
+		}
+		tk = ctx.currentToken()
+	}
 	if tk.Position.Column == key.GetToken().Position.Column && tk.Type == token.StringType {
 		// in this case,
 		// ----
 		// key: <value does not defined>
 		// next
+
 		nullToken := p.createNullToken(colonToken)
 		ctx.insertToken(ctx.idx, nullToken)
-		return ast.Null(nullToken), nil
+		nullNode := ast.Null(nullToken)
+
+		if comment != nil {
+			nullNode.SetComment(comment)
+		} else {
+			// If there is a comment, it is already bound to the key node,
+			// so remove the comment from the key to bind it to the null value.
+			keyComment := key.GetComment()
+			if keyComment != nil {
+				if err := key.SetComment(nil); err != nil {
+					return nil, err
+				}
+				nullNode.SetComment(keyComment)
+			}
+		}
+		return nullNode, nil
 	}
 
 	if tk.Position.Column < key.GetToken().Position.Column {
@@ -174,13 +197,20 @@ func (p *parser) createMapValueNode(ctx *context, key ast.MapKeyNode, colonToken
 		// next
 		nullToken := p.createNullToken(colonToken)
 		ctx.insertToken(ctx.idx, nullToken)
-		return ast.Null(nullToken), nil
+		nullNode := ast.Null(nullToken)
+		if comment != nil {
+			nullNode.SetComment(comment)
+		}
+		return nullNode, nil
 	}
 
 	value, err := p.parseToken(ctx, ctx.currentToken())
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to parse mapping 'value' node")
 	}
+	if comment != nil {
+		value.SetComment(comment)
+	}
 	return value, nil
 }
 
diff --git a/path.go b/path.go
index 7a0c3b1..72554bd 100644
--- a/path.go
+++ b/path.go
@@ -500,11 +500,29 @@ func newSelectorNode(selector string) *selectorNode {
 }
 
 func (n *selectorNode) filter(node ast.Node) (ast.Node, error) {
+	selector := n.selector
+	if len(selector) > 1 && selector[0] == '\'' && selector[len(selector)-1] == '\'' {
+		selector = selector[1 : len(selector)-1]
+	}
 	switch node.Type() {
 	case ast.MappingType:
 		for _, value := range node.(*ast.MappingNode).Values {
 			key := value.Key.GetToken().Value
-			if key == n.selector {
+			if len(key) > 0 {
+				switch key[0] {
+				case '"':
+					var err error
+					key, err = strconv.Unquote(key)
+					if err != nil {
+						return nil, errors.Wrapf(err, "failed to unquote")
+					}
+				case '\'':
+					if len(key) > 1 && key[len(key)-1] == '\'' {
+						key = key[1 : len(key)-1]
+					}
+				}
+			}
+			if key == selector {
 				if n.child == nil {
 					return value.Value, nil
 				}
@@ -518,7 +536,7 @@ func (n *selectorNode) filter(node ast.Node) (ast.Node, error) {
 	case ast.MappingValueType:
 		value := node.(*ast.MappingValueNode)
 		key := value.Key.GetToken().Value
-		if key == n.selector {
+		if key == selector {
 			if n.child == nil {
 				return value.Value, nil
 			}
@@ -571,7 +589,9 @@ func (n *selectorNode) replace(node ast.Node, target ast.Node) error {
 }
 
 func (n *selectorNode) String() string {
-	s := fmt.Sprintf(".%s", n.selector)
+	var builder PathBuilder
+	selector := builder.normalizeSelectorName(n.selector)
+	s := fmt.Sprintf(".%s", selector)
 	if n.child != nil {
 		s += n.child.String()
 	}
diff --git a/scanner/scanner.go b/scanner/scanner.go
index ce9c665..b0eac48 100644
--- a/scanner/scanner.go
+++ b/scanner/scanner.go
@@ -339,6 +339,11 @@ func (s *Scanner) scanDoubleQuote(ctx *Context) (tk *token.Token, pos int) {
 					value = append(value, '\n')
 					idx++
 					continue
+				case 'r':
+					ctx.addOriginBuf(nextChar)
+					value = append(value, '\r')
+					idx++
+					continue
 				case 'v':
 					ctx.addOriginBuf(nextChar)
 					value = append(value, '\v')
diff --git a/token/token.go b/token/token.go
index 182f4be..c86caab 100644
--- a/token/token.go
+++ b/token/token.go
@@ -623,12 +623,12 @@ func IsNeedQuoted(value string) bool {
 	}
 	first := value[0]
 	switch first {
-	case '*', '&', '[', '{', '}', ']', ',', '!', '|', '>', '%', '\'', '"', '@':
+	case '*', '&', '[', '{', '}', ']', ',', '!', '|', '>', '%', '\'', '"', '@', ' ':
 		return true
 	}
 	last := value[len(value)-1]
 	switch last {
-	case ':':
+	case ':', ' ':
 		return true
 	}
 	if looksLikeTimeValue(value) {


Test patch:
diff --git a/decode_test.go b/decode_test.go
index 2549bea..2373804 100644
--- a/decode_test.go
+++ b/decode_test.go
@@ -67,6 +67,18 @@ func TestDecoder(t *testing.T) {
 			"v: 1.234\n",
 			map[string]string{"v": "1.234"},
 		},
+		{
+			"v: \" foo\"\n",
+			map[string]string{"v": " foo"},
+		},
+		{
+			"v: \"foo \"\n",
+			map[string]string{"v": "foo "},
+		},
+		{
+			"v: \" foo \"\n",
+			map[string]string{"v": " foo "},
+		},
 		{
 			"v: false\n",
 			map[string]bool{"v": false},
@@ -425,6 +437,14 @@ func TestDecoder(t *testing.T) {
 			`"1": "a\x2Fb\u002Fc\U0000002Fd"`,
 			map[interface{}]interface{}{"1": `a/b/c/d`},
 		},
+		{
+			"'1': \"2\\n3\"",
+			map[interface{}]interface{}{"1": "2\n3"},
+		},
+		{
+			"'1': \"2\\r\\n3\"",
+			map[interface{}]interface{}{"1": "2\r\n3"},
+		},
 
 		{
 			"a: -b_c",
diff --git a/encode_test.go b/encode_test.go
index 74b1aa5..3ff6f1c 100644
--- a/encode_test.go
+++ b/encode_test.go
@@ -303,6 +303,21 @@ func TestEncoder(t *testing.T) {
 			map[string]string{"a": "Hello #comment"},
 			nil,
 		},
+		{
+			"a: \" b\"\n",
+			map[string]string{"a": " b"},
+			nil,
+		},
+		{
+			"a: \"b \"\n",
+			map[string]string{"a": "b "},
+			nil,
+		},
+		{
+			"a: \" b \"\n",
+			map[string]string{"a": " b "},
+			nil,
+		},
 		{
 			"a: 100.5\n",
 			map[string]interface{}{
@@ -980,6 +995,30 @@ c: true
 	}
 }
 
+func TestEncoder_InlineNil(t *testing.T) {
+	type base struct {
+		A int
+		B string
+	}
+	var buf bytes.Buffer
+	enc := yaml.NewEncoder(&buf)
+	if err := enc.Encode(struct {
+		*base `yaml:",inline"`
+		C     bool
+	}{
+		C: true,
+	}); err != nil {
+		t.Fatalf("%+v", err)
+	}
+	expect := `
+c: true
+`
+	actual := "\n" + buf.String()
+	if expect != actual {
+		t.Fatalf("inline marshal error: expect=[%s] actual=[%s]", expect, actual)
+	}
+}
+
 func TestEncoder_Flow(t *testing.T) {
 	var buf bytes.Buffer
 	enc := yaml.NewEncoder(&buf, yaml.Flow(true))
diff --git a/parser/parser_test.go b/parser/parser_test.go
index 6044699..13157c0 100644
--- a/parser/parser_test.go
+++ b/parser/parser_test.go
@@ -808,6 +808,74 @@ foo: > # comment
 	}
 }
 
+func TestCommentWithNull(t *testing.T) {
+	t.Run("same line", func(t *testing.T) {
+		content := `
+foo:
+  bar: # comment
+  baz: 1
+`
+		expected := `
+foo:
+  bar: null # comment
+  baz: 1`
+		f, err := parser.ParseBytes([]byte(content), parser.ParseComments)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if len(f.Docs) != 1 {
+			t.Fatal("failed to parse content with same line comment")
+		}
+		if f.Docs[0].String() != strings.TrimPrefix(expected, "\n") {
+			t.Fatal("failed to parse comment")
+		}
+	})
+	t.Run("next line", func(t *testing.T) {
+		content := `
+foo:
+  bar:
+    # comment
+  baz: 1
+`
+		expected := `
+foo:
+  bar: null # comment
+  baz: 1`
+		f, err := parser.ParseBytes([]byte(content), parser.ParseComments)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if len(f.Docs) != 1 {
+			t.Fatal("failed to parse content with next line comment")
+		}
+		if f.Docs[0].String() != strings.TrimPrefix(expected, "\n") {
+			t.Fatal("failed to parse comment")
+		}
+	})
+	t.Run("next line and different indent", func(t *testing.T) {
+		content := `
+foo:
+  bar:
+ # comment
+baz: 1`
+		f, err := parser.ParseBytes([]byte(content), parser.ParseComments)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if len(f.Docs) != 1 {
+			t.Fatal("failed to parse content with next line comment")
+		}
+		expected := `
+foo:
+  bar: null # comment
+baz: 1`
+		if f.Docs[0].String() != strings.TrimPrefix(expected, "\n") {
+			t.Fatal("failed to parse comment")
+		}
+	})
+
+}
+
 func TestNodePath(t *testing.T) {
 	yml := `
 a: # commentA
@@ -824,6 +892,8 @@ a: # commentA
   i: fuga # commentI
 j: piyo # commentJ
 k.l.m.n: moge # commentKLMN
+o#p: hogera # commentOP
+q#.r: hogehoge # commentQR
 `
 	f, err := parser.ParseBytes([]byte(yml), parser.ParseComments)
 	if err != nil {
@@ -854,6 +924,8 @@ k.l.m.n: moge # commentKLMN
 		"$.a.i",
 		"$.j",
 		"$.'k.l.m.n'",
+		"$.o#p",
+		"$.'q#.r'",
 	}
 	if !reflect.DeepEqual(expectedPaths, commentPaths) {
 		t.Fatalf("failed to get YAMLPath to the comment node:\nexpected[%s]\ngot     [%s]", expectedPaths, commentPaths)
diff --git a/path_test.go b/path_test.go
index 4271da5..c0073ce 100644
--- a/path_test.go
+++ b/path_test.go
@@ -61,6 +61,8 @@ store:
   bicycle:
     color: red
     price: 19.95
+  bicycle*unicycle:
+    price: 20.25
 `
 	tests := []struct {
 		name     string
@@ -97,6 +99,11 @@ store:
 			path:     builder().Root().Child("store").Child("bicycle").Child("price").Build(),
 			expected: float64(19.95),
 		},
+		{
+			name:     `$.store.'bicycle*unicycle'.price`,
+			path:     builder().Root().Child("store").Child(`bicycle*unicycle`).Child("price").Build(),
+			expected: float64(20.25),
+		},
 	}
 	t.Run("PathString", func(t *testing.T) {
 		for _, test := range tests {
diff --git a/token/token_test.go b/token/token_test.go
index 4f5764f..dc1b4df 100644
--- a/token/token_test.go
+++ b/token/token_test.go
@@ -118,6 +118,9 @@ func TestIsNeedQuoted(t *testing.T) {
 		"Off",
 		"OFF",
 		"@test",
+		" a",
+		" a ",
+		"a ",
 	}
 	for i, test := range needQuotedTests {
 		if !token.IsNeedQuoted(test) {
diff --git a/yaml_test.go b/yaml_test.go
index 5828629..4446d31 100644
--- a/yaml_test.go
+++ b/yaml_test.go
@@ -2,6 +2,7 @@ package yaml_test
 
 import (
 	"bytes"
+	"strings"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -1161,6 +1162,94 @@ hoge:
 	})
 }
 
+func TestCommentMapRoundTrip(t *testing.T) {
+	// test that an unmarshal and marshal round trip retains comments.
+	// if expect is empty, the test will use the input as the expected result.
+	tests := []struct {
+		name          string
+		source        string
+		expect        string
+		encodeOptions []yaml.EncodeOption
+	}{
+		{
+			name: "simple map",
+			source: `
+# head
+a: 1 # line
+# foot
+`,
+		},
+		{
+			name: "nesting",
+			source: `
+- 1 # one
+- foo:
+    a: b
+    # c comment
+    c: d # d comment
+    "e#f": g # g comment
+    h.i: j # j comment
+    "k.#l": m # m comment
+`,
+		},
+		{
+			name:          "single quotes",
+			source:        `'a#b': c # c comment`,
+			encodeOptions: []yaml.EncodeOption{yaml.UseSingleQuote(true)},
+		},
+		{
+			name:          "single quotes added in encode",
+			source:        `a#b: c # c comment`,
+			encodeOptions: []yaml.EncodeOption{yaml.UseSingleQuote(true)},
+			expect:        `'a#b': c # c comment`,
+		},
+		{
+			name:          "double quotes quotes transformed to single quotes",
+			source:        `"a#b": c # c comment`,
+			encodeOptions: []yaml.EncodeOption{yaml.UseSingleQuote(true)},
+			expect:        `'a#b': c # c comment`,
+		},
+		{
+			name:   "single quotes quotes transformed to double quotes",
+			source: `'a#b': c # c comment`,
+			expect: `"a#b": c # c comment`,
+		},
+		{
+			name:   "single quotes removed",
+			source: `'a': b # b comment`,
+			expect: `a: b # b comment`,
+		},
+		{
+			name:   "double quotes removed",
+			source: `"a": b # b comment`,
+			expect: `a: b # b comment`,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			var val any
+			cm := yaml.CommentMap{}
+			source := strings.TrimSpace(test.source)
+			if err := yaml.UnmarshalWithOptions([]byte(source), &val, yaml.CommentToMap(cm)); err != nil {
+				t.Fatalf("%+v", err)
+			}
+			marshaled, err := yaml.MarshalWithOptions(val, append(test.encodeOptions, yaml.WithComment(cm))...)
+			if err != nil {
+				t.Fatalf("%+v", err)
+			}
+			got := strings.TrimSpace(string(marshaled))
+			expect := strings.TrimSpace(test.expect)
+			if expect == "" {
+				expect = source
+			}
+			if got != expect {
+				t.Fatalf("expected:\n%s\ngot:\n%s\n", expect, got)
+			}
+		})
+
+	}
+}
+
 func TestRegisterCustomMarshaler(t *testing.T) {
 	type T struct {
 		Foo []byte `yaml:"foo"`