[asukaminato0721]

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.

[Copilot]

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_action into safe_delete_file_code_actions to return multiple actions, adding "Find usages" and "Delete anyway" options when imports are found
• Implemented collect_import_usage_locations to 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 using editor.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.

[stroxler]

I think there are issues here where the feature can misbehave:

• Incomplete references: indexed-but-closed importers have no retained AST, so they’re skipped. I reproduced this: 0 locations instead of 1. Path deduplication can also discard an open importer’s usable in-memory AST.
• Client integration: the server offers a VS Code-only command without negotiating support, and its missing kind hides “Find usages” from VS Code’s Refactor menu.
• Incorrect locations: notebook references bypass cell-URI mapping; attribute imports can also be mistaken for submodule usages.

One suggestion from an AI reviewer: make discovery on-demand, prefer current in-memory contents, and route locations through the server’s existing conversion helpers. The dependency graph still prevents these omissions from producing a false "Safe delete" action.

[stroxler]

Thanks! This is looking better.

Unfortunately it's a big PR, so the first round didn't find all concerns. I think there are still some things worth addressing based on another round of AI review:

1. Package re-exports can hide both new actions.
   With pkg/__init__.py containing from . import target, requesting actions on pkg/target.py returns neither “Find usages” nor “Delete anyway.” The [export matching](

   pyrefly/pyrefly/lib/lsp/non_wasm/safe_delete_file.rs

   Lines 250 to 269 in a2b90b0

   	let base_exports = self
   	.transaction
   	.import_handle(self.importer, base, None)
   	.finding()
   	.map(|handle| self.transaction.get_exports(&handle));
   	for alias in &import_from.names {
   	let imported_module = append_module(base, &alias.name.id);
   	let refers_to_target = match base_exports
   	.as_ref()
   	.and_then(|exports| exports.get(&alias.name.id))
   	{
   	Some(ExportLocation::ThisModule(_)) => false,
   	Some(ExportLocation::OtherModule(module, _)) => {
   	module_matches_target(*module, self.target_module)
   	}
   	None => module_matches_target(imported_module, self.target_module),
   	};
   	if refers_to_target {
   	self.ranges.push(import_from.range());
   	return;

   ) treats OtherModule(pkg, …) as the final destination, missing the actual submodule. This is a regression from the previous revision. Follow the solver’s self-import/submodule semantics rather than comparing only the first export edge.

2. Removing an import in an unsaved buffer still reports its old disk location.
   I indexed consumer.py containing import target, opened it, then sent didChange replacing its contents with answer = 123. “Find usages” still returned the old import at line 0. The [memory-first sorting](

   pyrefly/pyrefly/lib/lsp/non_wasm/safe_delete_file.rs

   Lines 170 to 189 in a2b90b0

   	let mut rdeps = rdeps.into_iter().collect::<Vec<_>>();
   	// Process an open buffer before its filesystem handle reaches path deduplication.
   	rdeps.sort_by_key(|handle| !handle.path().is_memory());
   	let mut seen = HashSet::new();
   	let mut usages = Vec::new();
   	for rdep_handle in rdeps {
   	if !seen.insert(rdep_handle.path().as_path().to_owned()) {
   	continue;
   	}
   	let Some(module_info) = transaction.get_module_info(&rdep_handle) else {
   	continue;
   	};
   	if !matches!(
   	module_info.path().details(),
   	ModulePathDetails::FileSystem(_) | ModulePathDetails::Memory(_)
   	) {
   	continue;
   	}
   	// Indexed closed files retain source text, but they do not always retain an AST.
   	let ast = Ast::parse(module_info.contents(), module_info.source_type()).0;

   ) cannot help when the current memory handle no longer depends on the target and therefore isn’t in rdeps. Consult current open-buffer contents independently of whether that handle remains a dependent.

3. Unsupported clients still reparse every importer.
   [Collection precedes the capability check](

   pyrefly/pyrefly/lib/lsp/non_wasm/safe_delete_file.rs

   Lines 98 to 115 in a2b90b0

   	let rdeps = transaction.get_rdeps(&handle);
   	if rdeps.is_empty() {
   	return Some(vec![delete_file_code_action(
   	format!("Safe delete file `{file_name}`"),
   	uri,
   	)]);
   	}
   	let usages = collect_import_usages(transaction, rdeps, module_name);
   	if usages.is_empty() {
   	return None;
   	}
   	let mut actions = Vec::new();
   	if supports_find_usages {
   	let locations: Vec<_> = usages.iter().filter_map(to_lsp_location).collect();
   	if !locations.is_empty() {
   	actions.push(find_usages_code_action(&file_name, uri, locations));
   	}
   	}

   ). With 50 importers of roughly 4,000 lines each, warm requests took about 34 ms even with findFileUsages: false, versus 0.16 ms for an unused file. Prefer on-demand location collection; at minimum, short-circuit unsupported clients and reuse retained ASTs.

The closed-file, capability/kind, and notebook-mapping fixes otherwise look good. The open-buffer test uses IndexingMode::None, so it misses the indexed disk/buffer collision above.

[github-actions]

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