From 46e749b08e3856719bbc8dd4951a5db3abc0ddea Mon Sep 17 00:00:00 2001 From: Kyle Into Date: Mon, 13 Oct 2025 13:40:42 -0700 Subject: [PATCH 1/4] get_transitive_rdeps should work on in-memory files Summary: it's confusing that callers have to know which type of handle to provide when all handles are allowed. I can't think of a reason someone would call this on an in-memory file and expect no rdeps. Differential Revision: D84531937 --- pyrefly/lib/state/lsp.rs | 47 +++++++++++--------------------------- pyrefly/lib/state/state.rs | 15 +++++++++--- 2 files changed, 25 insertions(+), 37 deletions(-) diff --git a/pyrefly/lib/state/lsp.rs b/pyrefly/lib/state/lsp.rs index 1796d6deb..ff52f52b2 100644 --- a/pyrefly/lib/state/lsp.rs +++ b/pyrefly/lib/state/lsp.rs @@ -2741,40 +2741,19 @@ impl<'a> CancellableTransaction<'a> { // General strategy: // 1: Compute the set of transitive rdeps. // 2. Find references in each one of them using the index computed during earlier checking - let mut transitive_rdeps = match definition.module.path().details() { - ModulePathDetails::Memory(path_buf) => { - let handle_of_filesystem_counterpart = Handle::new( - definition.module.name(), - ModulePath::filesystem(path_buf.clone()), - sys_info.dupe(), - ); - // In-memory files can never be found through import resolution (no rdeps), - // so we must compute the transitive rdeps of its filesystem counterpart instead. - let mut rdeps = self - .as_ref() - .get_transitive_rdeps(handle_of_filesystem_counterpart.dupe()); - // We still add itself to the rdeps set, so that we will still find local references - // within the file. - rdeps.insert(Handle::new( - definition.module.name(), - definition.module.path().dupe(), - sys_info.dupe(), - )); - rdeps - } - _ => { - let definition_handle = Handle::new( - definition.module.name(), - definition.module.path().dupe(), - sys_info.dupe(), - ); - let rdeps = self.as_ref().get_transitive_rdeps(definition_handle.dupe()); - // We still need to know everything about the definition file, because the index - // only contains non-local references. - self.run(&[definition_handle], Require::Everything)?; - rdeps - } - }; + let handle = Handle::new( + definition.module.name(), + definition.module.path().dupe(), + sys_info.dupe(), + ); + let mut transitive_rdeps = self.as_ref().get_transitive_rdeps(handle); + // We still add itself to the rdeps set, so that we will still find local references + // within the file. + transitive_rdeps.insert(Handle::new( + definition.module.name(), + definition.module.path().dupe(), + sys_info.dupe(), + )); // Remove the filesystem counterpart from candidate list, // otherwise we will have results from both filesystem and in-memory version of the file. for fs_counterpart_of_in_memory_handles in transitive_rdeps diff --git a/pyrefly/lib/state/state.rs b/pyrefly/lib/state/state.rs index ff7019510..e392710cc 100644 --- a/pyrefly/lib/state/state.rs +++ b/pyrefly/lib/state/state.rs @@ -457,10 +457,19 @@ impl<'a> Transaction<'a> { } /// Compute transitive dependency closure for the given handle. - /// Note that for IDE services, if the given handle is an in-memory one, then you are probably - /// not getting what you want, because the set of rdeps of in-memory file for IDE service will - /// only contain itself. pub fn get_transitive_rdeps(&self, handle: Handle) -> HashSet { + // The set of rdeps on an in-memory file is only itself. This is never what callers want, so they + // should not need to convert it themself. + let handle = if let ModulePathDetails::Memory(path) = handle.path().details() { + Handle::new( + handle.module(), + ModulePath::filesystem(path.clone()), + handle.sys_info().dupe(), + ) + } else { + handle + }; + let mut transitive_rdeps = HashSet::new(); let mut work_list = vec![handle]; loop { From b1a046a5bd7cd1b29cf20d7d6c1e24dd2b0c55ff Mon Sep 17 00:00:00 2001 From: Kyle Into Date: Mon, 13 Oct 2025 13:40:42 -0700 Subject: [PATCH 2/4] respond to will_rename_files Differential Revision: D84537595 --- pyrefly/lib/lsp/server.rs | 49 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/pyrefly/lib/lsp/server.rs b/pyrefly/lib/lsp/server.rs index 3738d728d..08e6b5622 100644 --- a/pyrefly/lib/lsp/server.rs +++ b/pyrefly/lib/lsp/server.rs @@ -79,6 +79,7 @@ use lsp_types::Registration; use lsp_types::RegistrationParams; use lsp_types::RelatedFullDocumentDiagnosticReport; use lsp_types::RelativePattern; +use lsp_types::RenameFilesParams; use lsp_types::RenameOptions; use lsp_types::RenameParams; use lsp_types::SemanticTokens; @@ -141,6 +142,7 @@ use lsp_types::request::SemanticTokensFullRequest; use lsp_types::request::SemanticTokensRangeRequest; use lsp_types::request::SignatureHelpRequest; use lsp_types::request::UnregisterCapability; +use lsp_types::request::WillRenameFiles; use lsp_types::request::WorkspaceConfiguration; use lsp_types::request::WorkspaceSymbolRequest; use pyrefly_build::handle::Handle; @@ -444,7 +446,20 @@ pub fn capabilities( supported: Some(true), change_notifications: Some(OneOf::Left(true)), }), - file_operations: None, + file_operations: Some(lsp_types::WorkspaceFileOperationsServerCapabilities { + will_rename: Some(lsp_types::FileOperationRegistrationOptions { + filters: vec![lsp_types::FileOperationFilter { + pattern: lsp_types::FileOperationPattern { + glob: "**/*.{py,pyi}".to_owned(), + + matches: Some(lsp_types::FileOperationPatternKind::File), + options: None, + }, + scheme: None, + }], + }), + ..Default::default() + }), }), ..Default::default() } @@ -882,6 +897,20 @@ impl Server { )); ide_transaction_manager.save(transaction); } + } else if let Some(params) = as_request::(&x) { + if let Some(params) = self + .extract_request_params_or_send_err_response::( + params, &x.id, + ) + { + let transaction = + ide_transaction_manager.non_committable_transaction(&self.state); + self.send_response(new_response( + x.id, + Ok(self.will_rename_files(&transaction, params)), + )); + ide_transaction_manager.save(transaction); + } } else if &x.method == "pyrefly/textDocument/typeErrorDisplayStatus" { let text_document: TextDocumentIdentifier = serde_json::from_value(x.params)?; self.send_response(new_response( @@ -2108,6 +2137,24 @@ impl Server { let _ = lsp_queue.send(LspEvent::RecheckFinished); })); } + + fn will_rename_files( + &self, + _transaction: &Transaction<'_>, + params: RenameFilesParams, + ) -> Option { + // TODO: Implement import updates when files are renamed + // For now, return None to indicate no edits are needed + // This is similar to how basedpyright initially implemented this feature + eprintln!( + "will_rename_files called with {} file(s)", + params.files.len() + ); + for file in ¶ms.files { + eprintln!(" Renaming: {} -> {}", file.old_uri, file.new_uri); + } + None + } } impl TspInterface for Server { From 42a5b05cfa8e3b0a71ff8f18edd9ca9b20b35598 Mon Sep 17 00:00:00 2001 From: Kyle Into Date: Mon, 13 Oct 2025 13:40:42 -0700 Subject: [PATCH 3/4] implement basic will_rename_files Summary: will_rename_files fixes imports when renaming files in the editor Differential Revision: D84537597 --- pyrefly/lib/lsp/features/mod.rs | 1 + pyrefly/lib/lsp/features/will_rename_files.rs | 314 ++++++++++++++++++ pyrefly/lib/lsp/server.rs | 15 +- 3 files changed, 318 insertions(+), 12 deletions(-) create mode 100644 pyrefly/lib/lsp/features/will_rename_files.rs diff --git a/pyrefly/lib/lsp/features/mod.rs b/pyrefly/lib/lsp/features/mod.rs index 826bb67a8..00ef88eae 100644 --- a/pyrefly/lib/lsp/features/mod.rs +++ b/pyrefly/lib/lsp/features/mod.rs @@ -7,3 +7,4 @@ pub mod hover; pub mod provide_type; +pub mod will_rename_files; diff --git a/pyrefly/lib/lsp/features/will_rename_files.rs b/pyrefly/lib/lsp/features/will_rename_files.rs new file mode 100644 index 000000000..fef36e813 --- /dev/null +++ b/pyrefly/lib/lsp/features/will_rename_files.rs @@ -0,0 +1,314 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +use std::collections::HashMap; +use std::sync::Arc; + +use lsp_types::RenameFilesParams; +use lsp_types::TextEdit; +use lsp_types::Url; +use lsp_types::WorkspaceEdit; +use pyrefly_python::PYTHON_EXTENSIONS; +use pyrefly_python::ast::Ast; +use pyrefly_python::module_name::ModuleName; +use pyrefly_util::lined_buffer::LinedBuffer; +use pyrefly_util::lock::RwLock; +use rayon::prelude::*; +use ruff_python_ast::Stmt; +use ruff_python_ast::name::Name; +use ruff_text_size::Ranged; + +use crate::lsp::module_helpers::make_open_handle; +use crate::lsp::module_helpers::module_info_to_uri; +use crate::state::state::State; +use crate::state::state::Transaction; + +/// Visitor that looks for imports of an old module name and creates TextEdits to update them +struct RenameUsageVisitor<'a> { + edits: Vec, + old_module_name: &'a ModuleName, + new_module_name: &'a ModuleName, + lined_buffer: &'a LinedBuffer, +} + +impl<'a> RenameUsageVisitor<'a> { + fn new( + old_module_name: &'a ModuleName, + new_module_name: &'a ModuleName, + lined_buffer: &'a LinedBuffer, + ) -> Self { + Self { + edits: Vec::new(), + old_module_name, + new_module_name, + lined_buffer, + } + } + + fn visit_stmt(&mut self, stmt: &Stmt) { + match stmt { + Stmt::Import(import) => { + for alias in &import.names { + let imported_module = ModuleName::from_name(&alias.name.id); + if imported_module == *self.old_module_name + || imported_module + .as_str() + .starts_with(&format!("{}.", self.old_module_name.as_str())) + { + // Replace the module name + let new_import_name = if imported_module == *self.old_module_name { + self.new_module_name.as_str().to_owned() + } else { + // Replace the prefix + imported_module.as_str().replace( + self.old_module_name.as_str(), + self.new_module_name.as_str(), + ) + }; + + self.edits.push(TextEdit { + range: self.lined_buffer.to_lsp_range(alias.name.range()), + new_text: new_import_name, + }); + } + } + } + Stmt::ImportFrom(import_from) => { + if let Some(module) = &import_from.module { + let imported_module = ModuleName::from_name(&module.id); + if imported_module == *self.old_module_name + || imported_module + .as_str() + .starts_with(&format!("{}.", self.old_module_name.as_str())) + { + // Replace the module name + let new_import_name = if imported_module == *self.old_module_name { + self.new_module_name.as_str().to_owned() + } else { + // Replace the prefix + imported_module.as_str().replace( + self.old_module_name.as_str(), + self.new_module_name.as_str(), + ) + }; + + self.edits.push(TextEdit { + range: self.lined_buffer.to_lsp_range(module.range()), + new_text: new_import_name, + }); + } + } + } + _ => {} + } + } + + fn take_edits(self) -> Vec { + self.edits + } +} + +/// Handle workspace/willRenameFiles request to update imports when files are renamed. +/// +/// This function: +/// 1. Converts file paths to module names +/// 2. Uses get_transitive_rdeps to find all files that depend on the renamed module +/// 3. Uses a visitor pattern to find imports of the old module and creates TextEdits +/// 4. Returns a WorkspaceEdit with all necessary changes +pub fn will_rename_files( + state: &Arc, + transaction: &Transaction<'_>, + _open_files: &Arc>>>, + params: RenameFilesParams, +) -> Option { + eprintln!( + "will_rename_files called with {} file(s)", + params.files.len() + ); + + let mut all_changes: HashMap> = HashMap::new(); + + for file_rename in ¶ms.files { + eprintln!( + " Processing rename: {} -> {}", + file_rename.old_uri, file_rename.new_uri + ); + + // Convert URLs to paths + let old_uri = match Url::parse(&file_rename.old_uri) { + Ok(uri) => uri, + Err(_) => { + eprintln!(" Failed to parse old_uri"); + continue; + } + }; + + let new_uri = match Url::parse(&file_rename.new_uri) { + Ok(uri) => uri, + Err(_) => { + eprintln!(" Failed to parse new_uri"); + continue; + } + }; + + let old_path = match old_uri.to_file_path() { + Ok(path) => path, + Err(_) => { + eprintln!(" Failed to convert old_uri to path"); + continue; + } + }; + + let new_path = match new_uri.to_file_path() { + Ok(path) => path, + Err(_) => { + eprintln!(" Failed to convert new_uri to path"); + continue; + } + }; + + // Only process Python files + if !PYTHON_EXTENSIONS + .iter() + .any(|ext| old_path.extension().and_then(|e| e.to_str()) == Some(*ext)) + { + eprintln!(" Skipping non-Python file"); + continue; + } + + // Get the config to find the search path + let old_handle = make_open_handle(state, &old_path); + let config = state + .config_finder() + .python_file(old_handle.module(), old_handle.path()); + + // Convert paths to module names + let old_module_name = + ModuleName::from_path(&old_path, config.search_path()).or_else(|| { + // Fallback: try to get module name from the handle + Some(old_handle.module()) + }); + + // For the new module name, we can't rely on from_path because the file doesn't exist yet. + // Instead, we compute the relative path from the old to new file and adjust the module name. + let new_module_name = if let Some(old_parent) = old_path.parent() { + if let Some(new_parent) = new_path.parent() { + // If both files are in the same directory, just replace the file name part + if old_parent == new_parent { + // Extract the module name from the new file name + if let Some(file_stem) = new_path.file_stem() { + if let Some(file_stem_str) = file_stem.to_str() { + // If the old file was in a module, replace just the last component + if let Some(old_mod) = &old_module_name { + let mut components = old_mod.components(); + if !components.is_empty() { + components.pop(); + components.push(Name::new(file_stem_str)); + Some(ModuleName::from_parts(components)) + } else { + Some(ModuleName::from_str(file_stem_str)) + } + } else { + Some(ModuleName::from_str(file_stem_str)) + } + } else { + None + } + } else { + None + } + } else { + // Files are in different directories, try from_path + ModuleName::from_path(&new_path, config.search_path()) + } + } else { + None + } + } else { + None + }; + + let (old_module_name, new_module_name) = match (old_module_name, new_module_name) { + (Some(old), Some(new)) => (old, new), + _ => { + eprintln!( + " Could not determine module names for the rename (old={:?}, new={:?})", + old_module_name, new_module_name + ); + continue; + } + }; + + eprintln!( + " Module rename: {} -> {}", + old_module_name, new_module_name + ); + + // If module names are the same, no need to update imports + if old_module_name == new_module_name { + eprintln!(" Module names are the same, skipping"); + continue; + } + + // Use get_transitive_rdeps to find all files that depend on this module + let rdeps = transaction.get_transitive_rdeps(old_handle.clone()); + + eprintln!(" Found {} transitive rdeps", rdeps.len()); + + // Visit each dependent file to find and update imports (parallelized) + let rdeps_changes: Vec<(Url, Vec)> = rdeps + .into_par_iter() + .filter_map(|rdep_handle| { + let module_info = transaction.get_module_info(&rdep_handle)?; + + let ast = Ast::parse(module_info.contents()).0; + let mut visitor = RenameUsageVisitor::new( + &old_module_name, + &new_module_name, + module_info.lined_buffer(), + ); + + for stmt in &ast.body { + visitor.visit_stmt(stmt); + } + + let edits_for_file = visitor.take_edits(); + + if !edits_for_file.is_empty() { + let uri = module_info_to_uri(&module_info)?; + eprintln!( + " Found {} import(s) to update in {}", + edits_for_file.len(), + uri + ); + Some((uri, edits_for_file)) + } else { + None + } + }) + .collect(); + + // Merge results into all_changes + for (uri, edits) in rdeps_changes { + all_changes.entry(uri).or_default().extend(edits); + } + } + + if all_changes.is_empty() { + eprintln!(" No import updates needed"); + None + } else { + eprintln!( + " Returning {} file(s) with import updates", + all_changes.len() + ); + Some(WorkspaceEdit { + changes: Some(all_changes), + ..Default::default() + }) + } +} diff --git a/pyrefly/lib/lsp/server.rs b/pyrefly/lib/lsp/server.rs index 08e6b5622..3bae7de16 100644 --- a/pyrefly/lib/lsp/server.rs +++ b/pyrefly/lib/lsp/server.rs @@ -176,6 +176,7 @@ use crate::lsp::features::hover::get_hover; use crate::lsp::features::provide_type::ProvideType; use crate::lsp::features::provide_type::ProvideTypeResponse; use crate::lsp::features::provide_type::provide_type; +use crate::lsp::features::will_rename_files::will_rename_files; use crate::lsp::lsp::apply_change_events; use crate::lsp::lsp::as_notification; use crate::lsp::lsp::as_request; @@ -2140,20 +2141,10 @@ impl Server { fn will_rename_files( &self, - _transaction: &Transaction<'_>, + transaction: &Transaction<'_>, params: RenameFilesParams, ) -> Option { - // TODO: Implement import updates when files are renamed - // For now, return None to indicate no edits are needed - // This is similar to how basedpyright initially implemented this feature - eprintln!( - "will_rename_files called with {} file(s)", - params.files.len() - ); - for file in ¶ms.files { - eprintln!(" Renaming: {} -> {}", file.old_uri, file.new_uri); - } - None + will_rename_files(&self.state, transaction, &self.open_files, params) } } From 177c9902a6d9c64f6eebfbcf20e1ba1041b1f815 Mon Sep 17 00:00:00 2001 From: Kyle Into Date: Mon, 13 Oct 2025 13:40:42 -0700 Subject: [PATCH 4/4] test will_rename_files (#1281) Summary: Pull Request resolved: https://github.com/facebook/pyrefly/pull/1281 Differential Revision: D84535941 --- pyrefly/lib/test/lsp/lsp_interaction/mod.rs | 1 + .../lsp/lsp_interaction/will_rename_files.rs | 73 +++++++++++++++++++ 2 files changed, 74 insertions(+) create mode 100644 pyrefly/lib/test/lsp/lsp_interaction/will_rename_files.rs diff --git a/pyrefly/lib/test/lsp/lsp_interaction/mod.rs b/pyrefly/lib/test/lsp/lsp_interaction/mod.rs index 44c7cd825..136c182d2 100644 --- a/pyrefly/lib/test/lsp/lsp_interaction/mod.rs +++ b/pyrefly/lib/test/lsp/lsp_interaction/mod.rs @@ -22,4 +22,5 @@ mod references; mod rename; mod type_definition; mod util; +mod will_rename_files; mod workspace_symbol; diff --git a/pyrefly/lib/test/lsp/lsp_interaction/will_rename_files.rs b/pyrefly/lib/test/lsp/lsp_interaction/will_rename_files.rs new file mode 100644 index 000000000..e5164fa38 --- /dev/null +++ b/pyrefly/lib/test/lsp/lsp_interaction/will_rename_files.rs @@ -0,0 +1,73 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +use lsp_server::Message; +use lsp_server::Request; +use lsp_server::RequestId; +use lsp_server::Response; +use lsp_types::Url; + +use crate::test::lsp::lsp_interaction::object_model::InitializeSettings; +use crate::test::lsp::lsp_interaction::object_model::LspInteraction; +use crate::test::lsp::lsp_interaction::util::get_test_files_root; + +#[test] +fn test_will_rename_files() { + let root = get_test_files_root(); + let mut interaction = LspInteraction::new(); + interaction.set_root(root.path().to_path_buf()); + interaction.initialize(InitializeSettings::default()); + + let foo = "tests_requiring_config/foo.py"; + let bar = "tests_requiring_config/bar.py"; + interaction.server.did_open(foo); + interaction.server.did_open(bar); + + let bar_path = root.path().join(bar); + let baz_path = root.path().join("tests_requiring_config/baz.py"); + let foo_path = root.path().join(foo); + + // Send will_rename_files request to rename bar.py to baz.py + interaction.server.send_message(Message::Request(Request { + id: RequestId::from(2), + method: "workspace/willRenameFiles".to_owned(), + params: serde_json::json!({ + "files": [{ + "oldUri": Url::from_file_path(&bar_path).unwrap().to_string(), + "newUri": Url::from_file_path(&baz_path).unwrap().to_string() + }] + }), + })); + + // Expect a response with edits to update imports in foo.py + interaction.client.expect_response(Response { + id: RequestId::from(2), + result: Some(serde_json::json!({ + "changes": { + Url::from_file_path(&foo_path).unwrap().to_string(): [ + { + "newText": "baz", + "range": { + "start": {"line": 5, "character": 7}, + "end": {"line": 5, "character": 10} + } + }, + { + "newText": "baz", + "range": { + "start": {"line": 6, "character": 5}, + "end": {"line": 6, "character": 8} + } + } + ] + } + })), + error: None, + }); + + interaction.shutdown(); +}