Implement PrintAST for Go

[aeisenberg]

This issue is about adding support to the CodeQL VS Code extension for viewing ASTs of Go files. See the parent issue github/vscode-codeql#492.

[aeisenberg]

See also #179.

[aeisenberg]

I have created a simple printAst.ql contextual query for Go and things are generally working quite nicely. However, there are a few rough edges:

1. Comments are included in the ast. I'm not very familiar with go, so maybe that's something users expect, but from my perspective it clutters things up.
2. Also, comment nodes all seem to be top-level (ie- even if they are inside a function, they don't have a parent node)
3. And, it looks like all files are being returned in the ast, even though I am only requesting a single file at a time.
4. Looks like lists of ParameterDecl and ResultVariableDecl are reversed order. Eg- func unquoteBytes(s []byte) (t []byte, ok bool) lists ok first and s last.

I'll create a PR for the query I am using and hopefully we can use this to debug these issues.

[smowton]

Re: no. 4, this should fix it: #263

[smowton]

Re: no. 3, how about #264 ?

Comments are still second level (children of Files but not of anything else). This is how the Go parser describes them -- we'd have to use source location information, which might be a bit brittle, to give them a better parent. Does that cause you much trouble? I could add a skipComments option to the config if you want?

[aeisenberg]

Thanks for the related PRs.

Regarding comments in the AST, I'm not an avid go user (I haven't done much more than the intro tutorials a few years ago), so it's hard to know how useful they are. My thoughts are that if comments are regularly returned as useful results in codeql-go queries, then we should include them in the AST. If they're not (or rarely are), then let's remove them. More precisely, only if comments are regularly involved with security issues, let's keep them.

It all depends on what users expect and what they find useful.

[smowton]

I'm not aware of any existing query where we pay attention to them. Do you want a flag in the config to omit them, or is it simple to omit them your side?

[aeisenberg]

I think it is reasonable to have the printAst.ql query from #262 determine which ast types it will include. Though, not sure how to specify that in the PrintAstConfiguration. Do you have a suggestion? Maybe a flag is the only way, though.

[sauyon]

This is how the Go parser describes them -- we'd have to use source location information, which might be a bit brittle, to give them a better parent.

Is that the case? The Go AST library provides CommentMap, which seems like the natural way to parent comment groups.

[smowton]

Ah nice, ok so we're using an old API that describes comments only as File children, but there's a better one that will give them best-possible parents. For now do you just want me to add a configuration filter for commentgroups?

[aeisenberg]

I think that would be easiest from my perspective.

[smowton]

Alright, coming up

[smowton]

Added to #264 along with the refinements suggested by @sauyon

[smowton]

@aeisenberg you ok with marking this done?

[aeisenberg]

We're done here. Thanks!