Agent-Logs-Url: https://github.com/github/codeql/sessions/bd4fa112-dbc3-47e8-9cef-9b1b13c7e549

Co-authored-by: owen-mc <62447351+owen-mc@users.noreply.github.com>

Agent-Logs-Url: https://github.com/github/codeql/sessions/3db201db-a1b5-4353-a94a-14a8d156dd3b

Co-authored-by: owen-mc <62447351+owen-mc@users.noreply.github.com>

These tests consist of various Python constructions (hopefully a
somewhat comprehensive set) with specific timestamp annotations
scattered throughout. When the tests are run using the Python 3
interpreter, these annotations are checked and compared to the "current
timestamp" to see that they are in agreement. This is what makes the
tests "self-validating".

There are a few different kinds of annotations: the basic `t[4]` style
(meaning this is executed at timestamp 4), the `t[dead(4)]` variant
(meaning this _would_ happen at timestamp 4, but it is in a dead
branch), and `t[never]` (meaning this is never executed at all).

In addition to this, there is a query, MissingAnnotations, which checks
whether we have applied these annotations maximally. Many expression
nodes are not actually annotatable, so there is a sizeable list of
excluded nodes for that query.

These use the annotated, self-verifying test files to check various
consistency requirements.

Some of these may be expressing the same thing in different ways, but
it's fairly cheap to keep them around, so I have not attempted to
produce a minimal set of queries for this.

This one demonstrates a bug in the current CFG. In a dictionary
comprehension `{k: v for k, v in d.items()}`, we evaluate the value
before the key, which is incorrect. (A fix for this bug has been
implemented in a separate PR.)

This looks for nodes annotated with `t[never]` in the test that are
reachable in the CFG. This should not happen (it messes with various
queries, e.g. the "mixed returns" query), but the test shows that in a
few particular cases (involving the `match` statement where all cases
contain `return`s), we _do_ have reachable nodes that shouldn't be.

This one is potentially a bit iffy -- it checks for a very powerful
property (that implies many of the other queries), but as the test
results show, it can produce false positives when there is in fact no
problem. We may want to get rid of it entirely, if it becomes too noisy.

Part 1 of N of "getting rid of $children" in node-types.yml

Note: in one of the cases the affected node still has the $children
field present. This is because there's some weirdness about recording
multiline comments as class member separators that I did not want to
figure out how to address right now.

I ended up also aliasing `_async_keyword` to a named node to make it
more consistent with the other node kinds that can be in this field (as
it would be awkward to have two named types and a token here).

Elsewhere in the node types, we'll still have `async?: "async"`, and I
think that's okay.

A lot of changes, but for the most part these are just adding named
fields in places where they make sense.

After this, there are still ~20 instances of unnamed children appearing.

This required a change in a different place, due to aliasing.

Some nodes with a single child (arguably redundant to do, but I think
it's nice to have the types be consistent), and also an instance of
ensuring that all branches of a `choice` expose consistent field names.

Not entirely happy about the mixed nature of the `kind` filed (having
both tokens and the named node `throw_keyword` in there), but that's a
problem for a different time.