Diagnostic `Location.file` fields contained absolute filesystem paths,
causing the GitHub UI to generate broken file links with runner paths
like `/home/runner/work/...`. Now paths are relativized against the
source root (`LGTM_SRC` or cwd), falling back to absolute if the file
is outside the source root.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
First, we extend the various location overriding hacks to also accept
list and dict splats in various places. Having done this, we then have
to tackle how to actually desugar these new comprehension forms (as this
is what we currently do for the old forms).
As a reminder, a list comprehension like `[x for x in y]` currently gets
desugared into a small local function, something like
```python
def listcomp(a):
for x in a:
yield x
listcomp(y)
```
For `[*x for x in y]`, the behaviour we want is that we unpack `x`
before yielding its elements in turn. This is essentially what we would
get if we were to use `yield from x` instead of `yield x` in the above
desugaring, so that's what we do. This also works for set
comprehensions.
For dict comprehensions, it's slightly more complicated. Here, the
generator function instead yields a stream of `(key, value)` tuples.
(And apparently the old parser got this wrong and emitted `(value, key)`
pairs instead, which we faithfully recreated in the new parser as well.
We fix that bug in both parsers while we're at it). So, a bare `yield
from` is not enough, we also need a `.items()` call to get the
double-starred expression to emit its items as a stream of tuples (that
we then `yield from`.
To make this (hopefully) less verbose in the implementation, we defer
the decision of whether to use `yield` or `yield from` by introducing a
`yield_kind` scoped variable that determines the type of the actual AST
node. And of course for dict comprehensions with unpacking we need to
synthesise the extra machinery mentioned above.
On the plus side, this means we don't have to mess with control-flow, as
the existing machinery should be able to handle the desugared syntax
just fine.
Adds a new `isLazy` predicate to the relevant classes, and adds the
relevant dbscheme (and up/downgrade) changes. On upgrades we do nothing,
and on downgrades we remove the `is_lazy` bits.
As defined in PEP-810. We implement this in much the same way as how we
handle `async` annotations currently. The relevant nodes get an
`is_lazy` field that defaults to being false.
An example (provided by @redsun82) is the string `f"{x:=^20}"`. Parsing
this (with unnamed nodes shown) illustrates the problem:
```
module [0, 0] - [2, 0]
expression_statement [0, 0] - [0, 11]
string [0, 0] - [0, 11]
string_start [0, 0] - [0, 2]
interpolation [0, 2] - [0, 10]
"{" [0, 2] - [0, 3]
expression: named_expression [0, 3] - [0, 9]
name: identifier [0, 3] - [0, 4]
":=" [0, 4] - [0, 6]
ERROR [0, 6] - [0, 7]
"^" [0, 6] - [0, 7]
value: integer [0, 7] - [0, 9]
"}" [0, 9] - [0, 10]
string_end [0, 10] - [0, 11]
```
Observe that we've managed to combine the format specifier token `:` and
the fill character `=` in a single token (which doesn't match the `:` we
expect in the grammar rule), and hence we get a syntax error.
If we change the `=` to some other character (e.g. a `-`), we instead
get
```
module [0, 0] - [2, 0]
expression_statement [0, 0] - [0, 11]
string [0, 0] - [0, 11]
string_start [0, 0] - [0, 2]
interpolation [0, 2] - [0, 10]
"{" [0, 2] - [0, 3]
expression: identifier [0, 3] - [0, 4]
format_specifier: format_specifier [0, 4] - [0, 9]
":" [0, 4] - [0, 5]
"}" [0, 9] - [0, 10]
string_end [0, 10] - [0, 11]
```
and in particular no syntax error.
To fix this, we want to ensure that the `:` is lexed on its own, and the
`token(prec(1, ...))` construction can be used to do exactly this.
Finally, you may wonder why `=` is special here. I think what's going on
is that the lexer knows that `:=` is a token on its own (because it's
used in the walrus operator), and so it greedily consumes the following
`=` with this in mind.
See https://peps.python.org/pep-0758/ for more details.
We implement this by extending the syntax for exceptions and exception
groups so that the `type` field can now contain either an expression
(which matches the old behaviour), or a comma-separated list of at least
two elements (representing the new behaviour).
We model the latter case using a new node type `exception_list`, which
in `tsg-python` is simply mapped to a tuple. This means it matches the
existing behaviour (when the tuple is surrounded by parentheses)
exactly, hence we don't need to change any other code.
As a consequence of this, however, we cannot directly parse the Python
2.7 syntax `except Foo, e: ...` as `except Foo as e: ...`, as this would
introduce an ambiguity in the grammar. Thus, we have removed support for
the (deprecated) 2.7-style syntax, and only allow `as` to indicate
binding of the exception. The syntax `except Foo, e: ...` continues to
be parsed (in particular, it's not suddenly a syntax error), but it will
be parsed as if it were `except (Foo, e): ...`, which may not give the
correct results.
In principle we could extend the QL libraries to account for this case
(specifically when analysing Python 2 code). In practice, however, I
expect this to have a minor impact on results, and not worth the
additional investment at this time.
Adds three new AST nodes to the mix:
- `TemplateString` represents a t-string in Python 3.14
- `TemplateStringPart` represents one of the string constituents of a
t-string. (The interpolated expressions are represented as `Expr` nodes,
just like f-strings.)
- `JoinedTemplateString` represents an implicit concatenation of
template strings.
Importantly, we _completely avoid_ the complicated construction we
currently do for format strings (as well as the confusing nomenclature).
No extra injection of empty strings (so that a template string is a
strict alternation of strings and expressions). A `JoinedTemplateString`
simply has a list of template string children, and a `TemplateString`
has a list of "values" which may be either `Expr` or
`TemplateStringPart` nodes.
If we ever find that we actually want the more complicated interface for
these strings, then I would much rather we reconstruct this inside of QL
rather than in the parser.
- Extends the scanner with a new token kind representing the start of a
template string. This is used to distinguish template strings from
regular strings (because only a template string will start with a
`_template_string_start` external token).
- Cleans up the logic surrounding interpolations (and the method names)
so that format strings and template strings behave the same in this
case.
Finally, we add two new node types in the tree-sitter grammar:
- `template_string` behaves like format strings, but is a distinct type
(mainly so that an implicit concatenation between template strings and
regular strings becomes a syntax error).
- `concatenated_template_string` is the counterpart of
`concatenated_string`.
However, internally, the string parts of a template strings are just the
same `string_content` nodes that are used in regular format strings. We
will disambiguate these inside `tsg-python`.
Follow-up to https://github.com/github/codeql/pull/20630
The fix didn't fully work since when we raise the ImportError in `find_module` we don't pass a named argument into the format string which causes a `KeyError`.
We need to use a format string without named arguments, like Python 3.13 and earlier did.
It seems `_ERR_MSG` was silently removed in Python 3.14, leading to an
`ImportError` when running the extractor.
To fix this, we explicitly set `_ERR_MSG` when the existing import fails
(using `_ERR_MSG_PREFIX` which is available in Python 3.14+, along with
the bits that make up the difference between this and `_ERR_MSG`).
- fall back to full extraction on overlay changes json read error
- we filter both root modules and (transitive) imports against the overlay-changes json.
Rather than relying on matching arbitrary nodes inside tree-sitter-graph
and then checking whether they are of type ERROR or MISSING (which seems
to have stopped working in later versions of tree-sitter), we now
explicitly go through the tree-sitter tree, locating all of the error
and missing nodes along the way. We then add these on to the graph
output in the same format as was previously produced by
tree-sitter-graph.
Note that it's very likely that some of the syntax errors will move
around a bit as a consequence of this change. In general, we don't
expect syntax errors to have stable locations, as small changes in the
grammar can cause an error to appear in a different position, even if
the underlying (erroneous) code has not changed.
It seems that with a newer version of tree-sitter, we no longer parse
the (not actually valid!) syntax `Spam[**P2]` as if the `**` is an
exponentiation operation (with a missing left operand).
Updates the Python extractor to depend on version 0.24.7 of tree-sitter
(and 0.12.0 of tree-sitter-graph).
A few changes were needed in order to make the code build and run after
updating the dependencies:
- In `main.rs`, the `Language` parameter is now passed as a reference.
- In `python.tsg`, many queries had captures that were not actually used
in the body of the stanza. This is no longer allowed (unless the
captures start with an underscore), as it may indicate an error. To fix
this, I added underscores in the appropriate places (and verified that
none of these unused captures were in fact bugs).
This previously only worked in certain circumstances. In particular,
assignments such as `match[1] = ...` or even just `match[1]` would fail
to parse correctly.
Fixing this turned out to be less trivial than anticipated. Consider the
fact that
```
match [1]: case (...)
```
can either look the start of a `match` statement, or it could be a type
ascription, ascribing the value of `case(...)` (a call) to the item at
index 1 of `match`.
To fix this, then, we give `match` the identifier and `match` the
statement the same precendence in the grammar, and additionally also
mark a conflict between `match_statement` and `primary_expression`. This
causes the conflict to be resolved dynamically, and seems to do the
right thing in all cases.
The previous version was tested on a version of the code where we had
temporarily removed the `glob.strip("/")` bit, and so the bug didn't
trigger then.
We now correctly remember if the glob ends in `/`, and add an extra part
in that case. This way, if the path ends with multiple slashes, they
effectively get consolidated into a single one, which results in the
correct semantics.
