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

Extracted method fails when it contains multi subscriptables #509

Closed
naokisz opened this issue Oct 5, 2022 · 2 comments · Fixed by #524
Closed

Extracted method fails when it contains multi subscriptables #509

naokisz opened this issue Oct 5, 2022 · 2 comments · Fixed by #524
Labels
bug Unexpected or incorrect user-visible behavior extract-refactor
Milestone

Comments

@naokisz
Copy link

naokisz commented Oct 5, 2022

Describe the bug

Error while extracting method that contains augmented assignment to multi subscriptables (dict, list etc.).

To Reproduce

Steps to reproduce the behavior:

  1. Code before refactoring:
def my_method():
    my_var = [[0], [1], [2]]
    my_var[0][0] += 1
    print(1)
  1. Describe the refactoring you want to do

Select the print(1) line, and then trigger Extract method.

  1. Expected code after refactoring:
def my_method():
    my_var = [[0], [1], [2]]
    my_var[0][0] += 1
    extracted_method()

def extracted_method():
    print(1)
  1. Describe the error or unexpected result that you are getting

Exception occured and code didn't change.

Traceback (most recent call last):
  File "/home/naokisz/python/rope_issue/__pypackages__/3.10/lib/pylsp_rope/plugin.py", line 152, in pylsp_execute_command
    return commands[command](workspace, **arguments[0])()
  File "/home/naokisz/python/rope_issue/__pypackages__/3.10/lib/pylsp_rope/plugin.py", line 176, in __call__
    rope_changeset = self.get_changes()
  File "/home/naokisz/python/rope_issue/__pypackages__/3.10/lib/pylsp_rope/plugin.py", line 240, in get_changes
    rope_changeset = refactoring.get_changes(
  File "/home/naokisz/python/rope_issue/__pypackages__/3.10/lib/rope/refactor/extract.py", line 83, in get_changes
    new_contents = _ExtractPerformer(info).extract()
  File "/home/naokisz/python/rope_issue/__pypackages__/3.10/lib/rope/refactor/extract.py", line 274, in extract
    extract_info = self._collect_info()
  File "/home/naokisz/python/rope_issue/__pypackages__/3.10/lib/rope/refactor/extract.py", line 300, in _collect_info
    self._find_definition(extract_collector)
  File "/home/naokisz/python/rope_issue/__pypackages__/3.10/lib/rope/refactor/extract.py", line 372, in _find_definition
    parts = _ExtractMethodParts(self.info)
  File "/home/naokisz/python/rope_issue/__pypackages__/3.10/lib/rope/refactor/extract.py", line 507, in __init__
    self.info_collector = self._create_info_collector()
  File "/home/naokisz/python/rope_issue/__pypackages__/3.10/lib/rope/refactor/extract.py", line 579, in _create_info_collector
    ast.walk(node, info_collector)
  File "/home/naokisz/python/rope_issue/__pypackages__/3.10/lib/rope/base/ast.py", line 41, in walk
    walk(child, walker)
  File "/home/naokisz/python/rope_issue/__pypackages__/3.10/lib/rope/base/ast.py", line 39, in walk
    return method(node)
  File "/home/naokisz/python/rope_issue/__pypackages__/3.10/lib/rope/refactor/extract.py", line 813, in _FunctionDef
    ast.walk(child, self)
  File "/home/naokisz/python/rope_issue/__pypackages__/3.10/lib/rope/base/ast.py", line 39, in walk
    return method(node)
  File "/home/naokisz/python/rope_issue/__pypackages__/3.10/lib/rope/refactor/extract.py", line 847, in _AugAssign
    target_id = node.target.value.id
AttributeError: 'Subscript' object has no attribute 'id'

Editor information (please complete the following information):

  • Project Python version: python 3.10.7
  • Rope Python version: python 3.10.7
  • Rope version: rope 1.3.0
  • Text editor/IDE and version: Neovim with pylsp-rope 0.1.10

Additional context

This issue may be related to #459 because same exception occured.
But this issue differs from #459 in that #459 doesn't contain multi subscriptables and contains try..except block.

@naokisz naokisz added the bug Unexpected or incorrect user-visible behavior label Oct 5, 2022
@lieryan
Copy link
Member

lieryan commented Oct 5, 2022

Thanks for writing the bug report @naokisz, the issue does seem to be related to #459, and really appreciate your taking the time to find that related ticket.

Issue #459 wasn't actually related to the try-except block itself, but rather the crux of the issue is that it is an error when rope is analysing the extracted code to find variables that are being read and/or written within the extracted code block, for determining the extracted function's argument list and return value. The try-except itself isn't really related, other than it somehow helps in triggering the bug.

It seems that the analyser doesn't currently handle Subscript nodes correctly, which manifested in both #459 and this issue. The pull request #460 which was created to fix #459 was incomplete, as it doesn't correctly handle nested subscriptions and other more complex left hand expressions.

I think _AugAssign will need to recursively walk the left hand expression (i.e. node.target) for it to handle these more complex expressions.

@lieryan
Copy link
Member

lieryan commented Nov 21, 2022

This has been fixed in master and will be included when 1.5.0 is released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unexpected or incorrect user-visible behavior extract-refactor
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants