dst removes line break between /* … */ comments and package statement

[stapelberg]

When taking the example program from the README, and adding a /* single line comment */ like so:

package main

import "github.com/dave/dst/decorator"

func main() {
	code := `/* single line comment */
package a

func main(){
	var a int    // foo
	var b string // bar
}
`
	f, err := decorator.Parse(code)
	if err != nil {
		panic(err)
	}
	if err := decorator.Print(f); err != nil {
		panic(err)
	}
}

…the output is:

/* single line comment */package a

func main() {
	var a int    // foo
	var b string // bar
}

Note how package a is now at the end of the previous line.

The same issue is reproducible with multi-line /* … */ comments.

I tested this with github.com/dave/dst v0.27.0, which is current at the time of writing.

[stapelberg]

When setting a breakpoint on

dst/decorator/restorer-generated.go

Line 672 in bc71a76

r.applySpace(n, "Before", n.Decs.Before)

in gdb and print n.Decs, I can see that Before is set to 0.

When I manually change the code to hard-code dst.NewLine, the rewrite works as expected:

		r.applySpace(n, "Before", dst.NewLine) // n.Decs.Before)

So I think the question is: why does the dst.File not get its Decs.Before set correctly?

[stapelberg]

Looking at this some more with the following test program:

package main

import "github.com/dave/dst/decorator"

func main() {
	code := `/*
first line
second line
last line
*/
package a

func main(){
	var a int
	/* first main line
	second main line
	last main line */
	var b string
}
`
	f, err := decorator.Parse(code)
	if err != nil {
		panic(err)
	}
	if err := decorator.Print(f); err != nil {
		panic(err)
	}
}

I see that for the FileRestorer.applyDecorations call for the second ast.DeclStmt in func main, the decorations look like this:

(dlv) p decorations
github.com/dave/dst.Decorations len: 2, cap: 2, [
	"/* first main line\n\tsecond main line\n\tlast main line */",
	"\n",
]

For comparison, the decorations for the ast.File node look like this:

(dlv) p decorations
github.com/dave/dst.Decorations len: 2, cap: 2, [
	"/*\nfirst line\nsecond line\nlast line\n*/",
	"\n",
]

So it seems like the \n is part of the dst.Decorations for multi-line comments, and n.Decs.Before being empty might be correct after all.

Maybe the issue is with restoring the decorations, then.

[dave]

I just pushed a fix to master... can you give it a try?

[dave]

I haven't worked on this codebase for several years so I'm not exactly familiar with the micro details... but I found incrementing the cursor after the decoration for any package comment fixed this problem, and didn't fail any of the other tests... If I can find some time in the next few days I'll spend a bit longer to do a little bit more research, because I'd love to understand a bit more before tagging this as a release.

[dave]

... and yes you're right, I believe the decorator is working correctly by treating an inline comment on the preceding line to the node as a comment decoration then a newline decoration (this is how it treats this type of comment elsewhere in the code).

[stapelberg]

I just pushed a fix to master... can you give it a try?

Thank you for the fix! I just tested it and successfully verified that it fixes the issue for me :)

[matthewhughes934]

Thanks for fixing this! I was wondering if there was any chance for a new tag to include the fix? I'm asking because I found this addresses a downstream bug I found: segmentio/golines#110

[dave]

OK done. v0.27.3

This has been sitting at the bottom of my todo list since I pushed that change!