Repository: goccy/go-yaml
Pull request #210 (2021-03-01 15:33:57)

Fixing Issue 186, scanner should account for newline characters when …
…processing multi-line text. Without this source annotations line/column number (for this and all subsequent tokens) is inconsistent with plain text editors. e.g. https://github.com/goccy/go-yaml/issues/186. This addresses the issue specifically for single and double quote text only.

1.) We would have preferred to avoid introducing an additional state in the loop (isNewLine), but this requires placing the column marker to an invalid position (0) and believe it is safer not to do so. This is done to avoid potential bugs in the future if the loop exits before the column marker can be brought back in-bounds.

2.) We debated internally whether the slice sorting in the tests should be done through a helper function or not. We didn't feel a convenience function improved code readability to merit consolidation of the logic. If you disagree, lmk. We add the sort to be token order agnostic in our tests.


Patch:
diff --git a/scanner/scanner.go b/scanner/scanner.go
index 9e22380..bb85d89 100644
--- a/scanner/scanner.go
+++ b/scanner/scanner.go
@@ -197,18 +197,25 @@ func (s *Scanner) scanSingleQuote(ctx *Context) (tk *token.Token, pos int) {
 	ctx.addOriginBuf('\'')
 	srcpos := s.pos()
 	startIndex := ctx.idx + 1
-	s.progressColumn(ctx, 1)
 	src := ctx.src
 	size := len(src)
 	value := []rune{}
 	isFirstLineChar := false
+	isNewLine := false
 	for idx := startIndex; idx < size; idx++ {
+		if !isNewLine {
+			s.progressColumn(ctx, 1)
+		} else {
+			isNewLine = false
+		}
 		c := src[idx]
 		pos = idx + 1
 		ctx.addOriginBuf(c)
 		if s.isNewLineChar(c) {
 			value = append(value, ' ')
 			isFirstLineChar = true
+			isNewLine = true
+			s.progressLine(ctx)
 			continue
 		} else if c == ' ' && isFirstLineChar {
 			continue
@@ -224,6 +231,7 @@ func (s *Scanner) scanSingleQuote(ctx *Context) (tk *token.Token, pos int) {
 			idx++
 			continue
 		}
+		s.progressColumn(ctx, 1)
 		tk = token.SingleQuote(string(value), string(ctx.obuf), srcpos)
 		pos = idx - startIndex + 1
 		return
@@ -253,18 +261,25 @@ func (s *Scanner) scanDoubleQuote(ctx *Context) (tk *token.Token, pos int) {
 	ctx.addOriginBuf('"')
 	srcpos := s.pos()
 	startIndex := ctx.idx + 1
-	s.progressColumn(ctx, 1)
 	src := ctx.src
 	size := len(src)
 	value := []rune{}
 	isFirstLineChar := false
+	isNewLine := false
 	for idx := startIndex; idx < size; idx++ {
+		if !isNewLine {
+			s.progressColumn(ctx, 1)
+		} else {
+			isNewLine = false
+		}
 		c := src[idx]
 		pos = idx + 1
 		ctx.addOriginBuf(c)
 		if s.isNewLineChar(c) {
 			value = append(value, ' ')
 			isFirstLineChar = true
+			isNewLine = true
+			s.progressLine(ctx)
 			continue
 		} else if c == ' ' && isFirstLineChar {
 			continue
@@ -365,6 +380,7 @@ func (s *Scanner) scanDoubleQuote(ctx *Context) (tk *token.Token, pos int) {
 			isFirstLineChar = false
 			continue
 		}
+		s.progressColumn(ctx, 1)
 		tk = token.DoubleQuote(string(value), string(ctx.obuf), srcpos)
 		pos = idx - startIndex + 1
 		return
@@ -728,7 +744,6 @@ func (s *Scanner) scan(ctx *Context) (pos int) {
 			if !ctx.existsBuffer() {
 				token, progress := s.scanQuote(ctx, c)
 				ctx.addToken(token)
-				s.progressColumn(ctx, progress)
 				pos += progress
 				return
 			}


Test patch:
diff --git a/lexer/lexer_test.go b/lexer/lexer_test.go
index ab7f863..bbe4e2f 100644
--- a/lexer/lexer_test.go
+++ b/lexer/lexer_test.go
@@ -68,7 +68,7 @@ type testToken struct {
 	value  string
 }
 
-func TestTokenValueAndLineColumnPosition(t *testing.T) {
+func TestSingleLineToken_ValueLineColumnPosition(t *testing.T) {
 	tests := []struct {
 		name   string
 		src    string
@@ -269,3 +269,199 @@ func tokenMatches(t *token.Token, e testToken) bool {
 		t.Position.Line == e.line &&
 		t.Position.Column == e.column
 }
+
+func TestMultiLineToken_ValueLineColumnPosition(t *testing.T) {
+	tests := []struct {
+		name   string
+		src    string
+		expect []testToken
+	}{
+		{
+			name: "double quote",
+			src: `one: "1 2 3 4 5"
+two: "1 2
+3 4
+5"
+three: "1 2 3 4
+5"`,
+			expect: []testToken{
+				{
+					line:   1,
+					column: 1,
+					value:  "one",
+				},
+				{
+					line:   1,
+					column: 4,
+					value:  ":",
+				},
+				{
+					line:   1,
+					column: 6,
+					value:  "1 2 3 4 5",
+				},
+				{
+					line:   2,
+					column: 1,
+					value:  "two",
+				},
+				{
+					line:   2,
+					column: 4,
+					value:  ":",
+				},
+				{
+					line:   2,
+					column: 6,
+					value:  "1 2 3 4 5",
+				},
+				{
+					line:   5,
+					column: 1,
+					value:  "three",
+				},
+				{
+					line:   5,
+					column: 6,
+					value:  ":",
+				},
+				{
+					line:   5,
+					column: 8,
+					value:  "1 2 3 4 5",
+				},
+			},
+		},
+		{
+			name: "single quote in an array",
+			src: `arr: ['1', 'and
+two']
+last: 'hello'`,
+			expect: []testToken{
+				{
+					line:   1,
+					column: 1,
+					value:  "arr",
+				},
+				{
+					line:   1,
+					column: 4,
+					value:  ":",
+				},
+				{
+					line:   1,
+					column: 6,
+					value:  "[",
+				},
+				{
+					line:   1,
+					column: 7,
+					value:  "1",
+				},
+				{
+					line:   1,
+					column: 10,
+					value:  ",",
+				},
+				{
+					line:   1,
+					column: 12,
+					value:  "and two",
+				},
+				{
+					line:   2,
+					column: 5,
+					value:  "]",
+				},
+				{
+					line:   3,
+					column: 1,
+					value:  "last",
+				},
+				{
+					line:   3,
+					column: 5,
+					value:  ":",
+				},
+				{
+					line:   3,
+					column: 7,
+					value:  "hello",
+				},
+			},
+		},
+		{
+			name: "single quote and double quote",
+			src: `foo: "test
+
+
+
+
+bar"
+foo2: 'bar2'`,
+			expect: []testToken{
+				{
+					line:   1,
+					column: 1,
+					value:  "foo",
+				},
+				{
+					line:   1,
+					column: 4,
+					value:  ":",
+				},
+				{
+					line:   1,
+					column: 6,
+					value:  "test     bar",
+				},
+				{
+					line:   7,
+					column: 1,
+					value:  "foo2",
+				},
+				{
+					line:   7,
+					column: 5,
+					value:  ":",
+				},
+				{
+					line:   7,
+					column: 7,
+					value:  "bar2",
+				},
+			},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			got := lexer.Tokenize(tc.src)
+			sort.Slice(got, func(i, j int) bool {
+				// sort by line, then column
+				if got[i].Position.Line < got[j].Position.Line {
+					return true
+				} else if got[i].Position.Line == got[j].Position.Line {
+					return got[i].Position.Column < got[j].Position.Column
+				}
+				return false
+			})
+			sort.Slice(tc.expect, func(i, j int) bool {
+				if tc.expect[i].line < tc.expect[j].line {
+					return true
+				} else if tc.expect[i].line == tc.expect[j].line {
+					return tc.expect[i].column < tc.expect[j].column
+				}
+				return false
+			})
+			if len(got) != len(tc.expect) {
+				t.Errorf("Tokenize() token count mismatch, expected:%d got:%d", len(tc.expect), len(got))
+			}
+			for i, tok := range got {
+				if !tokenMatches(tok, tc.expect[i]) {
+					t.Errorf("Tokenize() expected:%+v got line:%d column:%d value:%s", tc.expect[i], tok.Position.Line, tok.Position.Column, tok.Value)
+				}
+			}
+		})
+	}
+}
diff --git a/printer/printer_test.go b/printer/printer_test.go
index 02a304f..2afa74f 100644
--- a/printer/printer_test.go
+++ b/printer/printer_test.go
@@ -1,6 +1,7 @@
 package printer_test
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/goccy/go-yaml/lexer"
@@ -43,7 +44,7 @@ alias: *x
 			t.Fatalf("unexpected output: expect:[%s]\n actual:[%s]", expect, actual)
 		}
 	})
-	t.Run("print stargin from tokens[4]", func(t *testing.T) {
+	t.Run("print starting from tokens[4]", func(t *testing.T) {
 		tokens := lexer.Tokenize(yml)
 		var p printer.Printer
 		actual := "\n" + p.PrintErrorToken(tokens[4], false)
@@ -149,3 +150,74 @@ alias: *x`
 		t.Fatalf("unexpected output: expect:[%s]\n actual:[%s]", expected, got)
 	}
 }
+
+func Test_Printer_Multiline(t *testing.T) {
+	yml := `
+text1: 'aaaa
+ bbbb
+ cccc'
+text2: "ffff
+ gggg
+ hhhh"
+text3: hello
+`
+	tc := []struct {
+		token int
+		want  string
+	}{
+		{
+			token: 2,
+			want: `
+>  2 | text1: 'aaaa
+   3 |  bbbb
+   4 |  cccc'
+              ^
+   5 | text2: "ffff
+   6 |  gggg
+   7 |  hhhh"`,
+		},
+		{token: 3,
+			want: `
+   2 | text1: 'aaaa
+   3 |  bbbb
+   4 |  cccc'
+>  5 | text2: "ffff
+   6 |  gggg
+   7 |  hhhh"
+       ^
+   8 | text3: hello`,
+		},
+		{token: 5,
+			want: `
+   2 | text1: 'aaaa
+   3 |  bbbb
+   4 |  cccc'
+>  5 | text2: "ffff
+   6 |  gggg
+   7 |  hhhh"
+              ^
+   8 | text3: hello`,
+		},
+		{token: 6,
+			want: `
+   5 | text2: "ffff
+   6 |  gggg
+   7 |  hhhh"
+>  8 | text3: hello
+       ^
+`,
+		},
+	}
+	for _, tt := range tc {
+		name := fmt.Sprintf("print starting from tokens[%d]", tt.token)
+		t.Run(name, func(t *testing.T) {
+			tokens := lexer.Tokenize(yml)
+			var p printer.Printer
+			got := "\n" + p.PrintErrorToken(tokens[tt.token], false)
+			want := tt.want
+			if got != want {
+				t.Fatalf("PrintErrorToken() got: %s\n want:%s\n", want, got)
+			}
+		})
+	}
+}