Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

go/printer: Invalid reformatting of slice type with a /* comment. #67868

Open
JonasUnderscore opened this issue Jun 7, 2024 · 5 comments
Open
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@JonasUnderscore
Copy link

Go version

devel go1.23-7492290887

Output of go env in your module/workspace:

set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOOS=windows
set GOTOOLCHAIN=auto
set GOVERSION=devel go1.23-...

What did you do?

https://go.dev/play/p/ndVJVdo9nIW
Click format.

A realistic example could be formatting var s [/*<10*/]byte

What did you see happen?

Formatting code with a "/*" comment and line break inside "[" "]" of a slice either fails or produces invalid code.
Clicking format fails with "24:16: expected type, found newline (and 1 more errors)".

Having multiple comments moves them 2 lines down (no error).
Line comments and multi-line comments are moved to the next position after "]" where line breaks don't insert a ";".

Setting a fake position to the same line as the element type before writing "]" changes the behavior. "go/printer/nodes.go:1049"

ast.ArrayType lacks position information about "]".
The closing "]" must be on the same line as the array length ends. On a slice type this implicit positioning spans multiple lines and the chosen line affects ;'s.

What did you expect to see?

Comments not moving across tokens when formatted.
I would also like the ast types to have positions for more tokens. Guessing the position requires different logic for different types: last line for slice "]", first line for map "]", first line for type assert ".", unknown for <- chan "chan".

@ianlancetaylor
Copy link
Contributor

Specifically, gofmt turns this valid Go code

type Slice = [
	/*a*/]any

into this invalid code:

type Slice = []
/*a*/ any

CC @griesemer

@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 7, 2024
@mknyszek mknyszek added this to the Backlog milestone Jun 7, 2024
@mknyszek
Copy link
Contributor

mknyszek commented Jun 7, 2024

CC @mvdan as well, via https://dev.golang.org/owners

@gabyhelp
Copy link

gabyhelp commented Jun 8, 2024

Similar Issues

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

@griesemer
Copy link
Contributor

Fixing this will require additional position information in the go/ast.ArrayType node (position of the ], trivial but requires API change) or better "guessing" of the position of "]" based on the available comment and node positions (hard).

@rsc
Copy link
Contributor

rsc commented Jun 28, 2024

Even with bad position information it seems like the formatter should understand when it is safe to insert a line break and when it isn't; we should always emit valid Go code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

6 participants