cmd/fmt: single import and comment combined

[verdverm]

What version of CUE are you using (cue version)?

0.4.0

What did you do?

exec cue fmt file.cue
cmp stdout file.cue
-- file.cue --
import "strings"

// comment

s: string
u: strings.ToUpper(s)

What did you see instead?

import ( "strings"

	// comment
)

s: string
u: strings.ToUpper(s)

[jlongtine]

I've noticed versions of this, as well. Comments being moved around into odd places in the AST tree. I don't have any current examples, but I'll post them here if I come across them again.

[rudifa]

This issue is same as #720

1. The newline after the comment in the input file triggers the bug, as the comment gets associated with the previous import line, then the comment is misplaced into the import block.

cue % cat testdata/1447-2.cue                                        [issues-fmt-comments L|…1]
import "strings"
// comment

s: string
u: strings.ToUpper(s)

cue % cat testdata/1447-2.cue | go run . fmt -                       [issues-fmt-comments L|…1]
import ( "strings"
	// comment
)

s: string
u: strings.ToUpper(s)

2. In the absence of a newline between the comment and the code following it there is no problem, and the fmt output is similar to the input.

cue % cat testdata/1447-1.cue                                        [issues-fmt-comments L|…1]
import "strings"
// comment
s: string
u: strings.ToUpper(s)

cue % cat testdata/1447-1.cue | go run . fmt -                       [issues-fmt-comments L|…1]
import "strings"

// comment
s: string
u: strings.ToUpper(s)

3. I inserted the following code into fmt.go:

...
		files = append(files, f) // cue/cmd/cue/cmd/fmt.go line 83

		// TEMPORARY: print DebugStr for `f`, the decoded `*astFile`
		dbgstr := astinternal.DebugStr(f)
	        fmt.Fprintf(os.Stderr,"DebugStr: %s\n", dbgstr)
...

The DebugStrs for the bad input testdata/1447-2.cue and good input testdata/1447-1.cue are

