cue/parser: fix comment attachement issues

Both parser and formatter can be much simpler
now block comments are gone, but will leave
that for another CL.

Fixes #155.

Change-Id: I570fa93ba1147d7fabaed48a78bb3581b1139c8e
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/3782
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
diff --git a/cue/parser/parser.go b/cue/parser/parser.go
index 46e2344..0b1e895 100644
--- a/cue/parser/parser.go
+++ b/cue/parser/parser.go
@@ -103,13 +103,28 @@
 func (p *parser) openComments() *commentState {
 	if c := p.comments; c != nil && c.isList > 0 {
 		if c.lastChild != nil {
+			var groups []*ast.CommentGroup
 			for _, cg := range c.groups {
-				cg.Position = c.lastPos
-				c.lastChild.AddComment(cg)
+				if cg.Position == 0 {
+					groups = append(groups, cg)
+				}
 			}
+			groups = append(groups, c.lastChild.Comments()...)
+			for _, cg := range c.groups {
+				if cg.Position != 0 {
+					cg.Position = c.lastPos
+					groups = append(groups, cg)
+				}
+			}
+			ast.SetComments(c.lastChild, groups)
 			c.groups = nil
+		} else {
+			c.lastChild = nil
+			// attach before next
+			for _, cg := range c.groups {
+				cg.Position = 0
+			}
 		}
-		c.lastChild = nil
 	}
 	c := &commentState{
 		parent: p.comments,
@@ -158,7 +173,9 @@
 		}
 	case c.isList == 0:
 		parent := c.parent
-		parent.groups = append(parent.groups, c.groups...)
+		if len(c.groups) > 0 {
+			parent.groups = append(parent.groups, c.groups...)
+		}
 		parent.pos++
 		p.comments = parent
 	}
@@ -182,7 +199,9 @@
 	}
 	for _, cg := range c.groups {
 		if n != nil {
-			n.AddComment(cg)
+			if cg != nil {
+				n.AddComment(cg)
+			}
 		}
 	}
 	c.groups = nil
@@ -668,8 +687,11 @@
 	}
 
 	if p.tok == token.ELLIPSIS {
-		list = append(list, &ast.Ellipsis{Ellipsis: p.pos})
+		c := p.openComments()
+		ellipsis := &ast.Ellipsis{Ellipsis: p.pos}
 		p.next()
+		c.closeNode(p, ellipsis)
+		list = append(list, ellipsis)
 	}
 	return
 }
