The legacy CFG emitted two ControlFlowNodes for `x[i] += 42` (one load,
one store, with `load.strictlyDominates(store)`). The new CFG collapses
them to a single canonical node, mirroring Java's single-`VarAccess`
model where `isVarRead`/`isVarWrite` are non-disjoint on the same
expression. Reconcile two legacy two-node behaviours with the merged
single-node world:
1. `Cfg::ControlFlowNode.isLoad()` no longer excludes augmented
targets — both `isLoad` and `isStore` hold on the merged canonical
node, matching Java. `NameNode.defines` drops the now-redundant
`not isLoad` guard; `Py::Name.defines` already filters by
`isDefinition` (Store/Param/AugAssign-target ctx).
2. `LocalFlow::definitionFlowStep` is restricted to NameNode targets,
matching legacy ESSA's `assignment_definition` which required
`defn.(NameNode).defines(v)`. Subscript and attribute writes
(`x[i] = 42`, `obj.attr = 42`) no longer emit a local-flow step
*into* the LHS expression — that flow is handled by the AttrWrite
and content-flow machinery. This is essential for keeping augmented
Subscript/Attribute targets classifiable as `LocalSourceNode` on
the read side, which the API graph requires for emitting Use edges.
`StoreLoadTest.ql` is updated to filter `isAugLoad` out of the regular
`load` tag, mirroring the pre-existing `not isAugStore` filter on the
`store` tag so augmented-assignment expectations remain
`augload=n augstore=n` (not also `load=n store=n`).
Closes the three remaining ApiGraphs library-test failures
(`getSubscript.ql` semantically, plus cosmetic toString updates in
`ModuleImportWithDots.ql` and `test_crosstalk.ql`).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds API graph support for observing that in
```python
def foo(x : Bar): ...
```
The variable `x` is likely to be an instance of the type `Bar` inside
this function.
In particular, we add `getInstanceFromAnnotation` as a predicate on API
graph nodes that tracks this step (corresponding to a new edge type
labeled with "annotation" in the API graph), and extend the existing
`getAnInstance` predicate to also include instances arising from type
annotations.
A more complete solution would also add support for annotated
assignments (`x : Foo = ...` or just `x : Foo`) as well as track types
through type aliases (`type Foo = Bar`). This turns out to be
non-trivial, however, as these type constructs don't have any CFG nodes
(and so no data-flow nodes by default either). In order to not have
perfect be the enemy of good, this commit is only targeting the type
parameter case (which is also likely to be the most common use case
anyway).
The tests for API graphs have been extended accordingly, including tests
for the kinds of type ascriptions that we _don't_ currently model in API
graphs (marked with `MISSING:` in the inline tests).
Factor out use-use flow in order to do this.
Also improve names and comments.
I also wanted to change the types in `difinitionFlowStep`, but
that broke the module instantiation.
type tracking and the API graph.
- In `TypeTrackerSpecific.qll` we add a jump step
- to every scope entry definition
- from the value of any defining `DefinitionNode`
(In our example, the definition is the class name, `Users`,
while the assigned value is the class definition, and it is
the latter which receives flow in this case.)
- In `LocalSources.qll` we allow scope entry definitions as local sources.
- This feels natural enough, as they are a local source for the value, they represent.
It is perhaps a bit funne to see an Ssa variable here,
rather than a control flow node.
- This is necessary in order for type tracking to see the local flow
from the scope entry definition.
- In `ApiGraphs.qll` we no longer restrict the result of `trackUseNode`
to be an `ExprNode`. To keep the positive formulation, we do not
prohibit module variable nodes. Instead we restrict to the new
`LocalSourceNodeNotModule` which avoids those cases.
This is a condensed versio of the user reported example
found [here](eb377d5918/app.py (L278))
The `MISSING` annotation indicates where our API graph falls short.
A class decorator could change the class definition in any way.
In this specific case, it would be better if we allowed the subclass to
be found with API graphs still.
inspired by
c2250cfb80/tests/auth_tests/test_views.py (L40-L46)
