CC @griesemer

@zimmski The AST walker only visits comments that were associated with nodes (excluding the list of all comments that's in ast.File). At this point, only comments above declarations, and line comments in declarations (e.g. structs) are associated with the respective nodes; i.e., the parser assumes that those comments "belong" to those nodes.

In general, the parser doesn't make any further assumptions about other comments and does not associate them with any nodes (this is indeed a major shortcoming of the existing AST and we're investigating how to remedy this situation, possibly via a new AST representation).

Walk could be made more sophisticated and walk comments as they appear by position, relative to the other nodes. But this requires that position information is 100% correct (which is not always the case when the AST is manipulated after creation). It also is not clear with which node a comment should appear (this is something a future heuristic would have to address).

If you want to walk all comments, it's trivial to a) ignore comments encountered by Walk, and b) simply traverse the list of all comments attached to ast.File (i.e., the list of comments in ast.File contains all comments, even the ones associated with other nodes).

Closing this for now since it's working as originally intended; and with the understanding that a future implementation needs to do a better job.

Thanks for the detailed explanation! We have already a workaround which traverses all comments through the AST, remembers them, and then traverses all comments which have not been visited. This is needed because we analyse comments differently if they belong to a declaration.

I am wondering if these changes to the AST and ast.Walk can be implemented at all because of the backwards-compatible promise? Also, do comments really have to appear "with" a node? Can't they just reside inside the AST as an ordinary node? Additionally the position information must be somehow somewhere correct, because using go/printer will output the correct source even after a modification, or is this assumption wrong?

The suggested changes require a new AST - not entirely different, but capable of associating comments with nodes. I have a prototype but it needs a bit more work. Once I am confident enough that it can work well, I'll send out a proposal.

Comments can appear anywhere between two tokens (actually, from the scanner's point of view, a comment is just a token). Because they can appear anywhere, there's no obvious place to put them in the AST. For instance, an if statement "if expr /* comment */ { foo() }" consists of an IfStmt node and several child nodes (expr, then branch). The comment could be anywhere. In this case it could (and probably does) make sense to say that it "belongs" to the expr. In other words, the comment is said to be "associated" with the expr. If the comment appeared before the "if" it might instead be associated with the IfStmt node, etc. This association is not obvious from the grammar or language, and requires a heuristic (which may fail). However, if comments were associated with nodes, moving nodes would ensure the associated comments also move with them. This is currently not the case.

For an unmodified AST, all positions (including comment positions) are correct. If the AST is modified, but comment and AST node positions are not updated accordingly, the go/printer will print the comments in the wrong places. This is a common issue with AST manipulation and the major reason why comments should probably be attached to nodes. This was in some sense a design mistake (mea culpa) - the AST package is one of the very first Go packages and I didn't appreciate the need for easy manipulation (nor did I foresee all the AST manipulation tools).

Did you put the proposal you are working on somewhere public?

@zimmski I'm sorry, no, I have not made any progress on this front due to other more pressing 1.6 items. But it's on the forefront of my things to do.

No need to be sorry. I was just wondering how that is going since I stumbled on my "workaround" mark. Is anything easy on your 1.6? Maybe I can help out since I have some open source hours left in the coming months.