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.

Sign up for GitHub

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

Jump to bottom

go/ast: NewCommentMap doc doesn't apply to trailing line comments #33451

Closed
willfaught opened this issue Aug 3, 2019 · 1 comment
Closed

go/ast: NewCommentMap doc doesn't apply to trailing line comments #33451

willfaught opened this issue Aug 3, 2019 · 1 comment
Labels
FrozenDueToAge

Comments

@willfaught
Copy link
Contributor

willfaught commented Aug 3, 2019 •
edited

What version of Go are you using (go version)?

$ go version
go version go1.12.7 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/Will/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/Will/Developer/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/Cellar/go/1.12.7/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.12.7/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
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/bx/qk0phsxd265fqj512dnnpg080000gn/T/go-build968808452=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Test code:

func parseFileLines(content string) (*token.FileSet, *ast.File) {
	fset := token.NewFileSet()
	f, err := parser.ParseFile(fset, "test.go", content, parser.ParseComments)
	if err != nil {
		panic(err)
	}
	return fset, f
}

func TestCM2(t *testing.T) {
	fset, f := parseFileLines(`//1

//2
package p

//3

//4
var _ int

//5
`)
	cm := ast.NewCommentMap(fset, f, f.Comments)
	pretty.Println(cm)
	t.FailNow()
}

What did you expect to see?

It's unclear. The NewCommentMap doc doesn't seem to apply to trailing line comments in files:

$ go doc go/ast NewCommentMap
func NewCommentMap(fset *token.FileSet, node Node, comments []*CommentGroup) CommentMap
    NewCommentMap creates a new comment map by associating comment groups of the
    comments list with the nodes of the AST specified by node.

    A comment group g is associated with a node n if:

    - g starts on the same line as n ends
    - g starts on the line immediately following n, and there is
      at least one empty line after g and before the next node
    - g starts before n and is not associated to the node before n
      via the previous rules

    NewCommentMap tries to associate a comment group to the "largest" node
    possible: For instance, if the comment is a line comment trailing an
    assignment, the comment is associated with the entire assignment rather than
    just the last operand in the assignment.

None of those rules seem to apply to the //5 comment. NewCommentMap associates //5 with the int node (see below), even though they don't seem to satisfy any of the 3 rules in the NewCommentMap doc: (1) it doesn't start on the same line as the line on which the int node ends; (2) it doesn't start on the line immediately following the line of the int node; and (3) it doesn't start before int.

What did you see instead?

Test output:

ast.CommentMap{
    [...]
    &ast.Ident{
        NamePos: 36,
        Name:    "int",
        Obj:     (*ast.Object)(nil),
    }:  {
        &ast.CommentGroup{
            List: {
                &ast.Comment{Slash:41, Text:"//5"},
            },
        },
    },
}
--- FAIL: TestCM2 (0.00s)
FAIL
FAIL	github.com/willfaught/foo	0.007s
Error: Tests failed.

//5 is associated with int.
@willfaught
Copy link
Contributor Author

Seems too similar to #21755. Closing myself. I'd at least like the existing doc to be updated to fully describe the existing behavior, but I could see an admin arguing that that other issue might already encompass that work.

@golang golang locked and limited conversation to collaborators Aug 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@willfaught @gopherbot