Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename global var affects list comprehension #293

Closed
matmarczak opened this issue Mar 3, 2020 · 2 comments
Closed

Rename global var affects list comprehension #293

matmarczak opened this issue Mar 3, 2020 · 2 comments

Comments

@matmarczak
Copy link

With code:

some_var = 1
compr = [some_var for some_var in range(10)]

When renaming either some_var from global or some_var from list comprehension scope both variables get renamed.

@lieryan
Copy link
Member

lieryan commented Sep 6, 2021

Copying the investigation text from #249:

I took a look at this, it seems like there are a number of issues at play here:

  1. pyscopes.py::_HoldingScopeFinder currently assumes that a single logical line of code (i.e. a python statement) can only belong only to a single variable scope, but list/generator expression and lambdas are expressions that can create their own variable scope, so this assumption is incorrect. It may take quite a bit of internal rewrite of _HoldingScopeFinder and friends to fix this part of the issue.

  2. currently we don't create a variable scope for List/Generator Comprehension, this is in line with Python 2, where Comprehension leaks its loop variables to the surrounding scope, but it's incorrect for Python 3 code (Comprhensions with scope #422)

  3. The loop variable of a List/Generator Comprehension currently aren't considered an assignment statement, which means that unless you have an assignment for the same name elsewhere, rope can't resolve the variable (Implement renaming of walrus operator and loop variables in comprehensions #318)

Probably the quickest way to fix this is to fix that issue 3 first. That should make rope treat comprehension with Python 2 scope semantics (i.e. leaky loop variable), so renames will work most of the time. In Python 3 though, renames will be a bit overzealous if your loop variable shadows a name in the surrounding scope. We can fix the scoping issue later on.

@lieryan
Copy link
Member

lieryan commented Oct 9, 2021

This is now fixed in #430, which is merged in master and will be part of 0.21 release.

@lieryan lieryan closed this as completed Oct 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants