Add more metadata for C++ print AST queries

[aeisenberg]

Based on discussion with @dbartol there are some enhancements to the C++ Print AST query that I would like to propose.

1. Along with each child node in the AST should contain metadata that describes the property in its parent that returns this node. Eg- the LHS of an AssignExpr would have metadata containing something like getLValue(). This would be displayed somehow in the AST Viewer in VS Code. (Perhaps as a tooltip, perhaps in the title, or in some other way). If possible, the metadata would also contain enough information so that the IDE could navigate back to the definition of the predicate (maybe the metadata from below and the name of the predicate is sufficient).
2. Each node in the AST should contain metadata that describes its type so that it is possible for users to navigate to the declaration of that type. The type name already exists in the label, but it is not the fully qualified name, so there is not enough information to open that type in the IDE.

See also #4278.

[criemen]

re 1. I plan to add a predicate
abstract string getChildAccessor(int index);
to StmtParent.
The implementation of that, for example, in Assignment then would look like this:

  override string getChildAccessor(int index) {
    index = 0 and 
    result = "getLValue"
    or 
    index = 1 and 
    result = "getRValue"
  }

The main work will be auditing all subclasses of StmtParent to see which need an override.

This information then can be used to replace the (extractor-internal) child index number we currently use for printing the AST graph.
The VSCode AST viewer could then also leverage this information.
Right now I am leaning to not including () in the data, as that can be easily added when printing, if needed.

I have not investigated 2. yet, and will postpone working on that once 1. is addressed.

[aeisenberg]

I expect (2) is more controversial and should have some more thought as to exactly how it should be implemented.

Also, note that the second part of (1):

If possible, the metadata would also contain enough information so that the IDE could navigate back to the definition of the predicate (maybe the metadata from below and the name of the predicate is sufficient).

is currently blocked and possibly not necessary. See github/vscode-codeql#614. And if we can get something to work, we should be able to resolve the QL class using the unqualified name and the language of the database.

[criemen]

Also, note that the second part of (1):

If possible, the metadata would also contain enough information so that the IDE could navigate back to the definition of the predicate (maybe the metadata from below and the name of the predicate is sufficient).

is currently blocked and possibly not necessary. See github/vscode-codeql#614. And if we can get something to work, we should be able to resolve the QL class using the unqualified name and the language of the database.

I am not so sure about that, as you'd (minimum) would need to traverse the QL class hierarchy - for example, we can implement the predicate getChildAccessor on BinaryOperation, whereas the class exposed in the AST would be LogicalAndExpr.
If you have enough semantic information on the QL code available in the vscode extension, then I think this is viable without us providing more metadata.

[aeisenberg]

Our approach is to assume that the name in square brackets corresponds to the unqualified class name. If that's not the case, then we will need some extra metadata (and if that's not the case, I think we should carefully consider why we are doing it this way).

Currently, all we need is the unqualified class name. For example, if you create a *.ql file whose contents are:

import cpp
BinaryOperation

And then you pressed F12 with BinaryOperation selected, you will be navigated to its definition in Expr.qll. Unfortunately, due to limitations on how the language server works, we cannot invoke this operation unless we create a file inside of the vscode workspace.

[criemen]

The problem is that the predicate, like getLValue() will not be implemented on the downstream class, but somewhere higher in the hierarchy.
So in the example, the name in the square brackets will be LogicalAndExpr, but its implementation of getLValue() resides in BinaryOperation.

I do not see added value of navigating there, whereas I think navigating to the QL class of the node is very useful.

[aeisenberg]

That would work, too. I think the class name in square brackets should be the most appropriate/interesting class for the particular node.

If there is no class (ie- if the node is synthetic), then the name should be in parens. I think this is already the case.

[criemen]

re 1.: The PR #4562 just landed, replacing all labels with the predicates used to navigate in the AST.

There are two caveats:

• In some rare cases, there is no predicate available to access the child (for example, for the grouping of parameters of a function). We then supply a name in angle brackets to clearly distinguish that these names are not predicates
• In some cases, we suply a list of predicates to invoke, for example getExpr().getFullyConverted(). This might impact code that wants to jump-to-def on the predicate name.

[jbj]

I'll close this issue since I believe we've now implemented everything that's compatible with the AST viewer as it exists today. If that's not the case, feel free to reopen.

@aeisenberg If you have a detailed proposal for the jump-to-type functionality, or if we should figure out a design together with the other language teams, please open a new issue.