Use the value not the label for the print ast node

[aeisenberg]

Fixes #659

Checklist

• [n/a] CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
• Issues have been created for any UI or other user-facing changes made by this pull request.
• [n/a] @github/docs-content-dsp has been cc'd in all issues for UI or other user-facing changes made by this pull request.

[adityasharad]

Generally looks good, some questions about fallback behaviour and testing.

[adityasharad]

What happens if we have an AST query that doesn't set semmle.label on its nodes? For example

query predicate nodes(Node node) { ... }

In that case I would expect value here to be falsy, and I suggest we fall back to entity.label in that default case.

[aeisenberg]

Sounds reasonable.

[adityasharad]

To clarify: I think we should handle that in the default: case too, in case semmle.label is not set at all.

[aeisenberg]

I don't think that's the behaviour we want. Right now, I don't know what the other kind of labels there are, but we don't want them to overwrite whatever is in the semmle.label. And more importantly, we don't even know what would be in the value field and if it makes any sense to display.

[adityasharad]

If we can assume all AST queries follow this pattern:

query predicate nodes(Node node, string key, string value) {
  key = "semmle.label" and ...
}

then the nodes table will have tuples with semmle.label set, and this PR is good to go.

I am trying to be cautious about the possibility that no such label is set, which will happen if you write:

query predicate nodes(Node node) { ... }

In that case I think we'd have to create an item under the default: case of the switch in this code, which uses entity.label. Perhaps that's not something we need to handle in this PR if we know it won't arise from any of the implementations.

[aeisenberg]

If an entity does not have a semmle.label tupleType, then I think this is a bug. As far as I know existing languages all borrow from the cpp implementation which always creates a semmle.label for each node. This is not mentioned in the specs and maybe we should clarify.

[aeisenberg]

https://github.com/github/codeql-c-analysis-team/pull/187

[adityasharad]

I'm convinced! Ship it.