impl show file ref in safe delete #2415#2663
impl show file ref in safe delete #2415#2663asukaminato0721 wants to merge 1 commit intofacebook:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends the safe-delete file feature (from #2415) by adding two new code actions when a file has import usages: "Find usages of file ..." (which opens a reference list via a client command) and "Delete file ... anyway" (which performs the delete regardless of usages). It also includes the corresponding VS Code extension handler for the pyrefly.findFileUsages command and updates tests.
Changes:
- Refactored
safe_delete_file_code_actionintosafe_delete_file_code_actionsto return multiple actions, adding "Find usages" and "Delete anyway" options when imports are found - Implemented
collect_import_usage_locationsto gather exact reference locations (file URI + range) instead of just detecting presence of usages - Added VS Code extension command handler (
pyrefly.findFileUsages) to display references usingeditor.action.showReferences
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pyrefly/lib/lsp/non_wasm/safe_delete_file.rs | Refactored to return multiple code actions and collect usage locations with ranges |
| pyrefly/lib/lsp/non_wasm/server.rs | Updated import and call site for renamed function returning Vec |
| pyrefly/lib/test/lsp/lsp_interaction/safe_delete_file.rs | Updated tests to verify new "Find usages" and "Delete anyway" actions |
| lsp/src/extension.ts | Added pyrefly.findFileUsages command handler to show references |
| lsp/package.json | Registered pyrefly.findFileUsages command in extension manifest |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| CodeActionOrCommand::CodeAction(CodeAction { | ||
| title: title.clone(), | ||
| command: Some(Command { | ||
| title, | ||
| command: "pyrefly.findFileUsages".to_owned(), | ||
| arguments: Some(vec![json!({ | ||
| "uri": uri, | ||
| "locations": locations, | ||
| })]), | ||
| }), | ||
| ..Default::default() | ||
| }) |
There was a problem hiding this comment.
The find_usages_code_action is missing a kind field (defaults to None). Every other code action in this codebase sets a kind (e.g., refactor.delete, refactor.move, quickfix, source.fixAll). While kind is optional per the LSP spec, some clients use CodeActionContext.only to filter actions by kind. Actions without a kind may be unexpectedly hidden or shown depending on the client's filtering logic. Consider setting a kind here, such as CodeActionKind::new("refactor.delete") to be consistent with the sibling "delete anyway" action, or another appropriate kind.
This comment has been minimized.
This comment has been minimized.
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
Summary
Fixes #2415
Added two unsafe-delete actions for files: Find usages of file ...(opens a reference list via a client command) and Delete file
...anyway.Implemented import-usage collection to provide exact reference locations for the new command.
Updated the VS Code extension to handle pyrefly.findFileUsages and show the references list.
Test Plan
Adjusted LSP interaction tests to expect the new actions.