Sweep the last few uses of legacy AstNode.getAFlowNode() in tests over to
explicit ControlFlowNode joins after the shared-CFG migration. importflow.ql
needs the new Cfg::ControlFlowNode/CompareNode types because DataFlow::Node.
asCfgNode() now returns the shared-CFG node.
Also extend ImportResolution::allowedEssaImportStep to walk back through
uncertain-write SSA inputs, so that a later 'from X import *' does not hide
the preceding explicit (re)assignment from module-export resolution. Without
this, a reassigned name that survives a wildcard import was no longer
recognised as the module export. Rebless ModuleExport.expected to drop the
legacy 'ControlFlowNode for' toString prefix and pick up the two correct rows
exposed by the fix.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mostly removing of nodes from the graph.
One result lost:
```
check("submodule.submodule_attr", submodule.submodule_attr, "submodule_attr", globals()) #$ MISSING:prints=submodule_attr
```
This commit removes SSA nodes from the data flow graph. Specifically, for a definition and use such as
```python
x = expr
y = x + 2
```
we used to have flow from `expr` to an SSA variable representing x and from that SSA variable to the use of `x` in the definition of `y`. Now we instead have flow from `expr` to the control flow node for `x` at line 1 and from there to the control flow node for `x` at line 2.
Specific changes:
- `EssaNode` from the data flow layer no longer exists.
- Several glue steps between `EssaNode`s and `CfgNode`s have been deleted.
- Entry nodes are now admitted as `CfgNodes` in the data flow layer (they were filtered out before).
- Entry nodes now have a new `toString` taking into account that the module name may be ambigous.
- Some tests have been rewritten to accomodate the changes, but only `python/ql/test/experimental/dataflow/basic/maximalFlowsConfig.qll` should have semantic changes.
- Comments have been updated
- Test output has been updated, but apart from `python/ql/test/experimental/dataflow/basic/maximalFlows.expected` only `python/ql/test/experimental/dataflow/typetracking-summaries/summaries.py` should have a semantic change. This is a bonus fix, probably meaning that something was never connected up correctly.
It's nice that it fixes the `InsecureProtocol` test-case (which maybe
should have been a test-case for the import resolution library in the
first place?)
But it's not quite right:
1. it adds spurious flow for `clashing_attr`
2. it runs into huge problems for typetracking_imports/tracked.expected
3. it runs into the problem for
https://github.com/github/codeql/pull/10176 with an `from <pkg>
import *` blocking flow from previously defined variable, that is NOT
overridden. (simplistic_reexport.bar_attr)
Just like the one added for `py/insecure-protocol` in fb425b7, but
instead added in the import-resolution tests, such that we don't have to
remember it's in a completely different directory.
It's not very useful to look at, and it's a mess when you change any
tests to see all the changes lines in the expected output that you
really do not care about!
I guess we could have done this at the very start of introducing this
test in this PR, but I think the last commit was mostly inspired from
looking at all the things that evidently was re-exported from the trace
import, even when I knew they were not available because of the
`__all__` definition.
However, we can see that `from <pkg> import *` and `import pkg` are
handled differently. Would have liked `has_defined_all_indirection` to
behave in the same way no matter how the import was made.
Notice that `has_defined_all_indirection` all have both
`all_defined_bar_copy` and `all_defined_foo_copy` marked as exported,
even though only `all_defined_foo_copy` is available.
For `from <pkg> import <attr>` we would use to treat the `<pkg>`
(ImportExpr) as a definition of the name `<attr>`.
Since this removes bad import-flow, and nothing broke, I'm guessing this
was never intentional.