DebugStr: import <[2// comment] "strings">, s: string, u: strings.ToUpper(s)

DebugStr: import "strings", <[d0// comment] s: string>, u: strings.ToUpper(s)

Above suggests that the problem might start in the func (i *Decoder) File() *ast.File.

4. Is there a written spec for what the decoder should do with comments variously placed with respect to the cue code?

5. It is jpluscplusm who noticed that a newline between a comment and the code following it produces the misplaced comments, cmd/fmt: fmt emits invalid CUE when commenting struct references in lists #2274

[rudifa]

The following reproducer confirms the original complaint, namely that the comment // section 1 is about strings is pulled into the import section. The cue-ast-print output confirms that this comment is attached to the preceding node, ImportSpec.

The comment // section 2 will be added here looks good in the formatted output, although it is also attached to the preceding node, the Field with label u.

The title comment // the purpose of this file is ... also looks good; it is attached to the top level File node.

In my opinion, these 'floating' comments (neither Doc nor Line) should all be attached to the File node, with their Position indicating to the formatter where to place them w.r.t. other nodes.

A possible alternative would be to a separate node of a new type, comment-group-only for each of them. This might be helpful in issues like #2672, to prevent excessive simplification.

# cmd/fmt: single import and comment combined #1447

exec cue-ast-print 1447.cue
cmp stdout 1447.ast
exec cue fmt 1447.cue
cmp 1447.cue 1447.got.cue
exec cue vet 1447.cue           # pass

-- 1447.cue --
// the purpose of this file is ...

import "strings"

// section 1 is about strings

u: strings.ToUpper("cue rules")

// section 2 will be added here

-- 1447.got.cue --
// the purpose of this file is ...

import ( "strings"

	// section 1 is about strings
)

u: strings.ToUpper("cue rules")

// section 2 will be added here
-- 1447.ast --
*ast.File{
. Filename: "1447.cue"
. Decls: []ast.Decl{
. . *ast.ImportDecl{
. . . Import: token.Pos("1447.cue:3:1")
. . . Lparen: token.Pos("-")
. . . Specs: []*ast.ImportSpec{
. . . . *ast.ImportSpec{
. . . . . Name: nil
. . . . . Path: *ast.BasicLit{
. . . . . . ValuePos: token.Pos("1447.cue:3:8")
. . . . . . Kind: token.Token("STRING")
. . . . . . Value: "\"strings\""
. . . . . }
. . . . . EndPos: token.Pos("-")
. . . . . Comments: []*ast.CommentGroup{
. . . . . . *ast.CommentGroup{
. . . . . . . Doc: false
. . . . . . . Line: false
. . . . . . . Position: 2
. . . . . . . List: []*ast.Comment{
. . . . . . . . *ast.Comment{
. . . . . . . . . Slash: token.Pos("1447.cue:5:1")
. . . . . . . . . Text: "// section 1 is about strings"
. . . . . . . . }
. . . . . . . }
. . . . . . }
. . . . . }
. . . . }
. . . }
. . . Rparen: token.Pos("-")
. . }
. . *ast.Field{
. . . Label: *ast.Ident{
. . . . NamePos: token.Pos("1447.cue:7:1")
. . . . Name: "u"
. . . }
. . . Optional: token.Pos("-")
. . . Constraint: token.Token("ILLEGAL")
. . . TokenPos: token.Pos("1447.cue:7:2")
. . . Token: token.Token(":")
. . . Value: *ast.CallExpr{
. . . . Fun: *ast.SelectorExpr{
. . . . . X: *ast.Ident{
. . . . . . NamePos: token.Pos("1447.cue:7:4")
. . . . . . Name: "strings"
. . . . . }
. . . . . Sel: *ast.Ident{
. . . . . . NamePos: token.Pos("1447.cue:7:12")
. . . . . . Name: "ToUpper"
. . . . . }
. . . . }
. . . . Lparen: token.Pos("1447.cue:7:19")
. . . . Args: []ast.Expr{
. . . . . *ast.BasicLit{
. . . . . . ValuePos: token.Pos("1447.cue:7:20")
. . . . . . Kind: token.Token("STRING")
. . . . . . Value: "\"cue rules\""
. . . . . }
. . . . }
. . . . Rparen: token.Pos("1447.cue:7:31")
. . . }
. . . Attrs: []*ast.Attribute{
. . . }
. . . Comments: []*ast.CommentGroup{
. . . . *ast.CommentGroup{
. . . . . Doc: false
. . . . . Line: false
. . . . . Position: 5
. . . . . List: []*ast.Comment{
. . . . . . *ast.Comment{
. . . . . . . Slash: token.Pos("1447.cue:9:1")
. . . . . . . Text: "// section 2 will be added here"
. . . . . . }
. . . . . }
. . . . }
. . . }
. . }
. }
. Imports: []*ast.ImportSpec{
. . *ast.ImportSpec{
. . . Name: nil
. . . Path: *ast.BasicLit{
. . . . ValuePos: token.Pos("1447.cue:3:8")
. . . . Kind: token.Token("STRING")
. . . . Value: "\"strings\""
. . . }
. . . EndPos: token.Pos("-")
. . . Comments: []*ast.CommentGroup{
. . . . *ast.CommentGroup{
. . . . . Doc: false
. . . . . Line: false
. . . . . Position: 2
. . . . . List: []*ast.Comment{
. . . . . . *ast.Comment{
. . . . . . . Slash: token.Pos("1447.cue:5:1")
. . . . . . . Text: "// section 1 is about strings"
. . . . . . }
. . . . . }
. . . . }
. . . }
. . }
. }
. Comments: []*ast.CommentGroup{
. . *ast.CommentGroup{
. . . Doc: false
. . . Line: false
. . . Position: 0
. . . List: []*ast.Comment{
. . . . *ast.Comment{
. . . . . Slash: token.Pos("1447.cue:1:1")
. . . . . Text: "// the purpose of this file is ..."
. . . . }
. . . }
. . }
. }
}
-- end --