Comments that are placed inside empty function or loop bodies are associated with statements after the body, hence statements outside of the scope the comments are in. I would expect them to be associated with the function or loop declaration. This snipped demonstrates the issue:
package main
func main() {
}
func foo() {
// inside empty function, associated with function bar below
}
func bar() {
i := 1
for i < 2 {
// inside empty for, associated with if i == 3
}
// after empty for loop, this, however, is associated with for loop above
if i == 3 {
// inside empty if, associated with i = 4
}
i = 4
}
And here you can find code to reproduce the issue: https://play.golang.org/p/m4j-OTbdi-L
As @griesemer mentioned in #20744, the comment association heuristic is not straight forward to implement. Thus, I am wondering whether the above shown associations are intentional or whether this is a bug. Thanks for the clarification!
$ go version go version go1.14.2 darwin/amd64
go env Output
$ go env GO111MODULE="" GOARCH="amd64" GOBIN="" GOCACHE="/Users/.../Library/Caches/go-build" GOENV="/Users/.../Library/Application Support/go/env" GOEXE="" GOFLAGS="" GOHOSTARCH="amd64" GOHOSTOS="darwin" GOINSECURE="" GONOPROXY="" GONOSUMDB="" GOOS="darwin" GOPATH="/Users/.../go" GOPRIVATE="" GOPROXY="https://proxy.golang.org,direct" GOROOT="/usr/local/go" GOSUMDB="sum.golang.org" GOTMPDIR="" GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64" GCCGO="gccgo" AR="ar" CC="clang" CXX="clang++" CGO_ENABLED="1" GOMOD="/Users/.../go.mod" CGO_CFLAGS="-g -O2" CGO_CPPFLAGS="" CGO_CXXFLAGS="-g -O2" CGO_FFLAGS="-g -O2" CGO_LDFLAGS="-g -O2" PKG_CONFIG="pkg-config" GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/yb/hqncynqs0b5_3hyxcjpdsyr00000gr/T/go-build447564702=/tmp/go-build -gno-record-gcc-switches -fno-common"
Comment From: griesemer
Not having looked at this in detail, from your description this sounds like a bug; at least I don't see why this would be intentional.
Comment From: dave
In dst the algorithm to associate comments with decoration attachment points was by no means straightforward. If it finds nothing below the comment it bounces back and searches before the comment. In this example it would attach to the opening brace of the BlockStmt.
Comment From: ec-m
As @dave pointed out, the question is now, with which nodes we want to associate the comments in empty bodies.
I changed the function NewCommentMap(...) in go/ast/commentmap.go by slightly improving the association heuristic to also cover empty body cases:
Please see here for the full changes that also add an additional unit test: https://github.com/ec-m/go/commit/7b0f14cb18cbd27352140e2c34a985c6733afc17
With this fix, comments inside empty function/loop/if bodies are associated to the *ast.BlockStmt nodes that represent the empty bodies.
Do you think this would be an acceptable way to associate the comments, @griesemer? If so, I will open a pull request with Gerrit. Or would you expect the associated node to be something else, e.g. the function declaration itself? Thanks for the feedback.
Comment From: odeke-em
Howdy @ec-m @dave @griesemer!
Thank you for filing, and investigating this bug, @ec-m. I'd say please go for it, please mail that change; @griesemer's plate is heavily stacked, but with your CL up for review, that provides more actionable feedback and brings in more reviewers. It would be awesome to have you as a Go contributor, @ec-m! Happy holidays!
Comment From: ec-m
@odeke-em Thanks for the feedback. I will set up Gerrit in the next few days and send a PR :)
Comment From: gopherbot
Change https://golang.org/cl/281234 mentions this issue: go/ast: improve comment associations in empty function/loop/if bodies
Comment From: gopherbot
Change https://golang.org/cl/315370 mentions this issue: go/ast: print CommentMap contents in source order
Comment From: griesemer
cc: @mvdan