Skip to content

Preserve used imports during filtered import cleanup - #877

Open
yangfan-yf-yf wants to merge 1 commit into
python-rope:masterfrom
yangfan-yf-yf:fix-preserve-filtered-imports
Open

yangfan-yf-yf wants to merge 1 commit into
python-rope:masterfrom
yangfan-yf-yf:fix-preserve-filtered-imports

Conversation

@yangfan-yf-yf

Copy link
Copy Markdown
Contributor

Description

Fixes #811.

Moving a global from a package module can remove a still-used aliased import when another from import from the same package precedes it. In the reported example, moving module1_global_variable leaves proxy_module1_function() referencing utils_module after its import has been deleted, so calling it raises NameError.

I traced this to AddingVisitor: it reports a successful merge into a read-only import statement even though that statement rejects the update. Duplicate removal then deletes the other import. The visitor now accepts a read-only destination only when its existing names and aliases already cover the incoming import. New names, aliases, and star imports remain in their own statements; genuine duplicates can still be removed.

I added a move regression and import-filter tests covering distinct names, aliases, star imports, identical imports, and imports already covered by a superset or star. I also ran the original three-module example in separate Python processes before and after moving the variable: both executions now return 2 1 1.

Validation on Windows / Python 3.12.3:

  • The new move regression and three distinct-import subcases fail on the unmodified implementation.
  • Complete suite: 2119 passed, 12 skipped, 5 xfailed, with all six new import-filter subcases passing.
  • All five configured pre-commit checks pass for the changed files.

Checklist

  • I have added tests that prove my fix is effective
  • I have updated CHANGELOG.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor.move messing up existing imports

1 participant