@@ -1027,9 +1049,6 @@
 }
 
 func (p *parser) parseStruct() (expr ast.Expr) {
-	c := p.openComments()
-	defer func() { c.closeNode(p, expr) }()
-
 	lbrace := p.expect(token.LBRACE)
 
 	if p.trace {
@@ -1127,9 +1146,6 @@
 }
 
 func (p *parser) parseList() (expr ast.Expr) {
-	c := p.openComments()
-	defer func() { c.closeNode(p, expr) }()
-
 	lbrack := p.expect(token.LBRACK)
 
 	if p.trace {
@@ -1185,35 +1201,42 @@
 	defer p.closeList()
 
 	for p.tok != token.RBRACK && p.tok != token.ELLIPSIS && p.tok != token.EOF {
-		list = append(list, p.parseListElement())
-		// Enforce there is an explicit comma. We could also allow the
-		// omission of commas in lists, but this gives rise to some ambiguities
-		// with list comprehensions.
-		if p.tok == token.COMMA && p.lit != "," {
-			p.next()
-			// Allow missing comma for last element, though, to be compliant
-			// with JSON.
-			if p.tok == token.RBRACK || p.tok == token.FOR || p.tok == token.IF {
-				break
-			}
-			p.errf(p.pos, "missing ',' before newline in list literal")
-		} else if !p.atComma("list literal", token.RBRACK, token.FOR, token.IF) {
+		expr, ok := p.parseListElement()
+		list = append(list, expr)
+		if !ok {
 			break
 		}
-		p.next()
 	}
 
 	return
 }
 
-func (p *parser) parseListElement() (expr ast.Expr) {
+func (p *parser) parseListElement() (expr ast.Expr, ok bool) {
 	if p.trace {
 		defer un(trace(p, "ListElement"))
 	}
 	c := p.openComments()
 	defer func() { c.closeNode(p, expr) }()
 
-	return p.parseRHS()
+	expr = p.parseBinaryExprTail(false, token.LowestPrec+1, p.parseUnaryExpr())
+
+	// Enforce there is an explicit comma. We could also allow the
+	// omission of commas in lists, but this gives rise to some ambiguities
+	// with list comprehensions.
+	if p.tok == token.COMMA && p.lit != "," {
+		p.next()
+		// Allow missing comma for last element, though, to be compliant
+		// with JSON.
+		if p.tok == token.RBRACK || p.tok == token.FOR || p.tok == token.IF {
+			return expr, false
+		}
+		p.errf(p.pos, "missing ',' before newline in list literal")
+	} else if !p.atComma("list literal", token.RBRACK, token.FOR, token.IF) {
+		return expr, false
+	}
+	p.next()
+
+	return expr, true
 }
 
 // checkExpr checks that x is an expression (and not a type).
@@ -1415,11 +1438,14 @@
 }
 
 // Callers must check the result (using checkExpr), depending on context.
-func (p *parser) parseExpr(lhs bool) ast.Expr {
+func (p *parser) parseExpr(lhs bool) (expr ast.Expr) {
 	if p.trace {
 		defer un(trace(p, "Expression"))
 	}
 
+	c := p.openComments()
+	defer func() { c.closeExpr(p, expr) }()
+
 	return p.parseBinaryExpr(lhs, token.LowestPrec+1)
 }
 
diff --git a/cue/parser/parser_test.go b/cue/parser/parser_test.go
index 0c4f1d8..d892cc9 100644
--- a/cue/parser/parser_test.go
+++ b/cue/parser/parser_test.go
@@ -356,6 +356,7 @@
 			// end
 		]
 		c: [ 1, 2, 3, 4, // here
+			{ a: 3 }, // here
 			5, 6, 7, 8 // and here
 		]
 		d: {
@@ -370,11 +371,11 @@
 			// comment in struct body
 		}
 		`,
-		"a: <[d2// end] {a: 1, b: 2, c: 3, d: 4}>, " +
-			"b: <[d2// end] [1, 2, 3, 4, 5]>, " +
-			"c: [1, 2, 3, <[l1// here] 4>, 5, 6, 7, <[l1// and here] 8>], " +
+		"a: {a: 1, b: 2, c: 3, <[d5// end] d: 4>}, " +
+			"b: [1, 2, 3, 4, <[d2// end] 5>], " +
+			"c: [1, 2, 3, <[l2// here] 4>, <[l4// here] {a: 3}>, 5, 6, 7, <[l2// and here] 8>], " +
 			"d: {<[2/* 8 */] [l5// Hello] a: 1>, <[d0// Doc] b: 2>}, " +
-			"e1: <[d2// comment in list body] []>, " +
+			"e1: <[d1// comment in list body] []>, " +
 			"e2: <[d1// comment in struct body] {}>",
 	}, {
 		"attribute comments",
@@ -396,6 +397,24 @@
 		// a comment at the end of the file
 		`,
 		"<[0// a comment at the beginning of the file] [0// a second comment] <[d0// comment] a: 5>, <[2// a comment at the end of the file] {}>>",
+	}, {
+		"composite comments 2",
+		`
+	{
+// foo
+
+// fooo
+foo: 1
+
+bar: 2
+	}
+
+[
+	{"name": "value"}, // each element has a long
+	{"name": "next"}   // optional next element
+]
+`,
+		`{<[0// foo] [d0// fooo] foo: 1>, bar: 2}, [<[l4// each element has a long] {"name": "value"}>, <[l4// optional next element] {"name": "next"}>]`,
 	}}
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
diff --git a/encoding/protobuf/testdata/istio.io/api/pkg/github.com/golang/protobuf/protoc-gen-go/descriptor/descriptor_proto_gen.cue b/encoding/protobuf/testdata/istio.io/api/pkg/github.com/golang/protobuf/protoc-gen-go/descriptor/descriptor_proto_gen.cue
index 073dfde..0c599f5 100644
--- a/encoding/protobuf/testdata/istio.io/api/pkg/github.com/golang/protobuf/protoc-gen-go/descriptor/descriptor_proto_gen.cue
+++ b/encoding/protobuf/testdata/istio.io/api/pkg/github.com/golang/protobuf/protoc-gen-go/descriptor/descriptor_proto_gen.cue
@@ -187,8 +187,6 @@
 	options?:  FieldOptions @protobuf(8)
 }
 FieldDescriptorProto_Type:
-	// 0 is reserved for errors.
-	// Order is weird for historical reasons.
 	"TYPE_DOUBLE" |
 	"TYPE_FLOAT" |
 
@@ -219,6 +217,9 @@
 	"TYPE_SFIXED32" |
 	"TYPE_SFIXED64" |
 	"TYPE_SINT32" | // Uses ZigZag encoding.
+
+	// 0 is reserved for errors.
+	// Order is weird for historical reasons.
 	"TYPE_SINT64" // Uses ZigZag encoding.
 
 FieldDescriptorProto_Type_value: {