cue/parser: fix comment placement

A bug placed a floating non-doc comment preceeding
a field after this field instead.

This fixes the comment attachement for lists, structs,
and function calls.

Also tested in format as these tests are more "visual",
even though this was really a parser issue.

Fixes #478

Change-Id: I7aef2dadab16be9597d75ac03ab58598357f37bf
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/7822
Reviewed-by: CUE cueckoo <cueckoo@gmail.com>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>

cmd/cue/cmd/testdata/script/import_auto.txt

@@ -2,8 +2,6 @@ cue import -o - ./openapi.yaml
 cmp stdout expect-openapi
 
 -- expect-openapi --
-
-
 // An OpenAPI file
 
 info: {

cue/ast/astutil/apply_test.go

@@ -47,6 +47,7 @@ func TestApply(t *testing.T) {
 		`,
 		out: `
 iam: new
+
 // foo is a
 foo: {
 	iam: new

cue/format/node.go

@@ -386,7 +386,6 @@ func (f *formatter) decl(decl ast.Decl) {
 		f.print(n.At, n)
 
 	case *ast.CommentGroup:
-		f.print(newsection)
 		f.printComment(n)
 		f.print(newsection)
 

cue/format/testdata/comments.golden

@@ -42,3 +42,79 @@ m: {
 m: {
 	// empty with comment
 }
+
+// Issue #478
+struct1: {
+	// This is a comment
+
+	// This is a comment
+
+	// Another comment
+
+	something: {
+	}
+	// extra comment
+}
+
+struct2: {
+	// This is a comment
+
+	// This is a comment
+
+	// Another comment
+	something: {
+	}
+
+	// extra comment
+}
+
+list1: [
+	// Comment1
+
+	// Comment2
+
+	// Another comment
+	{
+	}
+
+	// Comment 3,
+]
+
+list2: [
+	// foo
+]
+
+list3: [
+	// foo
+
+	// bar
+]
+
+funcArg1: foo(
+		// Comment1
+
+	// Comment2
+	3,
+
+	// Comment3
+	)
+
+funcArg2: foo(
+		// Comment1
+
+	// Comment2
+
+	3,
+	// Comment3
+	)
+
+funcArg3: foo(
+		2,
+
+	// Comment1
+
+	// Comment2
+	3,
+
+	// Comment3
+	)

cue/format/testdata/comments.input

@@ -48,3 +48,79 @@ m: {
 m: {
     // empty with comment
 }
+
+// Issue #478
+struct1: {
+    // This is a comment
+
+    // This is a comment
+
+    // Another comment
+
+    something: {
+    }
+    // extra comment
+}
+
+struct2: {
+    // This is a comment
+
+    // This is a comment
+
+    // Another comment
+    something: {
+    }
+
+    // extra comment
+}
+
+list1: [
+    // Comment1
+
+    // Comment2
+
+    // Another comment
+    {
+    },
+
+    // Comment 3
+]
+
+list2: [
+    // foo
+]
+
+list3: [
+    // foo
+
+    // bar
+]
+
+funcArg1: foo(
+    // Comment1
+
+    // Comment2
+    3
+
+    // Comment3
+)
+
+funcArg2: foo(
+    // Comment1
+
+    // Comment2
+
+    3
+    // Comment3
+)
+
+funcArg3: foo(
+    2,
+
+    // Comment1
+
+    // Comment2
+    3
+
+    // Comment3
+)

cue/parser/parser.go

@@ -102,6 +102,9 @@ type commentState struct {
 
 // openComments reserves the next doc comment for the caller and flushes
 func (p *parser) openComments() *commentState {
+	child := &commentState{
+		parent: p.comments,
+	}
 	if c := p.comments; c != nil && c.isList > 0 {
 		if c.lastChild != nil {
 			var groups []*ast.CommentGroup
@@ -125,15 +128,16 @@ func (p *parser) openComments() *commentState {
 			for _, cg := range c.groups {
 				cg.Position = 0
 			}
+			child.groups = c.groups
+			c.groups = nil
 		}
 	}
-	c := &commentState{
-		parent: p.comments,
-		groups: []*ast.CommentGroup{p.leadComment},
+	if p.leadComment != nil {
+		child.groups = append(child.groups, p.leadComment)
+		p.leadComment = nil
 	}
-	p.comments = c
-	p.leadComment = nil
-	return c
+	p.comments = child
+	return child
 }
 
 // openList is used to treat a list of comments as a single comment
@@ -296,16 +300,26 @@ func (p *parser) consumeComment() (comment *ast.Comment, endline int) {
 // comments list, and return it together with the line at which
 // the last comment in the group ends. A non-comment token or n
 // empty lines terminate a comment group.
-func (p *parser) consumeCommentGroup(n int) (comments *ast.CommentGroup, endline int) {
+func (p *parser) consumeCommentGroup(prevLine, n int) (comments *ast.CommentGroup, endline int) {
 	var list []*ast.Comment
+	var rel token.RelPos
 	endline = p.file.Line(p.pos)
+	switch endline - prevLine {
+	case 0:
+		rel = token.Blank
+	case 1:
+		rel = token.Newline
+	default:
+		rel = token.NewSection
+	}
 	for p.tok == token.COMMENT && p.file.Line(p.pos) <= endline+n {
 		var comment *ast.Comment
 		comment, endline = p.consumeComment()
 		list = append(list, comment)
 	}
 
 	cg := &ast.CommentGroup{List: list}
+	ast.SetRelPos(cg, rel)
 	comments = cg
 	return
 }
@@ -338,10 +352,12 @@ func (p *parser) next() {
 		var comment *ast.CommentGroup
 		var endline int
 
-		if p.file.Line(p.pos) == p.file.Line(prev) {
+		currentLine := p.file.Line(p.pos)
+		prevLine := p.file.Line(prev)
+		if prevLine == currentLine {
 			// The comment is on same line as the previous token; it
 			// cannot be a lead comment but may be a line comment.
-			comment, endline = p.consumeCommentGroup(0)
+			comment, endline = p.consumeCommentGroup(prevLine, 0)
 			if p.file.Line(p.pos) != endline {
 				// The next token is on a different line, thus
 				// the last comment group is a line comment.
@@ -355,7 +371,10 @@ func (p *parser) next() {
 			if comment != nil {
 				p.comments.add(comment)
 			}
-			comment, endline = p.consumeCommentGroup(1)
+			comment, endline = p.consumeCommentGroup(prevLine, 1)
+			prevLine = currentLine
+			currentLine = p.file.Line(p.pos)
+
 		}
 
 		if endline+1 == p.file.Line(p.pos) && p.tok != token.EOF {
@@ -674,7 +693,11 @@ func (p *parser) parseCallOrConversion(fun ast.Expr) (expr *ast.CallExpr) {
 	c := p.openComments()
 	defer func() { c.closeNode(p, expr) }()
 
+	p.openList()
+	defer p.closeList()
+
 	lparen := p.expect(token.LPAREN)
+
 	p.exprLev++
 	var list []ast.Expr
 	for p.tok != token.RPAREN && p.tok != token.EOF {
@@ -1059,6 +1082,15 @@ func (p *parser) parseStructBody() []ast.Decl {
 
 	p.exprLev++
 	var elts []ast.Decl
+
+	// TODO: consider "stealing" non-lead comments.
+	// for _, cg := range p.comments.groups {
+	// 	if cg != nil {
+	// 		elts = append(elts, cg)
+	// 	}
+	// }
+	// p.comments.groups = p.comments.groups[:0]
+
 	if p.tok != token.RBRACE {
 		elts = p.parseFieldList()
 	}

cue/parser/parser_test.go

@@ -558,6 +558,50 @@ bar: 2
 		a: int=>2
 		`,
 		out: "a: int=>2\nalias \"int\" not allowed as value",
+	}, {
+		desc: "struct comments",
+		in: `
+		struct: {
+			// This is a comment
+		
+			// This is a comment
+		
+			// Another comment
+			something: {
+			}
+		
+			// extra comment
+		}`,
+		out: `struct: {<[0// This is a comment] [0// This is a comment] [d0// Another comment] [d5// extra comment] something: {}>}`,
+	}, {
+		desc: "list comments",
+		in: `
+		list: [
+			// Comment1
+
+			// Comment2
+
+			// Another comment
+			{
+			},
+
+			// Comment 3
+		]`,
+		out: "list: [<[0// Comment1] [0// Comment2] [d0// Another comment] [d3// Comment 3] {}>]",
+	}, {
+		desc: "call comments",
+		in: `
+		funcArg1: foo(
+			{},
+	
+			// Comment1
+
+			// Comment2
+			{}
+	
+			// Comment3
+		)`,
+		out: "funcArg1: foo(<[1// Comment1] {}>, <[d0// Comment2] [d1// Comment3] {}>)",
 	}}
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
@@ -751,7 +795,7 @@ func TestX(t *testing.T) {
 	t.Skip()
 
 	f, err := ParseFile("input", `
-	`)
+	`, ParseComments)
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}

encoding/protobuf/testdata/gateway.proto.out.cue

@@ -360,7 +360,6 @@ package v1alpha3
 		// secured using TLS. The value of this field determines how TLS is
 		// enforced.
 		mode?: #TLSmode @protobuf(2)
-
 		// Extra comment.
 
 		// REQUIRED if mode is `SIMPLE` or `MUTUAL`. The path to the file

encoding/protobuf/testdata/istio.io/api/cue.mod/gen/github.com/golang/protobuf/protoc-gen-go/descriptor/descriptor_proto_gen.cue

@@ -621,7 +621,6 @@ package descriptor
 }
 
 #ServiceOptions: {
-
 	// Note:  Field numbers 1 through 32 are reserved for Google's internal RPC
 	//   framework.  We apologize for hoarding these numbers to ourselves, but
 	//   we were already using them long before we decided to release Protocol
@@ -638,7 +637,6 @@ package descriptor
 }
 
 #MethodOptions: {
-
 	// Note:  Field numbers 1 through 32 are reserved for Google's internal RPC
 	//   framework.  We apologize for hoarding these numbers to ourselves, but
 	//   we were already using them long before we decided to release Protocol

encoding/protobuf/testdata/istio.io/api/mixer/v1/mixer_proto_gen.cue

@@ -195,7 +195,6 @@ import (
 
 // Used to report telemetry after performing one or more actions.
 #ReportRequest: {
-
 	// next value: 5
 
 	// Used to signal how the sets of compressed attributes should be reconstitued server-side.