Repository: goccy/go-yaml (go)

Issue:
comments are not properly preserved following folded/literal string
If a comment follows a field using folded or literal style strings, the first line of the comment is lost, for example (https://play.golang.com/p/mN4v86pSwP8 https://play.golang.com/p/xHeGkd9bxDu https://play.golang.com/p/uzQ8WGzA4gs https://play.golang.com/p/wDYKVpHPdSL)
```
document:
  - key1:
      key2: |
        Some text.

  # This is an important comment explaining key3.
  # Second line of comment.
  - key3:
      value: 1
```
becomes
```
document:
  - key1:
      key2: |
        Some text.
  # Second line of comment.
  - key3:
      value: 1
```

This does not happen with quoted or unquoted strings (https://play.golang.com/p/dc2RCZmRrIR https://play.golang.com/p/tjBLgB8OaG9).

This appears to be due to incorrect association of comments with nodes in the AST during parsing.

The YAML document above has the following AST comment associations (https://play.golang.com/p/3WtBBjARGXF)
```
comment: # This is an important comment explaining key3.
associated node:
      key2: |
        Some text.

comment: # Second line of comment.
associated node:
document:
  - key1:
      key2: |
        Some text.
  # Second line of comment.
  - key3:
      value: 1
```

Where we can see that the first line of the comments is incorrectly associated with the key2 node, but is not emitted, and the second line is correctly associated with the first element of the sequence via the `ValueComments` slice.

Pull request:
scanner: fix position calculation for literal, folded and raw folded strings
The approach to calculating the column is not ideal from an allocations perpective, but is similar to other calculations in the code base that do a temporary `[]rune` → `string` conversion. I would prefer to retain an offset field instead, but that is a more invasive change.

The setting of `token.Position.IndentNum` is left as is since it's not clear to me what that is there for; the field is never read in the code base and so is apparently not used internally (removal does not affect tests, though external client code may be using it for something).

Fixes #326.

Please take a look.

Patch:
diff --git a/encode.go b/encode.go
index 9536eeb..1772be4 100644
--- a/encode.go
+++ b/encode.go
@@ -481,8 +481,8 @@ func (e *Encoder) encodeMapItem(ctx context.Context, item MapItem, column int) (
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to encode MapItem")
 	}
-	if m, ok := value.(*ast.MappingNode); ok {
-		m.AddColumn(e.indent)
+	if e.isMapNode(value) {
+		value.AddColumn(e.indent)
 	}
 	return ast.MappingValue(
 		token.New("", "", e.pos(column)),
@@ -503,6 +503,11 @@ func (e *Encoder) encodeMapSlice(ctx context.Context, value MapSlice, column int
 	return node, nil
 }
 
+func (e *Encoder) isMapNode(node ast.Node) bool {
+	_, ok := node.(ast.MapNode)
+	return ok
+}
+
 func (e *Encoder) encodeMap(ctx context.Context, value reflect.Value, column int) ast.Node {
 	node := ast.Mapping(token.New("", "", e.pos(column)), e.isFlowStyle)
 	keys := make([]interface{}, len(value.MapKeys()))
@@ -519,7 +524,7 @@ func (e *Encoder) encodeMap(ctx context.Context, value reflect.Value, column int
 		if err != nil {
 			return nil
 		}
-		if value.Type() == ast.MappingType || value.Type() == ast.MappingValueType {
+		if e.isMapNode(value) {
 			value.AddColumn(e.indent)
 		}
 		node.Values = append(node.Values, ast.MappingValue(
@@ -642,7 +647,7 @@ func (e *Encoder) encodeStruct(ctx context.Context, value reflect.Value, column
 		if err != nil {
 			return nil, errors.Wrapf(err, "failed to encode value")
 		}
-		if _, ok := value.(*ast.MappingNode); ok {
+		if e.isMapNode(value) {
 			value.AddColumn(e.indent)
 		}
 		var key ast.MapKeyNode = e.encodeString(structField.RenderName, column)
diff --git a/go.sum b/go.sum
index 112c438..fbb7f81 100644
--- a/go.sum
+++ b/go.sum
@@ -28,7 +28,6 @@ golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn
 golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20200116001909-b77594299b42/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
-golang.org/x/sys v0.0.0-20200223170610-d5e6a3e2c0ae h1:/WDfKMnPU+m5M4xB+6x4kaepxRw6jWvR5iDRdvjHgy8=
 golang.org/x/sys v0.0.0-20200223170610-d5e6a3e2c0ae/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20220406163625-3f8b81556e12 h1:QyVthZKMsyaQwBTJE04jdNN0Pp5Fn9Qga0mrgxyERQM=
 golang.org/x/sys v0.0.0-20220406163625-3f8b81556e12/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
diff --git a/scanner/scanner.go b/scanner/scanner.go
index cf58bfb..066b795 100644
--- a/scanner/scanner.go
+++ b/scanner/scanner.go
@@ -61,13 +61,25 @@ func (s *Scanner) bufferedToken(ctx *Context) *token.Token {
 		s.savedPos = nil
 		return tk
 	}
-	size := len(ctx.buf)
+	line := s.line
+	column := s.column - len(ctx.buf)
+	level := s.indentLevel
+	if ctx.isSaveIndentMode() {
+		line -= s.newLineCount(ctx.buf)
+		column = strings.Index(string(ctx.obuf), string(ctx.buf)) + 1
+		// Since we are in a literal, folded or raw folded
+		// we can use the indent level from the last token.
+		last := ctx.lastToken()
+		if last != nil { // The last token should never be nil here.
+			level = last.Position.IndentLevel + 1
+		}
+	}
 	return ctx.bufferedToken(&token.Position{
-		Line:        s.line,
-		Column:      s.column - size,
-		Offset:      s.offset - size,
+		Line:        line,
+		Column:      column,
+		Offset:      s.offset - len(ctx.buf),
 		IndentNum:   s.indentNum,
-		IndentLevel: s.indentLevel,
+		IndentLevel: level,
 	})
 }
 
diff --git a/struct.go b/struct.go
index ac78f31..a3da8dd 100644
--- a/struct.go
+++ b/struct.go
@@ -95,7 +95,7 @@ type StructFieldMap map[string]*StructField
 
 func (m StructFieldMap) isIncludedRenderName(name string) bool {
 	for _, v := range m {
-		if v.RenderName == name {
+		if !v.IsInline && v.RenderName == name {
 			return true
 		}
 	}


Test patch:
diff --git a/decode_test.go b/decode_test.go
index e470e64..3938097 100644
--- a/decode_test.go
+++ b/decode_test.go
@@ -2804,3 +2804,21 @@ steps:
 		})
 	}
 }
+
+func TestSameNameInineStruct(t *testing.T) {
+	type X struct {
+		X float64 `yaml:"x"`
+	}
+
+	type T struct {
+		X `yaml:",inline"`
+	}
+
+	var v T
+	if err := yaml.Unmarshal([]byte(`x: 0.7`), &v); err != nil {
+		t.Fatal(err)
+	}
+	if fmt.Sprint(v.X.X) != "0.7" {
+		t.Fatalf("failed to decode")
+	}
+}
diff --git a/encode_test.go b/encode_test.go
index cfc3d8e..db36028 100644
--- a/encode_test.go
+++ b/encode_test.go
@@ -1473,3 +1473,45 @@ a:
 		t.Fatalf("failed to encode. expected %s but got %s", expected, got)
 	}
 }
+
+type customMapSliceOneItemMarshaler struct{}
+
+func (m *customMapSliceOneItemMarshaler) MarshalYAML() ([]byte, error) {
+	var v yaml.MapSlice
+	v = append(v, yaml.MapItem{"a", "b"})
+	return yaml.Marshal(v)
+}
+
+type customMapSliceTwoItemMarshaler struct{}
+
+func (m *customMapSliceTwoItemMarshaler) MarshalYAML() ([]byte, error) {
+	var v yaml.MapSlice
+	v = append(v, yaml.MapItem{"a", "b"})
+	v = append(v, yaml.MapItem{"b", "c"})
+	return yaml.Marshal(v)
+}
+
+func TestCustomMapSliceMarshaler(t *testing.T) {
+	type T struct {
+		A *customMapSliceOneItemMarshaler `yaml:"a"`
+		B *customMapSliceTwoItemMarshaler `yaml:"b"`
+	}
+	b, err := yaml.Marshal(&T{
+		A: &customMapSliceOneItemMarshaler{},
+		B: &customMapSliceTwoItemMarshaler{},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	expected := `
+a:
+  a: b
+b:
+  a: b
+  b: c
+`
+	got := "\n" + string(b)
+	if expected != got {
+		t.Fatalf("failed to encode. expected %s but got %s", expected, got)
+	}
+}
diff --git a/lexer/lexer_test.go b/lexer/lexer_test.go
index 03b741f..0d8f648 100644
--- a/lexer/lexer_test.go
+++ b/lexer/lexer_test.go
@@ -543,6 +543,49 @@ c: true`,
 				},
 			},
 		},
+		{
+			name: "issue326",
+			src: `a: |
+  Text
+b: 1`,
+			expect: []testToken{
+				{
+					line:   1,
+					column: 1,
+					value:  "a",
+				},
+				{
+					line:   1,
+					column: 2,
+					value:  ":",
+				},
+				{
+					line:   1,
+					column: 4,
+					value:  "|",
+				},
+				{
+					line:   2,
+					column: 3,
+					value:  "Text\n",
+				},
+				{
+					line:   3,
+					column: 1,
+					value:  "b",
+				},
+				{
+					line:   3,
+					column: 2,
+					value:  ":",
+				},
+				{
+					line:   3,
+					column: 4,
+					value:  "1",
+				},
+			},
+		},
 	}
 
 	for _, tc := range tests {