-
Notifications
You must be signed in to change notification settings - Fork 249
impl convert module #2102
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
base: main
Are you sure you want to change the base?
impl convert module #2102
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements LSP refactor actions for converting between Python modules and packages, mirroring PyCharm's behavior. A module (e.g., foo.py) can be converted to a package (foo/__init__.py), and an empty package (containing only __init__.py and optionally __pycache__) can be converted back to a module.
Changes:
- Added workspace edit support for file rename and delete operations
- Implemented bidirectional module-package conversion code actions
- Added comprehensive LSP interaction tests
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
pyrefly/lib/lsp/non_wasm/server.rs |
Added helper functions to check workspace edit capabilities, validate empty packages, and generate convert module/package code actions with appropriate rename and delete operations |
pyrefly/lib/test/lsp/lsp_interaction/convert_module_package.rs |
Added tests verifying both module-to-package and package-to-module conversion code actions with proper workspace edit operations |
pyrefly/lib/test/lsp/lsp_interaction/mod.rs |
Registered the new test module |
pyrefly/lib/test/lsp/lsp_interaction/test_files/convert_module_package/*.py |
Added test fixtures for module and empty package conversion scenarios |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
stroxler
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the new helpers be in a dedicated module rather than server.rs? I think in general we're trying to avoid too much growth in the central LSP modules as we add refactoring capabilities which often require a lot of code
4a272c5 to
cb39cb1
Compare
This comment has been minimized.
This comment has been minimized.
cb39cb1 to
9fc9223
Compare
This comment has been minimized.
This comment has been minimized.
9fc9223 to
b40e574
Compare
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
Summary
Fixes part of #364
Implemented LSP refactor actions to mirror PyCharm’s “convert module ↔ package” behavior: module→package renames foo.py to
foo/__init__.py, and package→module is only offered when the package directory contains only__init__.py(empty package), keeping content in__init__.pyas the module body.Added workspace-edit refactor actions (rename + delete operations; gated on client support for documentChanges/resourceOperations).
Test Plan
Added LSP interaction tests and fixtures.