nonlocal in locals #2617
nonlocal in locals #2617
Conversation
|
Hmmm... Why pylint consider |
There was a problem hiding this comment.
I also don't understand the pylint issue. Pinging @jacobtylerwalls to see if this sounds familiar, they have a very good grasp of recurring pylint issues
astroid/rebuilder.py
Outdated
| @@ -61,6 +61,11 @@ def __init__( | |||
| self._manager = manager | |||
| self._data = data.split("\n") if data else None | |||
| self._global_names: list[dict[str, list[nodes.Global]]] = [] | |||
| # In _nonlocal_names, | |||
There was a problem hiding this comment.
This comment is formatted a bit strange. Does it fit on two lines?
There was a problem hiding this comment.
I've modified it to
# In _nonlocal_names saves the FunctionDef where the variable is created,
# rather than Nonlocal, since we don't really need the Nonlocal statement.
There was a problem hiding this comment.
Thanks for working up the PR, but if you don't mind I'd like to pause to consider this in a little more detail first. My first impression is to wonder if we need to make a change here at all, see comment on issue.
Type of Changes
Description
Closes #2616
I did this by adding a
_nonlocal_namesproperty inTreeRebuilderto store those nonlocal names, like what_global_namesdoes. However it storesFunctionDefs where the variables are created, instead ofNonlocalnodes. (The savedGlobalnodes in_global_namesis not used for as well, but I didn't touch it, in case we may need that one day.)The items of
_nonlocal_namesare added invisit_nonlocal, which go through the tree and find the nearestFunctionDefwhoselocalscontains that name.And it is used in
_save_assignment. Ifnode.nameis inself._nonlocal_names[-1], then it will be put intoself._nonlocal_names[-1][node.name](the foundFunctionDef) rather than intonode.parent.I have also added a unit test
test_locals_with_global_and_nonlocalintest_builder.py, but I'm not really sure if I put it in the correct place.There might be some other problems since it's my first PR for this project. Really need someone to check it before merging.