cmd/fmt: fmt emits invalid CUE when commenting struct references in lists

[jpluscplusm]

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

cue version v0.5.0-beta.5

go version go1.19.3
       -compiler gc
       -trimpath true
     CGO_ENABLED 0
          GOARCH amd64
            GOOS linux
         GOAMD64 v1

Does this issue reproduce with the latest stable release?

Yes.

What did you do?

My editor runs cue fmt when it saves a .cue file.

Just occasionally I was being confused by cue command failures -- happening later on in the development process where I pointed the CUE CLI at a .cue file -- where I was /sure/ I'd not had cue fmt failures when saving the file and exiting ... but then I discovered syntax errors in the file.

Eventually I realised: I wasn't introducing the errors - cue fmt was.

Sometimes cue fmt gets confused about trailing comments, and moves required list item separators inside the comment. Here's a txtar repro where cue fmt emits invalidate CUE:

exec cue vet file.cue
exec cue fmt file.cue
exec cue vet file.cue
-- file.cue --
package p

#Number: two: 2

aList: [
  1,
  #Number.two, // comment
  3,
]

The bug seems to be picky about the lists it infects, and there's a slightly different list/comment scenario I've seen it fail in. Here, the blank line after the comment seems important: removing it stops the bug ocurring.

exec cue vet file.cue
exec cue fmt file.cue
exec cue vet file.cue
-- file.cue --
package p

#Number: two: 2

aList: [
  1,
  #Number.two,
  // comment

  3,
]

Both /seem/ to be related to that struct usage, and I've not managed to get the bug to affect a simpler list form.

What did you expect to see?

I expected to be able to put comments wherever I like; and for cue fmt either to tell me their form is not acceptable, or to emit valid CUE.

What did you see instead?

cue fmt emits invalid CUE.

[uhthomas]

I also observe this. It's annoying as the comma is appended every time cue fmt is run. Eventually there are hundreds of them!

A really minimal example still produces this.

a: [
    "a"
    // b,
]

[myitcv]

Thanks @jpluscplusm.

Here's a slight rework of your example to make it clearer what the output from fmt is, and why therefore it is problematic.

# Verify the file is initially valid
exec cue vet file.cue

# Format the file. This should not ever make it invalid.
exec cue fmt file.cue

# For the reader, compare the formatted result with a golden file as part of
# the test to make clear the problem that fmt introduces.
# Remove this line when attempting to fix the bug.
cmp file.cue file.cue.formatted

# Check the file is still valid after fmt
exec cue vet file.cue

-- file.cue --
package p

#Number: two: 2

aList: [
  1,
  #Number.two, // comment
  3,
]
-- file.cue.formatted --
package p

#Number: two: 2

aList: [
	1,
	#Number.two // comment,
	3,
]

Just to clarify however, my script above will fail when this bug is fixed because the cmp step will fail.

[uhthomas]

Another reproducer:

foo: [
	"bar" + 1 // some comment,,,,,,,
]

I don't believe this happens with basic elements though?

[NoamTD]

I took a look at the fmt implementation and managed to reproduce quite a few more cases where this breaks:

exec cue fmt file.cue
cmp file.cue bad-result.cue

-- file.cue --
import "strings"

someList: [
  a.b,                  // comment
  (a),                  // comment
  +1,                   // comment
  a[0],                 // comment
  strings.Sort([1, 2]), // comment
]
-- bad-result.cue --
import "strings"

someList: [
	a.b                  // comment,
	(a                   // comment),
	+1                   // comment,
	a[0                  // comment],
	strings.Sort([1, 2], // comment),
]