133: Visit functions in while test #186
Conversation
|
@bcaller let me know if this looks good to you. I think I covered all the cases you mentioned :) |
| :param loop_node: The loop node itself to connect to the new function nodes if any | ||
| :return: None | ||
| """ | ||
| if isinstance(comp_n, ast.Call) and get_call_names_as_string(comp_n.func) in self.function_names: |
There was a problem hiding this comment.
I know this was already here, but I don't understand why get_call_names_as_string(comp_n.func) in self.function_names is there. It means that range is ignored for instance. I'd be tempted to remove it, but maybe in a later PR.
There was a problem hiding this comment.
I didn't pay much mind to this detail but if removed, 4 of the test cases or for fail due to functions in the iter node being ignored (e.g. range).
I think the rationale here is to only link the while / for node to the function node if we've visited the function before, but I'm not sure why that would matter since we're immediately visiting the call node.
That said, we don't have a visit_Call method, so the default visitor will be invoked, which will in turn call visit_Name (which we don't have, so default again), then I think a call to visit_Load which will be default again, etc. I wonder if we'd need a visit_Call to handle this case before we can safely remove the in self.function_names.
|
@adrianbn If you make that change avoiding mutating the AST I'm happy to merge. |
Hi!
I've tried to resolve #133 with this PR. I hope I understood what the issue described, but this is my first foray into pyt (and contributions to static analysis tools) so take it with a grain of salt. In particular I'm not 100% convinced about the generated CFG and I'd appreciate a keen eye on the unit test's node relationships.
Let me know if you want me to make changes or if I completely missed the mark!
Regards,
Adrian