Preserve parameter references in unopened files Summary: An earlier change fixed incoming call hierarchy for files that the client did not open. It requested `Require::Everything` for every file that depends on the target module. This made the required ASTs available. However, the same helper also serves find references and find implementations. Those requests then performed full analysis on every dependent file. D117389890 (PR) limits that work. Incoming call hierarchy performs full analysis only on files that reference the target. Type hierarchy performs full analysis on its candidate files. Most find-reference requests and all find-implementation requests use only the retained index. This keeps incoming call hierarchy correct and makes common reference requests faster. D117389890 leaves one correctness problem. Parameter references do not use only the index. To find a keyword argument such as `greet(message="Hello")`, Pyrefly scans the caller AST. It also uses bindings to confirm that the keyword resolves to the selected parameter. An unopened file has neither at `Require::Indexing`. Find references therefore omits the keyword argument. Rename can change the parameter definition but leave `message=` unchanged in the unopened caller. This diff requests `Require::Everything` for all dependent files only when the selected definition is a parameter. Other definitions keep the index-only path from D117389890. This restores parameter references and rename without removing the main performance improvement. This diff also removes a duplicate type-hierarchy request for `Require::Everything`. The caller already makes the same request before it reads the candidate files. The tests cover parameter references and parameter rename in an unopened caller file. The call hierarchy test now has two unopened caller files. This verifies that one batch prepares every selected caller file. Reviewed By: stroxler Differential Revision: D117455220 fbshipit-source-id: f8ba135ace020fbcd0e4ba9b0e7509773ba832be
diff --git a/pyrefly/lib/lsp/non_wasm/server.rs b/pyrefly/lib/lsp/non_wasm/server.rs index 798006d..9b40b41 100644 --- a/pyrefly/lib/lsp/non_wasm/server.rs +++ b/pyrefly/lib/lsp/non_wasm/server.rs
@@ -6280,15 +6280,6 @@ handles.push(candidate); } } - // `type_hierarchy_subtype_items` reads each candidate's AST, solutions and bindings and - // silently `continue`s past any candidate missing them, so it needs `Require::Everything`. - // Escalate here rather than in `compute_transitive_rdeps_for_definition_impl`: this keeps - // type hierarchy's behaviour exactly as it is today while letting `textDocument/references` - // and `find_global_implementations_from_definition`, both answered entirely from the index, - // stop paying for ASTs they never read. - if !handles.is_empty() { - transaction.run(&handles, Require::Everything, None)?; - } Ok(handles) }
diff --git a/pyrefly/lib/state/lsp.rs b/pyrefly/lib/state/lsp.rs index 6981890..5fe7dab 100644 --- a/pyrefly/lib/state/lsp.rs +++ b/pyrefly/lib/state/lsp.rs
@@ -5015,6 +5015,8 @@ sys_info, ); let rdeps = transaction.transitive_rdeps(definition_handle.dupe()); + // Same-module reference discovery reads the definition's AST, bindings, and answers, + // even though most reverse dependencies can be answered from their retained indexes. transaction.run_for_handles(&[definition_handle], Require::Everything)?; rdeps } @@ -5078,11 +5080,25 @@ transaction: &mut T, sys_info: SysInfo, definition: &TextRangeWithModule, - mut process_fn: impl FnMut(&mut T, &Handle, &TextRangeWithModule) -> Option<R>, + process_fn: impl FnMut(&mut T, &Handle, &TextRangeWithModule) -> Option<R>, ) -> Result<Vec<R>, Cancelled> { let candidate_handles = compute_transitive_rdeps_for_definition_impl(transaction, sys_info, definition)?; + Ok(process_candidate_handles_with_definition_impl( + transaction, + candidate_handles, + definition, + process_fn, + )) +} + +fn process_candidate_handles_with_definition_impl<T: RdepTransaction, R>( + transaction: &mut T, + candidate_handles: Vec<Handle>, + definition: &TextRangeWithModule, + mut process_fn: impl FnMut(&mut T, &Handle, &TextRangeWithModule) -> Option<R>, +) -> Vec<R> { let mut results = Vec::new(); for handle in candidate_handles { let patched_definition = patch_definition_for_handle_impl(transaction, &handle, definition); @@ -5091,7 +5107,7 @@ } } - Ok(results) + results } fn find_global_references_from_definition_impl<T: RdepTransaction>( @@ -5101,9 +5117,16 @@ definition: TextRangeWithModule, options: ReferenceOptions, ) -> Result<Vec<(Module, Vec<TextRange>)>, Cancelled> { - let results = process_rdeps_with_definition_impl( + let candidate_handles = + compute_transitive_rdeps_for_definition_impl(transaction, sys_info, &definition)?; + if definition_kind.symbol_kind() == Some(SymbolKind::Parameter) { + // Keyword argument references require each candidate's AST and bindings to resolve the + // callee and refine the argument back to this parameter. + transaction.run_for_handles(&candidate_handles, Require::Everything)?; + } + let results = process_candidate_handles_with_definition_impl( transaction, - sys_info, + candidate_handles, &definition, |transaction, handle, patched_definition| { let mut module_refs: Vec<(Module, Vec<TextRange>)> = Vec::new(); @@ -5144,7 +5167,7 @@ Some(module_refs) } }, - )?; + ); let mut global_references: Vec<(Module, Vec<TextRange>)> = Vec::new(); for module_refs in results { @@ -5190,8 +5213,9 @@ impl<'a> CancellableTransaction<'a> { /// Processes each transitive reverse dependency for a given definition location. /// - /// This is a common pattern in workspace-wide - /// references-related features + /// This is a common pattern in workspace-wide references-related features. Candidates are + /// processed at their current requirement level; callers must explicitly request any data + /// beyond the retained index. pub(crate) fn process_rdeps_with_definition<T>( &mut self, sys_info: SysInfo,
diff --git a/pyrefly/lib/test/lsp/lsp_interaction/call_hierarchy.rs b/pyrefly/lib/test/lsp/lsp_interaction/call_hierarchy.rs index 21d4269..ca07b74 100644 --- a/pyrefly/lib/test/lsp/lsp_interaction/call_hierarchy.rs +++ b/pyrefly/lib/test/lsp/lsp_interaction/call_hierarchy.rs
@@ -302,19 +302,25 @@ .expect_response_with(|result| { let incoming_calls = result.expect("Expected Some(incoming_calls) for an unopened caller file"); - let caller_names: Vec<String> = incoming_calls + let mut caller_names: Vec<String> = incoming_calls .iter() .map(|call| call.from.name.clone()) .collect(); - - for expected in ["caller_one", "caller_two", "method_caller"] { - assert!( - caller_names.contains(&expected.to_owned()), - "Expected to find {} in an unopened file, got: {:?}", - expected, - caller_names - ); - } + caller_names.sort(); + assert_eq!( + caller_names, + vec![ + "caller_one".to_owned(), + "caller_three".to_owned(), + "caller_two".to_owned(), + "method_caller".to_owned(), + ] + ); + assert!( + incoming_calls + .iter() + .all(|call| call.from_ranges.len() == 1) + ); true })
diff --git a/pyrefly/lib/test/lsp/lsp_interaction/references.rs b/pyrefly/lib/test/lsp/lsp_interaction/references.rs index 702b654..0c48014 100644 --- a/pyrefly/lib/test/lsp/lsp_interaction/references.rs +++ b/pyrefly/lib/test/lsp/lsp_interaction/references.rs
@@ -16,6 +16,53 @@ use crate::test::lsp::lsp_interaction::util::get_test_files_root; #[test] +fn test_parameter_references_in_unopened_file() { + let root = get_test_files_root(); + let root_path = root.path().join("rename_kwargs_across_files"); + let scope_uri = Url::from_file_path(root_path.clone()).unwrap(); + let mut interaction = LspInteraction::new_with_args(LspInteractionArgs { + args: LspArgs { + indexing_mode: IndexingMode::LazyBlocking, + ..LspInteractionArgs::default().args + }, + ..Default::default() + }); + interaction.set_root(root_path.clone()); + interaction + .initialize(InitializeSettings { + workspace_folders: Some(vec![("test".to_owned(), scope_uri)]), + configuration: Some(None), + ..Default::default() + }) + .unwrap(); + + let defs = root_path.join("defs.py"); + let uses = root_path.join("uses.py"); + interaction.client.did_open("defs.py"); + + interaction + .client + .references("defs.py", 6, 16, true) + .expect_response(json!([ + { + "range": {"start":{"line":13,"character":31},"end":{"line":13,"character":38}}, + "uri": Url::from_file_path(&uses).unwrap().to_string() + }, + { + "range": {"start":{"line":6,"character":16},"end":{"line":6,"character":23}}, + "uri": Url::from_file_path(&defs).unwrap().to_string() + }, + { + "range": {"start":{"line":7,"character":14},"end":{"line":7,"character":21}}, + "uri": Url::from_file_path(&defs).unwrap().to_string() + }, + ])) + .unwrap(); + + interaction.shutdown().unwrap(); +} + +#[test] fn test_references_for_usage_with_config() { let root = get_test_files_root(); let root_path = root.path().join("tests_requiring_config");
diff --git a/pyrefly/lib/test/lsp/lsp_interaction/rename.rs b/pyrefly/lib/test/lsp/lsp_interaction/rename.rs index b7bfd56..e6385e9 100644 --- a/pyrefly/lib/test/lsp/lsp_interaction/rename.rs +++ b/pyrefly/lib/test/lsp/lsp_interaction/rename.rs
@@ -8,8 +8,11 @@ use lsp_types::Url; use lsp_types::request::PrepareRenameRequest; use lsp_types::request::Rename; +use pyrefly_lsp_test::IndexingMode; +use pyrefly_lsp_test::LspArgs; use pyrefly_lsp_test::object_model::InitializeSettings; use pyrefly_lsp_test::object_model::LspInteraction; +use pyrefly_lsp_test::object_model::LspInteractionArgs; use serde_json::json; use tempfile::TempDir; @@ -257,12 +260,18 @@ } #[test] -fn test_rename_kwarg_across_files() { +fn test_rename_kwarg_in_unopened_file() { let root = get_test_files_root(); let root_path = root.path().join("rename_kwargs_across_files"); let scope_uri = Url::from_file_path(root_path.clone()).unwrap(); - let mut interaction = LspInteraction::new(); + let mut interaction = LspInteraction::new_with_args(LspInteractionArgs { + args: LspArgs { + indexing_mode: IndexingMode::LazyBlocking, + ..LspInteractionArgs::default().args + }, + ..Default::default() + }); interaction.set_root(root_path.clone()); interaction .initialize(InitializeSettings { @@ -276,7 +285,6 @@ let uses = root_path.join("uses.py"); interaction.client.did_open("defs.py"); - interaction.client.did_open("uses.py"); interaction .client
diff --git a/pyrefly/lib/test/lsp/lsp_interaction/test_files/call_hierarchy_test/caller_extra.py b/pyrefly/lib/test/lsp/lsp_interaction/test_files/call_hierarchy_test/caller_extra.py new file mode 100644 index 0000000..f2a6fe8 --- /dev/null +++ b/pyrefly/lib/test/lsp/lsp_interaction/test_files/call_hierarchy_test/caller_extra.py
@@ -0,0 +1,10 @@ +# 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. + +from callee import my_function + + +def caller_three(): + my_function()