I'm slightly suspicious of this fix -- it seems to work, but it makes
me wonder if we're potentially missing other kinds of flow, by not
handling other kinds of definitions.
Also, I feel like this should really be attached to an appropriate
post-update node of the given argument. As it is written now, the flow
will go from the argument _before_ the call, which obviously misses a
step if the argument is modified by the call. In practice, I would
expect this to be rather rare.
This is the quick-and-dirty solution, as discussed.
An even quicker-and-dirtier solution would have used
`ModuleValue::attr` and take the `getOrigin` of that as the source of
the jump step. However, this turns out to be a bad choice, since
`attr` might fail to have a value for the given attribute (for a
variety of reasons). Thus, we instead appeal to a helper predicate
that keeps track of which names are defined by which right-hand-sides
in a given module. (Observe that type tracking works correctly for `x`
in `mymodule.py`, even though `x` is never assigned a value in the
eyes of the Value API.)
This means that points-to is only used to actually figure out if the
object we're looking an attribute up on is a module or not. This is
the next thing to replace in order to eliminate the dependence on
points-to, but this will require some care to ensure that all module
lookups are handled correctly.
Only two test files needed to be changed for the tests to pass. The
first was the fixed false negative in the type tracker, and the other
was a bunch of missing flow in the regression test. I have manually
removed the `# Flow not found` annotations to make them consistent
with the output. Pay particular attention to the annotation on line
117 -- I believe it was misplaced and should have been on line 106
instead (where, indeed, we now have flow where none appeared before).
A slightly more complicated version of the situation in
https://github.com/github/codeql/pull/2507 could cause the `toString`
calculation to diverge. Although the previous PR took tuples nested
inside tuples into account (and subscripted types cannot be nested
inside each other in our modelling), it did not account for having
this nesting be interleaved, and this is what caused the divergence.
I have not done the usual "test case first to show the problem
exists", since this would also diverge and take forever to fail. The
instance observed in `scipy` was likely caused by something akin to
```python
x = ()
while True:
x = x[(x,)]
```
Finally, to prevent this from happening with other types, I went
through and checked each instance where the string representation of
an `ObjectInternal` might potentially contain a reference to
itself (and thus explode). I encapsulated this in a
`bounded_toString` helper predicate, and used this in all the cases
where I was able to determine that the above _could_ happen.
