Technically we still depend on points-to in that we still mention
`PythonFunctionValue` and `ClassValue` in the query. However, we
immediately move to working with the corresponding `Function` and
`Class` AST nodes, and so we're not really using points-to. (The reason
for doing things this way is that otherwise the `.toString()` for all of
the alerts would change, which would make the diff hard to interpret.
This way, it should be fairly simple to see which changes are actually
relevant.)
We do lose some precision when moving away from points-to, and this is
reflected in the changes in the `.expected` file. In particular we no
longer do complicated tracking of values, but rather look at the
syntactic structure of the classes in question. This causes us to lose
out on some results where a special method is defined elsewhere, and
causes a single FP where a special method initially has the wrong
signature, but is subsequently overwritten with a function with the
correct signature.
We also lose out on results having to do with default values, as these
are now disabled.
Finally, it was necessary to add special handling of methods marked with
the `staticmethod` decorator, as these expect to receive fewer
arguments. This was motivated by a MRVA run, where e.g. sympy showed a
lot of examples along the lines of
```
@staticmethod
def __abs__():
return ...
```
Adds a new boolean parameter `is_unused_default` that indicates whether
the given result is one where a parameter to a special method has a
default value (which will never be used when invoked in the normal way).
These results are somewhat less useful (because the special method
_might_ be invoked directly, in which case the default value would still
be relevant), but it seemed like a shame to simply remove the code, so
instead I opted to disable it in this way.
Moves a bunch of `owner.declaredAttribute(name) = f` instances to the
top level, in the process greatly cleaning up the code. The behaviour
should be the unchanged.
Having done this, there's only one place where we depend on points-to,
and that's in the remaining `declaredAttribute` call. This should
greatly simplify the move away from points to.
Fixes the false positive reported in
https://github.com/github/codeql/issues/18910
Adds a new `Annotation` class (subclass of `Expr`) which encompasses all
possible kinds of annotations in Python.
Using this, we look for string literals which are part of an annotation,
and which have the same content as the name of a (potentially) unused
global variable, and in that case we do not produce an alert.
In future, we may want to support inspecting such string literals more
deeply (e.g. to support stuff like "list[unused_var]"), but I think for
now this level of support is sufficient.
Adds two test cases having to do with type annotations. The first one
demonstrates that type annotations (even if they are never executed by
the Python interpreter) count as uses for the purposes of the unused
variable query. The second one demonstrates that this is _not_ the case
if all such uses are inside strings (i.e. forward declarations), as we
do not currently inspect the content of these strings.
These seem generally useful outside of points-to, and so it might be
better to add them to the `Function` class instead.
I took the liberty of renaming these to say `Arguments` rather than
`Parameters`, as this is more in line with the nomenclature that we're
using elsewhere. (The internal points-to methods retain the old names.)
I'm somewhat ambivalent about the behaviour of `getMaxParameters` on
functions with `*varargs`. The hard-coded `INT_MAX` return value is
somewhat awkward, but the alternative (to only have the predicate
defined when a specific maximum exists) seems like it would potentially
cause a lot of headaches.
This required some code changes because of some breaking changes in
`clap` and `tree-sitter`.
Also needed to assign a new bazel repo name to the `crates_vendor` to
avoid name conflicts in `MODULE.bazel`.
Extends the mechanism introduced in
https://github.com/github/codeql/pull/18030
to behave the same for _all_ `MatchLiteralPattern`s, not just the ones
that happen to be the constant `True` or `False`.
Co-authored-by: yoff <yoff@github.com>
Observed on some test files in Nuitka/Nuitka, having `break` and
`continue` outside of loops in Python is (to Python) a syntax error, but
our parser happily accepted this broken syntax.
This then caused issues further downstream in the control-flow
construction, as it broke some invariants.
To fix this we now skip the code that would previously fail when the
invariants are broken.
Co-authored-by: yoff <yoff@github.com>
