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

Rope fails to rename variable identifier within a generator expression #249

Closed
d3r3kk opened this issue Jul 6, 2018 · 10 comments
Closed

Comments

@d3r3kk
Copy link

d3r3kk commented Jul 6, 2018

Issue found with renaming a variable within a generator expression. Exception thrown is:

RefactoringError: Rename refactoring should be performed on resolvable python identifiers.

Python version: 3.6.6
OS: Ubuntu 18.04
Rope Version: 0.10.7

Reproduction Steps

  1. Create a folder project
  2. cd project
  3. Create a python file called issue.py and copy the following code into it:
def do_stuff():
    my_list = [counter + 10 for counter in range(0, 10)]
    print(my_list)

do_stuff()
  1. Create another python file named renamer.py and copy the following code into it:
from rope.base import libutils
from rope.refactor.rename import Rename
import rope.base.project

proj = rope.base.project.Project(
    './',
    save_history=False,
    indent_size=4)

res = libutils.path_to_resource(proj, 'issue.py')

if res.exists:
    renamer = Rename(proj, res, 24)
    changes = renamer.get_changes('valid_count', resources=res)
    for c in changes.changes:
        print(c.new_contents)

  1. Run the renamer script via the command line > python renamer.py

Expected result

A printout of the issue.py file's contents, with the 2 instances of the 'countervariable renamed tovalid_count`.

Observed result

Thrown exception:

Traceback (most recent call last):
  File "renamer.py", line 14, in <module>
    renamer = Rename(proj, res, 31)
  File "/home/dekeeler/.local/lib/python3.6/site-packages/rope/refactor/rename.py", line 28, in __init__
    'Rename refactoring should be performed'
rope.base.exceptions.RefactoringError: Rename refactoring should be performed on resolvable python identifiers.
@phoenixsampras
Copy link

Hello and help, any workaround for this issue?

@soupytwist
Copy link
Contributor

Sorry, I haven't had much time to look into these issues further. If anyone else here is able to pick it up, that would be appreciated.

@wheelsandmetal
Copy link

Any news on this?

@thmsklngr
Copy link

I still encounter this behaviour in v0.16.0. Anyone got tipps? Using it within VSCode.

@rogedelgado
Copy link

Hello,

I can confirm the issue.I got the error in vim using LanguageCliente-neovim as client. The rope version that I'm using is v0.16.0.

Any advice for this?

@ArkadyWey
Copy link

Hi,

I am also still having this issue.

Does anyone have a fix?

@alexander-mart
Copy link

Other solution: https://stackoverflow.com/a/59570939/14522024

@mcepl
Copy link
Contributor

mcepl commented Oct 26, 2020

@lieryan Do you have any idea about this one? We seem to have many very unhappy customers here.

@lieryan
Copy link
Member

lieryan commented Oct 28, 2020

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

  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 Sep 6, 2021

Closing this ticket as the core renaming issue is already resolved by pull request #318. The slightly incorrect variable scoping issue will be tracked separately in #293.

@lieryan lieryan closed this as completed Sep 6, 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

10 participants