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>
diff --git a/cmd/cue/cmd/testdata/script/import_auto.txt b/cmd/cue/cmd/testdata/script/import_auto.txt
index 4c555f2..11bd70d 100644
--- a/cmd/cue/cmd/testdata/script/import_auto.txt
+++ b/cmd/cue/cmd/testdata/script/import_auto.txt
@@ -2,8 +2,6 @@
cmp stdout expect-openapi
-- expect-openapi --
-
-
// An OpenAPI file
info: {
diff --git a/cue/ast/astutil/apply_test.go b/cue/ast/astutil/apply_test.go
index a9dc94a..f7509b5 100644
--- a/cue/ast/astutil/apply_test.go
+++ b/cue/ast/astutil/apply_test.go
@@ -47,6 +47,7 @@
`,
out: `
iam: new
+
// foo is a
foo: {
iam: new
diff --git a/cue/format/node.go b/cue/format/node.go
index 8e9192f..cb865f4 100644
--- a/cue/format/node.go
+++ b/cue/format/node.go
@@ -386,7 +386,6 @@
f.print(n.At, n)
case *ast.CommentGroup:
- f.print(newsection)
f.printComment(n)
f.print(newsection)
diff --git a/cue/format/testdata/comments.golden b/cue/format/testdata/comments.golden
index 419f1fb..9b41967 100644
--- a/cue/format/testdata/comments.golden
+++ b/cue/format/testdata/comments.golden
@@ -42,3 +42,79 @@
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
+ )
diff --git a/cue/format/testdata/comments.input b/cue/format/testdata/comments.input
index 0f41c88..dc451ed 100644
--- a/cue/format/testdata/comments.input
+++ b/cue/format/testdata/comments.input
@@ -48,3 +48,79 @@
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
+)
diff --git a/cue/parser/parser.go b/cue/parser/parser.go
index 157b1b5..41daabd 100644
--- a/cue/parser/parser.go
+++ b/cue/parser/parser.go
@@ -102,6 +102,9 @@
// 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 @@
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,9 +300,18 @@
// 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()
@@ -306,6 +319,7 @@
}
cg := &ast.CommentGroup{List: list}
+ ast.SetRelPos(cg, rel)
comments = cg
return
}
@@ -338,10 +352,12 @@
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 @@
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 @@
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 @@
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()
}
diff --git a/cue/parser/parser_test.go b/cue/parser/parser_test.go
index 42c9c8e..42696c8 100644
--- a/cue/parser/parser_test.go
+++ b/cue/parser/parser_test.go
@@ -558,6 +558,50 @@
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 @@
t.Skip()
f, err := ParseFile("input", `
- `)
+ `, ParseComments)
if err != nil {
t.Errorf("unexpected error: %v", err)
}
diff --git a/encoding/protobuf/testdata/gateway.proto.out.cue b/encoding/protobuf/testdata/gateway.proto.out.cue
index d0d5330..b2992d8 100644
--- a/encoding/protobuf/testdata/gateway.proto.out.cue
+++ b/encoding/protobuf/testdata/gateway.proto.out.cue
@@ -360,7 +360,6 @@
// 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
diff --git a/encoding/protobuf/testdata/istio.io/api/cue.mod/gen/github.com/golang/protobuf/protoc-gen-go/descriptor/descriptor_proto_gen.cue b/encoding/protobuf/testdata/istio.io/api/cue.mod/gen/github.com/golang/protobuf/protoc-gen-go/descriptor/descriptor_proto_gen.cue
index ae8f090..6f068cd 100644
--- a/encoding/protobuf/testdata/istio.io/api/cue.mod/gen/github.com/golang/protobuf/protoc-gen-go/descriptor/descriptor_proto_gen.cue
+++ b/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 @@
}
#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 @@
}
#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
diff --git a/encoding/protobuf/testdata/istio.io/api/mixer/v1/mixer_proto_gen.cue b/encoding/protobuf/testdata/istio.io/api/mixer/v1/mixer_proto_gen.cue
index 47e96b9..b3f24df 100644
--- a/encoding/protobuf/testdata/istio.io/api/mixer/v1/mixer_proto_gen.cue
+++ b/encoding/protobuf/testdata/istio.io/api/mixer/v1/mixer_proto_gen.cue
@@ -195,7 +195,6 @@
// 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.
diff --git a/encoding/protobuf/testdata/istio.io/api/networking/v1alpha3/gateway_proto_gen.cue b/encoding/protobuf/testdata/istio.io/api/networking/v1alpha3/gateway_proto_gen.cue
index 5d26865..35fd9b1 100644
--- a/encoding/protobuf/testdata/istio.io/api/networking/v1alpha3/gateway_proto_gen.cue
+++ b/encoding/protobuf/testdata/istio.io/api/networking/v1alpha3/gateway_proto_gen.cue
@@ -360,7 +360,6 @@
// 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
diff --git a/encoding/protobuf/testdata/trailcomment.proto.out.cue b/encoding/protobuf/testdata/trailcomment.proto.out.cue
index 346f7b2..43228f1 100644
--- a/encoding/protobuf/testdata/trailcomment.proto.out.cue
+++ b/encoding/protobuf/testdata/trailcomment.proto.out.cue
@@ -14,7 +14,6 @@
}
c?: int32 @protobuf(3)
-
// hello world
}
