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

x/tools/go/ast/astutil: handle comments path enclosing node correctly #29780

Closed
anjmao opened this issue Jan 17, 2019 · 2 comments
Closed

x/tools/go/ast/astutil: handle comments path enclosing node correctly #29780

anjmao opened this issue Jan 17, 2019 · 2 comments
Milestone

Comments

@anjmao
Copy link

@anjmao anjmao commented Jan 17, 2019

Currently astutil.PathEnclosingInterval doesn't handle enclosing node for comments correctly. Examples:

  1. Pos on <> returns whole *ast.File as enclosing node, but should return *ast.CommentGroup.
// main is main <>
func main() {
}
  1. Pos on <> returns main function's *ast.BlockStmt with all statements (*ast.CommentGroup is not included in this node), but should return *ast.CommentGroup.
func main() {
     fmt.Print("hi") // Print prints hi <>
     fmt.Print("bye")
}

By looking at unit tests I can see that this was intentional behaviour https://github.com/golang/tools/blob/master/go/ast/astutil/enclosing_test.go#L156.

@alandonovan What do you think we can do here? Is it fixable? Was it some requirements for other tools to work this way? Thanks a lot.

Background:
x/tools/internal/lsp uses PathEnclosingInterval for autocompletion and I was working on preventing autocompletion inside comment blocks. I made a fix for 1 case which is still kind of hacky because it traverse *ast.File and checks if node on given pos is actually *ast.Comment , but for 2 case there is no easy way to fix it and the best way would be to fix PathEnclosingInterval.

/cc @stamblerre

@gopherbot gopherbot added this to the Unreleased milestone Jan 17, 2019
@alandonovan
Copy link
Contributor

@alandonovan alandonovan commented Jan 17, 2019

Comment handling in go/ast has been the bane of tool writers for years because they are stored off to the side. The correct solution to this problem (and a raft of others) is to make go/ast (optionally) save all comments in the tree. I don't think it should be too hard, but it will require a Go release.

As a workaround, you could seek through the File.Comments list to see whether the cursor position falls within a comment, and if so, prepend the Comment node to the path returned by PathEnclosingPosition.

@anjmao
Copy link
Author

@anjmao anjmao commented Jan 18, 2019

Thanks for suggestion, yes now I see that only comments on functions are preserved https://play.golang.org/p/UUy88P4jypL.

@anjmao anjmao closed this Jan 18, 2019
@golang golang locked and limited conversation to collaborators Jan 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.