Rename and inline silently skip modules with syntax errors when ignore_syntax_errors is set

Open
  • Is something wrong?
#923 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 7 days

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
35/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Active
Tech stack
python
Domain
developer-experience

Research direction

Start with the repro script, which uses Rename from rope.refactor.rename and inline.create_inline from rope.refactor.inline under ignore_syntax_errors=True, and compare with the run that raises ModuleSyntaxError when the flag is off. Trace where the occurrence finder skips unparseable modules and where the ChangeSet is built. Done means a caller can learn which module was skipped, either through a per-call override that raises or through a list of skipped resources, and the maintainers have picked one of those approaches.

Written by the indexing model from the issue text.

Description

With ignore_syntax_errors=True, cross-module refactorings treat a module that fails to parse as if it contained no occurrences. The module is left out of the ChangeSet and nothing tells the caller, so the refactoring leaves broken code behind.

Editors and language servers have to set this preference, because a project nearly always has a file that is half-edited. So every Rope-based tool can hit this, and the user only finds out when the skipped module fails at import time.

To Reproduce

import pathlib
import tempfile

from rope.base.project import Project
from rope.refactor import inline
from rope.refactor.rename import Rename

root = pathlib.Path(tempfile.mkdtemp())
(root / "a.py").write_text("def helper(x):\n    return x * 2\n")
(root / "b.py").write_text("from a import helper\nprint(helper(3))\n")
# c.py uses helper too, but has a syntax error further down
(root / "c.py").write_text("from a import helper\nprint(helper(4))\nif\n")

for name, make in [
    ("rename", lambda p, r: Rename(p, r, 4).get_changes("double")),
    ("inline", lambda p, r: inline.create_inline(p, r, 4).get_changes()),
]:
    project = Project(str(root), ropefolder=None, ignore_syntax_errors=True)
    changes = make(project, project.get_resource("a.py"))
    print(name, sorted(c.resource.path for c in changes.changes))

Output on master (46ebf37), Python 3.13:

rename ['a.py', 'b.py']
inline ['a.py', 'b.py']

After the rename, c.py still imports helper, which no longer exists. After the inline, a.py is empty and c.py imports a function that has been deleted. Without ignore_syntax_errors, both refactorings raise ModuleSyntaxError for c.py, which is the safe outcome.

Expected behaviour

The caller should learn that a module which may contain occurrences could not be analysed, so that it can refuse the refactoring or warn the user. Some possible approaches:

  1. A per-call override, such as get_changes(..., ignore_syntax_errors=False), that raises ModuleSyntaxError naming the module, even when the project preference is set.
  2. A list of the skipped resources, on the ChangeSet or the task handle.

Either would be enough. The occurrence finder already filters modules by a textual match before parsing them, so only modules that mention the name would need to be reported.

Editor information

  • Python 3.13
  • Rope master 46ebf37 (also 1.14.0)
  • Found while adding Rope refactorings to rope-language-server, which has to set ignore_syntax_errors
Dominant language
Python
Stars
2.2k
Forks
197
Avg merge
2d 13h
Merged PRs (30d)
5

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from python-rope/rope

All issues in python-rope/rope

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.